[PATCH] D78430: [InstSimplify] fold and/or of compares with equality to min/max constant

[Sanjay Patel via Phabricator via llvm-commits]

spatel marked an inline comment as done.
spatel added inline comments.


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:1675
+static Value *simplifyAndOrOfICmpsWithLimitConst(ICmpInst *Cmp0, ICmpInst *Cmp1,
+                                                 bool IsAnd) {
+  // Canonicalize an equality compare as Cmp0.
----------------
spatel wrote:
> lebedev.ri wrote:
> > You literally always check `!IsAnd`.
> > Should this be `IsOr` instead?
> I thought someone would call that out. :)
> My mind defaulted to the 'and' versions of the patterns when visualizing the logic, so that's how the code ended up like this. If we are comfortable inverting the pattern matching to the 'or' versions, then we can invert the first check at least to the positive 'IsAnd'. 
> Another option would duplicate all of the code predicated by the 'IsAnd' check.
> The alternative - make this 'IsOr' - would mean this function stands different/alone compared to all of the related compare simplifications. That didn't seem like a worthy option to me.
> Let me know which, if any, form you'd prefer.
Another option - we can change the later return lines to:
      return IsAnd ? getFalse(Cmp0->getType()) : getTrue(Cmp0->getType());



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

https://reviews.llvm.org/D78430