On Fri, Jul 15, 2022 at 3:28 PM Roger Sayle <ro...@nextmovesoftware.com> wrote:
>
>
>
> This patch resolves PR target/106273 which is a wrong code regression
>
> caused by the recent reorganization to split doubleword operations after
>
> reload on x86. For the failing test case, the constraints on the
>
> andnti3_doubleword_bmi pattern allow reload to allocate the output and
>
> operand in overlapping but non-identical registers, i.e.
>
>
>
> (insn 45 44 66 2 (parallel [
>
> (set (reg/v:TI 5 di [orig:96 i ] [96])
>
> (and:TI (not:TI (reg:TI 39 r11 [orig:83 _2 ] [83]))
>
> (reg/v:TI 4 si [orig:100 i ] [100])))
>
> (clobber (reg:CC 17 flags))
>
> ]) "pr106273.c":13:5 562 {*andnti3_doubleword_bmi}
>
>
>
> where the output is in registers 5 and 6, and the second operand is
>
> registers 4 and 5, which then leads to the incorrect split:
>
>
>
> (insn 113 44 114 2 (parallel [
>
> (set (reg:DI 5 di [orig:96 i ] [96])
>
> (and:DI (not:DI (reg:DI 39 r11 [orig:83 _2 ] [83]))
>
> (reg:DI 4 si [orig:100 i ] [100])))
>
> (clobber (reg:CC 17 flags))
>
> ]) "pr106273.c":13:5 566 {*andndi_1}
>
>
>
> (insn 114 113 66 2 (parallel [
>
> (set (reg:DI 6 bp [ i+8 ])
>
> (and:DI (not:DI (reg:DI 40 r12 [ _2+8 ]))
>
> (reg:DI 5 di [ i+8 ])))
>
> (clobber (reg:CC 17 flags))
>
> ]) "pr106273.c":13:5 566 {*andndi_1}
>
>
>
> [Notice that reg:DI 5 is set in the first instruction, but assumed
>
> to have its original value in the second]. My first thought was
>
> that this could be fixed by swapping the order of the split instructions
>
> (which works in this case), but in the general case, it's impossible
>
> to handle (set (reg:TI x) (op (reg:TI x+1) (reg:TI x-1)). Hence for
>
> correctness this pattern needs an earlyclobber "=&r", but we can also
>
> allow cases where the output is the same as one of the operands (using
>
> constraint "0"). The other binary logic operations (AND, IOR, XOR)
>
> are unaffected as they constrain the output to match the first
>
> operand, but BMI's andn is a three-operand instruction which can
>
> lead to the overlapping cases described above.
>
>
>
> 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?
>
>
>
> 2022-07-15 Roger Sayle <ro...@nextmovesoftware.com>
>
>
>
> gcc/ChangeLog
>
> PR target/106273
>
> * config/i386/i386.md (*andn<dwi>3_doubleword_bmi): Update the
>
> constraints to reflect the output is earlyclobber, unless it is
>
> the same register (pair) as one of the operands.
>
>
>
> gcc/testsuite/ChangeLog
>
> PR target/106273
>
> * gcc.target/i386/pr106273.c: New test case.
OK with a small testcase adjustment.
Thanks,
Uros.
+int
+main (void)
If possible, please name this function "bar", the "main" function
assumes a runtime test.
+{
+ u8 x;
+ foo (5, &x);
+ if (x != 5)
+ __builtin_abort ();
+ return 0;
+}
>
>
>
>
>
> Thanks again, and sorry for the inconvenience.
>
> Roger
>
> --
>
>
