This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] [match.pd]: missing optimization on comparison


Hello,

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

 (simplify
  (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.

+ (simplify
+  (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 a reviewer)

--
Marc Glisse


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]