Applied to master, thanks. --Philipp. On Tue, 13 Aug 2024 at 21:48, Jeff Law <jeffreya...@gmail.com> wrote:
> > > 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 >