[llvm-commits] [llvm] r52459 - in /llvm/trunk: lib/Transforms/IPO/DeadArgumentElimination.cpp test/Transforms/DeadArgElim/multdeadretval.ll

[Dan Gohman]

On Jun 18, 2008, at 4:13 AM, Matthijs Kooijman wrote:
> +/// MarkLive - Mark the given return value or argument as live.  
> Additionally,
> +/// mark any values that are used by this value (according to Uses)  
> live as
> +/// well.
> +void DAE::MarkLive(RetOrArg RA) {
> +  if (!LiveValues.insert(RA).second)
> +    return; // We were already marked Live
>
> +  if (RA.IsArg)
> +    DOUT << "DAE - Marking argument " << RA.Idx << " to function "  
> << RA.F->getNameStart() << " live\n";
> +  else
> +    DOUT << "DAE - Marking return value " << RA.Idx << " of  
> function " << RA.F->getNameStart() << " live\n";
>
> +  std::pair<UseMap::iterator, UseMap::iterator> Range =  
> Uses.equal_range(RA);
> +  UseMap::iterator E = Range.second;
> +  UseMap::iterator I = Range.first;
> +  for (; I != E; ++I)
> +    MarkLive(I->second);
> +  // Erase RA from the Uses map (from the lower bound to wherever  
> we ended up
> +  // after the loop).
> +  Uses.erase(Range.first, Range.second);
> }

I investigated this a little and found the subtle bug in this
bit of code that's triggering the infinite looping behavior.
The erase call is invalidating the Range.second iterator in
a MarkLive call earlier in the recursion. I guess your
original code which used lower_bound and just tested the
key at each iteration was a better approach after all, and I
shouldn't have suggested you change it :-}.

Dan