My recent change to synth_mult broke bootstrap on aarch64, as gimple's
array bounds checking pass reports a (false positive) problem in some
unreachable code. This leads to -Werror stopping in stage2 of the
bootstrap.
The solution/workaround is add some more conditionals to the code that
make it clear(er) to range analysis the conditions under which the new
code is run. I believe these tests are redundant, and hopefully will be
optimized away by the compiler (if not now then at some point in the future).
This patch has been tested on an aarch64-apple-darwin24.3.0, where I was
able to reproduce the failure. Committed to mainline as obvious (to
quickly resolve the current breakage).
2026-05-08 Roger Sayle <[email protected]>
gcc/ChangeLog
* expmed.cc (synth_mult): Add more constraints to recent change
to avoid "false positive" array bounds warnings during bootstrap.
Sincere apologies for the inconvenience.
Roger
--
> -----Original Message-----
> From: Alex Coplan <[email protected]>
> Sent: 08 May 2026 11:55
> To: Jeffrey Law <[email protected]>
> Cc: Roger Sayle <[email protected]>; [email protected]
> Subject: Re: [PATCH] PR middle-end/122871: Doubleword multiplication
> improvements
>
> Hi Roger,
>
> On 03/05/2026 17:36, Jeffrey Law wrote:
> >
> >
> > On 3/28/2026 7:09 AM, Roger Sayle wrote:
> > > This patch resolves PR middle-end/122871 by improving RTL expansion
> > > of doubleword multiplications. The main change is to synth_mult
> > > adding support for the case where the constant being multiplied has
> > > BITS_PER_WORD or more trailing zeros. The shift_cost tables in
> > > expmed are only parameterized for shifts less than BITS_PER_WORD, so
> > > doubleword shifts by more than this can't use the usual code path.
> > > This patch teaches synth_mult that for scalar doubleword
> > > multiplications, a doubleword shift by more than BITS_PER_WORD
> > > typically requires two instructions; one to set the result lowpart
> > > to zero, and the other a wordmode shift to calculate the result highpart.
>
> It looks like this patch breaks bootstrap on aarch64-linux-gnu. I see builds
> failing
> in stage2 with:
>
> In file included from /home/alecop01/toolchain/src/gcc/gcc/expmed.cc:35:
> In function ‘int* shift_cost_ptr(bool, machine_mode, int)’,
> inlined from ‘int shift_cost(bool, machine_mode, int)’ at
> /home/alecop01/toolchain/src/gcc/gcc/expmed.h:415:26,
> inlined from ‘void synth_mult(algorithm*, long unsigned int, const
> mult_cost*,
> machine_mode)’ at /home/alecop01/toolchain/src/gcc/gcc/expmed.cc:2969:30:
> /home/alecop01/toolchain/src/gcc/gcc/expmed.h:398:61: error: array subscript -
> 1 is below array bounds of ‘int [64]’ [-Werror=array-bounds=]
> 398 | return &this_target_expmed->x_shift_cost[speed][midx][bits];
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
> /home/alecop01/toolchain/src/gcc/gcc/expmed.h: In function ‘void
> synth_mult(algorithm*, long unsigned int, const mult_cost*, machine_mode)’:
> /home/alecop01/toolchain/src/gcc/gcc/expmed.h:168:7: note: while referencing
> ‘target_expmed::x_shift_cost’
> 168 | int x_shift_cost[2][NUM_MODE_IPV_INT][MAX_BITS_PER_WORD];
> | ^~~~~~~~~~~~
> cc1plus: all warnings being treated as errors
> make[3]: *** [Makefile:1217: expmed.o] Error 1
> make[3]: Leaving directory '/work/builds/bstrap/gcc'
> make[2]: *** [Makefile:5182: all-stage2-gcc] Error 2
> make[2]: Leaving directory '/work/builds/bstrap'
> make[1]: *** [Makefile:27065: stage2-bubble] Error 2
> make[1]: Leaving directory '/work/builds/bstrap'
> make: *** [Makefile:1140: all] Error 2
>
> Might you be able to take a look?
>
> Thanks,
> Alex
>
> > >
> > > For the testcase given in the PR:
> > >
> > > long long ashll_fn (long long a)
> > > {
> > > long long c;
> > >
> > > c = a << 33;
> > > c += a;
> > > return c;
> > > }
> > >
> > > GCC for arm-linux-gnueabihf currently generates with -O2:
> > >
> > > ashll_fn:
> > > lsl r2, r1, #11
> > > lsl ip, r0, #11
> > > subs ip, ip, r0
> > > orr r2, r2, r0, lsr #21
> > > sbc r2, r2, r1
> > > lsl r3, ip, #11
> > > lsl r2, r2, #11
> > > adds r3, r3, r0
> > > orr r2, r2, ip, lsr #21
> > > adc r1, r1, r2
> > > lsl r2, r1, #11
> > > lsl r0, r3, #11
> > > adds r0, r3, r0
> > > orr r2, r2, r3, lsr #21
> > > adc r1, r1, r2
> > > bx lr
> > >
> > > with this patch, we instead generate:
> > >
> > > ashll_fn:
> > > add r1, r1, r0, lsl #1
> > > bx lr
> > >
> > >
> > > Additionally, this patch includes a clean-up (identified by Andrew
> > > Pinski) to prevent RTL expansion of doubleword multiplications from
> > > initially emitting multiply instructions by immediate constants 0, 1
> > > or 2. These dubious multiplications eventually get tidied up by
> > > later RTL optimization passes, but being sensible during RTL
> > > expansion both speeds up the compiler and reduces unnecessary memory
> usage.
> > You might want to consider handling -1 and perhaps -2 as well if it's
> > trivial to do so. That could be a follow-up IMHO.
> >
> > >
> > >
> > > This patch has been tested on x86_64-pc-linux-gnu with make
> > > bootstrap and make -k check, both with and without
> > > --target_board=unix{-m32} with no new failures. Ok for mainline?
> > >
> > >
> > > 2026-03-28 Roger Sayle <[email protected]>
> > >
> > > gcc/ChangeLog
> > > PR middle-end/122871
> > > * expmed.cc (synth_mult): Handle doubleword left shifts by
> > > BITS_PER_MODE bits or more, for scalar modes.
> > > * optabs.cc (expand_doubleword_mult): Avoid generating multiply
> > > instructions by immediate constants 0, 1 or 2.
> > Consider adding the example as a testcase.
> >
> > OK with or without the testcase and handling of -1, -2.
> >
> > jeff
diff --git a/gcc/expmed.cc b/gcc/expmed.cc
index a75e406376a..fe39506b6bd 100644
--- a/gcc/expmed.cc
+++ b/gcc/expmed.cc
@@ -2962,7 +2962,9 @@ synth_mult (struct algorithm *alg_out, unsigned
HOST_WIDE_INT t,
}
}
}
- else if (GET_MODE_BITSIZE (imode) == 2 * BITS_PER_WORD
+ else if (2 * BITS_PER_WORD <= HOST_BITS_PER_WIDE_INT
+ && GET_MODE_BITSIZE (imode) == 2 * BITS_PER_WORD
+ && m >= BITS_PER_WORD
&& imode == mode)
{
q = t >> m;
