[PATCH] D38369: [InstSimplify] teach SimplifySelectInst() to fold more vector selects

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: lib/Analysis/InstructionSimplify.cpp:3585
+      if (Constant *CF = dyn_cast<Constant>(FalseVal))
+        return ConstantExpr::getSelect(CB, CT, CF);
     if (CB->isAllOnesValue())
----------------
haicheng wrote:
> spatel wrote:
> > I think we should use ConstantFoldSelectInstruction() here (to be more efficient?). That's the interface for other opcodes from what I can see in other parts of the file.
> I think I cannot use ConstantFoldSelectInstruction() which is declared in IR/ConstantFold.h rather than llvm/Analysis/ConstantFolding.h.  From the comments in IR/ConstantFold.h, ConstantFoldSelectInstruction() is used internal to ConstantExpr::get* methods.
> 
> Many ConstantFoldxxx methods used in this file are declared in llvm/Analysis/ConstantFolding.h.  I also find several places in this file calling ConstantExpr::get* methods.
Why not just add a line to ConstantFolding.h with the definition of ConstantFoldSelectInstruction()?

I'm looking at:
SimplifyExtractValueInst
SimplifyInsertValueInst
SimplifyExtractElementInst
SimplifyCastInst
etc.

They are all using this pattern, so why should SimplifySelectInst() be different? I think your way is fine too, but if we do that, then we should change everything else to be consistent.


Repository:
  rL LLVM

https://reviews.llvm.org/D38369