[llvm-commits] [PATCH] Clear unknown mem ops when merging stack slots (pr14090)

[Matthew Curtis]

On 10/17/2012 2:17 PM, Duncan Sands wrote:
> Hi Matthew Curtis,
>> --- a/lib/CodeGen/StackColoring.cpp
>> +++ b/lib/CodeGen/StackColoring.cpp
>> @@ -48,6 +48,7 @@
>>  #include "llvm/CodeGen/Passes.h"
>>  #include "llvm/CodeGen/SlotIndexes.h"
>>  #include "llvm/DebugInfo.h"
>> +#include "llvm/Instructions.h"
>>  #include "llvm/MC/MCInstrItineraries.h"
>>  #include "llvm/Target/TargetInstrInfo.h"
>>  #include "llvm/Target/TargetRegisterInfo.h"
>> @@ -511,14 +512,18 @@ void 
>> StackColoring::remapInstructions(DenseMap<int, int> &SlotRemap) {
>>            continue;
>>
>>          // Climb up and find the original alloca.
>> -        V = GetUnderlyingObject(V);
>> -        // If we did not find one, or if the one that we found is 
>> not in our
>> -        // map, then move on.
>> -        if (!V || !Allocas.count(V))
>> -          continue;
>> -
>> -        MMO->setValue(Allocas[V]);
>> -        FixedMemOp++;
>> +        const Value *UO = GetUnderlyingObject(V);
>> +
>> +        // Update value if it is in our map. Otherwise we may need 
>> to clear it
>> +        // if we don't know for sure that it doesn't alias a merged 
>> alloca.
>> +        DenseMap<const Value*, const Value*>::iterator NewV = 
>> Allocas.find(UO);
>
> If this map only contains AllocaInst's as the name suggests, then it 
> would be
> cheaper to first check if UO is an AllocaInst or not, and only do the 
> map lookup
> if it is one.
I'm not sure about whether Allocas only contains AllocaInsts. Keys are 
the returned by MachineFrameInfo::getObjectAllocation();

> Likewise for the null check: no point in doing a lookup if UO is
> none.
>
Done.

> Ciao, Duncan.

Thanks for the feedback.

Matthew C.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation