[PATCH] D8688: Update MergedLoadStoreMotion to use MemorySSA

[Daniel Berlin via llvm-commits]

On Wed, Jun 22, 2016 at 10:50 PM, Daniel Berlin <dberlin at dberlin.org> wrote:

>
>
> On Wed, Jun 22, 2016 at 7:07 PM, Gerolf Hoflehner <ghoflehner at apple.com>
> wrote:
>
>> Gerolf added a subscriber: Gerolf.
>>
>> ================
>> Comment at: lib/Transforms/Scalar/MergedLoadStoreMotion.cpp:297
>> @@ -231,3 +296,3 @@
>>
>>      MemoryLocation Loc0 = MemoryLocation::get(Load0);
>>      MemoryLocation Loc1 = MemoryLocation::get(Load1);
>> ----------------
>> Loc0,1 should be moved below the conditional. Then they are definitely
>> needed.
>>
>> ================
>> Comment at: lib/Transforms/Scalar/MergedLoadStoreMotion.cpp:346
>> @@ -271,1 +345,3 @@
>>    HoistedInst->insertBefore(HoistPt);
>> +  if (UseMemorySSA) {
>> +    // We also hoist operands of loads using this function, so check to
>> see if
>> ----------------
>> This is a MSSA update problem. How about
>> if (MSSA && MSSA->update(HoistCan, HoistedInst))
>>     MSSA->removeMemoryAccess(ElseInst);
>>
>> Sorry, i cut this answer off too early.

I also meant to say: we deliberately assert in removeMemoryAccess if you
try to remove something that has no memoryssa instruction, as it is a good
way to find updating bugs in clients, etc.

In any case, i've moved the check to be dependent on MSSA and not
UseMemorySSA, but i otherwise can't think of how to simplify it.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160622/d66b13d0/attachment.html>