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

[Sebastian Pop via llvm-commits]

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