Re: [PATCH] bitintlower: Fix up handle_plus_minus [PR126503]

Richard Biener <[email protected]>
Newsgroups gmane.comp.gcc.patches
Message-ID <[email protected]>

> Am 30.07.2026 um 23:32 schrieb Jakub Jelinek <[email protected]>:
> 
> 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?

Ok

Richard 

> 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.