[PATCH] D108408: [InstCombine] Transform X == 0 ? 0 : X * Y --> X * freeze(Y).

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:738-741
+  auto *TrueValC = dyn_cast<Constant>(TrueVal);
+  if (TrueValC == nullptr ||
+      !match(FalseVal, m_c_Mul(m_Specific(X), m_Value(Y))))
+    return nullptr;
----------------
spatel wrote:
> I'm still confused about this: we want to match a zero constant as the TrueVal based on the block comment for this function, so why not use m_Zero()?
> Usually, we can match vectors that include undef elements, but we may not be able to propagate those undefs through a transform (typically, we would replace with a ConstantInt::getNullValue() to get a "full" zero).
> 
> If the partial undef logic is complicated, I think that would be ok to defer to a follow-up patch. Ie, let's get the basic/common case right, then look at corner cases if that really matters.
Please read previous review comments in this patch. We are going in circles here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108408