[PATCH] D16743: This patch hijacks Danny's MemorySSA, a virtual SSA form for memory. Details on what it looks like are in MemorySSA.h
Daniel Berlin via llvm-commits
llvm-commits at lists.llvm.org
Sun Jan 31 16:04:04 PST 2016
And here's a version that applies against george's patch
On Sun, Jan 31, 2016 at 2:30 PM, Daniel Berlin <dberlin at dberlin.org> wrote:
> And since i had nothing to do this weekend, here's a version of my
> original patch (sorry george, it should be easy to port if you want it)
> that gets rid of the AccessType and UserList, and makes MemoryAccess's
> User's, and uses LLVM's use lists and operand handling instead of our own
> (if we want to go that way).
>
> 1. I did not include printing support (IE it still requires our dump
> functions to print them, ASMWriter will give unknown blah blah blah).
> 2. All the code in MemoryPhi is copied from PhiNode.
> 3. It passes the tests i had under ASAN, so it seems to work ...
>
> I'm also happy to do it as a followup to george's version.
>
>
>
>
> On Sat, Jan 30, 2016 at 11:00 AM, Daniel Berlin <dberlin at dberlin.org>
> wrote:
>
>>
>>> I see. Efficiency is certainly more important than purity. The reason
>>> I proposed that change is George's memory overhead concern -- the
>>> Users list currently has to be stored in the base class. The fixed
>>> cost per MemoryAccess using SmallPtrSet is 64 bytes.
>>>
>>
>>
>> This is solvable by using User/Value here, and is likely the preferred
>> solution over time.
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160131/d3a2de63/attachment.html>
-------------- next part --------------
diff --git a/include/llvm/IR/Value.def b/include/llvm/IR/Value.def
index 4c5d452..8fd32cb 100644
--- a/include/llvm/IR/Value.def
+++ b/include/llvm/IR/Value.def
@@ -54,6 +54,9 @@
HANDLE_VALUE(Argument)
HANDLE_VALUE(BasicBlock)
+HANDLE_VALUE(MemoryUse)
+HANDLE_VALUE(MemoryDef)
+HANDLE_VALUE(MemoryPhi)
HANDLE_GLOBAL_VALUE(Function)
HANDLE_GLOBAL_VALUE(GlobalAlias)
diff --git a/include/llvm/Transforms/Utils/MemorySSA.h b/include/llvm/Transforms/Utils/MemorySSA.h
index 45060b5..71b41ba 100644
--- a/include/llvm/Transforms/Utils/MemorySSA.h
+++ b/include/llvm/Transforms/Utils/MemorySSA.h
@@ -81,6 +81,10 @@
#include "llvm/Analysis/PHITransAddr.h"
#include "llvm/IR/Dominators.h"
#include "llvm/IR/Module.h"
+#include "llvm/IR/OperandTraits.h"
+#include "llvm/IR/Type.h"
+#include "llvm/IR/User.h"
+#include "llvm/IR/Value.h"
#include "llvm/Pass.h"
namespace llvm {
@@ -95,37 +99,29 @@ using const_memoryaccess_def_iterator =
// \brief The base for all memory accesses. All memory accesses in a block are
// linked together using an intrusive list.
-class MemoryAccess : public ilist_node<MemoryAccess> {
+class MemoryAccess : public User, public ilist_node<MemoryAccess> {
+ void *operator new(size_t, unsigned) = delete;
+ void *operator new(size_t) = delete;
+
public:
// Methods for support type inquiry through isa, cast, and
// dyn_cast
static inline bool classof(const MemoryAccess *) { return true; }
+ static inline bool classof(const Value *V) {
+ unsigned ID = V->getValueID();
+ return ID == Value::MemoryUseVal || ID == Value::MemoryPhiVal ||
+ ID == MemoryDefVal;
+ }
virtual ~MemoryAccess();
- BasicBlock *getBlock() const { return BlockAccessPair.getPointer(); }
+ BasicBlock *getBlock() const { return Block; }
virtual void print(raw_ostream &OS) const = 0;
virtual void dump() const;
- // FIXME: SmallSize=8 originally. Dropped to 4 until the FIXME above `Users`
- // gets sorted out.
- using UserListType = SmallPtrSet<MemoryAccess *, 4>;
- using iterator = UserListType::iterator;
- using const_iterator = UserListType::const_iterator;
-
- unsigned user_size() const { return Users.size(); }
- bool user_empty() const { return Users.empty(); }
- iterator user_begin() { return Users.begin(); }
- iterator user_end() { return Users.end(); }
- iterator_range<iterator> users() {
- return iterator_range<iterator>(user_begin(), user_end());
- }
-
- const_iterator user_begin() const { return Users.begin(); }
- const_iterator user_end() const { return Users.end(); }
- iterator_range<const_iterator> uses() const {
- return iterator_range<const_iterator>(user_begin(), user_end());
- }
+ /// \brief The user iterators for a memory access
+ typedef user_iterator iterator;
+ typedef const_user_iterator const_iterator;
/// \brief This iterator walks over all of the defs in a given
/// MemoryAccess. For MemoryPhi nodes, this walks arguments. For
@@ -136,45 +132,15 @@ public:
const_memoryaccess_def_iterator defs_end() const;
protected:
- enum MemoryAccessType : uint8_t { AccessUse, AccessDef, AccessPhi };
- constexpr static unsigned BitsInMemoryAccessType = 2;
-
friend class MemorySSA;
friend class MemoryUseOrDef;
friend class MemoryUse;
friend class MemoryDef;
friend class MemoryPhi;
- MemoryAccessType getAccessType() const { return BlockAccessPair.getInt(); }
-
- // More of an assumption than a hard constraint that will subtly break
- // existing code. Feel free to remove if this fires for valid reasons.
- void assumeIsNotUse() const {
- assert(getAccessType() != AccessUse && "MemoryUses can have users?");
- }
-
- /// \brief Add a use of this memory access to our list of uses.
- ///
- /// Note: We depend on being able to add the same use multiple times and not
- /// have it end up in our use list multiple times.
- void addUse(MemoryAccess *Use) {
- assumeIsNotUse();
- Users.insert(Use);
- }
-
- /// \brief Remove a use of this memory access from our list of uses.
- void removeUse(MemoryAccess *Use) {
- assumeIsNotUse();
- Users.erase(Use);
- }
-
- /// \brief Return true if \p Use is one of the uses of this memory access.
- bool hasUse(MemoryAccess *Use) const {
- assumeIsNotUse();
- return Users.count(Use);
- }
-
- MemoryAccess(MemoryAccessType AT, BasicBlock *BB) : BlockAccessPair(BB, AT) {}
+ MemoryAccess(LLVMContext &C, unsigned Vty, BasicBlock *BB,
+ unsigned NumOperands)
+ : User(Type::getVoidTy(C), Vty, nullptr, NumOperands), Block(BB) {}
/// \brief Used internally to give IDs to MemoryAccesses for printing
virtual unsigned getID() const = 0;
@@ -182,30 +148,7 @@ protected:
private:
MemoryAccess(const MemoryAccess &);
void operator=(const MemoryAccess &);
-
- // 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
- // assume that would be a bad tradeoff for speed vs memory consumption, and
- // it's mildly suboptimal for type safety (but no more than the current
- // approach).
- //
- // Switching to a traits-based system may be better for code duplication,
- // efficiency, and type-safety, but potentially more difficult to
- // grok/maintain/modify.
- //
- // We could also have an inheritance tree like
- // MemoryAccess
- // / \
- // UseOrDef DefOrPhi
- // / \ / \
- // Use Def Phi
- //
- // ...But then virtual base classes happen (diamond between MemoryAccess and
- // Def), and those are icky. Not sure if the trait system would need virtual
- // bases.
- UserListType Users;
- const PointerIntPair<BasicBlock *, BitsInMemoryAccessType, MemoryAccessType>
- BlockAccessPair;
+ BasicBlock *Block;
};
template <>
@@ -238,42 +181,41 @@ inline raw_ostream &operator<<(raw_ostream &OS, const MemoryAccess &MA) {
/// This class should never be instantiated directly; make a MemoryUse or
/// MemoryDef instead.
class MemoryUseOrDef : public MemoryAccess {
+ void *operator new(size_t, unsigned) = delete;
+ void *operator new(size_t) = delete;
+
public:
+ DECLARE_TRANSPARENT_OPERAND_ACCESSORS(MemoryAccess);
+
/// \brief Get the instruction that this MemoryUse represents.
Instruction *getMemoryInst() const { return MemoryInst; }
/// \brief Get the access that produces the memory state used by this Use.
- MemoryAccess *getDefiningAccess() const { return DefiningAccess; }
+ MemoryAccess *getDefiningAccess() const { return getOperand(0); }
static inline bool classof(const MemoryUseOrDef *) { return true; }
static inline bool classof(const MemoryAccess *MA) {
- return MA->getAccessType() == AccessUse || MA->getAccessType() == AccessDef;
+ return MA->getValueID() == MemoryUseVal || MA->getValueID() == MemoryDefVal;
}
protected:
friend class MemorySSA;
- MemoryUseOrDef(MemoryAccess *DMA, MemoryAccessType AT, Instruction *MI,
- BasicBlock *BB)
- : MemoryAccess(AT, BB), DefiningAccess(nullptr), MemoryInst(MI) {
+ MemoryUseOrDef(LLVMContext &C, MemoryAccess *DMA, unsigned Vty,
+ Instruction *MI, BasicBlock *BB)
+ : MemoryAccess(C, Vty, BB, 1), MemoryInst(MI) {
setDefiningAccess(DMA);
}
- void setDefiningAccess(MemoryAccess *DMA) {
- if (DefiningAccess == DMA)
- return;
-
- if (DefiningAccess)
- DefiningAccess->removeUse(this);
- if (DMA)
- DMA->addUse(this);
- DefiningAccess = DMA;
- }
+ void setDefiningAccess(MemoryAccess *DMA) { setOperand(0, DMA); }
private:
- MemoryAccess *DefiningAccess;
Instruction *MemoryInst;
};
+template <>
+struct OperandTraits<MemoryUseOrDef>
+ : public FixedNumOperandTraits<MemoryUseOrDef, 1> {};
+DEFINE_TRANSPARENT_OPERAND_ACCESSORS(MemoryUseOrDef, MemoryAccess)
/// \brief Represents read-only accesses to memory
///
@@ -281,13 +223,20 @@ private:
/// MemoryUse's is exactly the set of Instructions for which
/// AliasAnalysis::getModRefInfo returns "Ref".
class MemoryUse final : public MemoryUseOrDef {
+ void *operator new(size_t, unsigned) = delete;
+
public:
- MemoryUse(MemoryAccess *DMA, Instruction *MI, BasicBlock *BB)
- : MemoryUseOrDef(DMA, AccessUse, MI, BB) {}
+ DECLARE_TRANSPARENT_OPERAND_ACCESSORS(MemoryAccess);
+
+ // allocate space for exactly one operand
+ void *operator new(size_t s) { return User::operator new(s, 1); }
+
+ MemoryUse(LLVMContext &C, MemoryAccess *DMA, Instruction *MI, BasicBlock *BB)
+ : MemoryUseOrDef(C, DMA, MemoryUseVal, MI, BB) {}
static inline bool classof(const MemoryUse *) { return true; }
static inline bool classof(const MemoryAccess *MA) {
- return MA->getAccessType() == AccessUse;
+ return MA->getValueID() == MemoryUseVal;
}
void print(raw_ostream &OS) const override;
@@ -299,6 +248,9 @@ protected:
llvm_unreachable("MemoryUses do not have IDs");
}
};
+template <>
+struct OperandTraits<MemoryUse> : public FixedNumOperandTraits<MemoryUse, 1> {};
+DEFINE_TRANSPARENT_OPERAND_ACCESSORS(MemoryUse, MemoryAccess)
/// \brief Represents a read-write access to memory, whether it is a must-alias,
/// or a may-alias.
@@ -310,13 +262,21 @@ protected:
/// associated with them. This use points to the nearest reaching
/// MemoryDef/MemoryPhi.
class MemoryDef final : public MemoryUseOrDef {
+ void *operator new(size_t, unsigned) = delete;
+
public:
- MemoryDef(MemoryAccess *DMA, Instruction *MI, BasicBlock *BB, unsigned Ver)
- : MemoryUseOrDef(DMA, AccessDef, MI, BB), ID(Ver) {}
+ DECLARE_TRANSPARENT_OPERAND_ACCESSORS(MemoryAccess);
+
+ // allocate space for exactly one operand
+ void *operator new(size_t s) { return User::operator new(s, 1); }
+
+ MemoryDef(LLVMContext &C, MemoryAccess *DMA, Instruction *MI, BasicBlock *BB,
+ unsigned Ver)
+ : MemoryUseOrDef(C, DMA, MemoryDefVal, MI, BB), ID(Ver) {}
static inline bool classof(const MemoryDef *) { return true; }
static inline bool classof(const MemoryAccess *MA) {
- return MA->getAccessType() == AccessDef;
+ return MA->getValueID() == MemoryDefVal;
}
void print(raw_ostream &OS) const override;
@@ -330,8 +290,10 @@ protected:
private:
const unsigned ID;
- UserListType Users;
};
+template <>
+struct OperandTraits<MemoryDef> : public FixedNumOperandTraits<MemoryDef, 1> {};
+DEFINE_TRANSPARENT_OPERAND_ACCESSORS(MemoryDef, MemoryAccess)
/// \brief Represents phi nodes for memory accesses.
///
@@ -366,55 +328,131 @@ private:
/// Because MemoryUse's do not generate new definitions, they do not have this
/// issue.
class MemoryPhi final : public MemoryAccess {
+ void *operator new(size_t, unsigned) = delete;
+ // allocate space for exactly zero operands
+ void *operator new(size_t s) { return User::operator new(s); }
+
public:
- MemoryPhi(BasicBlock *BB, unsigned Ver, unsigned NumPreds = 0)
- : MemoryAccess(AccessPhi, BB), ID(Ver) {
- Operands.reserve(NumPreds);
+ /// Provide fast operand accessors
+ DECLARE_TRANSPARENT_OPERAND_ACCESSORS(MemoryAccess);
+
+ MemoryPhi(LLVMContext &C, BasicBlock *BB, unsigned Ver, unsigned NumPreds = 0)
+ : MemoryAccess(C, MemoryPhiVal, BB, 0), ID(Ver), ReservedSpace(NumPreds) {
+ allocHungoffUses(ReservedSpace);
}
+ // Block iterator interface. This provides access to the list of incoming
+ // basic blocks, which parallels the list of incoming values.
- /// \brief This is the number of incoming values currently in use
+ typedef BasicBlock **block_iterator;
+ typedef BasicBlock *const *const_block_iterator;
+
+ block_iterator block_begin() {
+ Use::UserRef *ref =
+ reinterpret_cast<Use::UserRef *>(op_begin() + ReservedSpace);
+ return reinterpret_cast<block_iterator>(ref + 1);
+ }
+
+ const_block_iterator block_begin() const {
+ const Use::UserRef *ref =
+ reinterpret_cast<const Use::UserRef *>(op_begin() + ReservedSpace);
+ return reinterpret_cast<const_block_iterator>(ref + 1);
+ }
+
+ block_iterator block_end() { return block_begin() + getNumOperands(); }
+
+ const_block_iterator block_end() const {
+ return block_begin() + getNumOperands();
+ }
+
+ op_range incoming_values() { return operands(); }
+
+ const_op_range incoming_values() const { return operands(); }
+
+ /// getNumIncomingValues - Return the number of incoming edges
+ ///
+ unsigned getNumIncomingValues() const { return getNumOperands(); }
+
+ /// getIncomingValue - Return incoming value number x
+ ///
+ MemoryAccess *getIncomingValue(unsigned i) const { return getOperand(i); }
+ void setIncomingValue(unsigned i, MemoryAccess *V) {
+ assert(V && "PHI node got a null value!");
+ assert(getType() == V->getType() &&
+ "All operands to PHI node must be the same type as the PHI node!");
+ setOperand(i, V);
+ }
+ static unsigned getOperandNumForIncomingValue(unsigned i) { return i; }
+ static unsigned getIncomingValueNumForOperand(unsigned i) { return i; }
+
+ /// getIncomingBlock - Return incoming basic block number @p i.
+ ///
+ BasicBlock *getIncomingBlock(unsigned i) const { return block_begin()[i]; }
+
+ /// getIncomingBlock - Return incoming basic block corresponding
+ /// to an operand of the PHI.
+ ///
+ BasicBlock *getIncomingBlock(const Use &U) const {
+ assert(this == U.getUser() && "Iterator doesn't point to PHI's Uses?");
+ return getIncomingBlock(unsigned(&U - op_begin()));
+ }
+
+ /// getIncomingBlock - Return incoming basic block corresponding
+ /// to value use iterator.
///
- /// During SSA construction, we differentiate between this and NumPreds to
- /// know when the PHI node is fully constructed.
- unsigned getNumIncomingValues() const { return Operands.size(); }
+ BasicBlock *getIncomingBlock(MemoryAccess::const_user_iterator I) const {
+ return getIncomingBlock(I.getUse());
+ }
+
+ void setIncomingBlock(unsigned i, BasicBlock *BB) {
+ assert(BB && "PHI node got a null basic block!");
+ block_begin()[i] = BB;
+ }
- MemoryAccess *getIncomingValue(unsigned V) const {
- return getOperand(V).second;
+ /// addIncoming - Add an incoming value to the end of the PHI list
+ ///
+ void addIncoming(MemoryAccess *V, BasicBlock *BB) {
+ if (getNumOperands() == ReservedSpace)
+ growOperands(); // Get more space!
+ // Initialize some new operands.
+ setNumHungOffUseOperands(getNumOperands() + 1);
+ setIncomingValue(getNumOperands() - 1, V);
+ setIncomingBlock(getNumOperands() - 1, BB);
}
- BasicBlock *getIncomingBlock(unsigned V) const { return getOperand(V).first; }
+ /// getBasicBlockIndex - Return the first index of the specified basic
+ /// block in the value list for this PHI. Returns -1 if no instance.
+ ///
+ int getBasicBlockIndex(const BasicBlock *BB) const {
+ for (unsigned i = 0, e = getNumOperands(); i != e; ++i)
+ if (block_begin()[i] == BB)
+ return i;
+ return -1;
+ }
- using Operand = std::pair<BasicBlock *, MemoryAccess *>;
- const Operand &getOperand(unsigned V) const {
- assert(V < Operands.size() && "Out of bounds Operand access");
- return Operands[V];
+ Value *getIncomingValueForBlock(const BasicBlock *BB) const {
+ int Idx = getBasicBlockIndex(BB);
+ assert(Idx >= 0 && "Invalid basic block argument!");
+ return getIncomingValue(Idx);
}
- using OperandsType = SmallVector<Operand, 4>;
- using const_op_iterator = OperandsType::const_iterator;
- inline const_op_iterator ops_begin() const { return Operands.begin(); }
- inline const_op_iterator ops_end() const { return Operands.end(); }
- inline iterator_range<const_op_iterator> operands() const {
- return iterator_range<const_op_iterator>(ops_begin(), ops_end());
+ static inline bool classof(const Value *V) {
+ return V->getValueID() == MemoryPhiVal;
}
static inline bool classof(const MemoryPhi *) { return true; }
static inline bool classof(const MemoryAccess *MA) {
- return MA->getAccessType() == AccessPhi;
+ return MA->getValueID() == MemoryPhiVal;
}
void print(raw_ostream &OS) const override;
protected:
friend class MemorySSA;
-
- // MemorySSA currently cannot handle modifications. This is protected to
- // ensure people don't try to modify things.
- void addIncoming(MemoryAccess *MA, BasicBlock *BB) {
- assert((isa<MemoryPhi>(MA) || isa<MemoryDef>(MA)) &&
- "Only MemoryPhis or MemoryDefs can be incoming values");
- Operands.push_back(std::make_pair(BB, MA));
- MA->addUse(this);
+ // allocHungoffUses - this is more complicated than the generic
+ // User::allocHungoffUses, because we have to allocate Uses for the incoming
+ // values and pointers to the incoming blocks, all in one allocation.
+ void allocHungoffUses(unsigned N) {
+ User::allocHungoffUses(N, /* IsPhi */ true);
}
// For debugging only. This gets used to give memory accesses pretty numbers
@@ -424,11 +462,26 @@ protected:
private:
// For debugging only
const unsigned ID;
- // FIXME(gbiv): Does having multiple of the same operand make a difference at
- // all?
- OperandsType Operands;
- UserListType Users;
+ unsigned ReservedSpace;
+
+ /// \brief growOperands - grow operands - This grows the operand list in
+ /// response
+ /// to a push_back style of operation. This grows the number of ops by 1.5
+ /// times.
+ ///
+ void growOperands() {
+ unsigned e = getNumOperands();
+ unsigned NumOps = e + e / 2;
+ if (NumOps < 2)
+ NumOps = 2; // 2 op PHI nodes are VERY common.
+
+ ReservedSpace = NumOps;
+ growHungoffUses(ReservedSpace, /* IsPhi */ true);
+ }
};
+template <> struct OperandTraits<MemoryPhi> : public HungoffOperandTraits<2> {};
+
+DEFINE_TRANSPARENT_OPERAND_ACCESSORS(MemoryPhi, MemoryAccess)
class MemorySSAWalker;
diff --git a/lib/Transforms/Utils/MemorySSA.cpp b/lib/Transforms/Utils/MemorySSA.cpp
index 988ed8b..fef4c47 100644
--- a/lib/Transforms/Utils/MemorySSA.cpp
+++ b/lib/Transforms/Utils/MemorySSA.cpp
@@ -109,11 +109,11 @@ MemoryAccess *MemorySSA::renameBlock(BasicBlock *BB,
if (It != PerBlockAccesses.end()) {
AccessListType *Accesses = It->second.get();
for (MemoryAccess &L : *Accesses) {
- switch (L.getAccessType()) {
- case MemoryAccess::AccessUse:
+ switch (L.getValueID()) {
+ case Value::MemoryUseVal:
cast<MemoryUse>(&L)->setDefiningAccess(IncomingVal);
break;
- case MemoryAccess::AccessDef:
+ case Value::MemoryDefVal:
// We can't legally optimize defs, because we only allow single
// memory phis/uses on operations, and if we optimize these, we can
// end up with multiple reaching defs. Uses do not have this
@@ -121,7 +121,7 @@ MemoryAccess *MemorySSA::renameBlock(BasicBlock *BB,
cast<MemoryDef>(&L)->setDefiningAccess(IncomingVal);
IncomingVal = &L;
break;
- case MemoryAccess::AccessPhi:
+ case Value::MemoryPhiVal:
IncomingVal = &L;
break;
}
@@ -208,10 +208,18 @@ MemorySSA::MemorySSA(Function &Func)
: AA(nullptr), DT(nullptr), F(Func), LiveOnEntryDef(nullptr),
Walker(nullptr), NextID(0) {}
-MemorySSA::~MemorySSA() {}
+MemorySSA::~MemorySSA() {
+ // Drop all our references
+ for (auto PBI = PerBlockAccesses.begin(), PBE = PerBlockAccesses.end();
+ PBI != PBE; ++PBI)
+ for (auto LI = PBI->second->begin(), LE = PBI->second->end(); LI != LE;
+ ++LI)
+ LI->dropAllReferences();
+}
MemorySSA::AccessListType *MemorySSA::getOrCreateAccessList(BasicBlock *BB) {
auto Res = PerBlockAccesses.insert(std::make_pair(BB, nullptr));
+
if (Res.second)
Res.first->second = make_unique<AccessListType>();
return Res.first->second.get();
@@ -236,8 +244,8 @@ MemorySSAWalker *MemorySSA::buildMemorySSA(AliasAnalysis *AA,
// semantics do *not* imply that something with no immediate uses can simply
// be removed.
BasicBlock &StartingPoint = F.getEntryBlock();
- LiveOnEntryDef =
- make_unique<MemoryDef>(nullptr, nullptr, &StartingPoint, NextID++);
+ LiveOnEntryDef = make_unique<MemoryDef>(F.getContext(), nullptr, nullptr,
+ &StartingPoint, NextID++);
// We maintain lists of memory accesses per-block, trading memory for time. We
// could just look up the memory access for every possible instruction in the
@@ -270,7 +278,7 @@ MemorySSAWalker *MemorySSA::buildMemorySSA(AliasAnalysis *AA,
for (auto &BB : IDFBlocks) {
// Insert phi node
AccessListType *Accesses = getOrCreateAccessList(BB);
- MemoryPhi *Phi = new MemoryPhi(BB, NextID++);
+ MemoryPhi *Phi = new MemoryPhi(F.getContext(), BB, NextID++);
InstructionToMemoryAccess.insert(std::make_pair(BB, Phi));
// Phi's always are placed at the front of the block.
Accesses->push_front(Phi);
@@ -326,9 +334,11 @@ MemoryAccess *MemorySSA::createNewAccess(Instruction *I, bool IgnoreNonMemory) {
MemoryUseOrDef *MA;
if (Def)
- MA = new MemoryDef(nullptr, I, I->getParent(), NextID++);
+ MA = new MemoryDef(I->getModule()->getContext(), nullptr, I, I->getParent(),
+ NextID++);
else
- MA = new MemoryUse(nullptr, I, I->getParent());
+ MA =
+ new MemoryUse(I->getModule()->getContext(), nullptr, I, I->getParent());
InstructionToMemoryAccess.insert(std::make_pair(I, MA));
return MA;
}
@@ -371,8 +381,8 @@ bool MemorySSA::dominatesUse(const MemoryAccess *Replacer,
// Since we may occur multiple times in the phi node, we have to check each
// operand to ensure Replacer dominates each operand where Replacee occurs.
for (const auto &Arg : MP->operands())
- if (Arg.second == Replacee)
- if (!DT->dominates(Replacer->getBlock(), Arg.first))
+ if (Arg == Replacee)
+ if (!DT->dominates(Replacer->getBlock(), MP->getIncomingBlock(Arg)))
return false;
return true;
}
@@ -393,19 +403,19 @@ void MemorySSA::verifyDomination(Function &F) {
for (BasicBlock &B : F) {
// Phi nodes are attached to basic blocks
if (MemoryPhi *MP = getMemoryAccess(&B)) {
- for (MemoryAccess *U : MP->users()) {
+ for (User *U : MP->users()) {
BasicBlock *UseBlock;
// Phi operands are used on edges, we simulate the right domination by
// acting as if the use occurred at the end of the predecessor block.
if (MemoryPhi *P = dyn_cast<MemoryPhi>(U)) {
for (const auto &Arg : P->operands()) {
- if (Arg.second == MP) {
- UseBlock = Arg.first;
+ if (Arg == MP) {
+ UseBlock = P->getIncomingBlock(Arg);
break;
}
}
} else {
- UseBlock = U->getBlock();
+ UseBlock = cast<MemoryAccess>(U)->getBlock();
}
assert(DT->dominates(MP->getBlock(), UseBlock) &&
"Memory PHI does not dominate it's uses");
@@ -423,13 +433,13 @@ void MemorySSA::verifyDomination(Function &F) {
// edge.
if (MemoryPhi *P = dyn_cast<MemoryPhi>(U)) {
for (const auto &Arg : P->operands()) {
- if (Arg.second == MD) {
- UseBlock = Arg.first;
+ if (Arg == MD) {
+ UseBlock = P->getIncomingBlock(Arg);
break;
}
}
} else {
- UseBlock = U->getBlock();
+ UseBlock = cast<MemoryAccess>(U)->getBlock();
}
assert(DT->dominates(MD->getBlock(), UseBlock) &&
"Memory Def does not dominate it's uses");
@@ -449,7 +459,8 @@ void MemorySSA::verifyUseInDefs(MemoryAccess *Def, MemoryAccess *Use) {
if (!Def) {
if (!isLiveOnEntryDef(Use))
llvm_unreachable("Null def but use not point to live on entry def");
- } else if (!Def->hasUse(Use)) {
+ } else if (std::find(Def->user_begin(), Def->user_end(), Use) ==
+ Def->user_end()) {
llvm_unreachable("Did not find use in def's use list");
}
}
@@ -461,18 +472,15 @@ void MemorySSA::verifyDefUses(Function &F) {
for (BasicBlock &B : F) {
// Phi nodes are attached to basic blocks
if (MemoryPhi *Phi = getMemoryAccess(&B))
- for (const MemoryPhi::Operand &Op : Phi->operands())
- verifyUseInDefs(Op.second, Phi);
+ for (unsigned i = 0, e = Phi->getNumIncomingValues(); i != e; ++i)
+ verifyUseInDefs(Phi->getIncomingValue(i), Phi);
for (Instruction &I : B) {
MemoryAccess *MA = getMemoryAccess(&I);
if (MA) {
- if (auto *MP = dyn_cast<MemoryPhi>(MA)) {
- for (unsigned I = 0, E = MP->getNumIncomingValues(); I != E; ++I)
- verifyUseInDefs(MP->getIncomingValue(I), MP);
- } else {
- verifyUseInDefs(cast<MemoryUseOrDef>(MA)->getDefiningAccess(), MA);
- }
+ assert(isa<MemoryUseOrDef>(MA) &&
+ "Found a phi node not attached to a bb");
+ verifyUseInDefs(cast<MemoryUseOrDef>(MA)->getDefiningAccess(), MA);
}
}
}
@@ -518,24 +526,11 @@ void MemoryDef::print(raw_ostream &OS) const {
}
void MemoryPhi::print(raw_ostream &OS) const {
- SmallVector<MemoryPhi::Operand, 8> Values(ops_begin(), ops_end());
-
- // Try to give operands some sort of predictable ordering, so tests are more
- // resilient.
- std::sort(Values.begin(), Values.end(),
- [](const MemoryPhi::Operand &P1, const MemoryPhi::Operand &P2) {
- unsigned ID1 = P1.second->getID();
- unsigned ID2 = P2.second->getID();
- if (ID1 != ID2)
- return ID1 < ID2;
- return P1.first->getName() < P2.first->getName();
- });
-
bool First = true;
OS << getID() << " = MemoryPhi(";
- for (const MemoryPhi::Operand &Op : Values) {
- BasicBlock *BB = Op.first;
- MemoryAccess *MA = Op.second;
+ for (const auto &Op : operands()) {
+ BasicBlock *BB = getIncomingBlock(Op);
+ MemoryAccess *MA = cast<MemoryAccess>(Op);
if (!First)
OS << ',';
else
@@ -882,7 +877,7 @@ CachingMemorySSAWalker::getClobberingMemoryAccess(MemoryAccess *StartingAccess,
Q.StartingLoc = Loc;
Q.Inst = StartingUseOrDef->getMemoryInst();
Q.IsCall = false;
- Q.DL = &Q.Inst->getParent()->getModule()->getDataLayout();
+ Q.DL = &Q.Inst->getModule()->getDataLayout();
if (auto CacheResult = doCacheLookup(StartingUseOrDef, Q, Q.StartingLoc))
return CacheResult;
@@ -920,7 +915,7 @@ CachingMemorySSAWalker::getClobberingMemoryAccess(const Instruction *I) {
if (!Q.IsCall)
Q.StartingLoc = MemoryLocation::get(I);
Q.Inst = I;
- Q.DL = &Q.Inst->getParent()->getModule()->getDataLayout();
+ Q.DL = &Q.Inst->getModule()->getDataLayout();
if (auto CacheResult = doCacheLookup(StartingAccess, Q, Q.StartingLoc))
return CacheResult;
diff --git a/test/Transforms/Util/MemorySSA/cyclicphi.ll b/test/Transforms/Util/MemorySSA/cyclicphi.ll
index e5a2012..51c2e4c 100644
--- a/test/Transforms/Util/MemorySSA/cyclicphi.ll
+++ b/test/Transforms/Util/MemorySSA/cyclicphi.ll
@@ -24,7 +24,7 @@ bb68: ; preds = %bb26
br label %bb77
bb77: ; preds = %bb68, %bb26
-; CHECK: 2 = MemoryPhi({bb68,1},{bb26,3})
+; CHECK: 2 = MemoryPhi({bb26,3},{bb68,1})
; CHECK: MemoryUse(2)
; CHECK-NEXT: %tmp78 = load i64*, i64** %tmp25, align 8
%tmp78 = load i64*, i64** %tmp25, align 8
diff --git a/test/Transforms/Util/MemorySSA/many-dom-backedge.ll b/test/Transforms/Util/MemorySSA/many-dom-backedge.ll
index 2c5c7bf..49f9145 100644
--- a/test/Transforms/Util/MemorySSA/many-dom-backedge.ll
+++ b/test/Transforms/Util/MemorySSA/many-dom-backedge.ll
@@ -40,7 +40,7 @@ sw.bb2:
br label %sw.epilog
sw.bb3:
-; CHECK: 10 = MemoryPhi({sw.almostexit,6},{loopbegin,9})
+; CHECK: 10 = MemoryPhi({loopbegin,9},{sw.almostexit,6})
; CHECK: 4 = MemoryDef(10)
; CHECK-NEXT: store i32 4
store i32 4, i32* %m, align 4
@@ -53,7 +53,7 @@ sw.default:
br label %sw.epilog
sw.epilog:
-; CHECK: 8 = MemoryPhi({sw.bb,1},{sw.bb1,2},{sw.bb2,3},{sw.bb3,4},{sw.default,5})
+; CHECK: 8 = MemoryPhi({sw.default,5},{sw.bb3,4},{sw.bb,1},{sw.bb1,2},{sw.bb2,3})
; CHECK-NEXT: MemoryUse(8)
; CHECK-NEXT: %0 =
%0 = load i32, i32* %m, align 4
diff --git a/test/Transforms/Util/MemorySSA/many-doms.ll b/test/Transforms/Util/MemorySSA/many-doms.ll
index 11694e4..733accd 100644
--- a/test/Transforms/Util/MemorySSA/many-doms.ll
+++ b/test/Transforms/Util/MemorySSA/many-doms.ll
@@ -51,7 +51,7 @@ sw.default:
br label %sw.epilog
sw.epilog:
-; CHECK: 7 = MemoryPhi({sw.bb,1},{sw.bb1,2},{sw.bb2,3},{sw.bb3,4},{sw.default,5})
+; CHECK: 7 = MemoryPhi({sw.default,5},{sw.bb,1},{sw.bb1,2},{sw.bb2,3},{sw.bb3,4})
; CHECK-NEXT: MemoryUse(7)
; CHECK-NEXT: %0 =
%0 = load i32, i32* %m, align 4
More information about the llvm-commits
mailing list