[PATCH] D34921: [ConstantHoisting] Remove dupliate logic in constant hoisting

[Leo Li via Phabricator via llvm-commits]

aoli added a comment.

For `alloca` instruction, shall we add special case here for constant hoisting? I saw some test cases which test constant hoisting for alloca instruction.



================
Comment at: lib/Transforms/Scalar/ConstantHoisting.cpp:412
+    // only and intrinsics which can not be freely called with constants. So
+    // it's safe for us to collect constant candidates from all IntrinsicInsts.
+    if (canReplaceOperandWithVariable(Inst, Idx)
----------------
efriedma wrote:
> Are you sure this is true...?  For example, llvm.memcpy has an "len" parameter which can be hoisted, and an "align" parameter which can't.
> 
> Or maybe I'm misinterpreting what this comment is talking about?
Yes, I checked all implementations of  method ` int getIntImmCost(Intrinsic::ID IID, unsigned Idx, const APInt &Imm, Type *Ty);`. There isn't any intrinsic which takes constant parameters only and can not be called freely with constant parameters.

The parameters in `llvm.memcpy` will never be hoisted because the cost of getting intrinsic immediate is always free for all targets. This is guaranteed in line 310

```
void ConstantHoistingPass::collectConstantCandidates(
    ConstCandMapType &ConstCandMap, Instruction *Inst, unsigned Idx,
    ConstantInt *ConstInt) {
  unsigned Cost;
  // Ask the target about the cost of materializing the constant for the given
  // instruction and operand index.
  if (auto IntrInst = dyn_cast<IntrinsicInst>(Inst))
    Cost = TTI->getIntImmCost(IntrInst->getIntrinsicID(), Idx,
                              ConstInt->getValue(), ConstInt->getType());
  else
    Cost = TTI->getIntImmCost(Inst->getOpcode(), Idx, ConstInt->getValue(),
                              ConstInt->getType());

  // Ignore cheap integer constants.
  if (Cost > TargetTransformInfo::TCC_Basic) {
```



https://reviews.llvm.org/D34921