[PATCH] Add define_insn_and_split for combine to detect x < 123U ? -1 : 0 (PR target/88425)

[Uros Bizjak]

On Tue, Dec 11, 2018 at 9:14 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Tue, Dec 11, 2018 at 09:03:08AM +0100, Uros Bizjak wrote:
> > On Tue, Dec 11, 2018 at 8:54 AM Jakub Jelinek <jakub@redhat.com> wrote:
> > >
> > > On Tue, Dec 11, 2018 at 08:44:00AM +0100, Uros Bizjak wrote:
> > > > > --- gcc/config/i386/i386.md.jj  2018-11-22 10:40:31.179683319 +0100
> > > > > +++ gcc/config/i386/i386.md     2018-12-10 11:24:49.785830186 +0100
> > > > > @@ -17195,6 +17195,24 @@ (define_insn "*x86_mov<mode>cc_0_m1_neg"
> > > > >     (set_attr "mode" "<MODE>")
> > > > >     (set_attr "length_immediate" "0")])
> > > > >
> > > > > +(define_insn_and_split "*x86_mov<SWI48:mode>cc_0_m1_neg_leu<SWI:mode>"
> > > > > +  [(set (match_operand:SWI48 0 "register_operand" "=r")
> > > > > +       (neg:SWI48
> > > > > +         (leu:SWI48
> > > > > +           (match_operand:SWI 1 "nonimmediate_operand" "<SWI:r>m")
> > > > > +           (match_operand:SWI 2 "<SWI:immediate_operand>" "<SWI:i>"))))
> > > >
> > > > You can use const_int_operand predicate with "n" constraint here.
> > >
> > > The point was to disallow 64-bit constants, so if I use const_int_operand
> > > above, I'd need to replace CONST_INT_P (operands[2]) test with
> > > trunc_int_for_mode (INTVAL (operands[2]), SImode) == INTVAL (operands[2])
> > > or similar, perhaps do it for the 64-bit mode only, so
> > >   (<SWI:MODE>mode != DImode
> > >    || (trunc_int_for_mode (INTVAL (operands[2]), SImode)
> > >        == INTVAL (operands[2])))
> >
> > The above is the preferred way (we already have a couple of instances
> > in predicates, where trunc_int_for_mode is used for the above case).
> > I'd recommend to put everything in the preparation statement and FAIL
> > in case the value is not supported.
> > Like:
> >
> > {
> >   HOST_WIDE_INT val = UINTVAL operands[2];
> >   if (trunc_int_for_mode (val, SImode) != val || val != -1)
> >   FAIL;
> >  ...
> > }
>
> If it is in preparation statements, then if it does FAIL, then it will ICE,
> because it matched as insn already, the FAIL would just mean it couldn't be
> split and we then would need to provide some pattern to handle it.

OK, let's go with your original patch then. Other approaches are more
complex that the original patch.

Thanks,
Uros.