Re: Help requested for htslib build failure on ppc64el
Hi Michael, Steffen,
On 12/30/2018 05:35 PM, Steffen Möller wrote:
I admit this is a bit over my head. It may be worthwhile to peek a boo at the latest gcc snapshots https://packages.debian.org/de/sid/ppc64/gcc-snapshot/download that may have a fix for that issue already.
I don't think it's a compiler issue... I can't explain it yet but it's probably
a float / double truncation issue exposed by some gcc-8 optimization. But trying
the snapshots can add more information to explain what's going on...
As a workaround to quickly proceed with what you are really after, and following the investigation by Gustavo, you may decide to have -O3 changed to -O2 if the arch is ppc64 and the compiler version is that faulty one.
Sorry, I meant -O0 instead of -O2, i.e. disabling any optimization, so it can't
be used as a workaround since it will destroy performance, specially for RISC.
That said, I dug a bit further and it looks like that sam_parse1() is actually
generating a different value for the floats in question, so when they are
loaded back for the comparison they are already screwed up, e.g.:
-- O3 --
0xc0490fcf
-3.14158988
-- O0 --
0xc0490fd0
-3.14159012 (expected)
because strtod() is used in float_to_le() and so also in u32_to_le(), which are
inlined and float_to_le() takes a float and not a double as the first argument
the use strtof() instead of strtod() in there looks the best, avoiding a
truncation from double to float, which might cause precision issues, specially
between different archs (like casts) plus optimization. So the following
change fixed the issue for me.
Michael, could you confirm that the following change fixes the issue at your
side please?
diff --git a/sam.c b/sam.c
index aa94776..23233a0 100644
--- a/sam.c
+++ b/sam.c
@@ -1408,7 +1408,7 @@ int sam_parse1(kstring_t *s, bam_hdr_t *h, bam1_t *b)
else if (type == 'S') while (q + 1 < p) { u16_to_le(strtoul(q + 1, &q, 0), (uint8_t *) str.s + str.l); str.l += 2; _skip_to_comma(q, p); }
else if (type == 'i') while (q + 1 < p) { i32_to_le(strtol(q + 1, &q, 0), (uint8_t *) str.s + str.l); str.l += 4; _skip_to_comma(q, p); }
else if (type == 'I') while (q + 1 < p) { u32_to_le(strtoul(q + 1, &q, 0), (uint8_t *) str.s + str.l); str.l += 4; _skip_to_comma(q, p); }
- else if (type == 'f') while (q + 1 < p) { float_to_le(strtod(q + 1, &q), (uint8_t *) str.s + str.l); str.l += 4; _skip_to_comma(q, p); }
+ else if (type == 'f') while (q + 1 < p) { float_to_le(strtof(q + 1, &q), (uint8_t *) str.s + str.l); str.l += 4; _skip_to_comma(q, p); }
else _parse_err_param(1, "unrecognized type B:%c", type);
#undef _skip_to_comma
Cheers,
Gustavo
Cheers,
Steffen
On 27.12.18 15:41, Michael Crusoe wrote:
https://buildd.debian.org/status/fetch.php?pkg=htslib&arch=ppc64el&ver=1.9-7&stamp=1545236716&raw=0
Can I get some assistance here? Rebuilding using Qemu and the earlier source packages produces the same error, so maybe this is a regression in the compiler?
--
Michael R. Crusoe
Co-founder & Lead, Common Workflow Language project <http://www.commonwl.org/>
Direktorius, VšĮ "Darbo eigos", Vilnius, Lithuania
https://orcid.org/0000-0002-2961-9670 <https://impactstory.org/u/0000-0002-2961-9670>
mrc@commonwl.org <mailto:mrc@commonwl.org>
+1 480 627 9108 / +370 653 11125
Reply to: