[MemorySSA] Don't do addr phi translation if addr value is not a loop invariant

[nikic]

You need to update phi-translation.ll for this change. From a cursory look I think the results improve while still being correct.

llvm/include/llvm/Analysis/MemorySSA.h

1258–1272

A slightly more accurate check would be if (Location.Size != LocationSize::beforeOrAfterPointer()). If the size is already unknown originally, then we don't need to bother with phi translation either.

[nikic]

In D133396#3773866, @nikic wrote:

You need to update phi-translation.ll for this change. From a cursory look I think the results improve while still being correct.

This is because PerformedPhiTranslation no longer gets set. After looking a bit closer, I think this flag is a leftover from an old implementation. Submitted https://reviews.llvm.org/D133404 to remove.

[StephenFan]

Make check more accurate

[StephenFan]

This patch seems wrong. It will make the test fail if D134161 is applied.

[StephenFan]

llvm/include/llvm/Analysis/MemorySSA.h

1269

These codes here seem to make sense. But deleting them would not cause any tests to fail. I think it is necessary to add test cases to cover it.

llvm/include/llvm/Analysis/MemorySSA.h

if (WalkingPhi && Location.Ptr) {
       // Mark size as unknown, if the location is not guaranteed to be
       // loop-invariant for any possible loop in the function. Setting the size
       // to unknown guarantees that any memory accesses that access locations
       // after the pointer are considered as clobbers, which is important to
       // catch loop carried dependences.
       if (!IsGuaranteedLoopInvariant(const_cast<Value *>(Location.Ptr)))
         CurrentPair.second =
             Location.getWithNewSize(LocationSize::beforeOrAfterPointer());
+      else if (Location.Size != LocationSize::beforeOrAfterPointer()){
         PHITransAddr Translator(
             const_cast<Value *>(Location.Ptr),
             OriginalAccess->getBlock()->getModule()->getDataLayout(), nullptr);
 
         if (!Translator.PHITranslateValue(OriginalAccess->getBlock(),
                                           DefIterator.getPhiArgBlock(), DT,
                                           true)) {
           Value *TransAddr = Translator.getAddr();
           if (TransAddr != Location.Ptr) {
             CurrentPair.second = CurrentPair.second.getWithNewPtr(TransAddr);
 
-          if (TransAddr &&
-              !IsGuaranteedLoopInvariant(const_cast<Value *>(TransAddr)))
-            CurrentPair.second = CurrentPair.second.getWithNewSize(
-                LocationSize::beforeOrAfterPointer());
-
             if (PerformedPhiTranslation)
               *PerformedPhiTranslation = true;
           }
         }
       }
     }
+  }
 
   MemoryAccessPair CurrentPair;
   memoryaccess_def_iterator DefIterator;
   MemoryLocation Location;
   MemoryAccess *OriginalAccess = nullptr;
   DominatorTree *DT = nullptr;
   bool WalkingPhi = false;
   bool *PerformedPhiTranslation = nullptr;