The recent if-conversion changes tripped a failure on the v850 port.
The core underlying issue is that while the if-conversion code tries to
do the right thing with noce_can_force_operand to determine if it can
force an arbitrary operand into a register, it's not really a sufficient
check.
Essentially for arithmetic codes, it checks the operands. If the
operands are force-able and there's a code_to_optab mapping, then it
returns true.
code_to_optab doesn't actually check anything other than the existence
of a mapping in the target. If the target pattern has restrictions
enforced by the condition or it's an expander that is allowed to FAIL,
then noce_can_force_operand can return true, even though we may not be
able to directly force the operand into a register.
This came up on the v850 when we had an operand that was a rotate by a
constant number of bits (I don't remember the count, all that's
important about it was the count was not 8 or 16).
The v850 port has this define_expand:
> (define_expand "rotlsi3"
[(parallel [(set (match_operand:SI 0 "register_operand" "")
(rotate:SI (match_operand:SI 1 "register_operand" "")
(match_operand:SI 2 "const_int_operand" "")))
(clobber (reg:CC CC_REGNUM))])]
"(TARGET_V850E_UP)"
{
if (INTVAL (operands[2]) != 16)
FAIL;
})
So the only rotate count allowed is 16 (there's a similar HI rotate with
a count of 8). AFAICT the rotate patterns are allowed to FAIL. So
naturally the expander fails and we get a testsuite regression:
Tests that now fail, but worked before (4 tests):
v850-sim/-mgcc-abi/-msoft-float/-mv850e3v5: gcc:
gcc.c-torture/execute/20100805-1.c -O3 -fomit-frame-pointer -funroll-loops
-fpeel-loops -ftracer -finline-functions (test for excess errors)
v850-sim/-mgcc-abi/-msoft-float/-mv850e3v5: gcc:
gcc.c-torture/execute/20100805-1.c -O3 -fomit-frame-pointer -funroll-loops
-fpeel-loops -ftracer -finline-functions (test for excess errors)
v850-sim/-msoft-float/-mv850e3v5: gcc: gcc.c-torture/execute/20100805-1.c -O3
-fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions
(test for excess errors)
v850-sim/-mv850e3v5: gcc: gcc.c-torture/execute/20100805-1.c -O3
-fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions
(test for excess errors)
This patch works around the problem by allowing the rotates in
additional cases, particularly for the V850E3V5+ variants which have a
general rotate capability. But let's be clear, this is just a
workaround and I expect we're going to have to revisit the code to test
if an operand can be forced into a register.
Pushing to the trunk.
commit abfc140579682598cd178eb9d0b0160bbfafc633
Author: Jeff Law <j...@ventanamicro.com>
Date: Sat Aug 17 10:30:48 2024 -0600
Adjust v850 rotate expander to allow more cases for V850E3V5
The recent if-conversion changes tripped a failure on the v850 port.
The core underlying issue is that while the if-conversion code tries to do
the
right thing with noce_can_force_operand to determine if it can force an
arbitrary operand into a register, it's not really a sufficient check.
Essentially for arithmetic codes, it checks the operands. If the operands
are
force-able and there's a code_to_optab mapping, then it returns true.
code_to_optab doesn't actually check anything other than the existence of a
mapping in the target. If the target pattern has restrictions enforced by
the
condition or it's an expander that is allowed to FAIL, then
noce_can_force_operand to be true, even though we may not be able to
directly
force the operand into a register.
This came up on the v850 when we had an operand that was a rotate by a
constant
number of bits (I don't remember the count, all that's important about it
was
the count was not 8 or 16).
The v850 port has this define_expand:
> (define_expand "rotlsi3"
> [(parallel [(set (match_operand:SI 0 "register_operand" "")
> (rotate:SI (match_operand:SI 1 "register_operand" "")
> (match_operand:SI 2 "const_int_operand"
"")))
> (clobber (reg:CC CC_REGNUM))])]
> "(TARGET_V850E_UP)"
> {
> if (INTVAL (operands[2]) != 16)
> FAIL;
> })
So the only rotate count allowed is 16 (there's a similar HI rotate with a
count of 8). AFAICT the rotate patterns are allowed to FAIL. So naturally the
expander fails and we get a testsuite regression:
> Tests that now fail, but worked before (4 tests):
>
> v850-sim/-mgcc-abi/-msoft-float/-mv850e3v5: gcc:
gcc.c-torture/execute/20100805-1.c -O3 -fomit-frame-pointer -funroll-loops
-fpeel-loops -ftracer -finline-functions (test for excess errors)
> v850-sim/-mgcc-abi/-msoft-float/-mv850e3v5: gcc:
gcc.c-torture/execute/20100805-1.c -O3 -fomit-frame-pointer -funroll-loops
-fpeel-loops -ftracer -finline-functions (test for excess errors)
> v850-sim/-msoft-float/-mv850e3v5: gcc: gcc.c-torture/execute/20100805-1.c
-O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer
-finline-functions (test for excess errors)
> v850-sim/-mv850e3v5: gcc: gcc.c-torture/execute/20100805-1.c -O3
-fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions
(test for excess errors)
This patch works around the problem by allowing the rotates in additional
cases, particularly for the V850E3V5+ variants which have a general rotate
capability. But let's be clear, this is just a workaround and I expect
we're
going to have to revisit the code to test if an operand can be forced into a
register.
gcc/
* config/v850/v850.md (rotlsi3): Allow more cases for V850E3V5+.
diff --git a/gcc/config/v850/v850.md b/gcc/config/v850/v850.md
index f810a27fa2e..83cc9972673 100644
--- a/gcc/config/v850/v850.md
+++ b/gcc/config/v850/v850.md
@@ -1352,7 +1352,9 @@ (define_expand "rotlsi3"
(clobber (reg:CC CC_REGNUM))])]
"(TARGET_V850E_UP)"
{
- if (INTVAL (operands[2]) != 16)
+ if (TARGET_V850E3V5_UP && e3v5_shift_operand (operands[2], SImode))
+ ;
+ else if (INTVAL (operands[2]) != 16)
FAIL;
})
