[PATCH] bitintlower: Fix up handle_plus_minus [PR126503]

Jakub Jelinek <[email protected]>
Newsgroups gmane.comp.gcc.patches
Message-ID <amvCymRNut-nd0rS@tucnak>
Hi!

The following testcase is miscompiled on aarch64 (but not on x86_64).
The difference is that x86_64/i686 define optabs that make it use IFN_UADDC
and IFN_USUBC, those are then used both in the loop and to perform the
most significant limb, so
  # _6 = PHI <0(2), _7(3)>
  # _9 = PHI <0(2), _10(3)>
  _8 = VIEW_CONVERT_EXPR<unsigned long[5]>(a)[_6];
  _11 = .USUBC (0, _8, _9);
  _12 = IMAGPART_EXPR <_11>;
  _13 = REALPART_EXPR <_11>;
  VIEW_CONVERT_EXPR<unsigned long[7]>(<retval>)[_6] = _13;
  _14 = _6 + 1;
  _15 = VIEW_CONVERT_EXPR<unsigned long[5]>(a)[_14];
  _16 = .USUBC (0, _15, _12);
  _10 = IMAGPART_EXPR <_16>;
  _17 = REALPART_EXPR <_16>;
  VIEW_CONVERT_EXPR<unsigned long[7]>(<retval>)[_14] = _17;
  _7 = _6 + 2;
  if (_7 != 4)
in the loop and
  _18 = MEM <unsigned long> [(_BitInt(257) *)&a + 32B];
  _19 = (<unnamed-signed:1>) _18;
  _20 = (<unnamed-unsigned:1>) _19;
  _21 = (unsigned long) _20;
  _22 = .USUBC (0, _21, _10);
  _23 = IMAGPART_EXPR <_22>;
  _24 = REALPART_EXPR <_22>;
  _25 = (<unnamed-signed:1>) _24;
  _26 = (unsigned long) _25;
  MEM <unsigned long> [(unsigned _BitInt(400) *)&<retval> + 32B] = _26;
...
after the loop.  Now, on targets which don't support the optab, we instead
use
  # _6 = PHI <0(2), _7(3)>
  # _9 = PHI <0(2), _10(3)>
  _8 = VIEW_CONVERT_EXPR<unsigned long[6]>(a)[_6];
  _11 = .SUB_OVERFLOW (0, _8);
  _13 = REALPART_EXPR <_11>;
  _14 = IMAGPART_EXPR <_11>;
  _15 = .SUB_OVERFLOW (_13, _9);
  _16 = IMAGPART_EXPR <_15>;
  _12 = _14 + _16;
  _17 = REALPART_EXPR <_15>;
  VIEW_CONVERT_EXPR<unsigned long[8]>(<retval>)[_6] = _17;
  _18 = _6 + 1;
  _19 = VIEW_CONVERT_EXPR<unsigned long[6]>(a)[_18];
  _20 = .SUB_OVERFLOW (0, _19);
  _21 = REALPART_EXPR <_20>;
  _22 = IMAGPART_EXPR <_20>;
  _23 = .SUB_OVERFLOW (_21, _12);
  _24 = IMAGPART_EXPR <_23>;
  _10 = _22 + _24;
  _25 = REALPART_EXPR <_23>;
  VIEW_CONVERT_EXPR<unsigned long[8]>(<retval>)[_18] = _25;
  _7 = _6 + 2;
  if (_7 != 4)
in the loop (i.e. instead of one .USUBC 2 .SUB_OVERFLOW) and then
after the loop for the most significant limb
  _26 = MEM <unsigned long> [(_BitInt(257) *)&a + 32B];
  _27 = (<unnamed-signed:1>) _26;
  _28 = (<unnamed-signed:1>) _10;
  _29 = 0 - _27;
  _30 = _29 - _28;
  _31 = (unsigned long) _30;
  MEM <unsigned long> [(unsigned _BitInt(400) *)&<retval> + 32B] = _31;
Now, the last thing is what is wrong.  We need to do two subtractions
(or after folding one negation and one subtraction), and while in the
original operation signed overflow is indeed undefined, it just means
that the two operations together don't overflow, but one of them can.
In this testcase (in foo function) on aarch64, _26 is 1 (the most
significant bit of 257-bit negative value) and _10 is also 1 (borrow
from within the loop).  When we perform this computation in signed 1-bit
precision, we have 0 - -1 (overflow) and -1 - -1 (another overflow).
The RTL emitted for this then results in miscompilation, but we really
shouldn't introduce UB into the IL for something that didn't have UB
originally.

So, the following patch forces use of unsigned type for these and casts
to the signed one only at the end.

Bootstrapped/regtested on x86_64-linux and i686-linux, bootstrapped
on aarch64-linux too, ok for trunk/16.2 if it passes there as well?

2026-07-30  Jakub Jelinek  <[email protected]>

	PR tree-optimization/126503
	* gimple-lower-bitint.cc (bitint_large_huge::handle_plus_minus): If
	IFN_ADDC/IFN_SUBC can't be used and rhs1_type is not the limb type
	and is signed, perform both additions or both subtractions in
	unsigned type for the rhs1_type and cast to rhs1_type at the end.

	* gcc.dg/torture/bitint-106.c: New test.

--- gcc/gimple-lower-bitint.cc.jj	2026-07-29 10:04:21.350852573 +0200
+++ gcc/gimple-lower-bitint.cc	2026-07-30 19:24:50.354031989 +0200
@@ -1287,16 +1287,27 @@ bitint_large_huge::handle_plus_minus (tr
     }
   else
     {
-      tree in = add_cast (rhs1_type, data_in);
-      lhs = make_ssa_name (rhs1_type);
+      /* Always perform the two additions or subtractions in
+	 unsigned type, avoid introducing a temporary UB.  While
+	 the end result of the 2 additions or 2 subtractions in
+	 a valid program should not overflow, temporarily it can.
+	 See PR126503.  */
+      tree utype = unsigned_type_for (rhs1_type);
+      tree in = add_cast (utype, data_in);
+      lhs = make_ssa_name (utype);
+      if (utype != rhs1_type)
+	{
+	  rhs1 = add_cast (utype, rhs1);
+	  rhs2 = add_cast (utype, rhs2);
+	}
       g = gimple_build_assign (lhs, code, rhs1, rhs2);
       insert_before (g);
-      rhs1 = make_ssa_name (rhs1_type);
+      rhs1 = make_ssa_name (utype);
       g = gimple_build_assign (rhs1, code, lhs, in);
       insert_before (g);
       m_data[m_data_cnt] = NULL_TREE;
       m_data_cnt += 2;
-      return rhs1;
+      return utype != rhs1_type ? add_cast (rhs1_type, rhs1) : rhs1;
     }
   rhs1 = make_ssa_name (m_limb_type);
   g = gimple_build_assign (rhs1, REALPART_EXPR,
--- gcc/testsuite/gcc.dg/torture/bitint-106.c.jj	2026-07-30 19:27:34.671045358 +0200
+++ gcc/testsuite/gcc.dg/torture/bitint-106.c	2026-07-30 19:26:43.798660414 +0200
@@ -0,0 +1,38 @@
+/* PR tree-optimization/126503 */
+/* { dg-do run { target bitint575 } } */
+
+[[gnu::noipa]] unsigned _BitInt(400)
+foo (_BitInt(257) a)
+{
+  return (unsigned _BitInt(400)) (-a);
+}
+
+[[gnu::noipa]] unsigned _BitInt(300)
+bar (_BitInt(7) a, unsigned _BitInt(17) b)
+{
+  _BitInt(257) x = (_BitInt(257)) a + -1;
+  return ~((unsigned _BitInt(300)) x ^ (unsigned _BitInt(300)) b);
+}
+
+[[gnu::noipa]] _BitInt(129)
+baz (_BitInt(129) x)
+{
+  return x - 24;
+}
+
+[[gnu::noipa]] int
+qux (_BitInt(129) x, _BitInt(129) y)
+{
+  return x == y;
+}
+
+int
+main ()
+{
+  if (foo (-1wb) != 1uwb)
+    __builtin_abort ();
+  if (bar (1wb, 3uwb) != (unsigned _BitInt(300)) -4wb)
+    __builtin_abort ();
+  if (!qux (baz (100), 76))
+    __builtin_abort ();
+}

	Jakub
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.