[llvm-commits] [llvm] r52520 - /llvm/trunk/lib/Transforms/Scalar/SCCP.cpp

[Matthijs Kooijman]

Hi Dan,

good to see SCCP now knows about insert/extractvalue, that means I can remove
my local hack to suppress its warnings :-)

Yet, isn't the handling it does now (mainly tracking return values) a
duplicate effort with IPConstProp? Or does SCCP do other things that make it
more useful than IPCP for this stuff?

> +void SCCPSolver::visitExtractValueInst(ExtractValueInst &EVI) {
> +  Value *Aggr = EVI.getOperand(0);
You'd better use getAggregateOperand() for that?


> +
> +  // If the operand to the getresult is an undef, the result is undef.
> +  if (isa<UndefValue>(Aggr))
> +    return;
> +
> +  // Currently only handle single-index extractvalues.
> +  if (EVI.getNumIndices() != 1) {
> +    markOverdefined(&EVI);
> +    return;
> +  }
> +  
> +  Function *F = 0;
> +  if (CallInst *CI = dyn_cast<CallInst>(Aggr))
> +    F = CI->getCalledFunction();
> +  else if (InvokeInst *II = dyn_cast<InvokeInst>(Aggr))
> +    F = II->getCalledFunction();
> +
> +  // TODO: If IPSCCP resolves the callee of this function, we could propagate a
> +  // result back!
What do you mean by this? Are you considering the case where you visit the
extractvalue instruction on the result before the insertvalue instruction for
the return value?

> +void SCCPSolver::visitInsertValueInst(InsertValueInst &IVI) {
> +  Value *Aggr = IVI.getOperand(0);
Again, you should use getAggregateOperand().
> +  Value *Val = IVI.getOperand(1);
And getInsertedValueOperand() here.

> +  // If the operand to the getresult is an undef, the result is undef.
That should be insertvalue, not getresult?

> +  if (isa<UndefValue>(Aggr))
> +    return;
> +
> +  // Currently only handle single-index insertvalues.
> +  if (IVI.getNumIndices() != 1) {
> +    markOverdefined(&IVI);
> +    return;
> +  }
> +  
> +  // See if we are tracking the result of the callee.
> +  Function *F = IVI.getParent()->getParent();
> +  std::map<std::pair<Function*, unsigned>, LatticeVal>::iterator
> +    It = TrackedMultipleRetVals.find(std::make_pair(F, *IVI.idx_begin()));
> +
> +  // Merge in the inserted member value.
> +  if (It != TrackedMultipleRetVals.end())
> +    mergeInValue(It->second, F, getValueState(Val));
> +
> +  // Mark the aggregate result of the IVI overdefined; any tracking that we do will
> +  // be done on the individual member values.
> +  markOverdefined(&IVI);
> +}
From looking at this code, it seems that you are assuming that all insertvalue
instructions are used to build return values. From the looks of it, I could
fool SCCP into propagting the wrong constant, though I don't really know how
SCPP works.

> +      // The aggregate value is used in a way not handled here. Assume nothing.
> +      markOverdefined(*UI);
Shouldn't that be return value isntead of aggregate value?

Gr.

Matthijs
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20080620/11c7568a/attachment.sig>