[PATCH] D16743: This patch hijacks Danny's MemorySSA, a virtual SSA form for memory. Details on what it looks like are in MemorySSA.h

George Burgess IV via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 1 11:54:14 PST 2016


george.burgess.iv abandoned this revision.
george.burgess.iv marked 3 inline comments as done.
george.burgess.iv added a comment.

Commandeered http://reviews.llvm.org/D7864, as requested.


================
Comment at: include/llvm/Transforms/Utils/MemorySSA.h:186
@@ +185,3 @@
+
+  // FIXME(gbiv): MemoryUses don't have users. The quick fix for this would be
+  // to make a virtual `getUsers()` with llvm_unreachable in MemoryUse, but I'd
----------------
davidxl wrote:
> In the context when user_begin() is called, the memoryAccess must be a def. Given that assumption, you can safely DownCast the memoryAccess to MemoryDef -- the problem is both Def and Phi need to be handled.
Marking as done, assuming that we want to go with dannyb's "everything is a subclass of User" approach. :)

================
Comment at: include/llvm/Transforms/Utils/MemorySSA.h:203
@@ +202,3 @@
+  //
+  // ...But then virtual base classes happen (diamond between MemoryAccess and
+  // Def), and those are icky. Not sure if the trait system would need virtual
----------------
davidxl wrote:
> I have another suggestion of the hierarchy:
> 
> Strictly speaking a MemoryDef  and MemoryUse is not the IsA relationship, but a hasA relationship -- basically a memory def is not a use, but it has any implicit may use to model the may-kill.   With that in mind,  it is probably not a good idea to factor out UseOrDef out as the common ancestor of MemoryDef and MemoryUse. Instead:
> 
> 1) MemoryDef should have a pointer to a memoryUse instance
> 2) MemoryDef and Phi Node should have a common ancestor DefOrPhi as drawn above.
> 
> Pros: the userList can now be moved into DefOrPhi
> Cons: get|setDefininingAccess method now needs to be declared as virtual in MemoryAccess base and overridden in MemoryUse and MemoryDef.  For MemoryDef, it just need to forward the call to the memoryUse it owns.
> 
> To avoid virtual call, the memoryAccess can do dispatching via DownCasting depending on memoryAccess type.
Marking as done, assuming that we want to go with dannyb's "everything is a subclass of User" approach.

================
Comment at: include/llvm/Transforms/Utils/MemorySSA.h:333
@@ +332,3 @@
+  const unsigned ID;
+  UserListType Users;
+};
----------------
davidxl wrote:
> Is this redundant? There is one in base class.
Nice catch! Remnant of trying different designs. 


http://reviews.llvm.org/D16743





More information about the llvm-commits mailing list