diff mbox series

[to-be-committed,PR,target/115921] Improve reassociation for rv64

Message ID 7c038242-8663-4d94-9175-ea23397faae2@gmail.com
State New
Headers show
Series [to-be-committed,PR,target/115921] Improve reassociation for rv64 | expand

Commit Message

Jeff Law Sept. 2, 2024, 5:53 p.m. UTC 
As Jovan pointed out in pr115921, we're not reassociating expressions 
like this on rv64:

(x & 0x3e) << 12

It generates something like this:

         li      a5,258048
         slli    a0,a0,12
         and     a0,a0,a5


We have a pattern that's designed to clean this up.  Essentially 
reassociating the operations so that we don't need to load the constant 
resulting in something like this:

         andi    a0,a0,63
         slli    a0,a0,12


That pattern wasn't working for certain constants due to its condition. 
The condition is trying to avoid cases where this kind of reassociation 
would hinder shadd generation on rv64.  That condition was just written 
poorly.

This patch tightens up that condition in a few ways.  First, there's no 
need to worry about shadd cases if ZBA is not enabled.  Second we can't 
use shadd if the shift value isn't 1, 2 or 3.  Finally rather than 
open-coding one of the tests, we can use an existing operand predicate.

The net is we'll start performing this transformation in more cases on 
rv64 while still avoiding reassociation if it would spoil shadd generation.

Waiting on the pre-commit testing before taking any further action.

Jeff
diff mbox series

Patch

diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
index 3289ed2155a..58b31658e0a 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -2925,7 +2925,9 @@  (define_insn_and_split "*<optab>si3_extend_mask"
 ;; for IOR/XOR.  It probably doesn't matter for AND.
 ;;
 ;; We also don't want to do this if the immediate already fits in a simm12
-;; field.
+;; field, or is a single bit operand, or when we might be able to generate
+;; a shift-add sequence via the splitter in bitmanip.md
+;; in bitmanip.md for masks that are a run of consecutive ones.
 (define_insn_and_split "<optab>_shift_reverse<X:mode>"
   [(set (match_operand:X 0 "register_operand" "=r")
     (any_bitwise:X (ashift:X (match_operand:X 1 "register_operand" "r")
@@ -2934,9 +2936,9 @@  (define_insn_and_split "<optab>_shift_reverse<X:mode>"
   "(!SMALL_OPERAND (INTVAL (operands[3]))
     && SMALL_OPERAND (INTVAL (operands[3]) >> INTVAL (operands[2]))
     && popcount_hwi (INTVAL (operands[3])) > 1
-    && (!TARGET_64BIT
-	|| (exact_log2 ((INTVAL (operands[3]) >> INTVAL (operands[2])) + 1)
-	     == -1))
+    && (!(TARGET_64BIT && TARGET_ZBA)
+	|| !consecutive_bits_operand (operands[3], VOIDmode)
+	|| !imm123_operand (operands[2], VOIDmode))
     && (INTVAL (operands[3]) & ((1ULL << INTVAL (operands[2])) - 1)) == 0)"
   "#"
   "&& 1"
diff --git a/gcc/testsuite/gcc.target/riscv/pr115921.c b/gcc/testsuite/gcc.target/riscv/pr115921.c
new file mode 100644
index 00000000000..e508e7ce790
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/pr115921.c
@@ -0,0 +1,13 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -march=rv64gc_zba" { target { rv64 } } }  */
+/* { dg-options "-O2 -march=rv32gc_zba" { target { rv32 } } }  */
+
+typedef unsigned long target_wide_uint_t;
+
+target_wide_uint_t test_ashift_and(target_wide_uint_t x) {
+    return (x & 0x3f) << 12;
+}
+
+/* { dg-final { scan-assembler-times "\\sandi" 1 } } */
+/* { dg-final { scan-assembler-times "\\sslli" 1 } } */
+