This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH] RISC-V: Fix for combine bug with shift and AND operations.
- From: Jim Wilson <jimw at sifive dot com>
- To: gcc-patches at gcc dot gnu dot org
- Cc: Jim Wilson <jimw at sifive dot com>
- Date: Mon, 2 Apr 2018 15:38:04 -0700
- Subject: [PATCH] RISC-V: Fix for combine bug with shift and AND operations.
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
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" } } */
--
2.14.1