On 9/10/19 1:19 PM, Marc Glisse wrote: > (some quick comments, I didn't check in details)
Thanks for them. > > + (and:c (code1 @0 INTEGER_CST@1) (code2 @0 INTEGER_CST@2)) > [...] > + (if (code1 == NE_EXPR && !val) (code2 @0 @2)))))))) > > How about > > (and:c (code1 @0 INTEGER_CST@1) (code2@3 @0 INTEGER_CST@2)) > [...] > (if (code1 == NE_EXPR && !val) @3))))))) > > instead? This is also a way to check how well the generator handles this, > when @3 may not really exist (was in a gimple_cond). I like the changes. I applied them to all patterns in the patch set. > > (and:c (eq@3 @0 INTEGER_CST@1) (code2 @0 INTEGER_CST@2)) > > could be simplified to > > (and @3 (code2 @1 @2)) > > always (a trivial transform) and let the rest of the machinery fold code2 on > constants and then and with a constant. This way you would only need to > handle code1==NE_EXPR in the complicated transform, it might be slightly more > readable. (I am not saying we absolutely have to do it this way, it is just a > suggestion) That would be possible, I can experiment with that after the patch set is applied. The current patch is more or less direct transformation of what we have in gimple-fold.c. > > + (and (code1:c @0 INTEGER_CST@1) (code2:c @0 INTEGER_CST@2)) > > Don't we canonicalize 3 < X to X > 3? That would make the :c unnecessary. > Actually I don't remember how :c works on non-commutative operations (there > was also :C I think). Dunno here, Richi? > > + /* Chose the more restrictive of two < or <= comparisons. */ > > Choose? Yep. Thank you, Martin >
