On 8/13/24 5:57 AM, Manolis Tsamis wrote:
Now that more operations are allowed for noce_convert_multiple_sets, we need to
check noce_can_force_operand on the sequence before calling try_emit_cmove_seq.
Otherwise an inappropriate argument may be given to copy_to_mode_reg and result
in an ICE.

Fix-up for the recent ifcvt commit 72c9b5f438f22cca493b4e2a8a2a31ff61bf1477

        PR tree-optimization/116353

gcc/ChangeLog:

        * ifcvt.cc (bb_ok_for_noce_convert_multiple_sets): Check
        noce_can_force_operand.

gcc/testsuite/ChangeLog:

        * gcc.target/i386/pr116353.c: New test.
OK.

Note I'm not entirely sure that noce_can_force_operand is sufficient based on what I've seen on the v8 port.

What I'm seeing on the v8 port is that we're trying to create a conditional move where one arm is a rotate expression. That in and of itself isn't an issue. The port does (of course) expect a register operand, so we use force_operand to get the result into a GPR. So far, so good.

The ifcvt code does check noce_can_force_operand which returns true. I don't remember the precise details other than it looked reasonable. So still, so far, so good.

The problem in the v850 doesn't have a generalized rotation pattern. The expander will FAIL for most rotate counts and there's no alternate synthesis currently defined in the optabs interface for a word mode rotate. So that in turn causes force_operand to return NULL_RTX to its caller and boom!

The rotation patterns are allowed to FAIL if I'm reading the docs correctly. So it seems like what the v8 port is doing is valid, but it is causing a segfault and this testsuite failure:

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)


But perhaps it isn't that bad in practice. I can fix this by removing a bit of what I expect is unnecessary code in the v850 port.

Jeff

Reply via email to