On Wed, Oct 14, 2020 at 11:49 AM Jakub Jelinek <ja...@redhat.com> wrote:
>
> On Wed, Oct 14, 2020 at 11:22:48AM +0200, Richard Biener wrote:
> > > + if (mode == CCCmode
> > > + && GET_CODE (XEXP (x, 0)) == NEG
> > > + && GET_CODE (XEXP (XEXP (x, 0), 0)) == GEU
> > > + && REG_P (XEXP (XEXP (XEXP (x, 0), 0), 0))
> > > + && (GET_MODE (XEXP (XEXP (XEXP (x, 0), 0), 0)) == CCCmode
> > > + || GET_MODE (XEXP (XEXP (XEXP (x, 0), 0), 0)) == CCmode)
> > > + && REGNO (XEXP (XEXP (XEXP (x, 0), 0), 0)) == FLAGS_REG
> > > + && XEXP (XEXP (XEXP (x, 0), 0), 1) == const0_rtx
> > > + && GET_CODE (XEXP (x, 1)) == LTU
> > > + && REG_P (XEXP (XEXP (x, 1), 0))
> > > + && (GET_MODE (XEXP (XEXP (x, 1), 0))
> > > + == GET_MODE (XEXP (XEXP (XEXP (x, 0), 0), 0)))
> > > + && REGNO (XEXP (XEXP (x, 1), 0)) == FLAGS_REG
> > > + && XEXP (XEXP (x, 1), 1) == const0_rtx)
> >
> > Meh ;) templates to the rescue?
> >
> > rtx_match < un<NEG, bin<GEU, reg, ..> > ().matches (x)
> >
> > and with fancy metaprogramming expand it to above? Not sure if it's easier
> > to read that way. Maybe
>
> It would certainly not match the style used elsewhere in the backend.
> >
> > rtx neg, geu;
> > if (mode == CCCmode
> > && (neg = XEXP (x, 0), GET_CODE (neg) == NEG)
> > && (geu = XEXP (neg, 0), GET_CODE (geu) == GEU)
> > ...
> >
> > or
> >
> > if (mode == CCCmode
> > && GET_CODE (neg = XEXP (x, 0)) == NEG
> >
> > thus some manual CSE and naming in this matching would help?
>
> Attached are two incremental patches, one just adds op0 and op1 for the
> COMPARE operand of all the costs COMPARE handling, which replaces all the
> XEXP (x, 0) with op0 and XEXP (x, 1) with op1, the other is that plus
> the geu you've suggested.
OK, so the visual appearance is not very much improved. I guess
the main issue is the checks do not "align" with a view of the expression
but I can't see how to improve that. When one actually looks at
the tests the CSEd vairants are easier to match-up and I'd pick the geu one.
Eventually intermixed comments would help the casual reader?
+ rtx geu;
/* (neg:CCC */
if (mode == CCCmode
+ && GET_CODE (op0) == NEG
/* (geu (reg:CC[C] cc) const0)) */
+ && GET_CODE (geu = XEXP (op0, 0)) == GEU
+ && REG_P (XEXP (geu, 0))
+ && (GET_MODE (XEXP (geu, 0)) == CCCmode
+ || GET_MODE (XEXP (geu, 0)) == CCmode)
+ && REGNO (XEXP (geu, 0)) == FLAGS_REG
+ && XEXP (geu, 1) == const0_rtx
/* (LTU (reg:CCCmode cc) const0)) */
+ && GET_CODE (op1) == LTU
+ && REG_P (XEXP (op1, 0))
+ && GET_MODE (XEXP (op1, 0)) == GET_MODE (XEXP (geu, 0))
+ && REGNO (XEXP (op1, 0)) == FLAGS_REG
+ && XEXP (op1, 1) == const0_rtx)
I'll leave the actual review to Uros of course.
Richard.
>
> Jakub
