[PATCH] D25542: GVN-hoist: avoid calling MemoryLocation::get() on a call (PR30499)

[Sebastian Pop via llvm-commits]

Committed as r284099.
Thanks Danny for your review!


On Wed, Oct 12, 2016 at 10:30 PM, Sebastian Pop <sebpop at gmail.com> wrote:
> On Wed, Oct 12, 2016 at 10:26 PM, Daniel Berlin <dberlin at dberlin.org> wrote:
>> You can get rid of UseInst too, MemoryLocOrCall will take a MemoryUse
>> directly :)
>
> Yes if I add the "const MU" to the class, like so:
>
> diff --git a/lib/Transforms/Utils/MemorySSA.cpp
> b/lib/Transforms/Utils/MemorySSA.cpp
> index 03faffd..b02a3f5 100644
> --- a/lib/Transforms/Utils/MemorySSA.cpp
> +++ b/lib/Transforms/Utils/MemorySSA.cpp
> @@ -105,6 +105,8 @@ public:
>    MemoryLocOrCall() : IsCall(false) {}
>    MemoryLocOrCall(MemoryUseOrDef *MUD)
>        : MemoryLocOrCall(MUD->getMemoryInst()) {}
> +  MemoryLocOrCall(const MemoryUseOrDef *MUD)
> +      : MemoryLocOrCall(MUD->getMemoryInst()) {}
>
>    MemoryLocOrCall(Instruction *Inst) {
>      if (ImmutableCallSite(Inst)) {
> @@ -269,8 +271,7 @@ static bool instructionClobbersQuery(MemoryDef
> *MD, const MemoryUseOrDef *MU,
>  // Return true when MD may alias MU, return false otherwise.
>  bool defClobbersUseOrDef(MemoryDef *MD, const MemoryUseOrDef *MU,
>                           AliasAnalysis &AA) {
> -  return instructionClobbersQuery(MD, MU, MemoryLocOrCall(MU->getMemoryInst()),
> -                                  AA);
> +  return instructionClobbersQuery(MD, MU, MemoryLocOrCall(MU), AA);
>  }
>  }
>
>
>
>>
>>
>> On Wed, Oct 12, 2016 at 8:20 PM, Sebastian Pop <sebpop at gmail.com> wrote:
>>>
>>> On Wed, Oct 12, 2016 at 10:05 PM, Daniel Berlin <dberlin at dberlin.org>
>>> wrote:
>>> > There is actually a much simpler fix you sholdl use.
>>> >
>>> > return instructionClobbersQuery(MD, MU MemoryLocOrCall(MU), AA);
>>> >
>>> > You need to move that version of instructionClobbersQuery up, but it was
>>> > built to always do the right thing.
>>> >
>>> > Also note it will properly handle a case you are not: Fences do not have
>>> > memorylocations, and calling get on them will also crash.
>>> >
>>>
>>> Right. I will commit the attached patch after testing.
>>>
>>> Thanks,
>>> Sebastian
>>
>>