public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Philipp Tomsich <philipp.tomsich@vrull.eu>
To: gcc-patches@gcc.gnu.org
Cc: Manolis Tsamis <manolis.tsamis@vrull.eu>,
	Jim Wilson <jim.wilson.gcc@gmail.com>,
	Vineet Gupta <vineetg@rivosinc.com>,
	Kito Cheng <kito.cheng@gmail.com>,
	Palmer Dabbelt <palmer@rivosinc.com>,
	Andrew Waterman <andrew@sifive.com>,
	Philipp Tomsich <philipp.tomsich@vrull.eu>
Subject: [PATCH v1 3/3] RISC-V: Replace zero_extendsidi2_shifted with generalized split
Date: Tue, 24 May 2022 23:47:03 +0200	[thread overview]
Message-ID: <20220524214703.4022737-4-philipp.tomsich@vrull.eu> (raw)
In-Reply-To: <20220524214703.4022737-1-philipp.tomsich@vrull.eu>

The current method of treating shifts of extended values on RISC-V
frequently causes sequences of 3 shifts, despite the presence of the
'zero_extendsidi2_shifted' pattern.

Consider:
    unsigned long f(unsigned int a, unsigned long b)
    {
            a = a << 1;
            unsigned long c = (unsigned long) a;
            c = b + (c<<4);
            return c;
    }
which will present at combine-time as:
    Trying 7, 8 -> 9:
        7: r78:SI=r81:DI#0<<0x1
          REG_DEAD r81:DI
        8: r79:DI=zero_extend(r78:SI)
          REG_DEAD r78:SI
        9: r72:DI=r79:DI<<0x4
          REG_DEAD r79:DI
    Failed to match this instruction:
    (set (reg:DI 72 [ _1 ])
        (and:DI (ashift:DI (reg:DI 81)
                (const_int 5 [0x5]))
    	(const_int 68719476704 [0xfffffffe0])))
and produce the following (optimized) assembly:
    f:
	slliw	a5,a0,1
	slli	a5,a5,32
	srli	a5,a5,28
	add	a0,a5,a1
	ret

The current way of handling this (in 'zero_extendsidi2_shifted')
doesn't apply for two reasons:
- this is seen before reload, and
- (more importantly) the constant mask is not 0xfffffffful.

To address this, we introduce a generalized version of shifting
zero-extended values that supports any mask of consecutive ones as
long as the number of training zeros is the inner shift-amount.

With this new split, we generate the following assembly for the
aforementioned function:
    f:
	slli	a0,a0,33
	srli	a0,a0,28
	add	a0,a0,a1
	ret

gcc/ChangeLog:

	* config/riscv/riscv.md (zero_extendsidi2_shifted): Replace
	  with a generalized split that requires no clobber, runs
	  before reload and works for smaller masks.

Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
---

 gcc/config/riscv/riscv.md | 37 ++++++++++++++++++++-----------------
 1 file changed, 20 insertions(+), 17 deletions(-)

diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
index b8ab0cf169a..cc10cd90a74 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -2119,23 +2119,26 @@ (define_split
 ;; occur when unsigned int is used for array indexing.  Split this into two
 ;; shifts.  Otherwise we can get 3 shifts.
 
-(define_insn_and_split "zero_extendsidi2_shifted"
-  [(set (match_operand:DI 0 "register_operand" "=r")
-	(and:DI (ashift:DI (match_operand:DI 1 "register_operand" "r")
-			   (match_operand:QI 2 "immediate_operand" "I"))
-		(match_operand 3 "immediate_operand" "")))
-   (clobber (match_scratch:DI 4 "=&r"))]
-  "TARGET_64BIT && !TARGET_ZBA
-   && ((INTVAL (operands[3]) >> INTVAL (operands[2])) == 0xffffffff)"
-  "#"
-  "&& reload_completed"
-  [(set (match_dup 4)
-	(ashift:DI (match_dup 1) (const_int 32)))
-   (set (match_dup 0)
-	(lshiftrt:DI (match_dup 4) (match_dup 5)))]
-  "operands[5] = GEN_INT (32 - (INTVAL (operands [2])));"
-  [(set_attr "type" "shift")
-   (set_attr "mode" "DI")])
+(define_split
+  [(set (match_operand:DI 0 "register_operand")
+	(and:DI (ashift:DI (match_operand:DI 1 "register_operand")
+			   (match_operand:QI 2 "immediate_operand"))
+	        (match_operand:DI 3 "consecutive_bits_operand")))]
+  "TARGET_64BIT"
+  [(set (match_dup 0) (ashift:DI (match_dup 1) (match_dup 4)))
+   (set (match_dup 0) (lshiftrt:DI (match_dup 0) (match_dup 5)))]
+{
+	unsigned HOST_WIDE_INT mask = UINTVAL (operands[3]);
+	int leading = clz_hwi (mask);
+	int trailing = ctz_hwi (mask);
+
+	/* The shift-amount must match the number of trailing bits */
+	if (trailing != UINTVAL (operands[2]))
+	   FAIL;
+
+	operands[4] = GEN_INT (leading + trailing);
+	operands[5] = GEN_INT (leading);
+})
 
 ;;
 ;;  ....................
-- 
2.34.1


  parent reply	other threads:[~2022-05-24 21:47 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-24 21:47 [PATCH v1 0/3] RISC-V: Improve sequences with shifted zero-extended operands Philipp Tomsich
2022-05-24 21:47 ` [PATCH v1 1/3] RISC-V: add consecutive_bits_operand predicate Philipp Tomsich
2022-06-07 10:25   ` Kito Cheng
2022-06-14 11:38     ` Philipp Tomsich
2022-05-24 21:47 ` [PATCH v1 2/3] RISC-V: Split slli+sh[123]add.uw opportunities to avoid zext.w Philipp Tomsich
2022-06-07 10:25   ` Kito Cheng
2022-06-14 11:39     ` Philipp Tomsich
2022-06-17  8:34   ` Andreas Schwab
2022-06-17 14:00     ` Kito Cheng
2022-06-17 14:19       ` Philipp Tomsich
2022-05-24 21:47 ` Philipp Tomsich [this message]
2022-06-07 10:24   ` [PATCH v1 3/3] RISC-V: Replace zero_extendsidi2_shifted with generalized split Kito Cheng
2022-06-07 10:50     ` Philipp Tomsich
2022-06-07 13:18       ` Kito Cheng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220524214703.4022737-4-philipp.tomsich@vrull.eu \
    --to=philipp.tomsich@vrull.eu \
    --cc=andrew@sifive.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jim.wilson.gcc@gmail.com \
    --cc=kito.cheng@gmail.com \
    --cc=manolis.tsamis@vrull.eu \
    --cc=palmer@rivosinc.com \
    --cc=vineetg@rivosinc.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).