Hi James,
On 22/02/19 00:09, James Greenhalgh wrote:
> On Mon, Feb 18, 2019 at 08:40:12AM -0600, Matthew Malcomson wrote:
>>
>> Additionally, this patch contains two tidy-ups (happy to remove them or put
>> in
>> a separate patch if people want):
>
> Generally, yes I prefer that.
>
I've removed the tidy ups and retested -- modified patch attached.
>>
>> OK for trunk?
>
> This patch is fine for GCC 10, so not on trunk yet please unless there is
> a corectness reason for the change.
>
The patch is a correctness change to fix the P1 regression that occurs
when the stack pointer was passed to the define_insn from the
define_peephole2.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89324
Ok for trunk?
Regards,
Matthew
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index b7f6fe0f1354f7aa19076a946ed2c633b9b9b8da..b7cd9fc787b8526cc7b11b9430fbfcd73103328a 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -1985,7 +1985,7 @@ (define_expand "uaddvti4"
(define_insn "add<mode>3_compare0"
[(set (reg:CC_NZ CC_REGNUM)
(compare:CC_NZ
- (plus:GPI (match_operand:GPI 1 "register_operand" "%r,r,r")
+ (plus:GPI (match_operand:GPI 1 "register_operand" "%rk,rk,rk")
(match_operand:GPI 2 "aarch64_plus_operand" "r,I,J"))
(const_int 0)))
(set (match_operand:GPI 0 "register_operand" "=r,r,r")
@@ -2002,7 +2002,7 @@ (define_insn "add<mode>3_compare0"
(define_insn "*addsi3_compare0_uxtw"
[(set (reg:CC_NZ CC_REGNUM)
(compare:CC_NZ
- (plus:SI (match_operand:SI 1 "register_operand" "%r,r,r")
+ (plus:SI (match_operand:SI 1 "register_operand" "%rk,rk,rk")
(match_operand:SI 2 "aarch64_plus_operand" "r,I,J"))
(const_int 0)))
(set (match_operand:DI 0 "register_operand" "=r,r,r")
@@ -2034,7 +2034,7 @@ (define_insn "add<mode>3_compareC"
[(set (reg:CC_C CC_REGNUM)
(compare:CC_C
(plus:GPI
- (match_operand:GPI 1 "register_operand" "r,r,r")
+ (match_operand:GPI 1 "register_operand" "rk,rk,rk")
(match_operand:GPI 2 "aarch64_plus_operand" "r,I,J"))
(match_dup 1)))
(set (match_operand:GPI 0 "register_operand" "=r,r,r")
@@ -2081,7 +2081,7 @@ (define_insn "add<mode>3_compareV_imm"
(compare:CC_V
(plus:<DWI>
(sign_extend:<DWI>
- (match_operand:GPI 1 "register_operand" "r,r"))
+ (match_operand:GPI 1 "register_operand" "rk,rk"))
(match_operand:GPI 2 "aarch64_plus_immediate" "I,J"))
(sign_extend:<DWI>
(plus:GPI (match_dup 1) (match_dup 2)))))
@@ -2098,7 +2098,7 @@ (define_insn "add<mode>3_compareV"
[(set (reg:CC_V CC_REGNUM)
(compare:CC_V
(plus:<DWI>
- (sign_extend:<DWI> (match_operand:GPI 1 "register_operand" "r"))
+ (sign_extend:<DWI> (match_operand:GPI 1 "register_operand" "rk"))
(sign_extend:<DWI> (match_operand:GPI 2 "register_operand" "r")))
(sign_extend:<DWI> (plus:GPI (match_dup 1) (match_dup 2)))))
(set (match_operand:GPI 0 "register_operand" "=r")
@@ -2177,7 +2177,7 @@ (define_insn "*adds_<optab><ALLX:mode>_<GPI:mode>"
(compare:CC_NZ
(plus:GPI
(ANY_EXTEND:GPI (match_operand:ALLX 1 "register_operand" "r"))
- (match_operand:GPI 2 "register_operand" "r"))
+ (match_operand:GPI 2 "register_operand" "rk"))
(const_int 0)))
(set (match_operand:GPI 0 "register_operand" "=r")
(plus:GPI (ANY_EXTEND:GPI (match_dup 1)) (match_dup 2)))]
@@ -2189,7 +2189,7 @@ (define_insn "*adds_<optab><ALLX:mode>_<GPI:mode>"
(define_insn "*subs_<optab><ALLX:mode>_<GPI:mode>"
[(set (reg:CC_NZ CC_REGNUM)
(compare:CC_NZ
- (minus:GPI (match_operand:GPI 1 "register_operand" "r")
+ (minus:GPI (match_operand:GPI 1 "register_operand" "rk")
(ANY_EXTEND:GPI
(match_operand:ALLX 2 "register_operand" "r")))
(const_int 0)))
@@ -2207,7 +2207,7 @@ (define_insn "*adds_<optab><ALLX:mode>_shift_<GPI:mode>"
(ANY_EXTEND:GPI
(match_operand:ALLX 1 "register_operand" "r"))
(match_operand 2 "aarch64_imm3" "Ui3"))
- (match_operand:GPI 3 "register_operand" "r"))
+ (match_operand:GPI 3 "register_operand" "rk"))
(const_int 0)))
(set (match_operand:GPI 0 "register_operand" "=rk")
(plus:GPI (ashift:GPI (ANY_EXTEND:GPI (match_dup 1))
@@ -2221,7 +2221,7 @@ (define_insn "*adds_<optab><ALLX:mode>_shift_<GPI:mode>"
(define_insn "*subs_<optab><ALLX:mode>_shift_<GPI:mode>"
[(set (reg:CC_NZ CC_REGNUM)
(compare:CC_NZ
- (minus:GPI (match_operand:GPI 1 "register_operand" "r")
+ (minus:GPI (match_operand:GPI 1 "register_operand" "rk")
(ashift:GPI
(ANY_EXTEND:GPI
(match_operand:ALLX 2 "register_operand" "r"))
@@ -2244,7 +2244,7 @@ (define_insn "*adds_<optab><mode>_multp2"
(match_operand 2 "aarch64_pwr_imm3" "Up3"))
(match_operand 3 "const_int_operand" "n")
(const_int 0))
- (match_operand:GPI 4 "register_operand" "r"))
+ (match_operand:GPI 4 "register_operand" "rk"))
(const_int 0)))
(set (match_operand:GPI 0 "register_operand" "=r")
(plus:GPI (ANY_EXTRACT:GPI (mult:GPI (match_dup 1) (match_dup 2))
@@ -2259,7 +2259,7 @@ (define_insn "*adds_<optab><mode>_multp2"
(define_insn "*subs_<optab><mode>_multp2"
[(set (reg:CC_NZ CC_REGNUM)
(compare:CC_NZ
- (minus:GPI (match_operand:GPI 4 "register_operand" "r")
+ (minus:GPI (match_operand:GPI 4 "register_operand" "rk")
(ANY_EXTRACT:GPI
(mult:GPI (match_operand:GPI 1 "register_operand" "r")
(match_operand 2 "aarch64_pwr_imm3" "Up3"))
@@ -2923,7 +2923,7 @@ (define_insn "negvdi_carryinV"
(define_insn "*sub<mode>3_compare0"
[(set (reg:CC_NZ CC_REGNUM)
- (compare:CC_NZ (minus:GPI (match_operand:GPI 1 "register_operand" "r")
+ (compare:CC_NZ (minus:GPI (match_operand:GPI 1 "register_operand" "rk")
(match_operand:GPI 2 "register_operand" "r"))
(const_int 0)))
(set (match_operand:GPI 0 "register_operand" "=r")
@@ -2936,7 +2936,7 @@ (define_insn "*sub<mode>3_compare0"
;; zero_extend version of above
(define_insn "*subsi3_compare0_uxtw"
[(set (reg:CC_NZ CC_REGNUM)
- (compare:CC_NZ (minus:SI (match_operand:SI 1 "register_operand" "r")
+ (compare:CC_NZ (minus:SI (match_operand:SI 1 "register_operand" "rk")
(match_operand:SI 2 "register_operand" "r"))
(const_int 0)))
(set (match_operand:DI 0 "register_operand" "=r")
@@ -2949,7 +2949,7 @@ (define_insn "*subsi3_compare0_uxtw"
(define_insn "sub<mode>3_compare1_imm"
[(set (reg:CC CC_REGNUM)
(compare:CC
- (match_operand:GPI 1 "aarch64_reg_or_zero" "rZ,rZ")
+ (match_operand:GPI 1 "aarch64_reg_or_zero" "rkZ,rkZ")
(match_operand:GPI 2 "aarch64_plus_immediate" "I,J")))
(set (match_operand:GPI 0 "register_operand" "=r,r")
(plus:GPI
@@ -2965,7 +2965,7 @@ (define_insn "sub<mode>3_compare1_imm"
(define_insn "sub<mode>3_compare1"
[(set (reg:CC CC_REGNUM)
(compare:CC
- (match_operand:GPI 1 "aarch64_reg_or_zero" "rZ")
+ (match_operand:GPI 1 "aarch64_reg_or_zero" "rkZ")
(match_operand:GPI 2 "aarch64_reg_or_zero" "rZ")))
(set (match_operand:GPI 0 "register_operand" "=r")
(minus:GPI (match_dup 1) (match_dup 2)))]
@@ -2975,7 +2975,7 @@ (define_insn "sub<mode>3_compare1"
)
(define_peephole2
- [(set (match_operand:GPI 0 "register_operand")
+ [(set (match_operand:GPI 0 "aarch64_general_reg")
(minus:GPI (match_operand:GPI 1 "aarch64_reg_or_zero")
(match_operand:GPI 2 "aarch64_reg_or_zero")))
(set (reg:CC CC_REGNUM)
@@ -3000,7 +3000,7 @@ (define_peephole2
(compare:CC
(match_operand:GPI 1 "aarch64_reg_or_zero")
(match_operand:GPI 2 "aarch64_reg_or_zero")))
- (set (match_operand:GPI 0 "register_operand")
+ (set (match_operand:GPI 0 "aarch64_general_reg")
(minus:GPI (match_dup 1)
(match_dup 2)))]
""
@@ -3013,7 +3013,7 @@ (define_peephole2
)
(define_peephole2
- [(set (match_operand:GPI 0 "register_operand")
+ [(set (match_operand:GPI 0 "aarch64_general_reg")
(plus:GPI (match_operand:GPI 1 "register_operand")
(match_operand:GPI 2 "aarch64_plus_immediate")))
(set (reg:CC CC_REGNUM)
@@ -3038,7 +3038,7 @@ (define_peephole2
(compare:CC
(match_operand:GPI 1 "register_operand")
(match_operand:GPI 3 "const_int_operand")))
- (set (match_operand:GPI 0 "register_operand")
+ (set (match_operand:GPI 0 "aarch64_general_reg")
(plus:GPI (match_dup 1)
(match_operand:GPI 2 "aarch64_plus_immediate")))]
"INTVAL (operands[3]) == -INTVAL (operands[2])"
diff --git a/gcc/config/aarch64/predicates.md b/gcc/config/aarch64/predicates.md
index b8e6d232ff6237a58addda1ec0e953a1054dc616..8e1b784217b0367dc647a87024e14e36de781008 100644
--- a/gcc/config/aarch64/predicates.md
+++ b/gcc/config/aarch64/predicates.md
@@ -30,6 +30,10 @@ (define_predicate "aarch64_call_insn_operand"
(ior (match_code "symbol_ref")
(match_operand 0 "register_operand")))
+(define_predicate "aarch64_general_reg"
+ (and (match_operand 0 "register_operand")
+ (match_test "REGNO_REG_CLASS (REGNO (op)) == GENERAL_REGS")))
+
;; Return true if OP a (const_int 0) operand.
(define_predicate "const0_operand"
(and (match_code "const_int")
diff --git a/gcc/testsuite/gcc.dg/rtl/aarch64/subs_adds_sp.c b/gcc/testsuite/gcc.dg/rtl/aarch64/subs_adds_sp.c
new file mode 100644
index 0000000000000000000000000000000000000000..f537771a271e1d33df29561a4b687d621e090bab
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/rtl/aarch64/subs_adds_sp.c
@@ -0,0 +1,153 @@
+/* { dg-do compile { target aarch64-*-* } } */
+/* { dg-options "-O2" } */
+/*
+ Tests are:
+ Patterns allow subs/adds with a stack pointer source.
+ define_peephole2's don't generate patterns for subs/adds with a stack
+ pointer destination.
+ */
+
+/* These functions used to ICE due to using the stack pointer as a source
+ register. */
+
+int __RTL (startwith ("final"))
+adds ()
+{
+(function "adds"
+ (insn-chain
+ (block 2
+ (edge-from entry (flags "FALLTHRU"))
+ (cnote 3 [bb 2] NOTE_INSN_BASIC_BLOCK)
+ (cinsn 101 (parallel [
+ (set (reg:CC cc)
+ (compare:CC (reg/f:DI sp)
+ (const_int -3)))
+ (set (reg/f:DI x19)
+ (plus:DI (reg/f:DI sp)
+ (const_int 3)))
+ ]))
+ ;; Extra insn to avoid the above being deleted by DCE.
+ (cinsn 10 (use (reg/i:SI x19)))
+ (cinsn 11 (use (reg/i:SI sp)))
+ (edge-to exit (flags "FALLTHRU"))
+ ) ;; block 2
+ ) ;; insn-chain
+) ;; function "main"
+}
+
+int __RTL (startwith ("final"))
+subs ()
+{
+(function "subs"
+ (insn-chain
+ (block 2
+ (edge-from entry (flags "FALLTHRU"))
+ (cnote 3 [bb 2] NOTE_INSN_BASIC_BLOCK)
+ (cinsn 101 (parallel [
+ (set (reg:CC cc)
+ (compare:CC (reg/f:DI sp)
+ (const_int 3)))
+ (set (reg/f:DI x19)
+ (plus:DI (reg/f:DI sp)
+ (const_int -3)))
+ ]))
+ ;; Extra insn to avoid the above being deleted by DCE.
+ (cinsn 10 (use (reg/i:SI x19)))
+ (cinsn 11 (use (reg/i:SI sp)))
+ (edge-to exit (flags "FALLTHRU"))
+ ) ;; block 2
+ ) ;; insn-chain
+) ;; function "main"
+}
+
+/* These functions used to trigger peepholes generating invalid SUBS patterns
+ that used the stack pointer for the destination register. */
+
+int __RTL (startwith ("peephole2")) sub3_compare1_peephole_1 ()
+{
+(function "sub3_compare1_peephole_1"
+ (insn-chain
+ (block 2
+ (edge-from entry (flags "FALLTHRU"))
+ (cnote 3 [bb 2] NOTE_INSN_BASIC_BLOCK)
+ (cinsn 89 (set (reg:DI sp)
+ (minus:DI (reg:DI x2) (reg:DI x5))))
+ (cinsn 90 (set (reg:CC cc)
+ (compare:CC (reg:DI x2) (reg:DI x5))))
+ ;; Extra insn to avoid the above being deleted by DCE.
+ (cinsn 12 (use (reg/i:DI cc)))
+ (cinsn 11 (use (reg/i:DI sp)))
+ (edge-to exit (flags "FALLTHRU"))
+ ) ;; block 2
+ ) ;; insn-chain
+) ;; function "main"
+}
+
+int __RTL (startwith ("peephole2")) sub3_compare1_peephole_2 ()
+{
+(function "sub3_compare1_peephole_2"
+ (insn-chain
+ (block 2
+ (edge-from entry (flags "FALLTHRU"))
+ (cnote 3 [bb 2] NOTE_INSN_BASIC_BLOCK)
+ (cinsn 90 (set (reg:CC cc)
+ (compare:CC (reg:DI x2) (reg:DI x5))))
+ (cinsn 89 (set (reg:DI sp)
+ (minus:DI (reg:DI x2) (reg:DI x5))))
+ ;; Extra insn to avoid the above being deleted by DCE.
+ (cinsn 12 (use (reg/i:DI cc)))
+ (cinsn 11 (use (reg/i:DI sp)))
+ (edge-to exit (flags "FALLTHRU"))
+ ) ;; block 2
+ ) ;; insn-chain
+) ;; function "main"
+}
+
+int __RTL (startwith ("peephole2")) sub3_compare1_imm_peephole_1 ()
+{
+(function "sub3_compare1_imm_peephole_1"
+ (insn-chain
+ (block 2
+ (edge-from entry (flags "FALLTHRU"))
+ (cnote 3 [bb 2] NOTE_INSN_BASIC_BLOCK)
+ (cinsn 90 (set (reg:CC cc)
+ (compare:CC (reg:DI x2) (reg:DI x5))))
+ (cinsn 89 (set (reg:DI sp)
+ (minus:DI (reg:DI x2) (reg:DI x5))))
+ ;; Extra insn to avoid the above being deleted by DCE.
+ (cinsn 12 (use (reg/i:DI cc)))
+ (cinsn 11 (use (reg/i:DI sp)))
+ (edge-to exit (flags "FALLTHRU"))
+ ) ;; block 2
+ ) ;; insn-chain
+) ;; function "main"
+}
+
+int __RTL (startwith ("peephole2")) sub3_compare1_imm_peephole_2 ()
+{
+(function "sub3_compare1_imm_peephole_1"
+ (insn-chain
+ (block 2
+ (edge-from entry (flags "FALLTHRU"))
+ (cnote 3 [bb 2] NOTE_INSN_BASIC_BLOCK)
+ (cinsn 89 (set (reg:DI sp)
+ (minus:DI (reg:DI x2) (reg:DI x5))))
+ (cinsn 90 (set (reg:CC cc)
+ (compare:CC (reg:DI x2) (reg:DI x5))))
+ ;; Extra insn to avoid the above being deleted by DCE.
+ (cinsn 12 (use (reg/i:DI cc)))
+ (cinsn 11 (use (reg/i:DI sp)))
+ (edge-to exit (flags "FALLTHRU"))
+ ) ;; block 2
+ ) ;; insn-chain
+) ;; function "main"
+}
+
+/* Verify that the adds and subs functions generated their respective
+ instructions, and that none of the other functions generated either since
+ they are setting the stack pointer. */
+/* { dg-final { scan-assembler-times {adds\tx[0-9]+, sp} 1 } } */
+/* { dg-final { scan-assembler-not {adds\tsp} } } */
+/* { dg-final { scan-assembler-times {subs\tx[0-9]+, sp} 1 } } */
+/* { dg-final { scan-assembler-not {subs\tsp} } } */
+
diff --git a/gcc/testsuite/gfortran.fortran-torture/compile/pr89324.f90 b/gcc/testsuite/gfortran.fortran-torture/compile/pr89324.f90
new file mode 100644
index 0000000000000000000000000000000000000000..014b655d5edae26689ff57eda808e2a0f6146d5b
--- /dev/null
+++ b/gcc/testsuite/gfortran.fortran-torture/compile/pr89324.f90
@@ -0,0 +1,15 @@
+module a
+contains
+ pure function myotherlen()
+ myotherlen = 99
+ end
+ subroutine b
+ characterx
+ block
+ character(myotherlen()) c
+ c = "abc"
+ x = c
+ end block
+
+ end
+end
