[PATCH] D75807: [InstCombine] fold gep-of-select-of-constants (PR45084)

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:1816
+                                               m_Constant(FalseC))))
+    return nullptr;
+
----------------
Should the select be limited to one use? I'm not sure whether this transform is favorable if we need to keep both selects. In either case, this should be tested.


================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:1822
+  TrueC = ConstantExpr::getGetElementPtr(GEPSrcTy, TrueC, IndexC);
+  FalseC = ConstantExpr::getGetElementPtr(GEPSrcTy, FalseC, IndexC);
+  return SelectInst::Create(Cond, TrueC, FalseC);
----------------
It is better to construct these GEPs via IRBuilder, so the resulting constants get folded right away (GEPs are target-dependent).

Also, aren't we losing inbounds here?


================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:1823
+  FalseC = ConstantExpr::getGetElementPtr(GEPSrcTy, FalseC, IndexC);
+  return SelectInst::Create(Cond, TrueC, FalseC);
+}
----------------
We can preserve branch weight metadata here.


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

https://reviews.llvm.org/D75807