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
>

Reply via email to