This is an archive of the discontinued LLVM Phabricator instance.

Add the beginnings of an update API for preserving MemorySSA
ClosedPublic

Authored by dberlin on Feb 11 2016, 10:59 AM.

Details

Summary

This adds the beginning of an update API to preserve MemorySSA. In particular,
this patch adds a way to remove memory SSA accesses when instructions are
deleted.

It also adds relevant unit testing infrastructure for MemorySSA's API.

(There is an actual user of this API, i will make that diff dependent on this one. In practice, a ton of opt passes remove memory instructions, so it's hopefully an obviously useful API :P)

Diff Detail

Event Timeline

dberlin retitled this revision from to Add the beginnings of an update API for preserving MemorySSA.
dberlin updated this object.
dberlin added reviewers: hfinkel, reames.
dberlin added a subscriber: llvm-commits.
  • Update header
  • Update comments

This tests structural parts right now (IE that memoryssa is consistent and correct). I will add the checks that the walker gives correct answers, but wanted to get a quick review before i proceeded much further down this path.

Woohoo! A few comments for you (mostly nits)

include/llvm/Transforms/Utils/MemorySSA.h
518

Nit: Please add a newline before this

679

Nit: space after ), useless ; at the end.

lib/Transforms/Utils/MemorySSA.cpp
492

It seems that if MA is a Phi with no Uses (e.g. they were removed, ...), NewDefTarget may be null here. Is this intentional/possible?

815

Nit: Can we use if (auto *MUD = dyn_cast<MemoryUseOrDef>(MA)) instead?

818

Nit: IsCall = bool(ImmutableCallSite(I))

dberlin marked 5 inline comments as done.Feb 12 2016, 11:34 AM
dberlin added inline comments.
lib/Transforms/Utils/MemorySSA.cpp
492

Yes, this is possible, and was originally intentional.
I used to handle the replacing logic myself, but shoved it off to replaceAllUsesWith, which looks like it does make an assert that you don't pass null.
(in my code, it simply did nothing, because it has a while (!use_empty) loop)
So i've modified this code to only do replacement if !use_empty.

dberlin marked an inline comment as done.
dberlin edited edge metadata.
  • Fix comments from george
  • Revert MLSM.cpp changes
george.burgess.iv edited edge metadata.

This LGTM, thanks for the patch!

This revision is now accepted and ready to land.Feb 12 2016, 12:44 PM
dberlin edited edge metadata.
  • Add walker tests
This revision was automatically updated to reflect the committed changes.