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

[Bryant Wong via llvm-commits]

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

> 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:
>
>
As my original post indicates, I wish to insert an access, not replace
anything. I've inserted a new memory instruction (memmove, memcpy, or
memset), into the IR and wish to keep the MSSA tree synchronized.


>            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.
>
> Really, I don't want to replace anything. I'm interested in insertions. We
have removals and replacements. So might insertion also be a valid concept
with MSSA?


> 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.
>

Not replacing, please believe me.


>
>
>
>
> 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 at gmai
>>> l.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/1cc7c662/attachment.html>