[polly] r248603 - Use per-Purpose overloads for MemoryAccess creation
Johannes Doerfert via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 25 16:46:33 PDT 2015
Idea and patch LGTM, some comments that you might want to address are
inlined.
> Author: meinersbur
> Date: Fri Sep 25 13:53:27 2015
> New Revision: 248603
>
> URL: http://llvm.org/viewvc/llvm-project?rev=248603&view=rev
> Log:
> Use per-Purpose overloads for MemoryAccess creation
>
> This makes the intent of each created object clearer and allows to add more specific asserts. The bug fixed in r248535 has been discovered this way.
>
> No functional change intended; everything should behave as before.
>
> Modified:
> polly/trunk/include/polly/ScopInfo.h
> polly/trunk/lib/Analysis/ScopInfo.cpp
>
> Modified: polly/trunk/include/polly/ScopInfo.h
> URL: http://llvm.org/viewvc/llvm-project/polly/trunk/include/polly/ScopInfo.h?rev=248603&r1=248602&r2=248603&view=diff
> ==============================================================================
> --- polly/trunk/include/polly/ScopInfo.h (original)
> +++ polly/trunk/include/polly/ScopInfo.h Fri Sep 25 13:53:27 2015
> @@ -1490,22 +1490,60 @@ class ScopInfo : public RegionPass {
> ArrayRef<const SCEV *> Subscripts,
> ArrayRef<const SCEV *> Sizes, bool IsPHI);
>
> - void addMemoryAccess(BasicBlock *BB, Instruction *Inst,
> - MemoryAccess::AccessType Type, Value *BaseAddress,
> - unsigned ElemBytes, bool Affine, Value *AccessValue,
> - bool IsPHI = false) {
> - addMemoryAccess(BB, Inst, Type, BaseAddress, ElemBytes, Affine, AccessValue,
> - ArrayRef<const SCEV *>(), ArrayRef<const SCEV *>(), IsPHI);
> - }
> -
> - void addMemoryAccess(BasicBlock *BB, Instruction *Inst,
> - MemoryAccess::AccessType Type, Value *BaseAddress,
> - unsigned ElemBytes, bool Affine,
> - ArrayRef<const SCEV *> Subscripts,
> - ArrayRef<const SCEV *> Sizes, Value *AccessValue) {
> - addMemoryAccess(BB, Inst, Type, BaseAddress, ElemBytes, Affine, AccessValue,
> - Subscripts, Sizes, false);
> - }
> + /// @brief Create a MemoryAccess that represents either a LoadInst or
> + /// StoreInst.
> + ///
> + /// @param MemAccInst The LoadInst or StoreInst.
> + /// @param Type The kind of access.
> + /// @param BaseAddress The accessed array's base address.
> + /// @param ElemBytes Size of accessed array element.
> + /// @param IsAffine Whether all subscripts are affine expressions.
> + /// @param Subscripts Access subscripts per dimension.
> + /// @param Sizes The array dimension's sizes.
> + /// @param AccessValue Value read or written.
> + void addExplicitAccess(Instruction *MemAccInst, MemoryAccess::AccessType Type,
> + Value *BaseAddress, unsigned ElemBytes, bool IsAffine,
> + ArrayRef<const SCEV *> Subscripts,
> + ArrayRef<const SCEV *> Sizes, Value *AccessValue);
Could we call this a MemoryAccess in contrast to ScalarAccesses? Hence,
Create an Access for a memory load or store.
> + /// @brief Create a MemoryAccess for writing to .s2a memory.
> + ///
> + /// The access will be created at the @p Value's definition.
> + ///
> + /// @param Value The value to be written into the .s2a memory.
> + void addScalarWriteAccess(Instruction *Value);
I would avoid .s2a as it is an internal artifact. Maybe just say
something like:
Create a write Access for the definition of a sclar.
> + /// @brief Create a MemoryAccess for reading from .s2a memory.
> + ///
> + /// @param Value The scalar expected to be loaded.
> + /// @param User User of the scalar, where the access is added.
> + void addScalarReadAccess(Value *Value, Instruction *User);
Similar to above.
> + /// @brief Create a MemoryAccess for reading from .s2a memory.
Similar to above.
> + /// This is for PHINodes using the scalar. It is used when leaving the
> + /// incoming block to write to the .phiops location.
> + ///
> + /// @param Value The scalar expected to be loaded.
> + /// @param User The PHI node referencing @p Value.
> + /// @param UserBB Incoming block for the incoming @p Value.
> + void addScalarReadAccess(Value *Value, PHINode *User, BasicBlock *UserBB);
Maybe we could name this differently to make the purpose clearer, e.g.,
addPHIOpReadAccess
> + /// @brief Create a MemoryAccess for writing to .phiops memory.
> + ///
> + /// @param PHI PHINode under consideration.
> + /// @param IncomingBlock Some predecessor block.
> + /// @param IncomingValue @p PHI's value when coming from @p IncomingBlock.
> + /// @param IsExitBlock When true, use the .s2a alloca instead. Used for
> + /// values escaping through a PHINode in the SCoP
> + /// region's exit block.
> + void addPHIWriteAccess(PHINode *PHI, BasicBlock *IncomingBlock,
> + Value *IncomingValue, bool IsExitBlock);
> +
> + /// @brief Create a MemoryAccess for reading from .phiops memory.
> + ///
> + /// @param PHI PHINode under consideration. READ access will be added here.
> + void addPHIReadAccess(PHINode *PHI);
Again avoid .phiops and make it something like: the phi operand
> public:
> static char ID;
>
> Modified: polly/trunk/lib/Analysis/ScopInfo.cpp
> URL: http://llvm.org/viewvc/llvm-project/polly/trunk/lib/Analysis/ScopInfo.cpp?rev=248603&r1=248602&r2=248603&view=diff
> ==============================================================================
> --- polly/trunk/lib/Analysis/ScopInfo.cpp (original)
> +++ polly/trunk/lib/Analysis/ScopInfo.cpp Fri Sep 25 13:53:27 2015
> @@ -2703,26 +2703,18 @@ void ScopInfo::buildPHIAccesses(PHINode
> // we have to insert a scalar dependence from the definition of OpI to
> // OpBB if the definition is not in OpBB.
> if (OpIBB != OpBB) {
> - addMemoryAccess(OpBB, PHI, MemoryAccess::READ, OpI, 1, true, OpI);
> - addMemoryAccess(OpIBB, OpI, MemoryAccess::MUST_WRITE, OpI, 1, true,
> - OpI);
> + addScalarReadAccess(OpI, PHI, OpBB);
> + addScalarWriteAccess(OpI);
> }
> } else if (ModelReadOnlyScalars && !isa<Constant>(Op)) {
> - addMemoryAccess(OpBB, PHI, MemoryAccess::READ, Op, 1, true, Op);
> + addScalarReadAccess(Op, PHI, OpBB);
> }
>
> - // Always use the terminator of the incoming basic block as the access
> - // instruction.
> - OpI = OpBB->getTerminator();
> -
> - addMemoryAccess(OpBB, OpI, MemoryAccess::MUST_WRITE, PHI, 1, true, Op,
> - /* IsPHI */ !IsExitBlock);
> + addPHIWriteAccess(PHI, OpBB, Op, IsExitBlock);
> }
>
> - if (!OnlyNonAffineSubRegionOperands) {
> - addMemoryAccess(PHI->getParent(), PHI, MemoryAccess::READ, PHI, 1, true,
> - PHI,
> - /* IsPHI */ !IsExitBlock);
> + if (!OnlyNonAffineSubRegionOperands && !IsExitBlock) {
> + addPHIReadAccess(PHI);
> }
> }
>
> @@ -2778,7 +2770,7 @@ bool ScopInfo::buildScalarDependences(In
> // Do not build a read access that is not in the current SCoP
> // Use the def instruction as base address of the MemoryAccess, so that it
> // will become the name of the scalar access in the polyhedral form.
> - addMemoryAccess(UseParent, UI, MemoryAccess::READ, Inst, 1, true, Inst);
> + addScalarReadAccess(Inst, UI);
> }
>
> if (ModelReadOnlyScalars && !isa<PHINode>(Inst)) {
> @@ -2793,8 +2785,7 @@ bool ScopInfo::buildScalarDependences(In
> if (isa<Constant>(Op))
> continue;
>
> - addMemoryAccess(Inst->getParent(), Inst, MemoryAccess::READ, Op, 1, true,
> - Op);
> + addScalarReadAccess(Op, Inst);
> }
> }
>
> @@ -2865,8 +2856,8 @@ void ScopInfo::buildMemoryAccess(
> SizesSCEV.push_back(SE->getSCEV(ConstantInt::get(
> IntegerType::getInt64Ty(BasePtr->getContext()), Size)));
>
> - addMemoryAccess(Inst->getParent(), Inst, Type, BasePointer->getValue(),
> - Size, true, Subscripts, SizesSCEV, Val);
> + addExplicitAccess(Inst, Type, BasePointer->getValue(), Size, true,
> + Subscripts, SizesSCEV, Val);
> return;
> }
> }
> @@ -2874,9 +2865,9 @@ void ScopInfo::buildMemoryAccess(
>
> auto AccItr = InsnToMemAcc.find(Inst);
> if (PollyDelinearize && AccItr != InsnToMemAcc.end()) {
> - addMemoryAccess(Inst->getParent(), Inst, Type, BasePointer->getValue(),
> - Size, true, AccItr->second.DelinearizedSubscripts,
> - AccItr->second.Shape->DelinearizedSizes, Val);
> + addExplicitAccess(Inst, Type, BasePointer->getValue(), Size, true,
> + AccItr->second.DelinearizedSubscripts,
> + AccItr->second.Shape->DelinearizedSizes, Val);
> return;
> }
>
> @@ -2893,15 +2884,16 @@ void ScopInfo::buildMemoryAccess(
> bool IsAffine = !isVariantInNonAffineLoop &&
> isAffineExpr(R, AccessFunction, *SE, BasePointer->getValue());
>
> - SmallVector<const SCEV *, 4> Subscripts, Sizes;
> - Subscripts.push_back(AccessFunction);
> - Sizes.push_back(SE->getConstant(ZeroOffset->getType(), Size));
> + // FIXME: Size if the number of bytes of an array element, not the number of
> + // elements as probably intended here.
What exactly does this FIXME mean and why is it added?
> + const SCEV *SizeSCEV = SE->getConstant(ZeroOffset->getType(), Size);
>
> if (!IsAffine && Type == MemoryAccess::MUST_WRITE)
> Type = MemoryAccess::MAY_WRITE;
>
> - addMemoryAccess(Inst->getParent(), Inst, Type, BasePointer->getValue(), Size,
> - IsAffine, Subscripts, Sizes, Val);
> + addExplicitAccess(Inst, Type, BasePointer->getValue(), Size, IsAffine,
> + ArrayRef<const SCEV *>(AccessFunction),
> + ArrayRef<const SCEV *>(SizeSCEV), Val);
> }
>
> void ScopInfo::buildAccessFunctions(Region &R, Region &SR) {
> @@ -2946,8 +2938,7 @@ void ScopInfo::buildAccessFunctions(Regi
>
> if (buildScalarDependences(Inst, &R, NonAffineSubRegion)) {
> if (!isa<StoreInst>(Inst))
> - addMemoryAccess(&BB, Inst, MemoryAccess::MUST_WRITE, Inst, 1, true,
> - Inst);
> + addScalarWriteAccess(Inst);
> }
> }
> }
> @@ -2957,8 +2948,7 @@ void ScopInfo::addMemoryAccess(BasicBloc
> Value *BaseAddress, unsigned ElemBytes,
> bool Affine, Value *AccessValue,
> ArrayRef<const SCEV *> Subscripts,
> - ArrayRef<const SCEV *> Sizes,
> - bool IsPHI = false) {
> + ArrayRef<const SCEV *> Sizes, bool IsPHI) {
> AccFuncSetType &AccList = AccFuncMap[BB];
> size_t Identifier = AccList.size();
>
> @@ -2972,6 +2962,43 @@ void ScopInfo::addMemoryAccess(BasicBloc
> Subscripts, Sizes, AccessValue, IsPHI, BaseName);
> }
>
> +void ScopInfo::addExplicitAccess(
> + Instruction *MemAccInst, MemoryAccess::AccessType Type, Value *BaseAddress,
> + unsigned ElemBytes, bool IsAffine, ArrayRef<const SCEV *> Subscripts,
> + ArrayRef<const SCEV *> Sizes, Value *AccessValue) {
> + assert(isa<LoadInst>(MemAccInst) || isa<StoreInst>(MemAccInst));
> + assert(isa<LoadInst>(MemAccInst) == (Type == MemoryAccess::READ));
> + addMemoryAccess(MemAccInst->getParent(), MemAccInst, Type, BaseAddress,
> + ElemBytes, IsAffine, AccessValue, Subscripts, Sizes, false);
> +}
> +void ScopInfo::addScalarWriteAccess(Instruction *Value) {
> + addMemoryAccess(Value->getParent(), Value, MemoryAccess::MUST_WRITE, Value, 1,
> + true, Value, ArrayRef<const SCEV *>(),
> + ArrayRef<const SCEV *>(), false);
> +}
> +void ScopInfo::addScalarReadAccess(Value *Value, Instruction *User) {
> + assert(!isa<PHINode>(User));
> + addMemoryAccess(User->getParent(), User, MemoryAccess::READ, Value, 1, true,
> + Value, ArrayRef<const SCEV *>(), ArrayRef<const SCEV *>(),
> + false);
> +}
> +void ScopInfo::addScalarReadAccess(Value *Value, PHINode *User,
> + BasicBlock *UserBB) {
> + addMemoryAccess(UserBB, User, MemoryAccess::READ, Value, 1, true, Value,
> + ArrayRef<const SCEV *>(), ArrayRef<const SCEV *>(), false);
> +}
> +void ScopInfo::addPHIWriteAccess(PHINode *PHI, BasicBlock *IncomingBlock,
> + Value *IncomingValue, bool IsExitBlock) {
> + addMemoryAccess(IncomingBlock, IncomingBlock->getTerminator(),
> + MemoryAccess::MUST_WRITE, PHI, 1, true, IncomingValue,
> + ArrayRef<const SCEV *>(), ArrayRef<const SCEV *>(),
> + !IsExitBlock);
> +}
> +void ScopInfo::addPHIReadAccess(PHINode *PHI) {
> + addMemoryAccess(PHI->getParent(), PHI, MemoryAccess::READ, PHI, 1, true, PHI,
> + ArrayRef<const SCEV *>(), ArrayRef<const SCEV *>(), true);
> +}
> +
> Scop *ScopInfo::buildScop(Region &R, DominatorTree &DT) {
> unsigned MaxLoopDepth = getMaxLoopDepthInRegion(R, *LI, *SD);
> Scop *S = new Scop(R, AccFuncMap, *SE, DT, ctx, MaxLoopDepth);
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
--
Johannes Doerfert
Researcher / PhD Student
Compiler Design Lab (Prof. Hack)
Saarland University, Computer Science
Building E1.3, Room 4.31
Tel. +49 (0)681 302-57521 : doerfert at cs.uni-saarland.de
Fax. +49 (0)681 302-3065 : http://www.cdl.uni-saarland.de/people/doerfert
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 213 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150926/43601328/attachment.sig>
More information about the llvm-commits
mailing list