Skip to content

Commit

Permalink
Keep track of ScopArrayInfo objects that model PHI node storage
Browse files Browse the repository at this point in the history 
Summary:
When translating PHI nodes into memory dependences during code generation we
require two kinds of memory. 'Normal memory' as for all scalar dependences and
'PHI node memory' to store the incoming values of the PHI node. With this
patch we now mark and track these two kinds of memories, which we previously
incorrectly marked as a single memory object.

Being aware of PHI node storage makes code generation easier, as we do not need
to guess what kind of storage a scalar reference requires. This simplifies the
code nicely.

Reviewers: jdoerfert

Subscribers: pollydev, llvm-commits

Differential Revision: http://reviews.llvm.org/D11554

llvm-svn: 243420
  • Loading branch information
@tobiasgrosser
tobiasgrosser committed Jul 28, 2015
1 parent 280ee91 commit 9224522
Show file tree
Hide file tree
Showing 18 changed files with 140 additions and 175 deletions.
13 changes: 7 additions & 6 deletions polly/include/polly/CodeGen/BlockGenerators.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,12 +162,13 @@ class BlockGenerator {
/// memory location in the 'PHIOpMap' table. The memory locations in
/// 'PHIOpMap' end with '.phiops'.
///
/// An access to/from memory that belongs to a PHI node is in the ScopInfo
/// always modeled with the name of the PHI node. However, in reality PHI
/// nodes can introduce reads/writes to two different memory locations, the
/// normal '.s2a' locations and the special '.phiops' locations. We do not
/// track this difference in the polyhedral description, but only through
/// the content of the two maps 'ScalarMap' and 'PHIOpMap'.
/// The ScopArrayInfo objects of accesses that belong to a PHI node may have
/// identical base pointers, even though they refer to two different memory
/// locations, the normal '.s2a' locations and the special '.phiops'
/// locations. For historic reasons we keep such accesses in two maps
/// 'ScalarMap' and 'PHIOpMap', index by the BasePointer. An alternative
/// implemenation, could use a single map that uses the ScopArrayInfo object
/// as index.
///
/// Example:
///
Expand Down
36 changes: 31 additions & 5 deletions polly/include/polly/ScopInfo.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,9 @@ class ScopArrayInfo {
/// @param ElementType The type of the elements stored in the array.
/// @param IslCtx The isl context used to create the base pointer id.
/// @param DimensionSizes A vector containing the size of each dimension.
/// @param IsPHI Is this a PHI node specific array info object.
ScopArrayInfo(Value *BasePtr, Type *ElementType, isl_ctx *IslCtx,
const SmallVector<const SCEV *, 4> &DimensionSizes);
const SmallVector<const SCEV *, 4> &DimensionSizes, bool IsPHI);

/// @brief Destructor to free the isl id of the base pointer.
~ScopArrayInfo();
Expand Down Expand Up @@ -106,6 +107,18 @@ class ScopArrayInfo {
/// @brief Return the isl id for the base pointer.
__isl_give isl_id *getBasePtrId() const;

/// @brief Is this array info modeling special PHI node memory?
///
/// During code generation of PHI nodes, there is a need for two kinds of
/// virtual storage. The normal one as it is used for all scalar dependences,
/// where the result of the PHI node is stored and later loaded from as well
/// as a second one where the incoming values of the PHI nodes are stored
/// into and reloaded when the PHI is executed. As both memories use the
/// original PHI node as virtual base pointer, we have this additional
/// attribute to distinguish the PHI node specific array modeling from the
/// normal scalar array modeling.
bool isPHI() const { return IsPHI; };

/// @brief Dump a readable representation to stderr.
void dump() const;

Expand All @@ -131,6 +144,9 @@ class ScopArrayInfo {

/// @brief The sizes of each dimension.
SmallVector<const SCEV *, 4> DimensionSizes;

/// @brief Is this PHI node specific storage?
bool IsPHI;
};

/// @brief Represent memory accesses in statements.
Expand Down Expand Up @@ -757,9 +773,13 @@ class Scop {
/// Constraints on parameters.
isl_set *Context;

typedef MapVector<const Value *, std::unique_ptr<ScopArrayInfo>>
ArrayInfoMapTy;
typedef MapVector<std::pair<const Value *, int>,
std::unique_ptr<ScopArrayInfo>> ArrayInfoMapTy;
/// @brief A map to remember ScopArrayInfo objects for all base pointers.
///
/// As PHI nodes may have two array info objects associated, we add a flag
/// that distinguishes between the PHI node specific ArrayInfo object
/// and the normal one.
ArrayInfoMapTy ScopArrayInfoMap;

/// @brief The assumptions under which this scop was built.
Expand Down Expand Up @@ -1031,12 +1051,18 @@ class Scop {
/// @brief Return the (possibly new) ScopArrayInfo object for @p Access.
///
/// @param ElementType The type of the elements stored in this array.
/// @param IsPHI Is this ScopArrayInfo object modeling special
/// PHI node storage.
const ScopArrayInfo *
getOrCreateScopArrayInfo(Value *BasePtr, Type *ElementType,
const SmallVector<const SCEV *, 4> &Sizes);
const SmallVector<const SCEV *, 4> &Sizes,
bool IsPHI = false);

/// @brief Return the cached ScopArrayInfo object for @p BasePtr.
const ScopArrayInfo *getScopArrayInfo(Value *BasePtr);
///
/// @param BasePtr The base pointer the object has been stored for
/// @param IsPHI Are we looking for special PHI storage.
const ScopArrayInfo *getScopArrayInfo(Value *BasePtr, bool IsPHI = false);

void setContext(isl_set *NewContext);

Expand Down
16 changes: 13 additions & 3 deletions polly/include/polly/TempScopInfo.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,20 +48,27 @@ class IRAccess {
TypeKind Type;
bool IsAffine;

/// @brief Is this IRAccess modeling special PHI node accesses?
bool IsPHI;

public:
SmallVector<const SCEV *, 4> Subscripts, Sizes;

/// @brief Create a new IRAccess
///
/// @param IsPHI Are we modeling special PHI node accesses?
explicit IRAccess(TypeKind Type, Value *BaseAddress, const SCEV *Offset,
unsigned elemBytes, bool Affine)
unsigned elemBytes, bool Affine, bool IsPHI = false)
: BaseAddress(BaseAddress), Offset(Offset), ElemBytes(elemBytes),
Type(Type), IsAffine(Affine) {}
Type(Type), IsAffine(Affine), IsPHI(IsPHI) {}

explicit IRAccess(TypeKind Type, Value *BaseAddress, const SCEV *Offset,
unsigned elemBytes, bool Affine,
SmallVector<const SCEV *, 4> Subscripts,
SmallVector<const SCEV *, 4> Sizes)
: BaseAddress(BaseAddress), Offset(Offset), ElemBytes(elemBytes),
Type(Type), IsAffine(Affine), Subscripts(Subscripts), Sizes(Sizes) {}
Type(Type), IsAffine(Affine), IsPHI(false), Subscripts(Subscripts),
Sizes(Sizes) {}

enum TypeKind getType() const { return Type; }

Expand All @@ -83,6 +90,9 @@ class IRAccess {

bool isScalar() const { return Subscripts.size() == 0; }

// @brief Is this IRAccess modeling special PHI node accesses?
bool isPHI() const { return IsPHI; }

void print(raw_ostream &OS) const;
};

Expand Down
22 changes: 13 additions & 9 deletions polly/lib/Analysis/ScopInfo.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -350,10 +350,12 @@ static __isl_give isl_set *addRangeBoundsToSet(__isl_take isl_set *S,
}

ScopArrayInfo::ScopArrayInfo(Value *BasePtr, Type *ElementType, isl_ctx *Ctx,
const SmallVector<const SCEV *, 4> &DimensionSizes)
const SmallVector<const SCEV *, 4> &DimensionSizes,
bool IsPHI)
: BasePtr(BasePtr), ElementType(ElementType),
DimensionSizes(DimensionSizes) {
const std::string BasePtrName = getIslCompatibleName("MemRef_", BasePtr, "");
DimensionSizes(DimensionSizes), IsPHI(IsPHI) {
std::string BasePtrName =
getIslCompatibleName("MemRef_", BasePtr, IsPHI ? "__phi" : "");
Id = isl_id_alloc(Ctx, BasePtrName.c_str(), this);
}

Expand Down Expand Up @@ -890,7 +892,7 @@ void ScopStmt::buildAccesses(TempScop &tempScop, BasicBlock *Block,

Type *ElementType = getAccessInstType(AccessInst);
const ScopArrayInfo *SAI = getParent()->getOrCreateScopArrayInfo(
Access.getBase(), ElementType, Access.Sizes);
Access.getBase(), ElementType, Access.Sizes, Access.isPHI());

if (isApproximated && Access.isWrite())
Access.setMayWrite();
Expand Down Expand Up @@ -1705,15 +1707,17 @@ Scop::~Scop() {

const ScopArrayInfo *
Scop::getOrCreateScopArrayInfo(Value *BasePtr, Type *AccessType,
const SmallVector<const SCEV *, 4> &Sizes) {
auto &SAI = ScopArrayInfoMap[BasePtr];
const SmallVector<const SCEV *, 4> &Sizes,
bool IsPHI) {
auto &SAI = ScopArrayInfoMap[std::make_pair(BasePtr, IsPHI)];
if (!SAI)
SAI.reset(new ScopArrayInfo(BasePtr, AccessType, getIslCtx(), Sizes));
SAI.reset(
new ScopArrayInfo(BasePtr, AccessType, getIslCtx(), Sizes, IsPHI));
return SAI.get();
}

const ScopArrayInfo *Scop::getScopArrayInfo(Value *BasePtr) {
const ScopArrayInfo *SAI = ScopArrayInfoMap[BasePtr].get();
const ScopArrayInfo *Scop::getScopArrayInfo(Value *BasePtr, bool IsPHI) {
auto *SAI = ScopArrayInfoMap[std::make_pair(BasePtr, IsPHI)].get();
assert(SAI && "No ScopArrayInfo available for this base pointer");
return SAI;
}
Expand Down
6 changes: 4 additions & 2 deletions polly/lib/Analysis/TempScopInfo.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,12 +141,14 @@ void TempScopInfo::buildPHIAccesses(PHINode *PHI, Region &R,

Written = true;

IRAccess ScalarAccess(IRAccess::MUST_WRITE, PHI, ZeroOffset, 1, true);
IRAccess ScalarAccess(IRAccess::MUST_WRITE, PHI, ZeroOffset, 1, true,
/* IsPHI */ true);
AccFuncMap[OpBB].push_back(std::make_pair(ScalarAccess, OpI));
}

if (Written) {
IRAccess ScalarAccess(IRAccess::READ, PHI, ZeroOffset, 1, true);
IRAccess ScalarAccess(IRAccess::READ, PHI, ZeroOffset, 1, true,
/* IsPHI */ true);
Functions.push_back(std::make_pair(ScalarAccess, PHI));
}
}
Expand Down
122 changes: 22 additions & 100 deletions polly/lib/CodeGen/BlockGenerators.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -440,9 +440,7 @@ void BlockGenerator::generateScalarLoads(ScopStmt &Stmt,

auto Base = cast<Instruction>(MA.getBaseAddr());

// This is either a common scalar use (second case) or the use of a phi
// operand by the PHI node (first case).
if (isa<PHINode>(Base) && Base == MA.getAccessInstruction())
if (MA.getScopArrayInfo()->isPHI())
Address = getOrCreateAlloca(Base, PHIOpMap, ".phiops");
else
Address = getOrCreateAlloca(Base, ScalarMap, ".s2a");
Expand Down Expand Up @@ -501,74 +499,27 @@ void BlockGenerator::generateScalarStores(ScopStmt &Stmt, BasicBlock *BB,
"Region statements need to use the generateScalarStores() "
"function in the RegionGenerator");

// Set to remember a store to the phiops alloca of a PHINode. It is needed as
// we might have multiple write accesses to the same PHI and while one is the
// self write of the PHI (to the ScalarMap alloca) the other is the write to
// the operand alloca (PHIOpMap).
SmallPtrSet<PHINode *, 4> SeenPHIs;

// Iterate over all accesses in the given statement.
for (MemoryAccess *MA : Stmt) {

// Skip non-scalar and read accesses.
if (!MA->isScalar() || MA->isRead())
continue;

Instruction *ScalarBase = cast<Instruction>(MA->getBaseAddr());
Instruction *ScalarInst = MA->getAccessInstruction();
PHINode *ScalarBasePHI = dyn_cast<PHINode>(ScalarBase);
Instruction *Base = cast<Instruction>(MA->getBaseAddr());
Instruction *Inst = MA->getAccessInstruction();

// Get the alloca node for the base instruction and the value we want to
// store. In total there are 4 options:
// (1) The base is no PHI, hence it is a simple scalar def-use chain.
// (2) The base is a PHI,
// (a) and the write is caused by an operand in the block.
// (b) and it is the PHI self write (same as case (1)).
// (c) (2a) and (2b) are not distinguishable.
// For case (1) and (2b) we get the alloca from the scalar map and the value
// we want to store is initialized with the instruction attached to the
// memory access. For case (2a) we get the alloca from the PHI operand map
// and the value we want to store is initialized with the incoming value for
// this block. The tricky case (2c) is when both (2a) and (2b) match. This
// happens if the PHI operand is in the same block as the PHI. To handle
// that we choose the alloca of (2a) first and (2b) for the next write
// access to that PHI (there must be 2).
Value *ScalarValue = nullptr;
AllocaInst *ScalarAddr = nullptr;
Value *Val = nullptr;
AllocaInst *Address = nullptr;

if (!ScalarBasePHI) {
// Case (1)
ScalarAddr = getOrCreateAlloca(ScalarBase, ScalarMap, ".s2a");
ScalarValue = ScalarInst;
if (MA->getScopArrayInfo()->isPHI()) {
PHINode *BasePHI = dyn_cast<PHINode>(Base);
int PHIIdx = BasePHI->getBasicBlockIndex(BB);
Address = getOrCreateAlloca(Base, PHIOpMap, ".phiops");
Val = BasePHI->getIncomingValue(PHIIdx);
} else {
int PHIIdx = ScalarBasePHI->getBasicBlockIndex(BB);
if (ScalarBasePHI != ScalarInst) {
// Case (2a)
assert(PHIIdx >= 0 && "Bad scalar write to PHI operand");
SeenPHIs.insert(ScalarBasePHI);
ScalarAddr = getOrCreateAlloca(ScalarBase, PHIOpMap, ".phiops");
ScalarValue = ScalarBasePHI->getIncomingValue(PHIIdx);
} else if (PHIIdx < 0) {
// Case (2b)
ScalarAddr = getOrCreateAlloca(ScalarBase, ScalarMap, ".s2a");
ScalarValue = ScalarInst;
} else {
// Case (2c)
if (SeenPHIs.insert(ScalarBasePHI).second) {
// First access ==> same as (2a)
ScalarAddr = getOrCreateAlloca(ScalarBase, PHIOpMap, ".phiops");
ScalarValue = ScalarBasePHI->getIncomingValue(PHIIdx);
} else {
// Second access ==> same as (2b)
ScalarAddr = getOrCreateAlloca(ScalarBase, ScalarMap, ".s2a");
ScalarValue = ScalarInst;
}
}
Address = getOrCreateAlloca(Base, ScalarMap, ".s2a");
Val = Inst;
}

ScalarValue =
getNewScalarValue(ScalarValue, R, ScalarMap, BBMap, GlobalMap);
Builder.CreateStore(ScalarValue, ScalarAddr);
Val = getNewScalarValue(Val, R, ScalarMap, BBMap, GlobalMap);
Builder.CreateStore(Val, Address);
}
}

Expand Down Expand Up @@ -1174,58 +1125,29 @@ void RegionGenerator::generateScalarStores(ScopStmt &Stmt, BasicBlock *BB,
assert(StmtR && "Block statements need to use the generateScalarStores() "
"function in the BlockGenerator");

BasicBlock *ExitBB = StmtR->getExit();

// For region statements three kinds of scalar stores exists:
// (1) A definition used by a non-phi instruction outside the region.
// (2) A phi-instruction in the region entry.
// (3) A write to a phi instruction in the region exit.
// The last case is the tricky one since we do not know anymore which
// predecessor of the exit needs to store the operand value that doesn't
// have a definition in the region. Therefore, we have to check in each
// block in the region if we should store the value or not.

// Iterate over all accesses in the given statement.
for (MemoryAccess *MA : Stmt) {

// Skip non-scalar and read accesses.
if (!MA->isScalar() || MA->isRead())
continue;

Instruction *ScalarBase = cast<Instruction>(MA->getBaseAddr());
Instruction *ScalarInst = MA->getAccessInstruction();
PHINode *ScalarBasePHI = dyn_cast<PHINode>(ScalarBase);

Value *ScalarValue = nullptr;
Value *Val = nullptr;
AllocaInst *ScalarAddr = nullptr;

if (!ScalarBasePHI) {
// Case (1)
ScalarAddr = getOrCreateAlloca(ScalarBase, ScalarMap, ".s2a");
ScalarValue = ScalarInst;
} else if (ScalarBasePHI->getParent() != ExitBB) {
// Case (2)
assert(ScalarBasePHI->getParent() == StmtR->getEntry() &&
"Bad PHI self write in non-affine region");
assert(ScalarBase == ScalarInst &&
"Bad PHI self write in non-affine region");
ScalarAddr = getOrCreateAlloca(ScalarBase, ScalarMap, ".s2a");
ScalarValue = ScalarInst;
} else {
if (MA->getScopArrayInfo()->isPHI()) {
int PHIIdx = ScalarBasePHI->getBasicBlockIndex(BB);
// Skip accesses we will not handle in this basic block but in another one
// in the statement region.
if (PHIIdx < 0)
continue;

// Case (3)
ScalarAddr = getOrCreateAlloca(ScalarBase, PHIOpMap, ".phiops");
ScalarValue = ScalarBasePHI->getIncomingValue(PHIIdx);
Val = ScalarBasePHI->getIncomingValue(PHIIdx);
} else {
ScalarAddr = getOrCreateAlloca(ScalarBase, ScalarMap, ".s2a");
Val = ScalarInst;
}

ScalarValue =
getNewScalarValue(ScalarValue, R, ScalarMap, BBMap, GlobalMap);
Builder.CreateStore(ScalarValue, ScalarAddr);
Val = getNewScalarValue(Val, R, ScalarMap, BBMap, GlobalMap);
Builder.CreateStore(Val, ScalarAddr);
}
}

Expand Down
Loading

0 comments on commit 9224522

Please sign in to comment.