This is an archive of the discontinued LLVM Phabricator instance.

[MemoryLocation] Move DSE's logic to new MemLoc::getForDest helper (NFC).
ClosedPublic

Authored by fhahn on Dec 1 2021, 6:20 AM.

Details

Summary

DSE has some extra logic to determine the write location of library
calls like str*cpy and str*cat. This patch moves the logic to a new
MemoryLocation:getForDest variant, which takes a call and TLI.

This patch should be NFC, because no other places take advantage of the
new helper yet.

Suggested by @reames post-commit 7eec832def571.

Diff Detail

Event Timeline

fhahn created this revision.Dec 1 2021, 6:20 AM
fhahn requested review of this revision.Dec 1 2021, 6:20 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 1 2021, 6:20 AM
nikic accepted this revision.Dec 1 2021, 7:04 AM

LG

llvm/lib/Analysis/MemoryLocation.cpp
149

nit: return None;

llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
1039

Move these two over as well?

This revision is now accepted and ready to land.Dec 1 2021, 7:04 AM
reames added inline comments.Dec 1 2021, 10:24 AM
llvm/lib/Analysis/MemoryLocation.cpp
143

I might be missing something, but it looks like these are only handled generically inside MemoryLocation::getForArgument at the moment. Given the original code specialized for the constant case, and only returned getAfter (not also before), I think this move is not NFC.

nikic added inline comments.Dec 1 2021, 12:29 PM
llvm/lib/Analysis/MemoryLocation.cpp
143

The strncpy logic is added in the parent patch. You're right though that we lose the getAfter/getBeforeOrAfter distinction (though it probably doesn't matter much).

fhahn added inline comments.Dec 1 2021, 12:33 PM
llvm/lib/Analysis/MemoryLocation.cpp
143

Yeah, I guess we could add the various LibFunc_str* variants to the switch in getForArgument to use getAfter for them.

This revision was landed with ongoing or failed builds.Dec 3 2021, 1:12 AM
This revision was automatically updated to reflect the committed changes.