This is an archive of the discontinued LLVM Phabricator instance.

[MemorySSA] Fix bug in CachingMemorySSAWalker::invalidateInfo
ClosedPublic

Authored by gberry on Apr 21 2016, 1:48 PM.

Details

Summary

CachingMemorySSAWalker::invalidateInfo was using IsCall to determine
which cache map needed to be cleared of entries referring to the invalidated
MemoryAccess, but there could also be entries referring to it in the
other cache map (value entries, not key entries). This change just
clears both tables to be conservatively correct.

Also add a verifyRemoved() function, called when expensive
checks (i.e. XDEBUG) are enabled to verify that the invalidated
MemoryAccess object is not referenced in any of the caches.

Diff Detail

Event Timeline

gberry updated this revision to Diff 54570.Apr 21 2016, 1:48 PM
gberry retitled this revision from to [MemorySSA] Fix bug in CachingMemorySSAWalker::invalidateInfo.
gberry updated this object.
gberry added a subscriber: llvm-commits.
george.burgess.iv edited edge metadata.

LGTM with one nit. Thank you!

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

It might be useful to be able to call this e.g. directly from GDB, so can we relax this to if NDEBUG isn't defined? (To be clear, I agree that the automatic check on removal should happen iff XDEBUG is given)

This revision is now accepted and ready to land.Apr 21 2016, 2:34 PM
This revision was automatically updated to reflect the committed changes.
dberlin edited edge metadata.Apr 22 2016, 8:06 AM
dberlin added a subscriber: dberlin.

Thank you for fixing this!

(It also occurs to me we can probably just use WeakVH since these are all
Value's now, and then check for null)