On Tue, 28 Jan 2025, Jakub Jelinek wrote:
> Hi!
>
> The following testcase is miscompiled at -Os on x86_64-linux.
> The problem is during make_compound_operation of
> (ashiftrt:SI (ashift:SI (mult:SI (reg:SI 107 [ a_5 ])
> (const_int 3 [0x3]))
> (const_int 31 [0x1f]))
> (const_int 31 [0x1f]))
> where it incorrectly returns
> (mult:SI (sign_extract:SI (reg:SI 107 [ a_5 ])
> (const_int 2 [0x2])
> (const_int 0 [0]))
> (const_int 3 [0x3]))
> which isn't obviously true, the former returns either 0 or -1 depending
> on the least significant bit of the multiplication,
> the latter returns either 0 or -3 depending on the second least significant
> bit of the multiplication argument.
>
> The bug has been introduced in PR96998 r11-4563, which added handling of x
> * (2^N) similar to x << N. In the above case, pos is 0 and len is 1,
> sign extracting a single least significant bit of the multiplication.
> As 3 is not a power of 2, shift_amt is -1.
> But IN_RANGE (-1, 1, 1 - 1) is still true, because the basic requirement of
> IN_RANGE that LOWER is not greater than UPPER is violated.
> The intention of using 1 as LOWER is to avoid matching multiplication by 1,
> that really shouldn't appear in the IL. But to avoid violating IN_RANGE
> requirement, we need to verify that len is at least 2.
>
> I've added this len > 1 check to the inner if rather than outer because I
> think for GCC 16 we should add a further optimization.
> In the particular case of 1 least significant bit sign extraction from
> multiplication by 3, we could actually say it is equivalent to
> (sign_extract:SI (reg:SI 107 [ a_5 ])
> (const_int 1 [0x2])
> (const_int 0 [0]))
> That is because 3 is an odd number and multiplication by 2 will yield the
> least significant bit 0 (we are sign extracting just one) and so the
> multiplication doesn't change anything on the outcome.
> More generally, even for larger len, multiplication by C which is
> (1 << X) + 1 where X is >= len should be optimizable just to extraction
> of the multiplicand's least significant len bits.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Thanks,
Richard.
> 2025-01-28 Jakub Jelinek <ja...@redhat.com>
>
> PR rtl-optimization/118638
> * combine.cc (make_extraction): Only optimize (mult x 2^n) if len is
> larger than 1.
>
> * gcc.c-torture/execute/pr118638.c: New test.
>
> --- gcc/combine.cc.jj 2025-01-27 16:38:47.000000000 +0100
> +++ gcc/combine.cc 2025-01-27 18:55:22.082925556 +0100
> @@ -7629,7 +7629,7 @@ make_extraction (machine_mode mode, rtx
> least significant (LEN - C) bits of X, giving an rtx
> whose mode is MODE, then multiply it by 2^C. */
> const HOST_WIDE_INT shift_amt = exact_log2 (INTVAL (XEXP (inner, 1)));
> - if (IN_RANGE (shift_amt, 1, len - 1))
> + if (len > 1 && IN_RANGE (shift_amt, 1, len - 1))
> {
> new_rtx = make_extraction (mode, XEXP (inner, 0),
> 0, 0, len - shift_amt,
> --- gcc/testsuite/gcc.c-torture/execute/pr118638.c.jj 2025-01-27
> 18:56:30.246986900 +0100
> +++ gcc/testsuite/gcc.c-torture/execute/pr118638.c 2025-01-27
> 18:56:13.660215308 +0100
> @@ -0,0 +1,20 @@
> +/* PR rtl-optimization/118638 */
> +
> +__attribute__((noipa)) int
> +foo (int x)
> +{
> + int a = x != -3, b, c;
> + a *= 3;
> + b = 2 * x - 9;
> + a = a + b;
> + a = ~a;
> + c = a & 1;
> + return -c;
> +}
> +
> +int
> +main ()
> +{
> + if (foo (0) != -1)
> + __builtin_abort ();
> +}
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
