Page MenuHomePhabricator

[MemorySSA] Reset location size if IsGuaranteedLoopInvariant after phi tranlation
ClosedPublic

Authored by StephenFan on Sep 18 2022, 11:26 PM.

Details

Summary

We set the Location size to beforeOrAfter if the Location value is not
guaranteed loop invariant. But in some cases, we need to reset the
location size if the location size is precise after phi tranlation of
location value. This will improve MemorySSA analysis results.

Diff Detail

Event Timeline

StephenFan created this revision.Sep 18 2022, 11:26 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 18 2022, 11:26 PM
StephenFan requested review of this revision.Sep 18 2022, 11:26 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 18 2022, 11:26 PM
nikic added inline comments.Sep 20 2022, 11:13 AM
llvm/include/llvm/Analysis/MemorySSA.h
1255

The logic here looks somewhat convoluted -- isn't this equivalent to doing the translation first and then doing the invariance check afterwards (dropping the size adjustment before phi translation entirely)?

StephenFan added inline comments.Sep 20 2022, 8:07 PM
llvm/include/llvm/Analysis/MemorySSA.h
1255

Only if the phi translator gets a value different from the original one, line 1271 would be executed. if drop the size adjustment before phi translation entirely, in the case that Location.Ptr is not guaranteed loop invariant and can not be phi translated, the location size of Location.Ptr would not be adjusted to beforeOrAfterPointer. I hope I understood your question correctly and answered it clearly. :)

nikic added inline comments.Sep 21 2022, 1:43 AM
llvm/include/llvm/Analysis/MemorySSA.h
1255

Maybe it's easier to explain with code, this is what I had in mind:

void fillInCurrentPair() {
  CurrentPair.first = *DefIterator;
  CurrentPair.second = Location;
  if (WalkingPhi && Location.Ptr) {
    PHITransAddr Translator(
        const_cast<Value *>(Location.Ptr),
        OriginalAccess->getBlock()->getModule()->getDataLayout(), nullptr);

    if (!Translator.PHITranslateValue(OriginalAccess->getBlock(),
                                      DefIterator.getPhiArgBlock(), DT, true))
      CurrentPair.second =
          CurrentPair.second.getWithNewPtr(Translator.getAddr());

    // 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(CurrentPair.second.Ptr))
      CurrentPair.second =
          Location.getWithNewSize(LocationSize::beforeOrAfterPointer());
  }
}

Thanks nikic for digging into to reduce the logic.

nikic accepted this revision.Sep 22 2022, 1:37 AM

LGTM

This revision is now accepted and ready to land.Sep 22 2022, 1:37 AM
This revision was landed with ongoing or failed builds.Sep 22 2022, 4:46 AM
This revision was automatically updated to reflect the committed changes.