This is the mail archive of the
mailing list for the GCC project.
Re: [patch] [match.pd]: missing optimization on comparison
- From: Marc Glisse <marc dot glisse at inria dot fr>
- To: Kai Tietz <ktietz70 at googlemail dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Thu, 30 Aug 2018 22:20:06 +0200 (CEST)
- Subject: Re: [patch] [match.pd]: missing optimization on comparison
- References: <CAEwic4bhtQECeixVzcFmgo5C1mHqGCOPB+L=qy8xrRFYYfvZxg@mail.gmail.com> <CAEwic4Z=iKpSohF1xy3WPjSvCCUT6s+Q42dfc+yY1Z03ztxn3g@mail.gmail.com>
- Reply-to: gcc-patches at gcc dot gnu dot org
why not put these right next to the one that does x==0 & y==0?
INTEGRALS_SIGN_PREC_MATCH: the name doesn't really match the semantics.
You may need to add a bunch of :s.
I always wonder if such transformations should be blocked until a late
gimple pass so we keep the more natural form longer, but we haven't done
that for other similar transformations, so probably not. Does VRP handle
the new form as well as the old one?
For all the a < 0 or a >= 0 comparisons, I wonder if we should check that
the types are signed, instead of checking that both have the same
signedness. If the transformations are in the right order, we should never
reach this anyway since for unsigned it should have been folded to
true/false already, though it is always better to be safe.
I tend to prefer integer_all_onesp to integer_minus_onep here. You don't
handle complex, but if you did, that would be the right test, so we might
as well use it.
I probably would have lazily written something like
(bit_and:c (eq@3 (bit_and:c @0 @1) integer_minus_onep@2) (cmp @0 INTEGER_CST@4))
(bit_and @3 (cmp @2 @4)))
(maybe with some checks) to avoid handling all the cases manually, but
that doesn't mean that handling them is wrong.
It should be possible to merge the 4 (a cmp 0) &| (b cmp 0) into just 1 or
2 using "for", unless you find it unreadable.
+ (bit_and:c (ge (bit_ior:c @0 @1) integer_zerop@2) (cmp @0 integer_zerop))
+ (bit_and (cmp @0 @2) (ge @1 @2))))
You can name the comparison in the input so you don't have to reconstruct
(cmp @0 @2) in the output. Same for "((a & b) < 0) & (a < 0)", etc.
The case cmp == GE_EXPR will be immediately folded back to the original
(@0|@1)>=0, so the formulation is mostly to save on the number of patterns
(I am not complaining, as you can see above I am often in favor of
writing less and letting further transformation finish the job).
We could probably transform more generally a==cst & cond(a) into a==cst &
cond(cst), but that's not easy to formulate.
I was surprised not to see the impossible cases like ((a & b) < 0) & (a >=
0), but I don't mind their absence.
Other than those details, I think the patch looks good, thanks. (I am not