]> git.ipfire.org Git - thirdparty/gcc.git/commit
[RISC-V][PR target/116283] Fix split code for recent Zbs improvements with masked...
authorJeff Law <jlaw@ventanamicro.com>
Fri, 9 Aug 2024 23:46:01 +0000 (17:46 -0600)
committerJeff Law <jlaw@ventanamicro.com>
Fri, 9 Aug 2024 23:46:01 +0000 (17:46 -0600)
commitd4e1290e5d603984e9b410c7d4cf21a9ffbd68fd
treee73cc9b6603381fc8cd4948c8fb2b49655d9a29d
parent4734c1bfe837b3e70bc783dafc442de3bca43d88
[RISC-V][PR target/116283] Fix split code for recent Zbs improvements with masked bit positions

So Patrick's fuzzer found an interesting little buglet in the Zbs improvements
I added a couple months back.

Specifically when we have masked bit position for a Zbs instruction.  If the
mask has extraneous bits set we'll generate an unrecognizable insn due to an
invalid constant.

More concretely, let's take this pattern:

> (define_insn_and_split ""
>   [(set (match_operand:DI 0 "register_operand" "=r")
>         (any_extend:DI
>          (ashift:SI (const_int 1)
>                     (subreg:QI                        (and:DI (match_operand:DI 1 "register_operand" "r")
>                               (match_operand 2 "const_int_operand")) 0))))]
What we need to know to transform this into bset for rv64.

After masking the shift count we want to know the low 5 bits aren't 0x1f.  If
they were 0x1f, then the constant generated would be 0x80000000 which would
then need sign extension out to 64bits, which the bset instruction will not do
for us.

We can ignore anything outside the low 5 bits.  The mode of the shift is SI, so
shifting by 32+ bits is undefined behavior.

It's also worth explicitly mentioning that the hardware is going to mask the
count against 0x3f.

The net is if (operands[2] & 0x1f) != 0x1f, then this transformation is safe.
So onto the generated split code...

>   [(set (match_dup 0) (and:DI (match_dup 1) (match_dup 2)))
>    (set (match_dup 0) (zero_extend:DI (ashift:SI
>                                      (const_int 1)
>                                      (subreg:QI (match_dup 0) 0))))]

Which would seemingly do exactly what we want.   The problem is the first split
insn.  If the constant does not fit into a simm12, that insn won't be
recognized resulting in the ICE.

The fix is simple, we just need to mask the constant before generating RTL.  We
can just mask it against 0x1f since we only care about the low 5 bits.

This affects multiple patterns.  I've added the appropriate fix to all of them.

Tested in my tester.  Waiting for the pre-commit bits to run before pushing.

PR target/116283
gcc/
* config/riscv/bitmanip.md (Zbs combiner patterns/splitters): Mask the
bit position in the split code appropriately.

gcc/testsuite/

* gcc.target/riscv/pr116283.c: New test
gcc/config/riscv/bitmanip.md
gcc/testsuite/gcc.target/riscv/pr116283.c [new file with mode: 0644]