On 4/1/25 12:20 AM, Jin Ma wrote:
Assuming we have the following variables:
unsigned long long a0, a1;
unsigned int a2;
For the expression:
a0 = (a0 << 50) >> 49; // slli a0, a0, 50 + srli a0, a0, 49
a2 = a1 + a0; // addw a2, a1, a0 + slli a2, a2, 32 + srli a2, a2, 32
In the optimization process of ZBA (combine pass), it would be optimized to:
a2 = a0 << 1 + a1; // sh1add a2, a0, a1 + zext.w a2, a2
This is clearly incorrect, as it overlooks the fact that a0=a0&0x7ffe, meaning
that the bits a0[32:14] are set to zero.
gcc/ChangeLog:
* config/riscv/bitmanip.md: The optimization can only be applied if
the high bit of operands[3] is set to 1.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/zba-shNadd-09.c: New test.
* gcc.target/riscv/zba-shNadd-10.c: New test.
---
gcc/config/riscv/bitmanip.md | 3 ++-
.../gcc.target/riscv/zba-shNadd-09.c | 12 +++++++++++
.../gcc.target/riscv/zba-shNadd-10.c | 20 +++++++++++++++++++
3 files changed, 34 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.target/riscv/zba-shNadd-09.c
create mode 100644 gcc/testsuite/gcc.target/riscv/zba-shNadd-10.c
diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md
index b29c127bcb8..9091c48b106 100644
--- a/gcc/config/riscv/bitmanip.md
+++ b/gcc/config/riscv/bitmanip.md
@@ -80,7 +80,8 @@ (define_split
(match_operand:DI 3
"consecutive_bits_operand")) 0)
(subreg:SI (match_operand:DI 4
"register_operand") 0))))]
"TARGET_64BIT && TARGET_ZBA
- && riscv_shamt_matches_mask_p (INTVAL (operands[2]), INTVAL (operands[3]))"
+ && riscv_shamt_matches_mask_p (INTVAL (operands[2]), INTVAL (operands[3]))
+ && ((INTVAL (operands[3]) & (1 << 31)) != 0)"
[(set (match_dup 0) (plus:DI (ashift:DI (match_dup 1) (match_dup 2))
(match_dup 4)))
(set (match_dup 0) (zero_extend:DI (subreg:SI (match_dup 0) 0)))])
So I would add a comment to that new condition. Something like
/* Ensure the mask includes all the bits in SImode. */
We need to be careful with constants like 1 << 31. Something like
(HOST_WIDE_INT_1U << 31) would be better from a type safety standpoint
(INTVAL returns a HOST_WIDE_INT object).
+#include <stdio.h>
In general, avoid #includes if you can in tests. Remove the <stdio.h>
include.
+ printf("%llu\n", d);
Instead of a printf and using dg-output, the standard way we test for
correctness is by either aborting or calling exit (0). So rather than
the printf use
if (d != 3023282)
__builtin_abort ();
__builtin_exit (0);
And drop the dg-output line.
With those changes this patch is probably OK. We'd want to post the V2
so that the pre-commit tester can chew on it.
Thanks!
Jeff
