[llvm] r275357 - [SCCP] Generalize tryToReplaceInstWithConstant to work also with arguments.

[Davide Italiano via llvm-commits]

On Wed, Jul 13, 2016 at 6:51 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
> On Wed, Jul 13, 2016 at 6:27 PM, Davide Italiano via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
>>
>> Author: davide
>> Date: Wed Jul 13 20:27:29 2016
>> New Revision: 275357
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=275357&view=rev
>> Log:
>> [SCCP] Generalize tryToReplaceInstWithConstant to work also with
>> arguments.
>>
>> Modified:
>>     llvm/trunk/lib/Transforms/Scalar/SCCP.cpp
>>
>> Modified: llvm/trunk/lib/Transforms/Scalar/SCCP.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/SCCP.cpp?rev=275357&r1=275356&r2=275357&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/Scalar/SCCP.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Scalar/SCCP.cpp Wed Jul 13 20:27:29 2016
>> @@ -1510,16 +1510,16 @@ bool SCCPSolver::ResolvedUndefsIn(Functi
>>    return false;
>>  }
>>
>> -static bool tryToReplaceInstWithConstant(SCCPSolver Solver, Instruction
>> *Inst,
>> -                                         bool shouldEraseFromParent) {
>> +template <typename ArgOrInst>
>> +static bool tryToReplaceWithConstant(SCCPSolver Solver, ArgOrInst *AI) {
>
>
> Can you just use a Value* here?
>

Sure, I'll change that.

>>
>>    Constant *Const = nullptr;
>> -  if (Inst->getType()->isStructTy()) {
>> -    std::vector<LatticeVal> IVs = Solver.getStructLatticeValueFor(Inst);
>> +  if (AI->getType()->isStructTy()) {
>> +    std::vector<LatticeVal> IVs = Solver.getStructLatticeValueFor(AI);
>>      if (std::any_of(IVs.begin(), IVs.end(),
>>                      [](LatticeVal &LV) { return LV.isOverdefined(); }))
>>        return false;
>>      std::vector<Constant *> ConstVals;
>> -    StructType *ST = dyn_cast<StructType>(Inst->getType());
>> +    StructType *ST = dyn_cast<StructType>(AI->getType());
>>      for (unsigned i = 0, e = ST->getNumElements(); i != e; ++i) {
>>        LatticeVal V = IVs[i];
>>        ConstVals.push_back(V.isConstant()
>> @@ -1528,18 +1528,23 @@ static bool tryToReplaceInstWithConstant
>>      }
>>      Const = ConstantStruct::get(ST, ConstVals);
>>    } else {
>> -    LatticeVal IV = Solver.getLatticeValueFor(Inst);
>> +    LatticeVal IV = Solver.getLatticeValueFor(AI);
>>      if (IV.isOverdefined())
>>        return false;
>> -
>> -    Const =
>> -        IV.isConstant() ? IV.getConstant() :
>> UndefValue::get(Inst->getType());
>> +    Const = IV.isConstant() ? IV.getConstant() :
>> UndefValue::get(AI->getType());
>>    }
>>    assert(Const && "Constant is nullptr here!");
>> -  DEBUG(dbgs() << "  Constant: " << *Const << " = " << *Inst << '\n');
>> +  DEBUG(dbgs() << "  Constant: " << *Const << " = " << *AI << '\n');
>>
>>    // Replaces all of the uses of a variable with uses of the constant.
>> -  Inst->replaceAllUsesWith(Const);
>> +  AI->replaceAllUsesWith(Const);
>> +  return true;
>> +}
>> +
>> +static bool tryToReplaceInstWithConstant(SCCPSolver Solver, Instruction
>> *Inst,
>> +                                         bool shouldEraseFromParent) {
>> +  if (!tryToReplaceWithConstant(Solver, Inst))
>> +    return false;
>>
>>    // Delete the instruction.
>>    if (shouldEraseFromParent)
>> @@ -1766,17 +1771,8 @@ static bool runIPSCCP(Module &M, const D
>>          // TODO: Could use getStructLatticeValueFor to find out if the
>> entire
>>          // result is a constant and replace it entirely if so.
>>
>> -        LatticeVal IV = Solver.getLatticeValueFor(&*AI);
>> -        if (IV.isOverdefined()) continue;
>> -
>> -        Constant *CST = IV.isConstant() ?
>> -        IV.getConstant() : UndefValue::get(AI->getType());
>> -        DEBUG(dbgs() << "***  Arg " << *AI << " = " << *CST <<"\n");
>> -
>> -        // Replaces all of the uses of a variable with uses of the
>> -        // constant.
>> -        AI->replaceAllUsesWith(CST);
>> -        ++IPNumArgsElimed;
>> +        if (tryToReplaceWithConstant(Solver, &*AI))
>> +          ++IPNumArgsElimed;
>>        }
>>      }
>
>
> Testcase?
>

Sorry if I failed to mention on this commit, but this is NFC as we
skip the replacement phase if the argument is of struct type (unless
I'm missing something).
My next patch, http://reviews.llvm.org/D22329 , changes the behaviour
and add test cases.


Thanks for your review!

-- 
Davide

"There are no solved problems; there are only problems that are more
or less solved" -- Henri Poincare