[llvm-commits] [llvm] r72364 - in /llvm/trunk: include/llvm/Analysis/ScalarEvolutionExpander.h lib/Analysis/ScalarEvolutionExpander.cpp lib/Transforms/Scalar/IndVarSimplify.cpp test/Transforms/IndVarSimplify/2009-05-24-useafterfree.ll

[Török Edwin]

On 2009-05-24 22:55, Dan Gohman wrote:
> On May 24, 2009, at 12:41 PM, Török Edwin wrote:
>
>   
>> On 2009-05-24 22:05, Török Edwin wrote:
>>     
>>> Maybe instead of AssertingVH we could use CallbackVH, and have it  
>>> assert
>>> only if you
>>> want to dereference a deleted Value*. Does that sound reasonable?
>>>
>>>       
>> Actually I think the Rewriter.clear() could be moved before those  
>> last 2
>> functions that use the Rewriter.
>> FixUsesBeforeDefs and SinkUnusedInvariants only need  
>> isInsertedInstruction.
>> That one is uses the InsertedValues set which doesn't use AssertingVH,
>> it is also not cleared by Rewriter.clear().
>>
>> In fact only RewriteIVExpressions needs isInsertedExpression, so I  
>> think
>> it would be safe to clear the Rewriter map right after calling
>> RewriteIVExpressions.
>>     
>
> Ok, this sounds reasonable.  I'm concerned about the
> RecursivelyDeleteTriviallyDeadInstructions call at the end of
> RewriteIVExpressions, but this can be covered by making the
> clear() happen even earlier.
>   

I've put the .clear() at the end of RewriteIVExpressions, before the
deletion loop.
Please see r72372.

Best regards,
--Edwin