[PATCH] D26127: [MemorySSA] Repair AccessList invariants after insertion of new MemoryUseOrDef.

[Daniel Berlin via llvm-commits]

In particular:
"
I'm not so sure that it's sufficient. Suppose, for instance, that I wanted
to insert a MemoryDef between 1 and 2 in the below AccessList:


  0 = MemoryDef(liveOnEntry)
  1 = MemoryDef(0)
  2 = MemoryDef(1)
  3 = MemoryUse(1)

Invoking createMemoryAccess followed by RAUW of 1's uses with 4 would
result in:
....
"

Which is why this is not the way one does this, anywhere.
If you wish to *replace* an existing access, the normal way is:

           MemoryAccess *Def = OldMemAcc->getDefiningAccess();
           NewMemAcc =
               MSSA->createMemoryAccessAfter(Repl, Def,
OldMemAccess->getMemoryInst());
           OldMemAcc->replaceAllUsesWith(NewMemAcc);

It doesn't make any sense to create a replacing access with the thing you
are going to replace as the definition.

The same thing, btw, would happen in normal LLVM SSA IR if you used the
thing you are replacing in the definition of the replacement.

%c = add %a, %b
%e = add %c, %d

If you wish to replace the first, you can't do this:

%c = add %a, %b
%tmp = add %a, %c
%e = add %c, %d

because calling %c->replaceAllUsesWith %tmp
will result in:

%tmp = add %a, %tmp
%e = add %c, %d


IE the behavior you are citing is completely consistent with existing LLVM
IR. If you define a replacement partially in terms of the thing it is
replacing, it breaks.




On Sun, Oct 30, 2016 at 1:56 PM, Daniel Berlin <dberlin at dberlin.org> wrote:

> For starters, createDefinedAccess is private, so nobody can use it.
> Second, the API calls that use it explicitly take a defining access to
> avoid the problem you cite.
>
> Can you produce a case where normal usage of the update API actually does
> the wrong thing?
>
> I'm entirely unsure what actual usage problem you are trying to solve, so
> I can't help explain how to do it.
>
>
> On Sun, Oct 30, 2016, 1:17 PM Bryant Wong <314472 at gmail.com> wrote:
>
>> Thanks for clearing up my misunderstanding regarding MemoryUses.
>>
>> On Sun, Oct 30, 2016 at 2:04 PM, Daniel Berlin <dberlin at dberlin.org>
>> wrote:
>>
>>
>>
>> On Sun, Oct 30, 2016 at 3:03 AM, bryant <3.14472+reviews.llvm.org@
>> gmail.com> wrote:
>>
>> bryant created this revision.
>> bryant added reviewers: dberlin, george.burgess.iv.
>> bryant added a subscriber: llvm-commits.
>> bryant set the repository for this revision to rL LLVM.
>>
>> An invariant of AccessLists is that the defining access of a Use or Def
>> is the nearest preceding Def node.
>>
>> False
>> This is correct for def's but not for uses.
>>
>>
>>
>> For instance, within a BasicBlock:
>>
>>   0 = MemoryDef(liveOnEntry)
>>   1 = MemoryDef(0)
>>   2 = MemoryUse(n)
>>   3 = MemoryDef(m)
>>
>> 1 is the nearest parent Def of 2 and 3, and m and n must be 1.
>>
>>
>> Given the above, this is incorrect.
>> n may be 0
>>
>> IE the following is perfectly legal
>>   0 = MemoryDef(liveOnEntry)
>>   1 = MemoryDef(0)
>>   2 = MemoryUse(0)
>>   3 = MemoryDef(1)
>>
>>
>>
>> This patch simplifies the interfaces of createMemoryAccessBefore/After,
>> and augments them to maintain this invariant.
>>
>> Additionally, when inserting a new Def after an existing Def, there is
>> currently no (clean) way to update the users of the old Def to use the new
>> Def.
>>
>>
>> RAUW works exactly to do this case.
>>
>>
>>
>> I'm not so sure that it's sufficient. Suppose, for instance, that I
>> wanted to insert a MemoryDef between 1 and 2 in the below AccessList:
>>
>>
>>   0 = MemoryDef(liveOnEntry)
>>   1 = MemoryDef(0)
>>   2 = MemoryDef(1)
>>   3 = MemoryUse(1)
>>
>> Invoking createMemoryAccess followed by RAUW of 1's uses with 4 would
>> result in:
>>
>>
>> So RAUW needs to happen before setting 4's defining access to 1, but this
>> isn't possible with the current createDefiningAccess. What other options
>> are there? RAUW with a bogus Def, create, then re-RAUW to the newly created
>> Def? I feel like I'm missing something obvious, so clarification would be
>> much appreciated.
>>
>>
>> So createDefiningAccess now permits the option of updating the users.
>>
>>
>> Please, no.
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161030/9d92cc98/attachment.html>