Page MenuHomePhabricator

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

Authored by StephenFan on Sep 6 2022, 7:44 PM.

Details

Diff Detail

Event Timeline

StephenFan created this revision.Sep 6 2022, 7:44 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 6 2022, 7:44 PM
StephenFan requested review of this revision.Sep 6 2022, 7:44 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 6 2022, 7:44 PM
nikic added a comment.Sep 7 2022, 1:11 AM

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 added a comment.Sep 7 2022, 1:24 AM

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.

Make check more accurate

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

StephenFan added inline comments.Sep 19 2022, 12:07 AM
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.

StephenFan planned changes to this revision.Sep 19 2022, 12:07 AM