[PATCH] D25126: [InstCombine] fold select X, (ext X), C

David Majnemer via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 7 10:35:11 PDT 2016


majnemer added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineSelect.cpp:966
+      Constant *AllOnesOrOne = ConstantExpr::getCast(ExtOpcode, One, SelType);
+      NewSel = SelectInst::Create(Cond, AllOnesOrOne, C);
+    } else {
----------------
bjope wrote:
> Why aren't you using the Builder here? Is it to avoid an implicit fold by the CreateSelect?
> To me it looks like you are using a different style compared to the optimization above involving the trunc.
> 
> And I guess that if you use the Builder, then you won't need to explicitly copyMetadata? Or is that missing from the optimization involving the trunc. There was not a single explicit copyMetadata in this file until now.
The code added is in the style of the rest of InstCombine. The builder is used for intermediate instructions but not the final instruction which is RAUW. This has to do with the particular behavior InstCombine has augmented the builder with.

I'm pretty sure the copyMetadata is around to make sure we don't discard the !prof. Other select combines probably drop this information.


https://reviews.llvm.org/D25126





More information about the llvm-commits mailing list