On 01/02/2020 08:30, Jakub Jelinek wrote:
Hi!
The following testcase ICEs on arm. The problem is that the following
peephole tries to match adjacent *arm_movsi_insn + *arm_cmpsi_insn;
the *arm_movsi_insn uses =rk, rk constraints and *arm_cmpsi_insn
uses r, IL constraints for what is matched by the peephole2. Peephole2s
work only with predicates and not constraints, and the instruction that
the peephole2 turns those two into is *movsi_compare0, which uses
=r, r constraints. As operands[1] is the same between the two insns,
sp (k constraint) is not possible for it, but as appears in the testcase,
we try to match sp = non-sp; cc = cmp (non-sp, 0) into
[ cc = cmp (non-sp, 0); sp = non-sp ] insn which isn't then recognized,
because it doesn't match the =r constraint.
I couldn't find a predicate that would do what we need here, the peephole2
uses arm_general_register_operand which matches SUBREGs as well as REGs
and requires <= 15 REGNO or pseudo register. There is also
arm_hard_general_register_operand predicate that matches only REGs and
requires <= 15 REGNO, so slightly closer to what we need, but nothing
that would require <= LAST_ARM_REGNUM && != SP_REGNUM.
The following patch fixes it just by excluding SP destination in the
peephole2 condition, bootstrapped/regtested on armv7hl-linux-gnueabi,
ok for trunk?
Yet another option would be
s/arm_general_register_operand/arm_hard_general_register_operand/g
in the peephole2 and use
"TARGET_ARM && REGNO (operands[0]) != SP_REGNUM"
as condition.
And yet another option would be to add some predicate that would be
arm_hard_general_register_operand and would require that REGNO != SP_REGNUM
and use it in this peephole2. But I'd need a suggestion how to call
that predicate, because I have no idea how arm calls core? registers
except sp.
And yet another option would be specify the constraints to
movsi_compare0 correctly...
The restriction on using SP applies only to Thumb code. In Arm state SP
can be used just fine (as was the case when this pattern was originally
written and SP was permitted in constraint 'r'. When thumb2 support was
added we split SP into a separate class, but not all patterns were fully
updated for this.
Since the peephole applies only to Arm state (TARGET_ARM), there's no
need to change that, but the pattern it hits needs some additional
variants so that it will match.
I'll commit a patch to do that shortly, but feel free to commit your
testcase.
Thanks for looking into this though.
R.
(sorry for the delay replying, was in meetings nearly all last week).
2020-02-01 Jakub Jelinek <ja...@redhat.com>
PR target/91913
* config/arm/arm.md (movsi + cmpsi -> movsi_compare0 peephole2):
Punt if destination is sp register.
* gfortran.dg/pr91913.f90: New test.
--- gcc/config/arm/arm.md.jj 2020-01-30 12:57:52.062102735 +0100
+++ gcc/config/arm/arm.md 2020-01-31 15:55:47.199979148 +0100
@@ -11227,7 +11227,7 @@ (define_peephole2
(match_operand:SI 1 "arm_general_register_operand" ""))
(set (reg:CC CC_REGNUM)
(compare:CC (match_dup 1) (const_int 0)))]
- "TARGET_ARM"
+ "TARGET_ARM && REG_P (operands[0]) && REGNO (operands[0]) != SP_REGNUM"
[(parallel [(set (reg:CC CC_REGNUM) (compare:CC (match_dup 1) (const_int
0)))
(set (match_dup 0) (match_dup 1))])]
""
--- gcc/testsuite/gfortran.dg/pr91913.f90.jj 2020-01-31 15:54:06.839475636
+0100
+++ gcc/testsuite/gfortran.dg/pr91913.f90 2020-01-31 15:53:52.265692947
+0100
@@ -0,0 +1,5 @@
+! PR target/91913
+! { dg-do compile }
+! { dg-options "-std=legacy -Ofast --param max-cse-insns=0 -fno-schedule-insns
-fsanitize=null" }
+
+include 'string_ctor_1.f90'
Jakub
