This is an archive of the discontinued LLVM Phabricator instance.

[AST] Don't assert instruction reads/writes memory (PR51333)
ClosedPublic

Authored by nikic on Jun 16 2022, 1:24 AM.

Details

Summary

This function is well-defined for an instruction that doesn't access memory (and thus trivially doesn't alias anything in the AST), so drop the assert. We can end up with a readnone call here if we originally created a MemoryDef for an indirect call, which was later replaced with a direct readnone call.

Fixes https://github.com/llvm/llvm-project/issues/51333.

Diff Detail

Event Timeline

nikic created this revision.Jun 16 2022, 1:24 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 16 2022, 1:24 AM
nikic requested review of this revision.Jun 16 2022, 1:24 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 16 2022, 1:24 AM
nikic added a comment.Jun 24 2022, 2:01 AM

Ping

llvm/lib/Analysis/AliasSetTracker.cpp
237

If desired, I could also just drop the check completely and let getModRefInfo do its thing.

fhahn accepted this revision.Jul 1 2022, 7:21 AM

LGTM, thanks! Handling instructions not accessing memory should be fine there and pushing the responsibility to remove MemmoryDefs that become non-defs to transforms making changes seems unnecessarily strict.

llvm/test/Transforms/LICM/pr51333.ll
24

can this use opaque pointers?

This revision is now accepted and ready to land.Jul 1 2022, 7:21 AM
nikic added inline comments.Jul 1 2022, 7:31 AM
llvm/test/Transforms/LICM/pr51333.ll
24

Yes, but this will need a (rustc) backport, so it's easier to keep typed pointers in the initial commit.

This revision was landed with ongoing or failed builds.Jul 1 2022, 8:04 AM
This revision was automatically updated to reflect the committed changes.