[PATCH] D109807: [InstCombine] Narrow type of logical operation chains in certain cases

[Usman Nadeem via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:1613
   CastInst *Cast0 = dyn_cast<CastInst>(Op0);
-  if (!Cast0)
-    return nullptr;
+  if (!Cast0) {
+    std::swap(Op0, Op1);
----------------
spatel wrote:
> It's not clear to me why this swap is necessary. Do you have a test case where a logic binop has a cast as operand 0 and a binop as operand 1? Complexity-based canonicalization is supposed to prevent that. See InstCombinerImpl::SimplifyAssociativeOrCommutative().
I think you have it the other way around, the code currently only checks for the cast to be on the LHS.
This swap handles the case when the Op0 (higher complexity) is another binop.


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

https://reviews.llvm.org/D109807