Vlad's recent IRA change twiddled register assignments on the visium
port causing a regression on the bit_shift.c test.

Essentially the change in register assignments have resulted in changing
where register->register moves are occurring.  I have verified we have
the same number of insns on each of the paths through the testcase at
the split2 pass.

Prior to Vlad's change we'd have the following at the start of the function:

> (insn 56 7 3 2 (parallel [
>             (set (reg/v:SI 10 r10 [orig:62 x ] [62])
>                 (reg:SI 1 r1 [66]))
>             (clobber (reg:CC 50 flags))
>         ]) "j.c":5:1 19 {*movsi_insn_flags}
>      (nil))
> (note 3 56 57 2 NOTE_INSN_FUNCTION_BEG)
> (insn 57 3 58 2 (set (reg:CC 50 flags)
>         (compare:CC (reg/v:SI 10 r10 [orig:62 x ] [62])
>             (const_int 0 [0]))) "j.c":8:3 210 {*cmpsi}
>      (nil))
> (jump_insn 58 57 28 2 (set (pc)
>         (if_then_else (eq (reg:CC 50 flags)
>                 (const_int 0 [0]))
>             (label_ref:SI 33)
>             (pc))) "j.c":8:3 247 {*normal_branch}
>      (int_list:REG_BR_PROB 59055804 (nil))

That sequence is advantageous because the reg->reg copy at insn 56 can
be used to eliminate the compare at insn 57.

After Vlad's changes, pseudos 66 and 62 get the same hard reg, so we
have something like this:

> (note 3 7 57 2 NOTE_INSN_FUNCTION_BEG)
> (insn 57 3 58 2 (set (reg:CC 50 flags)
>         (compare:CC (reg/v:SI 1 r1 [orig:62 x ] [62])
>             (const_int 0 [0]))) "k.c":8:3 210 {*cmpsi}
>      (nil))
> (jump_insn 58 57 28 2 (set (pc)
>         (if_then_else (eq (reg:CC 50 flags)
>                 (const_int 0 [0]))
>             (label_ref:SI 33)
>             (pc))) "k.c":8:3 247 {*normal_branch}
>      (int_list:REG_BR_PROB 59055804 (nil))

And of course we can't eliminate the compare because there's no reg->reg
copy to set the condition codes before the compare. And the test fails
(because it's designed to test for elimination of the compares).
There's a copy into the return register at the end of the function after
Vlad's change -- we have the same number of each class of insn
post-reload, both in terms of static counts and in terms of dynamic
counts on each path.

The final assembly is better prior to Vlad's change (because of the
comparison elimination).  So while it's a regression, I don't think it
should rise to the point of reverting Vlad's change or requiring Vlad to
fix it via some other mechanism (we might look at improve compare-elim
for gcc-10).

The test is still valid, so I don't want to remove it.  So I'm just
xfailing it.


Jeff

        * gcc.target/visium/bit_shift.c: xfail.

index bf373428de8..dbda64dd5f1 100644
--- a/gcc/testsuite/gcc.target/visium/bit_shift.c
+++ b/gcc/testsuite/gcc.target/visium/bit_shift.c
@@ -10,4 +10,4 @@ int bit_shift (long int x)
   return n;
 }
 
-/* { dg-final { scan-assembler-not "cmp" } } */
+/* { dg-final { scan-assembler-not "cmp" { xfail *-*-* } } } */

Reply via email to