[PATCH 0/4] [ARC] Collection Of Bug Fixes

[PATCH 0/4] [ARC] Collection Of Bug Fixes

[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

[PATCH 3/4] gcc/arc: Remove store_update_operand predicate

[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

Re: [PATCH 3/4] gcc/arc: Remove store_update_operand predicate

[Andrew Burgess]

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

Re: [PATCH 3/4] gcc/arc: Remove store_update_operand predicate

[Joern Wolfgang Rennecke]

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.

[PATCH 4/4] gcc/arc: Avoid JUMP_LABEL_AS_INSN for possible return jumps

[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

Re: [PATCH 4/4] gcc/arc: Avoid JUMP_LABEL_AS_INSN for possible return jumps

[Joern Wolfgang Rennecke]

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.

[PATCH 1/4] gcc/arc: Fix warning in test

[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

Re: [PATCH 1/4] gcc/arc: Fix warning in test

[Joern Wolfgang Rennecke]

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.

[PATCH 2/4] gcc/arc: Remove load_update_operand predicate

[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

Re: [PATCH 2/4] gcc/arc: Remove load_update_operand predicate

[Joern Wolfgang Rennecke]

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.

Re: [PATCH 2/4] gcc/arc: Remove load_update_operand predicate

[Andrew Burgess]

* 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

Re: [PATCH 2/4] gcc/arc: Remove load_update_operand predicate

[Joern Wolfgang Rennecke]

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.

Re: [PATCH 0/4] [ARC] Collection Of Bug Fixes

[Joern Wolfgang Rennecke]

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.