[libc] Workaround for gcc complaining about implicit conversions with the ternary ?: operator.

[lntue]

Fixes #120427, #122745

This is caused by a -Wconversion false-positive bug in gcc: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101537

[llvmbot]

@llvm/pr-subscribers-libc

Author: None (lntue)

Changes

Fixes #120427.

---

Full diff: https://github.com/llvm/llvm-project/pull/124820.diff

1 Files Affected:

• (modified) libc/src/__support/FPUtil/except_value_utils.h (+8-4)

diff --git a/libc/src/__support/FPUtil/except_value_utils.h b/libc/src/__support/FPUtil/except_value_utils.h
index f8e4e92d3e1fb3..2e58bf2b287fdf 100644
--- a/libc/src/__support/FPUtil/except_value_utils.h
+++ b/libc/src/__support/FPUtil/except_value_utils.h
@@ -81,12 +81,16 @@ template <typename T, size_t N> struct ExceptValues {
         StorageType out_bits = values[i].rnd_towardzero_result;
         switch (fputil::quick_get_round()) {
         case FE_UPWARD:
-          out_bits += sign ? values[i].rnd_downward_offset
-                           : values[i].rnd_upward_offset;
+          if (sign)
+            out_bits += values[i].rnd_downward_offset;
+          else
+            out_bits += values[i].rnd_upward_offset;
           break;
         case FE_DOWNWARD:
-          out_bits += sign ? values[i].rnd_upward_offset
-                           : values[i].rnd_downward_offset;
+          if (sign)
+            out_bits += values[i].rnd_upward_offset;
+          else
+            out_bits += values[i].rnd_downward_offset;
           break;
         case FE_TONEAREST:
           out_bits += values[i].rnd_tonearest_offset;

[nickdesaulniers]

Can you walk me through here (or in the PR/commit description) what's going on here? Where were the implicit conversions; how does this patch fix them?

[lntue]

Can you walk me through here (or in the PR/commit description) what's going on here? Where were the implicit conversions; how does this patch fix them?

For some reason, gcc promotes the ternary

sign ? values[i].rnd_downward_offset : values[i].rnd_upward_offset

to int when adding to out_bits +=, even though all of them are of StorageType, aka uint16_t, here.

And they stop the integer promotion when splitting the ternary operations to conditionals.

[nickdesaulniers]

Hmm...the relevant parts of the ASTs still look the same between the two cases though.
https://godbolt.org/z/jEo97qPK1

It's still unclear...oh, I think https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101537 is the relevant bug.

[nickdesaulniers]

Looking at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101537, I think you can keep the ternary, and instead work around this as such:

Suggested change

	if (sign)
	out_bits += values[i].rnd_downward_offset;
	else
	out_bits += values[i].rnd_upward_offset;
	// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101537
	out_bits = out_bits + (sign ? values[i].rnd_downward_offset : values[i].rnd_upward_offset);

ah, but that's not much more readable. Please cite https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101537 in either a comment in the sources and/or the PR description.

[lntue]

Thanks for finding the root cause of it! I updated the comments.