]> git.ipfire.org Git - thirdparty/gcc.git/commit
testsuite: Tweak mem-shift-canonical.c
authorRichard Sandiford <richard.sandiford@arm.com>
Fri, 9 Apr 2021 12:43:16 +0000 (13:43 +0100)
committerRichard Sandiford <richard.sandiford@arm.com>
Fri, 9 Apr 2021 12:43:16 +0000 (13:43 +0100)
commit7e45c45d9ccf9d0af2ce29fc5016506ba30676c0
treea0d4e71a24b5ee19301dbfd187a619bc3e26bf37
parent9a54db29387c4e936ab99499bf4d3e1649e92800
testsuite: Tweak mem-shift-canonical.c

mem-shift-canonical.c started failing after the fix for PR97684.
We used to generate things like:

        add     x7, x1, x5, lsl 2       // 54   [c=4 l=4]  *add_lsl_di
        ld1     {v0.s}[1], [x7] // 18   [c=4 l=4]  aarch64_simd_vec_setv4si/2

where the add_lsl_di was generated by LRA.  After PR97684 we generate:

        ldr     s1, [x1, x5, lsl 2]     // 17   [c=4 l=4]  *zero_extendsidi2_aarch64/3
        ins     v0.s[1], v1.s[0]        // 18   [c=4 l=4]  aarch64_simd_vec_setv4si/0

Which one is better is an interesting question.  However, it was really
only a fluke that we generated the original code.  The pseudo that
becomes s1 in the new code above has a REG_EQUIV note:

(insn 17 16 18 3 (set (reg:SI 111 [ MEM[(int *)b_25(D) + ivtmp.9_30 * 4] ])
        (mem:SI (plus:DI (mult:DI (reg:DI 101 [ ivtmp.9 ])
                    (const_int 4 [0x4]))
                (reg/v/f:DI 105 [ b ])) [2 MEM[(int *)b_25(D) + ivtmp.9_30 * 4]+0 S4 A32])) "gcc/testsuite/gcc.target/aarch64/mem-shift-canonical.c":21:18 52 {*movsi_aarch64}
     (expr_list:REG_EQUIV (mem:SI (plus:DI (mult:DI (reg:DI 101 [ ivtmp.9 ])
                    (const_int 4 [0x4]))
                (reg/v/f:DI 105 [ b ])) [2 MEM[(int *)b_25(D) + ivtmp.9_30 * 4]+0 S4 A32])
        (nil)))
(insn 18 17 19 3 (set (reg:V4SI 109 [ MEM[(int *)a_23(D) + ivtmp.9_30 * 4] ])
        (vec_merge:V4SI (vec_duplicate:V4SI (reg:SI 111 [ MEM[(int *)b_25(D) + ivtmp.9_30 * 4] ]))
            (subreg:V4SI (reg:SI 110 [ MEM[(int *)a_23(D) + ivtmp.9_30 * 4] ]) 0)
            (const_int 2 [0x2]))) "gcc/testsuite/gcc.target/aarch64/mem-shift-canonical.c":21:18 1683 {aarch64_simd_vec_setv4si}
     (expr_list:REG_DEAD (reg:SI 111 [ MEM[(int *)b_25(D) + ivtmp.9_30 * 4] ])
        (expr_list:REG_DEAD (reg:SI 110 [ MEM[(int *)a_23(D) + ivtmp.9_30 * 4] ])
            (nil))))

Before the PR, IRA didn't allocate a register to r111 and so LRA
rematerialised the REG_EQUIV note inside insn 18, leading to the
reload.  Now IRA allocates a register instead.

So I think this is working as expected, in the sense that IRA is now
doing what the backend asked it to do.  If the backend prefers the first
version (and it might not), it needs to do more than it's currently
doing to promote the use of lane loads.  E.g. it should probably have a
combine define_split that splits the combination of insn 17 and insn 18
into an ADD + an LD1.

I think for now the best thing is to use a different approach to
triggering the original bug.  The asm in the new test ICEs with the
r11-2903 LRA patch reverted and passes with it applied.

gcc/testsuite/
* gcc.target/aarch64/mem-shift-canonical.c: Use an asm instead
of relying on vectorisation.
gcc/testsuite/gcc.target/aarch64/mem-shift-canonical.c