[PATCH] D40304: [InstCombine] PR35354: Convert load bitcast (select (Cond, &V1, &V2)) --> select(Cond, load bitcast &V1, load bitcast &V2)

[Sanjay Patel via Phabricator via llvm-commits]

spatel accepted this revision.
spatel added a comment.
This revision is now accepted and ready to land.

I don't see a better way to avoid the problem, so LGTM. See inline for some small issues. Wait a day to commit in case anyone else has ideas.



================
Comment at: include/llvm/IR/PatternMatch.h:960-979
+// Matcher for LoadInst classes
+//
+
+template <typename Op_t> struct LoadClass_match {
+  Op_t Op;
+
+  LoadClass_match(const Op_t &OpMatch) : Op(OpMatch) {}
----------------
Please commit this part as an NFC preliminary step.


================
Comment at: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:568
+///   select ((cmp load V1, load V2), V1, V2).
+static bool isMinMaxPattern(Value *V) {
+  assert(V->getType()->isPointerTy() && "Expected pointer type.");
----------------
I still can't read this without thinking it's a duplicate of something in matchSelectPattern(). Include "load" in the name? "isMinMaxWithLoads()"?


================
Comment at: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:1377
 
+  // Try to decanonicalize store (bitcast (load (bitcast (select ...)))).
+  if (removeBitcastsFromLoadStoreOnMinMax(*this, SI))
----------------
Remove comment - the description above the function body is enough.


https://reviews.llvm.org/D40304