The use of the arc specific predicate load_update_operand is broken, this commit fixes the error, and in the process removes the need for load_update_operand altogether.
Currently load_update_operand is used with match_operator, the load_update_operand checks that the operand is a MEM operand, with an operand that is a plus, the plus in turn has operands that are a register and an immediate. However, the match_operator already checks the structure of the rtl tree, only in this case a different rtl pattern is checked for, in this case the operand must have two child operands, one a register operand and one an immediate operand. The mistake here is that the plus part of the rtl tree has been missed from the define_insn rtl pattern. The consequence of this mistake is that a MEM operand will match the load_update_operand predicate, then the second operand of the MEM insn will then be passed to the nonmemory_operand predicate, which assumes it will be passed an rtl_insn. However, the second operand of a MEM insn is the alias set for the address, not an rtl_insn. When fixing the rtl pattern within the define_insn it becomes obvious that all of the checks currently contained within the load_update_operand predicate are now contains within the rtl pattern, if the use of load_update_operand is replaced with the memory_operand predicate. I added a new test that exposes the issue that originally highlighted this bug for me. Having fixed this, I am now seeing some (but not all) of these instructions used within the wider GCC test suite. It would be great to add more tests targeting the specific instructions that are not covered in the wider GCC test suite, but so far I've been unable to come up with any usable tests. If anyone has advice on how to trigger the generation of specific instructions that would be great. gcc/ChangeLog: * config/arc/arc.md (*loadqi_update): Use 'memory_operand' and fix RTL pattern to include the plus. (*load_zeroextendqisi_update): Likewise. (*load_signextendqisi_update): Likewise. (*loadhi_update): Likewise. (*load_zeroextendhisi_update): Likewise. (*load_signextendhisi_update): Likewise. (*loadsi_update): Likewise. (*loadsf_update): Likewise. * config/arc/predicates.md (load_update_operand): Delete. gcc/testsuite/ChangeLog: * gcc.target/arc/load-update.c: New file. --- gcc/ChangeLog | 13 ++++++++ gcc/config/arc/arc.md | 48 +++++++++++++++--------------- gcc/config/arc/predicates.md | 18 ----------- gcc/testsuite/ChangeLog | 4 +++ gcc/testsuite/gcc.target/arc/load-update.c | 20 +++++++++++++ 5 files changed, 61 insertions(+), 42 deletions(-) create mode 100644 gcc/testsuite/gcc.target/arc/load-update.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 0a77807..705d4e9 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,16 @@ +2015-12-09 Andrew Burgess <andrew.burg...@embecosm.com> + + * config/arc/arc.md (*loadqi_update): Use 'memory_operand' and fix + RTL pattern to include the plus. + (*load_zeroextendqisi_update): Likewise. + (*load_signextendqisi_update): Likewise. + (*loadhi_update): Likewise. + (*load_zeroextendhisi_update): Likewise. + (*load_signextendhisi_update): Likewise. + (*loadsi_update): Likewise. + (*loadsf_update): Likewise. + * config/arc/predicates.md (load_update_operand): Delete. + 2015-12-10 Jeff Law <l...@redhat.com> PR tree-optimization/68619 diff --git a/gcc/config/arc/arc.md b/gcc/config/arc/arc.md index ac181a9..ef82007 100644 --- a/gcc/config/arc/arc.md +++ b/gcc/config/arc/arc.md @@ -1114,9 +1114,9 @@ ;; Note: loadqi_update has no 16-bit variant (define_insn "*loadqi_update" [(set (match_operand:QI 3 "dest_reg_operand" "=r,r") - (match_operator:QI 4 "load_update_operand" - [(match_operand:SI 1 "register_operand" "0,0") - (match_operand:SI 2 "nonmemory_operand" "rI,Cal")])) + (match_operator:QI 4 "memory_operand" + [(plus:SI (match_operand:SI 1 "register_operand" "0,0") + (match_operand:SI 2 "nonmemory_operand" "rI,Cal"))])) (set (match_operand:SI 0 "dest_reg_operand" "=r,r") (plus:SI (match_dup 1) (match_dup 2)))] "" @@ -1126,9 +1126,9 @@ (define_insn "*load_zeroextendqisi_update" [(set (match_operand:SI 3 "dest_reg_operand" "=r,r") - (zero_extend:SI (match_operator:QI 4 "load_update_operand" - [(match_operand:SI 1 "register_operand" "0,0") - (match_operand:SI 2 "nonmemory_operand" "rI,Cal")]))) + (zero_extend:SI (match_operator:QI 4 "memory_operand" + [(plus:SI (match_operand:SI 1 "register_operand" "0,0") + (match_operand:SI 2 "nonmemory_operand" "rI,Cal"))]))) (set (match_operand:SI 0 "dest_reg_operand" "=r,r") (plus:SI (match_dup 1) (match_dup 2)))] "" @@ -1138,9 +1138,9 @@ (define_insn "*load_signextendqisi_update" [(set (match_operand:SI 3 "dest_reg_operand" "=r,r") - (sign_extend:SI (match_operator:QI 4 "load_update_operand" - [(match_operand:SI 1 "register_operand" "0,0") - (match_operand:SI 2 "nonmemory_operand" "rI,Cal")]))) + (sign_extend:SI (match_operator:QI 4 "memory_operand" + [(plus:SI (match_operand:SI 1 "register_operand" "0,0") + (match_operand:SI 2 "nonmemory_operand" "rI,Cal"))]))) (set (match_operand:SI 0 "dest_reg_operand" "=r,r") (plus:SI (match_dup 1) (match_dup 2)))] "" @@ -1164,9 +1164,9 @@ ;; Note: no 16-bit variant for this pattern (define_insn "*loadhi_update" [(set (match_operand:HI 3 "dest_reg_operand" "=r,r") - (match_operator:HI 4 "load_update_operand" - [(match_operand:SI 1 "register_operand" "0,0") - (match_operand:SI 2 "nonmemory_operand" "rI,Cal")])) + (match_operator:HI 4 "memory_operand" + [(plus:SI (match_operand:SI 1 "register_operand" "0,0") + (match_operand:SI 2 "nonmemory_operand" "rI,Cal"))])) (set (match_operand:SI 0 "dest_reg_operand" "=w,w") (plus:SI (match_dup 1) (match_dup 2)))] "" @@ -1176,9 +1176,9 @@ (define_insn "*load_zeroextendhisi_update" [(set (match_operand:SI 3 "dest_reg_operand" "=r,r") - (zero_extend:SI (match_operator:HI 4 "load_update_operand" - [(match_operand:SI 1 "register_operand" "0,0") - (match_operand:SI 2 "nonmemory_operand" "rI,Cal")]))) + (zero_extend:SI (match_operator:HI 4 "memory_operand" + [(plus:SI (match_operand:SI 1 "register_operand" "0,0") + (match_operand:SI 2 "nonmemory_operand" "rI,Cal"))]))) (set (match_operand:SI 0 "dest_reg_operand" "=r,r") (plus:SI (match_dup 1) (match_dup 2)))] "" @@ -1189,9 +1189,9 @@ ;; Note: no 16-bit variant for this instruction (define_insn "*load_signextendhisi_update" [(set (match_operand:SI 3 "dest_reg_operand" "=r,r") - (sign_extend:SI (match_operator:HI 4 "load_update_operand" - [(match_operand:SI 1 "register_operand" "0,0") - (match_operand:SI 2 "nonmemory_operand" "rI,Cal")]))) + (sign_extend:SI (match_operator:HI 4 "memory_operand" + [(plus:SI (match_operand:SI 1 "register_operand" "0,0") + (match_operand:SI 2 "nonmemory_operand" "rI,Cal"))]))) (set (match_operand:SI 0 "dest_reg_operand" "=w,w") (plus:SI (match_dup 1) (match_dup 2)))] "" @@ -1214,9 +1214,9 @@ ;; No 16-bit variant for this instruction pattern (define_insn "*loadsi_update" [(set (match_operand:SI 3 "dest_reg_operand" "=r,r") - (match_operator:SI 4 "load_update_operand" - [(match_operand:SI 1 "register_operand" "0,0") - (match_operand:SI 2 "nonmemory_operand" "rI,Cal")])) + (match_operator:SI 4 "memory_operand" + [(plus:SI (match_operand:SI 1 "register_operand" "0,0") + (match_operand:SI 2 "nonmemory_operand" "rI,Cal"))])) (set (match_operand:SI 0 "dest_reg_operand" "=w,w") (plus:SI (match_dup 1) (match_dup 2)))] "" @@ -1238,9 +1238,9 @@ (define_insn "*loadsf_update" [(set (match_operand:SF 3 "dest_reg_operand" "=r,r") - (match_operator:SF 4 "load_update_operand" - [(match_operand:SI 1 "register_operand" "0,0") - (match_operand:SI 2 "nonmemory_operand" "rI,Cal")])) + (match_operator:SF 4 "memory_operand" + [(plus:SI (match_operand:SI 1 "register_operand" "0,0") + (match_operand:SI 2 "nonmemory_operand" "rI,Cal"))])) (set (match_operand:SI 0 "dest_reg_operand" "=w,w") (plus:SI (match_dup 1) (match_dup 2)))] "" diff --git a/gcc/config/arc/predicates.md b/gcc/config/arc/predicates.md index de0735a..bdad135 100644 --- a/gcc/config/arc/predicates.md +++ b/gcc/config/arc/predicates.md @@ -460,24 +460,6 @@ } ) -;; Return true if OP is valid load with update operand. -(define_predicate "load_update_operand" - (match_code "mem") -{ - if (GET_CODE (op) != MEM - || GET_MODE (op) != mode) - return 0; - op = XEXP (op, 0); - if (GET_CODE (op) != PLUS - || GET_MODE (op) != Pmode - || !register_operand (XEXP (op, 0), Pmode) - || !nonmemory_operand (XEXP (op, 1), Pmode)) - return 0; - return 1; - -} -) - ;; Return true if OP is valid store with update operand. (define_predicate "store_update_operand" (match_code "mem") diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index bf4d198..6ab629a 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,5 +1,9 @@ 2015-12-09 Andrew Burgess <andrew.burg...@embecosm.com> + * gcc.target/arc/load-update.c: New file. + +2015-12-09 Andrew Burgess <andrew.burg...@embecosm.com> + * gcc.target/arc/jump-around-jump.c (rtc_set_time): Declare. 2015-12-10 Jeff Law <l...@redhat.com> diff --git a/gcc/testsuite/gcc.target/arc/load-update.c b/gcc/testsuite/gcc.target/arc/load-update.c new file mode 100644 index 0000000..8299cb7 --- /dev/null +++ b/gcc/testsuite/gcc.target/arc/load-update.c @@ -0,0 +1,20 @@ +/* { dg-do compile } */ +/* { dg-options "-Os" } */ + +/* This caused a segfault due to incorrect rtl pattern in some + instructions. */ + +int a, d; +char *b; + +void fn1() +{ + char *e = 0; + for (; d; ++a) + { + char c = b [0]; + *e++ = '.'; + *e++ = 4; + *e++ = "0123456789abcdef" [c & 5]; + } +} -- 2.5.1