Re: Canonicalization of compares performed as side-effect operations

["Richard Earnshaw (lists)" <Richard dot Earnshaw at arm dot com>]

On 06/08/2019 17:22, Richard Earnshaw (lists) wrote:
> On 06/08/2019 17:17, Segher Boessenkool wrote:
> > Hi Richard,
> >
> > On Tue, Aug 06, 2019 at 04:35:04PM +0100, Richard Earnshaw (lists) wrote:
> > > Arm has an instruction that performs the following operation:
> > >
> > > (parallel [
> > >          (set (reg:CC 100 cc)
> > >              (compare:CC (const_int 0 [0])
> > >                  (reg:SI 121)))
> > >          (set (reg:SI 113)
> > >              (neg:SI (reg:SI 121)))
> > >      ])
> > >
> > > This is simply a reverse subtract from the constant zero, and setting
> > > the condition flags.  It's the low part of a negdi2 expansion.
> > >
> > > However, combine will rip this up and try to transform the compare into
> > > 'canonical' form, ie
> > >
> > > (parallel [
> > >          (set (reg:CC 100 cc)
> > >              (compare:CC (reg:SI 121)
> > >                  (const_int 0 [0])))
> > >          (set (reg:SI 113)
> > >              (neg:SI (reg:SI 121)))
> > >      ])
> > >
> > > (and obviously swapping the condition on the instruction that uses the
> > > comparison result).
> > >
> > > This, of course, doesn't match the behaviour of the instruction and
> > > no-longer matches the pattern in the md file.
> >
> > It is, however, canonical RTL:
> >
> > (from md.texi:)
> >
> >    In addition to algebraic simplifications, following canonicalizations
> >    are performed:
> >
> >    @itemize @bullet
> >    @item
> >    For commutative and comparison operators, a constant is always made 
> > the
> >    second operand.  If a machine only supports a constant as the second
> >    operand, only patterns that match a constant in the second operand 
> > need
> >    be supplied.
> >
> > Putting the constant first is non-canonical RTL and will in general not
> > match any instructions generated by GCC.
> >
> > > So is there a way to describe this instruction within the compiler, or a
> > > way to stop simplify_set from making this sort of simplification?
> >
> > What's wrong with describing the canonical form in your MD?  You'll need
> > some reversed condition code thingy, but that's it?
>
> It doesn't describe what the instruction does.  The negation has a side 
> effect of setting the flags, but the flags are swapped because the 
> side-effect comparison is swapped from a normal compare.  As I 
> mentioned, SELECT_CC_MODE doesn't help because it can't see the context 
> and the comparison just looks 'normal'.
>
> R.


See, for example, this comment in combine.c:

  /* Many machines that don't use CC0 have insns that can both perform an
     arithmetic operation and set the condition code.  These operations 
will
     be represented as a PARALLEL with the first element of the vector
     being a COMPARE of an arithmetic operation with the constant zero.
     The second element of the vector will set some pseudo to the result
     of the same arithmetic operation.  If we simplify the COMPARE, we 
won't
     match such a pattern and so will generate an extra insn.   Here we 
test
     for this case, where both the comparison and the operation result are
     needed, and make the PARALLEL by just replacing I2DEST in I3SRC with
     I2SRC.  Later we will make the PARALLEL that contains I2.  */

This is exactly the scenario we have here, but despite the comment, this 
is still happening in other places in combine.