diff mbox series

RISC-V: Optimize masking with two clear bits not a SMALL_OPERAND

Message ID 20221110213617.3592572-1-philipp.tomsich@vrull.eu
State Unresolved
Headers
Series RISC-V: Optimize masking with two clear bits not a SMALL_OPERAND |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Philipp Tomsich Nov. 10, 2022, 9:36 p.m. UTC 
  Add a split for cases where we can use two bclri (or one bclri and an
andi) to clear two bits.

gcc/ChangeLog:

	* config/riscv/bitmanip.md (*bclri<mode>_nottwobits): New pattern.
	(*bclridisi_nottwobits): New pattern, handling the sign-bit.
	* config/riscv/predicates.md (const_nottwobits_operand):
	New predicate.

gcc/testsuite/ChangeLog:

	* gcc.target/riscv/zbs-bclri.c: New test.

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

 gcc/config/riscv/bitmanip.md               | 38 ++++++++++++++++++++++
 gcc/config/riscv/predicates.md             |  5 +++
 gcc/testsuite/gcc.target/riscv/zbs-bclri.c | 12 +++++++
 3 files changed, 55 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/riscv/zbs-bclri.c

Comments

Jeff Law Nov. 17, 2022, 2:30 p.m. UTC | #1 
On 11/10/22 14:36, Philipp Tomsich wrote:
> Add a split for cases where we can use two bclri (or one bclri and an
> andi) to clear two bits.
>
> gcc/ChangeLog:
>
> 	* config/riscv/bitmanip.md (*bclri<mode>_nottwobits): New pattern.
> 	(*bclridisi_nottwobits): New pattern, handling the sign-bit.
> 	* config/riscv/predicates.md (const_nottwobits_operand):
> 	New predicate.
>
> gcc/testsuite/ChangeLog:
>
> 	* gcc.target/riscv/zbs-bclri.c: New test.

Don't we only have to worry about (subreg:DI (reg:SI )) to preserve the 
extension constraints?  Not that I think there's any value in allowing 
HI/QI, mostly wanting to double check my understanding of the extension 
constraints.


OK

jeff
Philipp Tomsich Nov. 17, 2022, 2:43 p.m. UTC | #2 
On Thu, 17 Nov 2022 at 15:30, Jeff Law <jeffreyalaw@gmail.com> wrote:
>
>
> On 11/10/22 14:36, Philipp Tomsich wrote:
> > Add a split for cases where we can use two bclri (or one bclri and an
> > andi) to clear two bits.
> >
> > gcc/ChangeLog:
> >
> >       * config/riscv/bitmanip.md (*bclri<mode>_nottwobits): New pattern.
> >       (*bclridisi_nottwobits): New pattern, handling the sign-bit.
> >       * config/riscv/predicates.md (const_nottwobits_operand):
> >       New predicate.
> >
> > gcc/testsuite/ChangeLog:
> >
> >       * gcc.target/riscv/zbs-bclri.c: New test.
>
> Don't we only have to worry about (subreg:DI (reg:SI )) to preserve the
> extension constraints?  Not that I think there's any value in allowing

That is the reason for the !paradoxical_subreg_p(...) check in
"bclri<mode>_nottwobits"... so at pattern will always be safe.

Do you see a risk on the "*bclridisi_nottwobits"?

> HI/QI, mostly wanting to double check my understanding of the extension
> constraints.

QI will be fully covered by simm12 (e.g., using the andi instruction).

For HI, we might in fact want to have a special case (similar to
"*bclridisi_nottwobits") — but the reach of the simm12 seems to cover
enough of 16-bit space that this hasn't shown up (yet) as enough of a
problem to warrant a special case.

Philipp.
Jeff Law Nov. 17, 2022, 2:53 p.m. UTC | #3 
On 11/17/22 07:43, Philipp Tomsich wrote:
> On Thu, 17 Nov 2022 at 15:30, Jeff Law <jeffreyalaw@gmail.com> wrote:
>>
>> On 11/10/22 14:36, Philipp Tomsich wrote:
>>> Add a split for cases where we can use two bclri (or one bclri and an
>>> andi) to clear two bits.
>>>
>>> gcc/ChangeLog:
>>>
>>>        * config/riscv/bitmanip.md (*bclri<mode>_nottwobits): New pattern.
>>>        (*bclridisi_nottwobits): New pattern, handling the sign-bit.
>>>        * config/riscv/predicates.md (const_nottwobits_operand):
>>>        New predicate.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>>        * gcc.target/riscv/zbs-bclri.c: New test.
>> Don't we only have to worry about (subreg:DI (reg:SI )) to preserve the
>> extension constraints?  Not that I think there's any value in allowing
> That is the reason for the !paradoxical_subreg_p(...) check in
> "bclri<mode>_nottwobits"... so at pattern will always be safe.
>
> Do you see a risk on the "*bclridisi_nottwobits"?

It's jut a sanity check for me, I don't have any concerns since you're 
avoiding this when working on a paradoxical.  But it does make me wonder 
if we need a paradoxical check on the other bit twiddles patterns to 
prevent them from changing the SImode sign bit in a paradoxical.


Jeff
Philipp Tomsich Nov. 17, 2022, 3:37 p.m. UTC | #4 
Applied to master. Thanks!
Philipp.

On Thu, 17 Nov 2022 at 15:30, Jeff Law <jeffreyalaw@gmail.com> wrote:
>
>
> On 11/10/22 14:36, Philipp Tomsich wrote:
> > Add a split for cases where we can use two bclri (or one bclri and an
> > andi) to clear two bits.
> >
> > gcc/ChangeLog:
> >
> >       * config/riscv/bitmanip.md (*bclri<mode>_nottwobits): New pattern.
> >       (*bclridisi_nottwobits): New pattern, handling the sign-bit.
> >       * config/riscv/predicates.md (const_nottwobits_operand):
> >       New predicate.
> >
> > gcc/testsuite/ChangeLog:
> >
> >       * gcc.target/riscv/zbs-bclri.c: New test.
>
> Don't we only have to worry about (subreg:DI (reg:SI )) to preserve the
> extension constraints?  Not that I think there's any value in allowing
> HI/QI, mostly wanting to double check my understanding of the extension
> constraints.
>
>
> OK
>
> jeff
>
diff mbox series

Patch

diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md
index 7fa8461bb71..f1d8f24c2d3 100644
--- a/gcc/config/riscv/bitmanip.md
+++ b/gcc/config/riscv/bitmanip.md
@@ -560,6 +560,44 @@ 
   "bclri\t%0,%1,%T2"
   [(set_attr "type" "bitmanip")])
 
+;; In case we have "val & ~IMM" where ~IMM has 2 bits set.
+(define_insn_and_split "*bclri<mode>_nottwobits"
+  [(set (match_operand:X 0 "register_operand" "=r")
+	(and:X (match_operand:X 1 "register_operand" "r")
+	       (match_operand:X 2 "const_nottwobits_operand" "i")))]
+  "TARGET_ZBS && !paradoxical_subreg_p (operands[1])"
+  "#"
+  "&& reload_completed"
+  [(set (match_dup 0) (and:X (match_dup 1) (match_dup 3)))
+   (set (match_dup 0) (and:X (match_dup 0) (match_dup 4)))]
+{
+	unsigned HOST_WIDE_INT bits = ~UINTVAL (operands[2]);
+	unsigned HOST_WIDE_INT topbit = HOST_WIDE_INT_1U << floor_log2 (bits);
+
+	operands[3] = GEN_INT (~bits | topbit);
+	operands[4] = GEN_INT (~topbit);
+})
+
+;; In case of a paradoxical subreg, the sign bit and the high bits are
+;; not allowed to be changed
+(define_insn_and_split "*bclridisi_nottwobits"
+  [(set (match_operand:DI 0 "register_operand" "=r")
+	(and:DI (match_operand:DI 1 "register_operand" "r")
+		(match_operand:DI 2 "const_nottwobits_operand" "i")))]
+  "TARGET_64BIT && TARGET_ZBS
+   && clz_hwi (~UINTVAL (operands[2])) > 33"
+  "#"
+  "&& reload_completed"
+  [(set (match_dup 0) (and:DI (match_dup 1) (match_dup 3)))
+   (set (match_dup 0) (and:DI (match_dup 0) (match_dup 4)))]
+{
+	unsigned HOST_WIDE_INT bits = ~UINTVAL (operands[2]);
+	unsigned HOST_WIDE_INT topbit = HOST_WIDE_INT_1U << floor_log2 (bits);
+
+	operands[3] = GEN_INT (~bits | topbit);
+	operands[4] = GEN_INT (~topbit);
+})
+
 (define_insn "*binv<mode>"
   [(set (match_operand:X 0 "register_operand" "=r")
 	(xor:X (ashift:X (const_int 1)
diff --git a/gcc/config/riscv/predicates.md b/gcc/config/riscv/predicates.md
index 6de9b39e39b..b368c11c930 100644
--- a/gcc/config/riscv/predicates.md
+++ b/gcc/config/riscv/predicates.md
@@ -304,6 +304,11 @@ 
        (match_test "ctz_hwi (INTVAL (op)) > 0")
        (match_test "SMALL_OPERAND (INTVAL (op) >> ctz_hwi (INTVAL (op)))")))
 
+;; A CONST_INT operand that has exactly two bits cleared.
+(define_predicate "const_nottwobits_operand"
+  (and (match_code "const_int")
+       (match_test "popcount_hwi (~UINTVAL (op)) == 2")))
+
 ;; A CONST_INT operand that fits into the unsigned half of a
 ;; signed-immediate after the top bit has been cleared.
 (define_predicate "uimm_extra_bit_operand"
diff --git a/gcc/testsuite/gcc.target/riscv/zbs-bclri.c b/gcc/testsuite/gcc.target/riscv/zbs-bclri.c
new file mode 100644
index 00000000000..12e2063436c
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/zbs-bclri.c
@@ -0,0 +1,12 @@ 
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gc_zbs -mabi=lp64" } */
+/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" } } */
+
+/* bclri + bclri */
+long long f5 (long long a)
+{
+  return a & ~0x11000;
+}
+
+/* { dg-final { scan-assembler-times "bclri\t" 2 } } */
+