Page MenuHomePhabricator

[DSE] Rewrite function memoryIsNotModified using MemorySSA
Needs ReviewPublic

Authored by StephenFan on Sep 13 2022, 8:16 PM.

Details

Reviewers
nikic
fhahn

Diff Detail

Event Timeline

StephenFan created this revision.Sep 13 2022, 8:16 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 13 2022, 8:16 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
StephenFan requested review of this revision.Sep 13 2022, 8:16 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 13 2022, 8:16 PM
nikic added inline comments.Sep 14 2022, 12:58 AM
llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
386

Do I understand correctly that another precondition of this code is now that FirstI must mod SecondI? Otherwise the walk may go past FirstI and an incorrect modification would be reported?

I think it would be better to do a domination check, as in https://github.com/llvm/llvm-project/blob/419580c699481de40f1e819c396fe07a63885b43/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp#L369.

We should also assert that SecondI is a MemoryDef, for MemoryUses different logic is required.

I'm also slightly concerned that this may produce incorrect results for the loop varying case if SecondI is a call, as MSSA has a known issue in this area.

StephenFan added inline comments.Sep 14 2022, 2:19 AM
llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
386

Thanks for reviewing! Would you mind posting a test case that can reflect the known issue?

nikic added inline comments.Sep 14 2022, 3:52 AM
llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
386