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

Andrew Burgess andrew.burgess@embecosm.com
Wed Dec 16 00:15:00 GMT 2015


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



More information about the Gcc-patches mailing list