RISC-V: Fix for combine bug with shift and AND operations.

Message ID 20180402223804.2228-1-jimw@sifive.com
State New
Headers show
Series
  • RISC-V: Fix for combine bug with shift and AND operations.
Related show

Commit Message

Jim Wilson April 2, 2018, 10:38 p.m.
This is a fix for PR 84660 which is a combine bug with SHIFT_COUNT_TRUNCATED.
It seems that use of SHIFT_COUNT_TRUNCATED is discouraged now, so rather than
fix combine this fixes the RISC-V backend to stop using SHIFT_COUNT_TRUNCATED.
This required adding 6 new patterns, and changing the existing patterns to use
QImode shift counts instead of SImode/SImode shift counts.  This also adds
the original testcase, and 6 minimal testcases which each test one of the new
patterns.

This was tested with riscv{32,64}-{linux,elf} builds and make checks.  There
are no regressions.  It was tested with a linux kernel and buildroot build and
boot on qemu.  It was tested by looking at libstdc++ code with and without the
patch, and trying to get the exact same code, though in the end I had to settle
for the same instructions with slightly different register allocation and
slightly different order in a few cases.

Committed.

Jim

	PR rtl-optimization/84660
	gcc/
	* config/riscv/riscv.h (SHIFT_COUNT_TRUNCATED): Set to zero.
	* config/riscv/riscv.md (<optab>si3): Use QImode shift count.
	(<optab>di3, <optab>si3_extend): Likewise.
	(<optab>si3_mask, <optab>si3_mask_1): New.
	(<optab>di3_mask, <optab>di3_mask_1): New.
	(<optab>si3_extend_mask, <optab>si3_extend_mask_1): New.
	(lshrsi3_zero_extend_1): Use VOIDmode shift count.
	* config/riscv/sync.md (atomic_test_and_set): Emit QImode shift count.
	gcc/testsuite/
	* gcc.target/riscv/pr84660.c: New.
	* gcc.target/riscv/shift-and-1.c: New.
	* gcc.target/riscv/shift-and-2.c: New.
---
 gcc/config/riscv/riscv.h                     |   4 +-
 gcc/config/riscv/riscv.md                    | 136 ++++++++++++++++++++++++++-
 gcc/config/riscv/sync.md                     |   5 +-
 gcc/testsuite/gcc.target/riscv/pr84660.c     |  17 ++++
 gcc/testsuite/gcc.target/riscv/shift-and-1.c |  10 ++
 gcc/testsuite/gcc.target/riscv/shift-and-2.c |  41 ++++++++
 6 files changed, 206 insertions(+), 7 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/pr84660.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/shift-and-1.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/shift-and-2.c

-- 
2.14.1

Patch

diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h
index ebd80c0a5f2..62279ff2cde 100644
--- a/gcc/config/riscv/riscv.h
+++ b/gcc/config/riscv/riscv.h
@@ -636,7 +636,9 @@  typedef struct {
    and maybe make use of that.  */
 #define SLOW_BYTE_ACCESS 1
 
-#define SHIFT_COUNT_TRUNCATED 1
+/* Using SHIFT_COUNT_TRUNCATED is discouraged, so we handle this with patterns
+   in the md file instead.  */
+#define SHIFT_COUNT_TRUNCATED 0
 
 /* Specify the machine mode that pointers have.
    After generation of rtl, the compiler makes no further distinction
diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
index bffe78dd837..9d222731a06 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -1483,11 +1483,16 @@ 
 ;;
 ;;  ....................
 
+;; Use a QImode shift count, to avoid generating sign or zero extend
+;; instructions for shift counts, and to avoid dropping subregs.
+;; expand_shift_1 can do this automatically when SHIFT_COUNT_TRUNCATED is
+;; defined, but use of that is discouraged.
+
 (define_insn "<optab>si3"
   [(set (match_operand:SI     0 "register_operand" "= r")
 	(any_shift:SI
 	    (match_operand:SI 1 "register_operand" "  r")
-	    (match_operand:SI 2 "arith_operand"    " rI")))]
+	    (match_operand:QI 2 "arith_operand"    " rI")))]
   ""
 {
   if (GET_CODE (operands[2]) == CONST_INT)
@@ -1499,11 +1504,50 @@ 
   [(set_attr "type" "shift")
    (set_attr "mode" "SI")])
 
+(define_insn_and_split "<optab>si3_mask"
+  [(set (match_operand:SI     0 "register_operand" "= r")
+	(any_shift:SI
+	    (match_operand:SI 1 "register_operand" "  r")
+	    (subreg:QI
+	     (and:SI
+	      (match_operand:SI 2 "register_operand"  "r")
+	      (match_operand 3 "const_int_operand")) 0)))]
+  "(INTVAL (operands[3]) & (GET_MODE_BITSIZE (SImode)-1))
+   == GET_MODE_BITSIZE (SImode)-1"
+  "#"
+  "&& 1"
+  [(set (match_dup 0)
+	(any_shift:SI (match_dup 1)
+		      (match_dup 2)))]
+  "operands[2] = gen_lowpart (QImode, operands[2]);"
+  [(set_attr "type" "shift")
+   (set_attr "mode" "SI")])
+
+(define_insn_and_split "<optab>si3_mask_1"
+  [(set (match_operand:SI     0 "register_operand" "= r")
+	(any_shift:SI
+	    (match_operand:SI 1 "register_operand" "  r")
+	    (subreg:QI
+	     (and:DI
+	      (match_operand:DI 2 "register_operand"  "r")
+	      (match_operand 3 "const_int_operand")) 0)))]
+  "TARGET_64BIT
+   && (INTVAL (operands[3]) & (GET_MODE_BITSIZE (SImode)-1))
+       == GET_MODE_BITSIZE (SImode)-1"
+  "#"
+  "&& 1"
+  [(set (match_dup 0)
+	(any_shift:SI (match_dup 1)
+		      (match_dup 2)))]
+  "operands[2] = gen_lowpart (QImode, operands[2]);"
+  [(set_attr "type" "shift")
+   (set_attr "mode" "SI")])
+
 (define_insn "<optab>di3"
   [(set (match_operand:DI 0 "register_operand"     "= r")
 	(any_shift:DI
 	    (match_operand:DI 1 "register_operand" "  r")
-	    (match_operand:DI 2 "arith_operand"    " rI")))]
+	    (match_operand:QI 2 "arith_operand"    " rI")))]
   "TARGET_64BIT"
 {
   if (GET_CODE (operands[2]) == CONST_INT)
@@ -1515,11 +1559,51 @@ 
   [(set_attr "type" "shift")
    (set_attr "mode" "DI")])
 
+(define_insn_and_split "<optab>di3_mask"
+  [(set (match_operand:DI     0 "register_operand" "= r")
+	(any_shift:DI
+	    (match_operand:DI 1 "register_operand" "  r")
+	    (subreg:QI
+	     (and:SI
+	      (match_operand:SI 2 "register_operand"  "r")
+	      (match_operand 3 "const_int_operand")) 0)))]
+  "TARGET_64BIT
+   && (INTVAL (operands[3]) & (GET_MODE_BITSIZE (DImode)-1))
+       == GET_MODE_BITSIZE (DImode)-1"
+  "#"
+  "&& 1"
+  [(set (match_dup 0)
+	(any_shift:DI (match_dup 1)
+		      (match_dup 2)))]
+  "operands[2] = gen_lowpart (QImode, operands[2]);"
+  [(set_attr "type" "shift")
+   (set_attr "mode" "DI")])
+
+(define_insn_and_split "<optab>di3_mask_1"
+  [(set (match_operand:DI     0 "register_operand" "= r")
+	(any_shift:DI
+	    (match_operand:DI 1 "register_operand" "  r")
+	    (subreg:QI
+	     (and:DI
+	      (match_operand:DI 2 "register_operand"  "r")
+	      (match_operand 3 "const_int_operand")) 0)))]
+  "TARGET_64BIT
+   && (INTVAL (operands[3]) & (GET_MODE_BITSIZE (DImode)-1))
+       == GET_MODE_BITSIZE (DImode)-1"
+  "#"
+  "&& 1"
+  [(set (match_dup 0)
+	(any_shift:DI (match_dup 1)
+		      (match_dup 2)))]
+  "operands[2] = gen_lowpart (QImode, operands[2]);"
+  [(set_attr "type" "shift")
+   (set_attr "mode" "DI")])
+
 (define_insn "*<optab>si3_extend"
   [(set (match_operand:DI                   0 "register_operand" "= r")
 	(sign_extend:DI
 	    (any_shift:SI (match_operand:SI 1 "register_operand" "  r")
-			  (match_operand:SI 2 "arith_operand"    " rI"))))]
+			  (match_operand:QI 2 "arith_operand"    " rI"))))]
   "TARGET_64BIT"
 {
   if (GET_CODE (operands[2]) == CONST_INT)
@@ -1530,13 +1614,57 @@ 
   [(set_attr "type" "shift")
    (set_attr "mode" "SI")])
 
+(define_insn_and_split "*<optab>si3_extend_mask"
+  [(set (match_operand:DI                   0 "register_operand" "= r")
+	(sign_extend:DI
+	    (any_shift:SI
+	     (match_operand:SI 1 "register_operand" "  r")
+	     (subreg:QI
+	      (and:SI
+	       (match_operand:SI 2 "register_operand" " r")
+	       (match_operand 3 "const_int_operand")) 0))))]
+  "TARGET_64BIT
+   && (INTVAL (operands[3]) & (GET_MODE_BITSIZE (SImode)-1))
+       == GET_MODE_BITSIZE (SImode)-1"
+  "#"
+  "&& 1"
+  [(set (match_dup 0)
+	(sign_extend:DI
+	 (any_shift:SI (match_dup 1)
+		       (match_dup 2))))]
+  "operands[2] = gen_lowpart (QImode, operands[2]);"
+  [(set_attr "type" "shift")
+   (set_attr "mode" "SI")])
+
+(define_insn_and_split "*<optab>si3_extend_mask_1"
+  [(set (match_operand:DI                   0 "register_operand" "= r")
+	(sign_extend:DI
+	    (any_shift:SI
+	     (match_operand:SI 1 "register_operand" "  r")
+	     (subreg:QI
+	      (and:DI
+	       (match_operand:DI 2 "register_operand" " r")
+	       (match_operand 3 "const_int_operand")) 0))))]
+  "TARGET_64BIT
+   && (INTVAL (operands[3]) & (GET_MODE_BITSIZE (SImode)-1))
+       == GET_MODE_BITSIZE (SImode)-1"
+  "#"
+  "&& 1"
+  [(set (match_dup 0)
+	(sign_extend:DI
+	 (any_shift:SI (match_dup 1)
+		       (match_dup 2))))]
+  "operands[2] = gen_lowpart (QImode, operands[2]);"
+  [(set_attr "type" "shift")
+   (set_attr "mode" "SI")])
+
 ;; Non-canonical, but can be formed by ree when combine is not successful at
 ;; producing one of the two canonical patterns below.
 (define_insn "*lshrsi3_zero_extend_1"
   [(set (match_operand:DI                   0 "register_operand" "=r")
 	(zero_extend:DI
 	 (lshiftrt:SI (match_operand:SI     1 "register_operand" " r")
-		      (match_operand:SI     2 "const_int_operand"))))]
+		      (match_operand        2 "const_int_operand"))))]
   "TARGET_64BIT && (INTVAL (operands[2]) & 0x1f) > 0"
 {
   operands[2] = GEN_INT (INTVAL (operands[2]) & 0x1f);
diff --git a/gcc/config/riscv/sync.md b/gcc/config/riscv/sync.md
index b6916bde65b..8e8c37702aa 100644
--- a/gcc/config/riscv/sync.md
+++ b/gcc/config/riscv/sync.md
@@ -182,13 +182,14 @@ 
   emit_move_insn (shmt, gen_rtx_ASHIFT (SImode, offset, GEN_INT (3)));
 
   rtx word = gen_reg_rtx (SImode);
-  emit_move_insn (word, gen_rtx_ASHIFT (SImode, tmp, shmt));
+  emit_move_insn (word, gen_rtx_ASHIFT (SImode, tmp,
+					gen_lowpart (QImode, shmt)));
 
   tmp = gen_reg_rtx (SImode);
   emit_insn (gen_atomic_fetch_orsi (tmp, aligned_mem, word, model));
 
   emit_move_insn (gen_lowpart (SImode, result),
 		  gen_rtx_LSHIFTRT (SImode, tmp,
-				    gen_lowpart (SImode, shmt)));
+				    gen_lowpart (QImode, shmt)));
   DONE;
 })
diff --git a/gcc/testsuite/gcc.target/riscv/pr84660.c b/gcc/testsuite/gcc.target/riscv/pr84660.c
new file mode 100644
index 00000000000..a87fa0a914d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/pr84660.c
@@ -0,0 +1,17 @@ 
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+extern void abort (void);
+extern void exit (int);
+
+unsigned int __attribute__ ((noinline, noclone))
+foo(unsigned int i) {
+
+  return 0xFFFF & (0xd066 << (((i & 0x1) ^ 0x2f) & 0xf));
+}
+
+int main() {
+  if (foo (1) != 0x8000)
+    abort ();
+  exit (0);
+}
diff --git a/gcc/testsuite/gcc.target/riscv/shift-and-1.c b/gcc/testsuite/gcc.target/riscv/shift-and-1.c
new file mode 100644
index 00000000000..d1f3a05db2c
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/shift-and-1.c
@@ -0,0 +1,10 @@ 
+/* { dg-do compile } */
+/* { dg-options "-march=rv32gc -mabi=ilp32 -O" } */
+
+/* Test for <optab>si3_mask.  */
+int
+sub1 (int i, int j)
+{
+  return i << (j & 0x1f);
+}
+/* { dg-final { scan-assembler-not "andi" } } */
diff --git a/gcc/testsuite/gcc.target/riscv/shift-and-2.c b/gcc/testsuite/gcc.target/riscv/shift-and-2.c
new file mode 100644
index 00000000000..2c98e50101b
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/shift-and-2.c
@@ -0,0 +1,41 @@ 
+/* { dg-do compile { target { riscv64*-*-* } } } */
+/* { dg-options "-march=rv64gc -mabi=lp64 -O" } */
+
+/* Test for <optab>si3_mask_1.  */
+extern int k;
+void
+sub2 (int i, long j)
+{
+  k = i << (j & 0x1f);
+}
+
+/* Test for <optab>si3_extend_mask.  */
+unsigned long
+sub3 (int mask)
+{
+  return 1 << (mask & 0xff);
+}
+
+/* Test for <optab>si3_extend_mask_1.  */
+int
+sub4 (int i, int j)
+{
+  return i << (j & 0x1f);
+}
+
+/* Test for <optab>di3_mask.  */
+long
+sub5 (long i, int j)
+{
+  char k = j & 0x3f;
+  return i << k;
+}
+
+/* Test for <optab>di3_mask_1.  */
+long
+sub6 (long i, long j)
+{
+  return i << (j & 0x3f);
+}
+/* { dg-final { scan-assembler-not "andi" } } */
+/* { dg-final { scan-assembler-not "sext.w" } } */