[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 added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:566
+///   select ((cmp load V1, load V2), V1, V2).
+static bool isMinMaxPattern(Value *V) {
+  assert(V->getType()->isPointerTy() && "Expected pointer type.");
----------------
Why do we need to match min/max? Does something break if we just match select?

If we need to restrict to min/max, this isn't the way to do it. Use pattern matches (m_c_SMax, etc)  or value tracking's matchSelectPattern().


================
Comment at: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:1333
+/// select ((cmp load V1, load V2), V1, V2).
+bool decanonicalizeLoadStoreOnSelect(InstCombiner &IC, StoreInst &SI) {
+  // bitcast?
----------------
It doesn't make sense to call this  "decanonicalize". Whatever we choose to do here is redefining canonical. "removeBitcastsFromLoadStoreOnSelect"?


https://reviews.llvm.org/D40304