[PATCH] D25172: [MSSA] Initial version of avoiding unnecessary use walks when calling getClobberingMemoryAccess

[George Burgess IV via llvm-commits]

> Right now we don't use the id's for anything but debugging. this would
give them sudden semantic meaning.

If they're currently otherwise unused, I don't see why this is a bad thing;
they have all of the properties we need, and we rely on them being stable
and present for our tests. If we want to add a second
semantically-important ID field, it looks like we have 4 bytes free in Defs
(on 64-bit), but Phis would get a bit bigger, and that would seem redundant.

If our only issue is functions inside of User/Value/..., making
MemoryAccess inherit privately from User might also solve this problem. I'm
not sure how much friction it would cause, but I think I remember you
saying that using a MemoryAccess as a User wasn't quite a great experience.
:)

On Mon, Oct 3, 2016 at 9:08 PM, Daniel Berlin <dberlin at dberlin.org> wrote:

>
> On Mon, Oct 3, 2016 at 9:04 PM, George Burgess IV <
> george.burgess.iv at gmail.com> wrote:
>
>> > So then isOptimized is "OldDef == Def"?
>>
>> "OldDef == getDefiningAccess()->getID()", yeah. Pointer equality
>> initially seems a bit harder to get right, since we could delete a Def and,
>> purely by chance, later make a new Def at the same address. Though, maybe
>> I'm just being paranoid. :)
>>
>
> Right now we don't use the id's for anything but debugging. this would
> give them sudden semantic meaning.
> Not sure what is best to do here.
>
>
>>
>> > Oh, it's just more efficient than having a densemap based cache if you
>> can make it work
>>
>> SGTM; thanks. Until we get an implementation for Defs, would it be better
>> to put these methods on `MemoryUse`, then?
>>
> Yes.
>
>
>>
>> On Mon, Oct 3, 2016 at 8:24 PM, Daniel Berlin <dberlin at dberlin.org>
>> wrote:
>>
>>>
>>>
>>> On Mon, Oct 3, 2016 at 5:09 PM, George Burgess IV <
>>> george.burgess.iv at gmail.com> wrote:
>>>
>>>> george.burgess.iv added inline comments.
>>>>
>>>>
>>>> > MemorySSA.cpp:2185
>>>> > +  // Note: Currently, we do not store the optimized def result
>>>> because we'd need
>>>> > +  // a separate field, since we can't use it as the defining access.
>>>> > +  if (StartingAccess->isOptimized())
>>>>
>>>> RE: `Note: ...`: can you bring me through why having this on a Def is
>>>> would be useful?
>>>
>>>
>>> Oh, it's just more efficient than having a densemap based cache if you
>>> can make it work :)
>>>
>>> It's optimally efficient space wise, but  you may pay the price in
>>> invalidaion time.
>>>
>>> Is the cache just invalidated too aggressively?
>>>
>>>
>>>>
>>> https://reviews.llvm.org/D25172
>>>>
>>>>
>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161004/910c32bc/attachment.html>