* [PATCH 0/4] [ARC] Collection Of Bug Fixes @ 2015-12-16 0:15 Andrew Burgess 2015-12-16 0:15 ` [PATCH 2/4] gcc/arc: Remove load_update_operand predicate Andrew Burgess ` (4 more replies) 0 siblings, 5 replies; 13+ messages in thread From: Andrew Burgess @ 2015-12-16 0:15 UTC (permalink / raw) To: gcc-patches; +Cc: Claudiu.Zissulescu, gnu, Andrew Burgess This is a collection of 4 bug fix patches for arc. All 4 patches are really stand-alone, I've only grouped them together as they all only effect arc. I don't have write access to the GCC repository, so if they get approved could they also be applied please. Thanks, Andrew -- Andrew Burgess (4): gcc/arc: Fix warning in test gcc/arc: Remove load_update_operand predicate gcc/arc: Remove store_update_operand predicate gcc/arc: Avoid JUMP_LABEL_AS_INSN for possible return jumps gcc/ChangeLog | 27 ++++++++++ gcc/config/arc/arc.c | 15 +++--- gcc/config/arc/arc.md | 72 ++++++++++++------------- gcc/config/arc/predicates.md | 36 ------------- gcc/testsuite/ChangeLog | 12 +++++ gcc/testsuite/gcc.target/arc/jump-around-jump.c | 2 +- gcc/testsuite/gcc.target/arc/load-update.c | 20 +++++++ gcc/testsuite/gcc.target/arc/loop-hazard-1.c | 16 ++++++ 8 files changed, 120 insertions(+), 80 deletions(-) create mode 100644 gcc/testsuite/gcc.target/arc/load-update.c create mode 100644 gcc/testsuite/gcc.target/arc/loop-hazard-1.c -- 2.5.1 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/4] gcc/arc: Remove load_update_operand predicate 2015-12-16 0:15 [PATCH 0/4] [ARC] Collection Of Bug Fixes Andrew Burgess @ 2015-12-16 0:15 ` Andrew Burgess 2015-12-17 10:20 ` Joern Wolfgang Rennecke 2015-12-16 0:15 ` [PATCH 1/4] gcc/arc: Fix warning in test Andrew Burgess ` (3 subsequent siblings) 4 siblings, 1 reply; 13+ messages in thread From: Andrew Burgess @ 2015-12-16 0:15 UTC (permalink / raw) To: gcc-patches; +Cc: Claudiu.Zissulescu, gnu, Andrew Burgess 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.burgess@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 <law@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.burgess@embecosm.com> + * gcc.target/arc/load-update.c: New file. + +2015-12-09 Andrew Burgess <andrew.burgess@embecosm.com> + * gcc.target/arc/jump-around-jump.c (rtc_set_time): Declare. 2015-12-10 Jeff Law <law@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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] gcc/arc: Remove load_update_operand predicate 2015-12-16 0:15 ` [PATCH 2/4] gcc/arc: Remove load_update_operand predicate Andrew Burgess @ 2015-12-17 10:20 ` Joern Wolfgang Rennecke 2015-12-18 12:52 ` Andrew Burgess 0 siblings, 1 reply; 13+ messages in thread From: Joern Wolfgang Rennecke @ 2015-12-17 10:20 UTC (permalink / raw) To: Andrew Burgess, gcc-patches; +Cc: Claudiu.Zissulescu On 16/12/15 00:15, Andrew Burgess wrote: > > > * 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. Store_update_operand has the very same problem, so it would make sense to fix that in the same check-in. FWIW, while using "memory_operand" makes for simple source code, it introduces duplicated checks (and they are appropriate only because the the update and some of the non-update addressing modes on the ARC are different modes of the same encoding). It checks that the inside of the MEM is a valid memory address, which is redundant with the pattern-provided checks that there's a plus with appropriate base and index/update operand inside. Also, by using memory_operand, you are adding a check to reject volatile memory operands during most optimization passes. Note that ARC's move_src_operand and move_dest_operand are fine with volatile MEMs irrespective of the setting of volatile_ok. Problems with volatiles can rally only be expected if there are multiple MEMs in a single pattern, that might alias, arithmetic on MEM that might result from simplifications using a different set of MEMs, or if the machine instructions that a pattern corresponds to are intrinsically unsuitable for volatile. Needlessly rejecting volatile MEMs will reduce optimization potential; this tends to be more visible on embedded platforms than with embedded code than with typical workstation code. Less reliance on this addressing modes orthogonality, no change of volatile behaviour, and maybe a slightly faster compiler, would be to just have an "update_operand" or "any_mem_operand" predicate checking that the inside is a MEM. and leave the address processing entirely to the instruction pattern and its operand predicates. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] gcc/arc: Remove load_update_operand predicate 2015-12-17 10:20 ` Joern Wolfgang Rennecke @ 2015-12-18 12:52 ` Andrew Burgess 2015-12-19 19:35 ` Joern Wolfgang Rennecke 0 siblings, 1 reply; 13+ messages in thread From: Andrew Burgess @ 2015-12-18 12:52 UTC (permalink / raw) To: Joern Wolfgang Rennecke; +Cc: gcc-patches, Claudiu.Zissulescu * Joern Wolfgang Rennecke <gnu@amylaar.uk> [2015-12-17 10:20:44 +0000]: > On 16/12/15 00:15, Andrew Burgess wrote: > > > > * 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. > > > Less reliance on this addressing modes orthogonality, no change of volatile > behaviour, > and maybe a slightly faster compiler, would be to just have an > "update_operand" or > "any_mem_operand" predicate checking that the inside is a MEM. and leave the > address > processing entirely to the instruction pattern and its operand predicates. A revised patch is below, updated to use a new any_mem_operand predicate. I've updated store_update_operand patch in the same say, and will send that as a follow-up to to the 3/4 email. Thanks, Andrew -- The current use of the arc specific predicate load_update_operand is broken, this commit fixes the error, and in the process moves to a more generic arc specific predicate any_mem_operand. 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, the only thing that a predicate now needs to do is to confirm that the operand being matched by the match_operator is a memory operand. This is what the new predicate 'any_mem_operand' does. The reasons for creating a new predicate 'any_mem_operand' rather than using the common 'memory_operand' predicate are described in this mailing list post: https://gcc.gnu.org/ml/gcc-patches/2015-12/msg01729.html gcc/ChangeLog: * config/arc/arc.md (*loadqi_update): Use new 'any_mem_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. (any_mem_operand): New predicate. gcc/testsuite/ChangeLog: * gcc.target/arc/load-update.c: New file. --- gcc/config/arc/arc.md | 48 +++++++++++++++--------------- gcc/config/arc/predicates.md | 21 ++----------- gcc/testsuite/gcc.target/arc/load-update.c | 20 +++++++++++++ 5 files changed, 65 insertions(+), 42 deletions(-) create mode 100644 gcc/testsuite/gcc.target/arc/load-update.c diff --git a/gcc/config/arc/arc.md b/gcc/config/arc/arc.md index ac181a9..7ca4431 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 "any_mem_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 "any_mem_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 "any_mem_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 "any_mem_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 "any_mem_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 "any_mem_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 "any_mem_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 "any_mem_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..268ff7e 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") @@ -817,3 +799,6 @@ (define_predicate "mem_noofs_operand" (and (match_code "mem") (match_code "reg" "0"))) + +(define_predicate "any_mem_operand" + (match_code "mem")) \ No newline at end of file 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.2.2 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] gcc/arc: Remove load_update_operand predicate 2015-12-18 12:52 ` Andrew Burgess @ 2015-12-19 19:35 ` Joern Wolfgang Rennecke 0 siblings, 0 replies; 13+ messages in thread From: Joern Wolfgang Rennecke @ 2015-12-19 19:35 UTC (permalink / raw) To: Andrew Burgess; +Cc: gcc-patches, Claudiu.Zissulescu On 18/12/15 12:52, Andrew Burgess wrote: > gcc/ChangeLog: > > * config/arc/arc.md (*loadqi_update): Use new 'any_mem_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. > (any_mem_operand): New predicate. > > gcc/testsuite/ChangeLog: > > * gcc.target/arc/load-update.c: New file. Thanks. I have applied this patch. ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/4] gcc/arc: Fix warning in test 2015-12-16 0:15 [PATCH 0/4] [ARC] Collection Of Bug Fixes Andrew Burgess 2015-12-16 0:15 ` [PATCH 2/4] gcc/arc: Remove load_update_operand predicate Andrew Burgess @ 2015-12-16 0:15 ` Andrew Burgess 2015-12-17 9:07 ` Joern Wolfgang Rennecke 2015-12-16 0:15 ` [PATCH 3/4] gcc/arc: Remove store_update_operand predicate Andrew Burgess ` (2 subsequent siblings) 4 siblings, 1 reply; 13+ messages in thread From: Andrew Burgess @ 2015-12-16 0:15 UTC (permalink / raw) To: gcc-patches; +Cc: Claudiu.Zissulescu, gnu, Andrew Burgess Missing function declaration causes a warning, that results in test failure. gcc/testsuite/ChangeLog: * gcc.target/arc/jump-around-jump.c (rtc_set_time): Declare. --- gcc/testsuite/ChangeLog | 4 ++++ gcc/testsuite/gcc.target/arc/jump-around-jump.c | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 6bcacab..bf4d198 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,7 @@ +2015-12-09 Andrew Burgess <andrew.burgess@embecosm.com> + + * gcc.target/arc/jump-around-jump.c (rtc_set_time): Declare. + 2015-12-10 Jeff Law <law@redhat.com> PR tree-optimization/68619 diff --git a/gcc/testsuite/gcc.target/arc/jump-around-jump.c b/gcc/testsuite/gcc.target/arc/jump-around-jump.c index 1b45328..338c667 100644 --- a/gcc/testsuite/gcc.target/arc/jump-around-jump.c +++ b/gcc/testsuite/gcc.target/arc/jump-around-jump.c @@ -97,7 +97,7 @@ struct rtc_device extern void rtc_time_to_tm(unsigned long time, struct rtc_time *tm); extern struct rtc_device *rtc_class_open(const char *name); extern void rtc_class_close(struct rtc_device *rtc); - +extern int rtc_set_time (struct rtc_device *rtc, struct rtc_time *tm); int rtc_set_ntp_time(struct timespec now) { -- 2.5.1 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] gcc/arc: Fix warning in test 2015-12-16 0:15 ` [PATCH 1/4] gcc/arc: Fix warning in test Andrew Burgess @ 2015-12-17 9:07 ` Joern Wolfgang Rennecke 0 siblings, 0 replies; 13+ messages in thread From: Joern Wolfgang Rennecke @ 2015-12-17 9:07 UTC (permalink / raw) To: Andrew Burgess, gcc-patches; +Cc: Claudiu.Zissulescu On 16/12/15 00:15, Andrew Burgess wrote: > Missing function declaration causes a warning, that results in test > failure. Ah, this test was affected when the default language was changed to gnu11 in October last year. > > gcc/testsuite/ChangeLog: > > * gcc.target/arc/jump-around-jump.c (rtc_set_time): Declare. Thanks, I've checked this in. ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/4] gcc/arc: Remove store_update_operand predicate 2015-12-16 0:15 [PATCH 0/4] [ARC] Collection Of Bug Fixes Andrew Burgess 2015-12-16 0:15 ` [PATCH 2/4] gcc/arc: Remove load_update_operand predicate Andrew Burgess 2015-12-16 0:15 ` [PATCH 1/4] gcc/arc: Fix warning in test Andrew Burgess @ 2015-12-16 0:15 ` Andrew Burgess 2015-12-18 12:53 ` Andrew Burgess 2015-12-16 0:15 ` [PATCH 4/4] gcc/arc: Avoid JUMP_LABEL_AS_INSN for possible return jumps Andrew Burgess 2015-12-17 8:59 ` [PATCH 0/4] [ARC] Collection Of Bug Fixes Joern Wolfgang Rennecke 4 siblings, 1 reply; 13+ messages in thread From: Andrew Burgess @ 2015-12-16 0:15 UTC (permalink / raw) To: gcc-patches; +Cc: Claudiu.Zissulescu, gnu, Andrew Burgess The use of the arc specific predicate store_update_operand is broken, this commit fixes the error, and in the process removes the need for store_update_operand altogether. Currently store_update_operand is used with match_operator, the store_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 store_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 store_update_operand predicate are now contains within the rtl pattern, if the use of store_update_operand is replaced with the memory_operand predicate. As with the previous patch in this series, once this patch is applied I see almost all of these instructions being used in the wider GCC testsuite. As with the previous patch, if anyone knows a good way to trigger the generation of specific instructions, tha would be great. gcc/ChangeLog: * config/arc/arc.md (*storeqi_update): Use 'memory_operand' and fix RTL pattern to include the plus. (*storehi_update): Likewise. (*storesi_update): Likewise. (*storesf_update): Likewise. * config/arc/predicates.md (store_update_operand): Delete. --- gcc/ChangeLog | 9 +++++++++ gcc/config/arc/arc.md | 24 ++++++++++++------------ gcc/config/arc/predicates.md | 18 ------------------ 3 files changed, 21 insertions(+), 30 deletions(-) diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 705d4e9..dcc0930 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,5 +1,14 @@ 2015-12-09 Andrew Burgess <andrew.burgess@embecosm.com> + * config/arc/arc.md (*storeqi_update): Use 'memory_operand' and + fix RTL pattern to include the plus. + (*storehi_update): Likewise. + (*storesi_update): Likewise. + (*storesf_update): Likewise. + * config/arc/predicates.md (store_update_operand): Delete. + +2015-12-09 Andrew Burgess <andrew.burgess@embecosm.com> + * config/arc/arc.md (*loadqi_update): Use 'memory_operand' and fix RTL pattern to include the plus. (*load_zeroextendqisi_update): Likewise. diff --git a/gcc/config/arc/arc.md b/gcc/config/arc/arc.md index ef82007..2ca4d1d 100644 --- a/gcc/config/arc/arc.md +++ b/gcc/config/arc/arc.md @@ -1149,9 +1149,9 @@ (set_attr "length" "4,8")]) (define_insn "*storeqi_update" - [(set (match_operator:QI 4 "store_update_operand" - [(match_operand:SI 1 "register_operand" "0") - (match_operand:SI 2 "short_immediate_operand" "I")]) + [(set (match_operator:QI 4 "memory_operand" + [(plus:SI (match_operand:SI 1 "register_operand" "0") + (match_operand:SI 2 "short_immediate_operand" "I"))]) (match_operand:QI 3 "register_operand" "c")) (set (match_operand:SI 0 "dest_reg_operand" "=w") (plus:SI (match_dup 1) (match_dup 2)))] @@ -1200,9 +1200,9 @@ (set_attr "length" "4,8")]) (define_insn "*storehi_update" - [(set (match_operator:HI 4 "store_update_operand" - [(match_operand:SI 1 "register_operand" "0") - (match_operand:SI 2 "short_immediate_operand" "I")]) + [(set (match_operator:HI 4 "memory_operand" + [(plus:SI (match_operand:SI 1 "register_operand" "0") + (match_operand:SI 2 "short_immediate_operand" "I"))]) (match_operand:HI 3 "register_operand" "c")) (set (match_operand:SI 0 "dest_reg_operand" "=w") (plus:SI (match_dup 1) (match_dup 2)))] @@ -1225,9 +1225,9 @@ (set_attr "length" "4,8")]) (define_insn "*storesi_update" - [(set (match_operator:SI 4 "store_update_operand" - [(match_operand:SI 1 "register_operand" "0") - (match_operand:SI 2 "short_immediate_operand" "I")]) + [(set (match_operator:SI 4 "memory_operand" + [(plus:SI (match_operand:SI 1 "register_operand" "0") + (match_operand:SI 2 "short_immediate_operand" "I"))]) (match_operand:SI 3 "register_operand" "c")) (set (match_operand:SI 0 "dest_reg_operand" "=w") (plus:SI (match_dup 1) (match_dup 2)))] @@ -1249,9 +1249,9 @@ (set_attr "length" "4,8")]) (define_insn "*storesf_update" - [(set (match_operator:SF 4 "store_update_operand" - [(match_operand:SI 1 "register_operand" "0") - (match_operand:SI 2 "short_immediate_operand" "I")]) + [(set (match_operator:SF 4 "memory_operand" + [(plus:SI (match_operand:SI 1 "register_operand" "0") + (match_operand:SI 2 "short_immediate_operand" "I"))]) (match_operand:SF 3 "register_operand" "c")) (set (match_operand:SI 0 "dest_reg_operand" "=w") (plus:SI (match_dup 1) (match_dup 2)))] diff --git a/gcc/config/arc/predicates.md b/gcc/config/arc/predicates.md index bdad135..bb488d1 100644 --- a/gcc/config/arc/predicates.md +++ b/gcc/config/arc/predicates.md @@ -460,24 +460,6 @@ } ) -;; Return true if OP is valid store with update operand. -(define_predicate "store_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) - || !(GET_CODE (XEXP (op, 1)) == CONST_INT - && SMALL_INT (INTVAL (XEXP (op, 1))))) - return 0; - return 1; -} -) - ;; Return true if OP is a non-volatile non-immediate operand. ;; Volatile memory refs require a special "cache-bypass" instruction ;; and only the standard movXX patterns are set up to handle them. -- 2.5.1 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/4] gcc/arc: Remove store_update_operand predicate 2015-12-16 0:15 ` [PATCH 3/4] gcc/arc: Remove store_update_operand predicate Andrew Burgess @ 2015-12-18 12:53 ` Andrew Burgess 2015-12-19 19:35 ` Joern Wolfgang Rennecke 0 siblings, 1 reply; 13+ messages in thread From: Andrew Burgess @ 2015-12-18 12:53 UTC (permalink / raw) To: gcc-patches; +Cc: Claudiu.Zissulescu, gnu Here's a revised version of this patch, written to use the any_mem_operand predicate. Thanks, Andrew -- The use of the arc specific predicate store_update_operand is broken, this commit fixes the error, switching to use 'any_mem_operand' instead. Currently store_update_operand is used with match_operator, the store_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 store_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 store_update_operand predicate are now contains within the rtl pattern, the only thing that a predicate now needs to do is to confirm that the operand being matched by the match_operator is a memory operand. This is achieved by switching to the 'any_mem_operand' predicate. gcc/ChangeLog: * config/arc/arc.md (*storeqi_update): Use 'memory_operand' and fix RTL pattern to include the plus. (*storehi_update): Likewise. (*storesi_update): Likewise. (*storesf_update): Likewise. * config/arc/predicates.md (store_update_operand): Delete. --- gcc/ChangeLog | 9 +++++++++ gcc/config/arc/arc.md | 24 ++++++++++++------------ gcc/config/arc/predicates.md | 18 ------------------ 3 files changed, 21 insertions(+), 30 deletions(-) diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 5c14a5a..b2dff58 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,5 +1,14 @@ 2015-12-09 Andrew Burgess <andrew.burgess@embecosm.com> + * config/arc/arc.md (*storeqi_update): Use 'any_mem_operand' and + fix RTL pattern to include the plus. + (*storehi_update): Likewise. + (*storesi_update): Likewise. + (*storesf_update): Likewise. + * config/arc/predicates.md (store_update_operand): Delete. + +2015-12-09 Andrew Burgess <andrew.burgess@embecosm.com> + * config/arc/arc.md (*loadqi_update): Use new 'any_mem_operand' and fix RTL pattern to include the plus. (*load_zeroextendqisi_update): Likewise. diff --git a/gcc/config/arc/arc.md b/gcc/config/arc/arc.md index 7ca4431..9e73d02 100644 --- a/gcc/config/arc/arc.md +++ b/gcc/config/arc/arc.md @@ -1149,9 +1149,9 @@ (set_attr "length" "4,8")]) (define_insn "*storeqi_update" - [(set (match_operator:QI 4 "store_update_operand" - [(match_operand:SI 1 "register_operand" "0") - (match_operand:SI 2 "short_immediate_operand" "I")]) + [(set (match_operator:QI 4 "any_mem_operand" + [(plus:SI (match_operand:SI 1 "register_operand" "0") + (match_operand:SI 2 "short_immediate_operand" "I"))]) (match_operand:QI 3 "register_operand" "c")) (set (match_operand:SI 0 "dest_reg_operand" "=w") (plus:SI (match_dup 1) (match_dup 2)))] @@ -1200,9 +1200,9 @@ (set_attr "length" "4,8")]) (define_insn "*storehi_update" - [(set (match_operator:HI 4 "store_update_operand" - [(match_operand:SI 1 "register_operand" "0") - (match_operand:SI 2 "short_immediate_operand" "I")]) + [(set (match_operator:HI 4 "any_mem_operand" + [(plus:SI (match_operand:SI 1 "register_operand" "0") + (match_operand:SI 2 "short_immediate_operand" "I"))]) (match_operand:HI 3 "register_operand" "c")) (set (match_operand:SI 0 "dest_reg_operand" "=w") (plus:SI (match_dup 1) (match_dup 2)))] @@ -1225,9 +1225,9 @@ (set_attr "length" "4,8")]) (define_insn "*storesi_update" - [(set (match_operator:SI 4 "store_update_operand" - [(match_operand:SI 1 "register_operand" "0") - (match_operand:SI 2 "short_immediate_operand" "I")]) + [(set (match_operator:SI 4 "any_mem_operand" + [(plus:SI (match_operand:SI 1 "register_operand" "0") + (match_operand:SI 2 "short_immediate_operand" "I"))]) (match_operand:SI 3 "register_operand" "c")) (set (match_operand:SI 0 "dest_reg_operand" "=w") (plus:SI (match_dup 1) (match_dup 2)))] @@ -1249,9 +1249,9 @@ (set_attr "length" "4,8")]) (define_insn "*storesf_update" - [(set (match_operator:SF 4 "store_update_operand" - [(match_operand:SI 1 "register_operand" "0") - (match_operand:SI 2 "short_immediate_operand" "I")]) + [(set (match_operator:SF 4 "any_mem_operand" + [(plus:SI (match_operand:SI 1 "register_operand" "0") + (match_operand:SI 2 "short_immediate_operand" "I"))]) (match_operand:SF 3 "register_operand" "c")) (set (match_operand:SI 0 "dest_reg_operand" "=w") (plus:SI (match_dup 1) (match_dup 2)))] diff --git a/gcc/config/arc/predicates.md b/gcc/config/arc/predicates.md index 268ff7e..ba11cd1 100644 --- a/gcc/config/arc/predicates.md +++ b/gcc/config/arc/predicates.md @@ -460,24 +460,6 @@ } ) -;; Return true if OP is valid store with update operand. -(define_predicate "store_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) - || !(GET_CODE (XEXP (op, 1)) == CONST_INT - && SMALL_INT (INTVAL (XEXP (op, 1))))) - return 0; - return 1; -} -) - ;; Return true if OP is a non-volatile non-immediate operand. ;; Volatile memory refs require a special "cache-bypass" instruction ;; and only the standard movXX patterns are set up to handle them. -- 2.2.2 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/4] gcc/arc: Remove store_update_operand predicate 2015-12-18 12:53 ` Andrew Burgess @ 2015-12-19 19:35 ` Joern Wolfgang Rennecke 0 siblings, 0 replies; 13+ messages in thread From: Joern Wolfgang Rennecke @ 2015-12-19 19:35 UTC (permalink / raw) To: Andrew Burgess, gcc-patches; +Cc: Claudiu.Zissulescu On 18/12/15 12:53, Andrew Burgess wrote: > > > * config/arc/arc.md (*storeqi_update): Use 'memory_operand' and > fix RTL pattern to include the plus. > (*storehi_update): Likewise. > (*storesi_update): Likewise. > (*storesf_update): Likewise. > * config/arc/predicates.md (store_update_operand): Delete. > Thanks. I have applied this patch. ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/4] gcc/arc: Avoid JUMP_LABEL_AS_INSN for possible return jumps 2015-12-16 0:15 [PATCH 0/4] [ARC] Collection Of Bug Fixes Andrew Burgess ` (2 preceding siblings ...) 2015-12-16 0:15 ` [PATCH 3/4] gcc/arc: Remove store_update_operand predicate Andrew Burgess @ 2015-12-16 0:15 ` Andrew Burgess 2015-12-17 10:55 ` Joern Wolfgang Rennecke 2015-12-17 8:59 ` [PATCH 0/4] [ARC] Collection Of Bug Fixes Joern Wolfgang Rennecke 4 siblings, 1 reply; 13+ messages in thread From: Andrew Burgess @ 2015-12-16 0:15 UTC (permalink / raw) To: gcc-patches; +Cc: Claudiu.Zissulescu, gnu, Andrew Burgess We currently call JUMP_LABEL_AS_INSN on a jump instruction that might have SIMPLE_RETURN as it's jump label, this triggers the assertions as SIMPLE_RETURN is of type rtx_extra, not rtx_insn. This commit first calls JUMP_LABEL then uses ANY_RETURN_P to catch all of the return style jump labels. After this we can use the safe_as_a cast mechanism to safely convert the jump label to an rtx_insn. There's a test included, but this issue is also hit in the tests: gcc.c-torture/execute/20000605-2.c gcc.dg/torture/pr68083.c gcc/ChangeLog: * config/arc/arc.c (arc_loop_hazard): Don't convert the jump label rtx to an rtx_insn until we confirm it's not a return rtx. gcc/testsuite/ChangeLog: * gcc.target/arc/loop-hazard-1.c: New file. --- gcc/ChangeLog | 5 +++++ gcc/config/arc/arc.c | 15 ++++++++------- gcc/testsuite/ChangeLog | 4 ++++ gcc/testsuite/gcc.target/arc/loop-hazard-1.c | 16 ++++++++++++++++ 4 files changed, 33 insertions(+), 7 deletions(-) create mode 100644 gcc/testsuite/gcc.target/arc/loop-hazard-1.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index dcc0930..bd2621d 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,5 +1,10 @@ 2015-12-09 Andrew Burgess <andrew.burgess@embecosm.com> + * config/arc/arc.c (arc_loop_hazard): Don't convert the jump label + rtx to an rtx_insn until we confirm it's not a return rtx. + +2015-12-09 Andrew Burgess <andrew.burgess@embecosm.com> + * config/arc/arc.md (*storeqi_update): Use 'memory_operand' and fix RTL pattern to include the plus. (*storehi_update): Likewise. diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c index 5bc2bce..2c0f8b9 100644 --- a/gcc/config/arc/arc.c +++ b/gcc/config/arc/arc.c @@ -7987,6 +7987,7 @@ static bool arc_loop_hazard (rtx_insn *pred, rtx_insn *succ) { rtx_insn *jump = NULL; + rtx label_rtx = NULL_RTX; rtx_insn *label = NULL; basic_block succ_bb; @@ -8013,22 +8014,22 @@ arc_loop_hazard (rtx_insn *pred, rtx_insn *succ) else return false; - label = JUMP_LABEL_AS_INSN (jump); - if (!label) - return false; - /* Phase 2b: Make sure is not a millicode jump. */ if ((GET_CODE (PATTERN (jump)) == PARALLEL) && (XVECEXP (PATTERN (jump), 0, 0) == ret_rtx)) return false; - /* Phase 2c: Make sure is not a simple_return. */ - if ((GET_CODE (PATTERN (jump)) == SIMPLE_RETURN) - || (GET_CODE (label) == SIMPLE_RETURN)) + label_rtx = JUMP_LABEL (jump); + if (!label_rtx) + return false; + + /* Phase 2c: Make sure is not a return. */ + if (ANY_RETURN_P (label_rtx)) return false; /* Pahse 2d: Go to the target of the jump and check for aliveness of LP_COUNT register. */ + label = safe_as_a <rtx_insn *> (label_rtx); succ_bb = BLOCK_FOR_INSN (label); if (!succ_bb) { diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 6ab629a..b98706f 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,5 +1,9 @@ 2015-12-09 Andrew Burgess <andrew.burgess@embecosm.com> + * gcc.target/arc/loop-hazard-1.c: New file. + +2015-12-09 Andrew Burgess <andrew.burgess@embecosm.com> + * gcc.target/arc/load-update.c: New file. 2015-12-09 Andrew Burgess <andrew.burgess@embecosm.com> diff --git a/gcc/testsuite/gcc.target/arc/loop-hazard-1.c b/gcc/testsuite/gcc.target/arc/loop-hazard-1.c new file mode 100644 index 0000000..7c688bb --- /dev/null +++ b/gcc/testsuite/gcc.target/arc/loop-hazard-1.c @@ -0,0 +1,16 @@ +/* { dg-do compile } */ +/* { dg-options "-Os" } */ + +/* This caused an assertion within arc_loop_hazard. */ + +unsigned a, b; + +long fn1() +{ + long c = 1, d = 0; + while (a && c && b) + c <<= 1; + while (c) + d |= c; + return d; +} -- 2.5.1 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 4/4] gcc/arc: Avoid JUMP_LABEL_AS_INSN for possible return jumps 2015-12-16 0:15 ` [PATCH 4/4] gcc/arc: Avoid JUMP_LABEL_AS_INSN for possible return jumps Andrew Burgess @ 2015-12-17 10:55 ` Joern Wolfgang Rennecke 0 siblings, 0 replies; 13+ messages in thread From: Joern Wolfgang Rennecke @ 2015-12-17 10:55 UTC (permalink / raw) To: Andrew Burgess, gcc-patches; +Cc: Claudiu.Zissulescu On 16/12/15 00:15, Andrew Burgess wrote: > > > gcc/ChangeLog: > > * config/arc/arc.c (arc_loop_hazard): Don't convert the jump label > rtx to an rtx_insn until we confirm it's not a return rtx. > > gcc/testsuite/ChangeLog: > > * gcc.target/arc/loop-hazard-1.c: New file. Thanks, I've checked this in. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/4] [ARC] Collection Of Bug Fixes 2015-12-16 0:15 [PATCH 0/4] [ARC] Collection Of Bug Fixes Andrew Burgess ` (3 preceding siblings ...) 2015-12-16 0:15 ` [PATCH 4/4] gcc/arc: Avoid JUMP_LABEL_AS_INSN for possible return jumps Andrew Burgess @ 2015-12-17 8:59 ` Joern Wolfgang Rennecke 4 siblings, 0 replies; 13+ messages in thread From: Joern Wolfgang Rennecke @ 2015-12-17 8:59 UTC (permalink / raw) To: Andrew Burgess, gcc-patches; +Cc: Claudiu.Zissulescu On 16/12/15 00:15, Andrew Burgess wrote: > This is a collection of 4 bug fix patches for arc. All 4 patches are > really stand-alone, I've only grouped them together as they all only > effect arc. Note for future postings: ChangeLog entries are supposed to appear as plain text, not as diff. ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2015-12-19 19:35 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-12-16 0:15 [PATCH 0/4] [ARC] Collection Of Bug Fixes Andrew Burgess 2015-12-16 0:15 ` [PATCH 2/4] gcc/arc: Remove load_update_operand predicate Andrew Burgess 2015-12-17 10:20 ` Joern Wolfgang Rennecke 2015-12-18 12:52 ` Andrew Burgess 2015-12-19 19:35 ` Joern Wolfgang Rennecke 2015-12-16 0:15 ` [PATCH 1/4] gcc/arc: Fix warning in test Andrew Burgess 2015-12-17 9:07 ` Joern Wolfgang Rennecke 2015-12-16 0:15 ` [PATCH 3/4] gcc/arc: Remove store_update_operand predicate Andrew Burgess 2015-12-18 12:53 ` Andrew Burgess 2015-12-19 19:35 ` Joern Wolfgang Rennecke 2015-12-16 0:15 ` [PATCH 4/4] gcc/arc: Avoid JUMP_LABEL_AS_INSN for possible return jumps Andrew Burgess 2015-12-17 10:55 ` Joern Wolfgang Rennecke 2015-12-17 8:59 ` [PATCH 0/4] [ARC] Collection Of Bug Fixes Joern Wolfgang Rennecke
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).