[PATCH] D142847: [InstCombine] reduce icmp_eq0-of-and-of-select-of-constants

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:1896
+  // TODO: Generalize for non-constant values.
+  // TODO: Invert with a "ne" predicate.
+  // TODO: Extend to handle a non-zero compare constant.
----------------
spatel wrote:
> goldstein.w.n wrote:
> > You could also match other predicates i.e `((x?1:4)&(y?1:4))<2`
> > 
> > Also it seems we already have some logic for match some other predicates in this case (although I can't find it):
> > https://godbolt.org/z/hjzMGKqKz
> > 
> > although it seems to have an off by-1 issue (see `ugt` and `ugt3`)
> From what I see in the debug spew, some of those examples reduce via demanded bits - we shrink a select constant because some bit(s) is/are irrelevant given the compare constants. That leads to further folding via FoldOpIntoSelect or some other transform. 
> 
> But there's no general transform along these lines that I see. Ie, if we choose some other arbitrary set of constants in the tests, it may or may not fold.
> From what I see in the debug spew, some of those examples reduce via demanded bits - we shrink a select constant because some bit(s) is/are irrelevant given the compare constants. That leads to further folding via FoldOpIntoSelect or some other transform. 
> 
> But there's no general transform along these lines that I see. Ie, if we choose some other arbitrary set of constants in the tests, it may or may not fold.

I see, then I would definetly add as a TODO to handle "other predicates" instead of just "ne".


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142847/new/

https://reviews.llvm.org/D142847