On Wed, 07 Dec 2022 12:55:17 PST (-0800), rzin...@ventanamicro.com wrote:
Due to RISC-V limitations on operations with big constants combine
is failing to match such operations and is not being able to
produce optimal code as it keeps splitting them. By pretending we
can do those operations we can get more opportunities for
simplification of surrounding instructions.
I saw Jeff's comments. This is always the kind of thing that worries
me: we're essentially lying to the optimizer in order to trick it into
generating better code, which might just make it generate worse code.
It's always easy to see a small example that improves, but those could
be wiped out by secondary effects in real code. So I'd usually want to
have some benchmarking for a patch like this.
That said, if this is just the standard way of doing things then maybe
it's just fine?
2022-12-06 Raphael Moreira Zinsly <rzin...@ventanamicro.com>
Jeff Law <j...@ventanamicro.com>
gcc/Changelog:
PR target/95632
PR target/106602
* config/riscv/riscv.md: New pattern to simulate complex
const_int loads.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/pr95632.c: New test.
* gcc.target/riscv/pr106602.c: Likewise.
---
gcc/config/riscv/riscv.md | 16 ++++++++++++++++
gcc/testsuite/gcc.target/riscv/pr106602.c | 14 ++++++++++++++
gcc/testsuite/gcc.target/riscv/pr95632.c | 15 +++++++++++++++
3 files changed, 45 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/riscv/pr106602.c
create mode 100644 gcc/testsuite/gcc.target/riscv/pr95632.c
diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
index df57e2b0b4a..0a9b5ec22b0 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -1667,6 +1667,22 @@
MAX_MACHINE_MODE, &operands[3], TRUE);
})
+;; Pretend to have the ability to load complex const_int in order to get
+;; better code generation around them.
+(define_insn_and_split ""
+ [(set (match_operand:GPR 0 "register_operand" "=r")
+ (match_operand:GPR 1 "splittable_const_int_operand" "i"))]
+ "cse_not_expected"
+ "#"
+ "&& 1"
+ [(const_int 0)]
+
+{
+ riscv_move_integer (operands[0], operands[0], INTVAL (operands[1]),
+ <GPR:MODE>mode, TRUE);
+ DONE;
+})
There's some comments from Jakub on this, I don't see any additional
issues with the code (aside from the "does it help" stuff from above).
+
;; 64-bit integer moves
(define_expand "movdi"
diff --git a/gcc/testsuite/gcc.target/riscv/pr106602.c
b/gcc/testsuite/gcc.target/riscv/pr106602.c
new file mode 100644
index 00000000000..83b70877012
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/pr106602.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -march=rv64gc" } */
There's a DG hook to limit this to 64-bit targets, that way it'll run
with whatever target is being tested.
+
+unsigned long
+foo2 (unsigned long a)
+{
+ return (unsigned long)(unsigned int) a << 6;
+}
+
+/* { dg-final { scan-assembler-times "slli\t" 1 } } */
+/* { dg-final { scan-assembler-times "srli\t" 1 } } */
+/* { dg-final { scan-assembler-not "\tli\t" } } */
+/* { dg-final { scan-assembler-not "addi\t" } } */
+/* { dg-final { scan-assembler-not "and\t" } } */
diff --git a/gcc/testsuite/gcc.target/riscv/pr95632.c
b/gcc/testsuite/gcc.target/riscv/pr95632.c
new file mode 100644
index 00000000000..bd316ab1d7b
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/pr95632.c
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -march=rv32imafc -mabi=ilp32f" } */
Is there a reason to make this rv32-only? Unless I'm missing something
this should generate pretty much the same code for rv64.
+
+unsigned short
+foo (unsigned short crc)
+{
+ crc ^= 0x4002;
+ crc >>= 1;
+ crc |= 0x8000;
+
+ return crc;
+}
+
+/* { dg-final { scan-assembler-times "srli\t" 1 } } */
+/* { dg-final { scan-assembler-not "slli\t" } } */
