This is an archive of the discontinued LLVM Phabricator instance.

Allow None as a MemoryLocation to getModRefInfo
ClosedPublic

Authored by asbirlea on Jul 14 2017, 3:20 PM.

Details

Summary

Adding part of the changes in D30369 (needed to make progress):
Current patch updates AliasAnalysis and MemoryLocation, but does _not_ clean up MemorySSA.

Original summary from D30369, by dberlin:
Currently, we have instructions which affect memory but have no memory
location. If you call, for example, MemoryLocation::get on a fence,
it asserts. This means things specifically have to avoid that. It
also means we end up with a copy of each API, one taking a memory
location, one not.

This starts to fix that.

We add MemoryLocation::getOrNone as a new call, and reimplement the
old asserting version in terms of it.

We make MemoryLocation optional in the (Instruction, MemoryLocation)
version of getModRefInfo, and kill the old one argument version in
favor of passing None (it had one caller). Now both can handle fences
because you can just use MemoryLocation::getOrNone on an instruction
and it will return a correct answer.

We use all this to clean up part of MemorySSA that had to handle this difference.

Note that literally every actual getModRefInfo interface we have could be made private and replaced with:

getModRefInfo(Instruction, Optional<MemoryLocation>)
and
getModRefInfo(Instruction, Optional<MemoryLocation>, Instruction, Optional<MemoryLocation>)

and delegating to the right ones, if we wanted to.

I have not attempted to do this yet.

Event Timeline

asbirlea created this revision.Jul 14 2017, 3:20 PM
dberlin accepted this revision.Jul 31 2017, 1:42 PM

So, this is a subset of an already approved patch, so LGTM.

This revision is now accepted and ready to land.Jul 31 2017, 1:42 PM
davide accepted this revision.Jul 31 2017, 1:45 PM

Sorry, I thought I already LGTM'd this one (I definitely did on the original)

This revision was automatically updated to reflect the committed changes.