[llvm-commits] [llvm] r164636 - /llvm/trunk/lib/Transforms/Scalar/SROA.cpp

Chandler Carruth chandlerc at google.com
Wed Sep 26 00:15:40 PDT 2012


On Tue, Sep 25, 2012 at 11:45 PM, Chandler Carruth <chandlerc at google.com>wrote:

> On Tue, Sep 25, 2012 at 2:50 PM, Nick Lewycky <nicholas at mxc.ca> wrote:
>
>> Author: nicholas
>> Date: Tue Sep 25 16:50:37 2012
>> New Revision: 164636
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=164636&view=rev
>> Log:
>> Revert the business end of r164634, and replace it with a different fix.
>> The
>> reason we were getting two of the same alloca is because of a
>> memmove/memcpy
>> which had the same alloca in both the src and dest. Now we detect that
>> case
>> directly. This has the same testcase as before, but fixes a clang test
>> CodeGenObjC/exceptions.m which runs clang -O2.
>>
>> Modified:
>>     llvm/trunk/lib/Transforms/Scalar/SROA.cpp
>>
>> Modified: llvm/trunk/lib/Transforms/Scalar/SROA.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/SROA.cpp?rev=164636&r1=164635&r2=164636&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/Scalar/SROA.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Scalar/SROA.cpp Tue Sep 25 16:50:37 2012
>> @@ -2228,7 +2228,10 @@
>>      // alloca that should be re-examined after rewriting this
>> instruction.
>>      if (AllocaInst *AI
>>            = dyn_cast<AllocaInst>(OtherPtr->stripInBoundsOffsets()))
>> -      Pass.Worklist.insert(AI);
>> +      // Don't revisit the alloca if both sides of the memory transfer
>> are
>> +      // referring to the same alloca.
>> +      if (AI != &NewAI)
>> +        Pass.Worklist.insert(AI);
>>
>
> Also, this doesn't make a lot of sense to me. Worklist is a SetVector, and
> we insert NewAI into it anyways... Were you trying to do something else
> here? I don't want to misunderstand the intent of the patch.
>

Heh, this happens to work with the test case we have because of other
factors. It just so happens that this is one of the cases where NewAI ==
OldAI, and thus isn't automatically re-queued for visiting, and this
prevents ever revisiting it. Sorry for the noise, but I understand this now.

That said, I also understand the real problem and fix. I'll be reverting
this and applying the full fix momentarily which should cope with the
strange sequence of events that led to this.


>
>
>>
>>      if (EmitMemCpy) {
>>        Value *OurPtr
>> @@ -3108,12 +3111,6 @@
>>    if (PromotableAllocas.empty())
>>      return false;
>>
>> -  // Ensure that the list is unique.
>> -  std::sort(PromotableAllocas.begin(), PromotableAllocas.end());
>> -  PromotableAllocas.erase(std::unique(PromotableAllocas.begin(),
>> -                                      PromotableAllocas.end()),
>> -                          PromotableAllocas.end());
>> -
>>    NumPromoted += PromotableAllocas.size();
>>
>>    if (DT && !ForceSSAUpdater) {
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120926/2c87a56a/attachment.html>


More information about the llvm-commits mailing list