[Polly] Introduce MemAccInst helper class; NFC

[Meinersbur]

Summary

MemAccInst wraps the common members of LoadInst and StoreInst. Add uses of this class to

• ScopInfo::buildMemoryAccess
• BlockGenerator::generateLocationAccessed
• ScopInfo::addArrayAccess
• Scop::buildAliasGroups
• wherever polly::getPointerOperand was used

The class has been extracted out of D12975 (DeLICM) where it is used more extensively.

[llvm-commits]

+/ getValueOperand() and getPointerOperand() have different implementations in
+/ LoadInst and StoreInst, but for most other members the implementation is
+/ identical. Those have been reproduced here to avoid useless branching to
+/ identical implementations. If those implementations change in
+/// LoadInst/StoreInst, they must be changed here as well.

The last sentence alone guarantees problems. Duplication is (almost)
never a solution, especially not to save a call/jump/branch.

• signature.asc219 BDownload
• msg-5106-603.txt162 BDownload

[Meinersbur]

In D16530#334955, @llvm-commits wrote:

+/ getValueOperand() and getPointerOperand() have different implementations in
+/ LoadInst and StoreInst, but for most other members the implementation is
+/ identical. Those have been reproduced here to avoid useless branching to
+/ identical implementations. If those implementations change in
+/// LoadInst/StoreInst, they must be changed here as well.

The last sentence alone guarantees problems. Duplication is (almost)
never a solution, especially not to save a call/jump/branch.

We'd notice immediately, but I can change to calling the proper functions, no problem.

Note that this is also duplicated in LLVM itself.

[Meinersbur]

Do not duplicate code from LLVM.

[grosser]

Hi Michael,

I think this patch makes a lot of sense. Unfortunately all these casts make it look rather ugly as they introduce more noise than is actually removed. I just looked quickly at the implementation of CallSite and it seems the implementation is slightly different. Specifically, it does not inherit from instruction, but rather creates a proxy class around the CallInst/InvokeInst. Like this it is possible to add default constructors for CallSite(Invoke Ty) and CallSite(CallTy) that allow for automatic implicit type conversion. Could this be a solution that would allow us to remove the need for explicit casts?

Tobias

[Meinersbur]

In D16530#335899, @grosser wrote:

Hi Michael,

I think this patch makes a lot of sense. Unfortunately all these casts make it look rather ugly as they introduce more noise than is actually removed. I just looked quickly at the implementation of CallSite and it seems the implementation is slightly different. Specifically, it does not inherit from instruction, but rather creates a proxy class around the CallInst/InvokeInst. Like this it is possible to add default constructors for CallSite(Invoke Ty) and CallSite(CallTy) that allow for automatic implicit type conversion. Could this be a solution that would allow us to remove the need for explicit casts?

Unfortunately their type is LoadInst* instead of LoadInst&. C++ does not support overloading operators/implicit conversions of pointer types. I could change their declarations to references and implement these implicit constructors, but this would create a new MemAccInst every time, i.e. create a new instruction because it is derived from Instruction.

The implementation of CallSite is quite different. It is a stack object containing a pointer to the CallInst or InvokeInst (you called it proxy). This allows "it can also create a null initialized CallSiteBase object for something which is NOT a call site". I don't think we need this, we can just test for nullptr. It also does not allow implicit conversions from pointers and users then are required to think about the lifetime of the stack object. It would allow implicit conversions from LoadInst& though.

Anyway, it is only one of the uses where I thought it could make sense. We could also just drop the BlockGenerator::generateLocationAccessed part.

One more use could be InstructionToAccess mapping, because since D13676 we only map loads/stores.

[grosser]

Meinersbur added a comment.

In http://reviews.llvm.org/D16530#335899, @grosser wrote:

Hi Michael,

I think this patch makes a lot of sense. Unfortunately all these casts make it look rather ugly as they introduce more noise than is actually removed. I just looked quickly at the implementation of CallSite and it seems the implementation is slightly different. Specifically, it does not inherit from instruction, but rather creates a proxy class around the CallInst/InvokeInst. Like this it is possible to add default constructors for CallSite(Invoke Ty) and CallSite(CallTy) that allow for automatic implicit type conversion. Could this be a solution that would allow us to remove the need for explicit casts?

Unfortunately their type is LoadInst* instead of LoadInst&. C++ does not support overloading operators/implicit conversions of pointer types. I could change their declarations to references and implement these implicit constructors, but this would create a new MemAccInst every time, i.e. create a new instruction because it is derived from Instruction.

I think it would be OK to change the type to LoadInst& and to construct
new instances of MemAccInst that keep just a pointer to LoadInst/StoreInst.

Now, I am not sure if there is a real need to actually derive from
Instruction at all. This seems to add overhead and does not seem to be
used. CallSiteInst works e.g. also without being derived from Instruction.

The implementation of CallSite is quite different. It is a stack object containing a pointer to the CallInst or InvokeInst (you called it proxy). This allows "it can also create a null initialized CallSiteBase object for something which is NOT a call site". I don't think we need this, we can just test for nullptr. It also does not allow implicit conversions from pointers and users then are required to think about the lifetime of the stack object. It would allow implicit conversions from LoadInst& though.

Right, the last point is what I am after.

Anyway, it is only one of the uses where I thought it could make sense. We could also just drop the BlockGenerator::generateLocationAccessed part.

If it can not even be used here, this new class does not seem to be very
useful. Hence, I am trying to figure out if we can address my last concerns.

One more use could be InstructionToAccess mapping, because since http://reviews.llvm.org/D13676 we only map loads/stores.

Possibly. However, I think the use in generationLocationAccessed would
be good enough to justify its introduction.

Best,
Tobias

[Meinersbur]

Proxy object variant (like CallSite, StringRef, ArrayRef)

[grosser]

Hi Michael,

this looks a lot better. Johannes pointed out to me that there is already a function polly::getPointerOperand in ScopHelper.cpp. Having both is probably redundant. I think the GetElementPtrInst is never used in this function (please check). So we can probably eliminate it and just use your class instead of the getPointerOperand instruction. Otherwise, this is probably good to go.

[Meinersbur]

Replace polly::getPointerOperand(); Move class MemAccInst to ScopHelper.h to make it available in ScopDetection.h

[grosser]

LGTM.

include/polly/CodeGen/BlockGenerators.h

 }
 
 namespace polly {
 using namespace llvm;
 class ScopStmt;
 class MemoryAccess;
 class ScopArrayInfo;
 class IslExprBuilder;
+class MemAccInst;
 
 /// @brief Generate a new basic block for a polyhedral statement.
 class BlockGenerator {
 public:
   typedef llvm::SmallVector<ValueMapT, 8> VectorValueMapT;
 
   /// @brief Map types to resolve scalar dependences.
   ///
protected:
   /// @param Inst The instruction for which we get the loop.
   /// @return The innermost loop that surrounds the instruction.
   Loop *getLoopForInst(const Instruction *Inst);
 
   /// @brief Generate the operand address
   /// @param NewAccesses A map from memory access ids to new ast expressions,
   ///                    which may contain new access expressions for certain
   ///                    memory accesses.
-  Value *generateLocationAccessed(ScopStmt &Stmt, const Instruction *Inst,
-                                  Value *Pointer, ValueMapT &BBMap,
-                                  LoopToScevMapT &LTS,
+  Value *generateLocationAccessed(ScopStmt &Stmt, MemAccInst Inst,
+                                  ValueMapT &BBMap, LoopToScevMapT &LTS,
                                   isl_id_to_ast_expr *NewAccesses);
 
   /// @param NewAccesses A map from memory access ids to new ast expressions,
   ///                    which may contain new access expressions for certain
   ///                    memory accesses.
   Value *generateScalarLoad(ScopStmt &Stmt, LoadInst *load, ValueMapT &BBMap,
                             LoopToScevMapT &LTS,
                             isl_id_to_ast_expr *NewAccesses);

include/polly/ScopInfo.h

 
 namespace polly {
 
 class MemoryAccess;
 class Scop;
 class ScopStmt;
 class ScopInfo;
 
+/// @brief Utility class to wrap the common members of LoadInst and StoreInst.
+///
+/// This works like LLVM utility classes like CallSite, IntrinsicInst,
+/// MemIntrinsic, MemTransferInst, etc.
+/// Unfortunately it is not possible to have an implicit conversion from
+/// LoadInst*/StoreInst* in C++. Such casts must be written explicitely using
+/// llvm::cast.
+class MemAccInst {
+private:
+  Instruction *I;
+
+public:
+  MemAccInst() : I(nullptr) {}
+  MemAccInst(const MemAccInst &Inst) : I(Inst.I) {}
+  /* implicit */ MemAccInst(LoadInst &LI) : I(&LI) {}
+  /* implicit */ MemAccInst(LoadInst *LI) : I(LI) {}
+  /* implicit */ MemAccInst(StoreInst &SI) : I(&SI) {}
+  /* implicit */ MemAccInst(StoreInst *SI) : I(SI) {}
+  explicit MemAccInst(Instruction &I) : I(&I) {
+    assert(isa<LoadInst>(I) || isa<StoreInst>(I));
+  }
+  explicit MemAccInst(Instruction *I) : I(I) {
+    assert(isa<LoadInst>(I) || isa<StoreInst>(I));
+  }
+
+  MemAccInst &operator=(const MemAccInst &Inst) {
+    I = Inst.I;
+    return *this;
+  }
+  MemAccInst &operator=(LoadInst &LI) {
+    I = &LI;
+    return *this;
+  }
+  MemAccInst &operator=(LoadInst *LI) {
+    I = LI;
+    return *this;
+  }
+  MemAccInst &operator=(StoreInst &SI) {
+    I = &SI;
+    return *this;
+  }
+  MemAccInst &operator=(StoreInst *SI) {
+    I = SI;
+    return *this;
+  }
+
+  operator Instruction *() const { return asInstruction(); }
+
+  Value *getOperand(unsigned i) const { return I->getOperand(i); }
+  BasicBlock *getParent() const { return I->getParent(); }
+  LLVMContext &getContext() const { return I->getContext(); }
+
+  Value *getValueOperand() {
+    return isStore() ? asStore()->getValueOperand() : asLoad();
+  }
+  const Value *getValueOperand() const {
+    return isStore() ? asStore()->getValueOperand() : asLoad();
+  }
+
+  Value *getPointerOperand() {
+    return getOperand(isStore() ? StoreInst::getPointerOperandIndex()
+                                : LoadInst::getPointerOperandIndex());
+  }
+  const Value *getPointerOperand() const {
+    return getOperand(isStore() ? StoreInst::getPointerOperandIndex()
+                                : LoadInst::getPointerOperandIndex());
+  }
+
+  unsigned getAlignment() const {
+    return isStore() ? asStore()->getAlignment() : asLoad()->getAlignment();
+  }
+  bool isVolatile() const {
+    return isStore() ? asStore()->isVolatile() : asLoad()->isVolatile();
+  }
+  bool isSimple() const {
+    return isStore() ? asStore()->isSimple() : asLoad()->isSimple();
+  }
+  AtomicOrdering getOrdering() const {
+    return isStore() ? asStore()->getOrdering() : asLoad()->getOrdering();
+  }
+  bool isUnordered() const {
+    return isStore() ? asStore()->isUnordered() : asLoad()->isUnordered();
+  }
+
+  bool isLoad() const { return isa<LoadInst>(I); }
+  LoadInst *asLoad() const { return cast<LoadInst>(I); }
+
+  bool isStore() const { return isa<StoreInst>(I); }
+  StoreInst *asStore() const { return cast<StoreInst>(I); }
+
+  Instruction *asInstruction() const { return I; }
+};
+
+typedef DenseMap<ScopStmt *, Value *> OutgoingValueMapTy;
+
 //===---------------------------------------------------------------------===//
 
 /// @brief Enumeration of assumptions Polly can take.
 enum AssumptionKind {
   ALIASING,
   INBOUNDS,
   WRAPPING,
   ALIGNMENT,
class ScopInfo : public RegionPass {
 
   /// @brief Build an instance of MemoryAccess from the Load/Store instruction.
   ///
   /// @param Inst       The Load/Store instruction that access the memory
   /// @param L          The parent loop of the instruction
   /// @param R          The region on which to build the data access dictionary.
   /// @param BoxedLoops The set of loops that are overapproximated in @p R.
   /// @param ScopRIL    The required invariant loads equivalence classes.
-  void buildMemoryAccess(Instruction *Inst, Loop *L, Region *R,
+  void buildMemoryAccess(MemAccInst Inst, Loop *L, Region *R,
                          const ScopDetection::BoxedLoopsSetTy *BoxedLoops,
                          const InvariantLoadsSetTy &ScopRIL);
 
   /// @brief Analyze and extract the cross-BB scalar dependences (or,
   ///        dataflow dependencies) of an instruction.
   ///
   /// @param Inst               The instruction to be analyzed
   /// @param R                  The SCoP region
class ScopInfo : public RegionPass {
   /// @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.
   /// @see ScopArrayInfo::MemoryKind
-  void addArrayAccess(Instruction *MemAccInst, MemoryAccess::AccessType Type,
+  void addArrayAccess(MemAccInst MemAccInst, MemoryAccess::AccessType Type,
                       Value *BaseAddress, unsigned ElemBytes, bool IsAffine,
                       ArrayRef<const SCEV *> Subscripts,
                       ArrayRef<const SCEV *> Sizes, Value *AccessValue);
 
   /// @brief Create a MemoryAccess for writing an llvm::Value.
   ///
   /// The access will be created at the @p Value's definition.
   ///

lib/Analysis/ScopInfo.cpp

for (ScopStmt &Stmt : *this) {
     if (StmtDomainEmpty)
       continue;
 
     for (MemoryAccess *MA : Stmt) {
       if (MA->isScalarKind())
         continue;
       if (!MA->isRead())
         HasWriteAccess.insert(MA->getBaseAddr());
-      Instruction *Acc = MA->getAccessInstruction();
-      PtrToAcc[getPointerOperand(*Acc)] = MA;
+      MemAccInst Acc(MA->getAccessInstruction());
+      PtrToAcc[Acc.getPointerOperand()] = MA;
       AST.add(Acc);
     }
   }
 
   SmallVector<AliasGroupTy, 4> AliasGroups;
   for (AliasSet &AS : AST) {
     if (AS.isMustAlias() || AS.isForwardingAliasSet())
       continue;
bool ScopInfo::buildScalarDependences(Instruction *Inst, Region *R,
   }
 
   return AnyCrossStmtUse;
 }
 
 extern MapInsnToMemAcc InsnToMemAcc;
 
 void ScopInfo::buildMemoryAccess(
-    Instruction *Inst, Loop *L, Region *R,
+    MemAccInst Inst, Loop *L, Region *R,
     const ScopDetection::BoxedLoopsSetTy *BoxedLoops,
     const InvariantLoadsSetTy &ScopRIL) {
-  unsigned Size;
-  Type *SizeType;
-  Value *Val;
-  enum MemoryAccess::AccessType Type;
-
-  if (LoadInst *Load = dyn_cast<LoadInst>(Inst)) {
-    SizeType = Load->getType();
-    Size = TD->getTypeAllocSize(SizeType);
-    Type = MemoryAccess::READ;
-    Val = Load;
-  } else {
-    StoreInst *Store = cast<StoreInst>(Inst);
-    SizeType = Store->getValueOperand()->getType();
-    Size = TD->getTypeAllocSize(SizeType);
-    Type = MemoryAccess::MUST_WRITE;
-    Val = Store->getValueOperand();
-  }
 
-  auto Address = getPointerOperand(*Inst);
+  Value *Address = Inst.getPointerOperand();
+  Value *Val = Inst.getValueOperand();
+  Type *SizeType = Val->getType();
+  unsigned Size = TD->getTypeAllocSize(SizeType);
+  enum MemoryAccess::AccessType Type =
+      Inst.isLoad() ? MemoryAccess::READ : MemoryAccess::MUST_WRITE;
 
   const SCEV *AccessFunction = SE->getSCEVAtScope(Address, L);
   const SCEVUnknown *BasePointer =
       dyn_cast<SCEVUnknown>(SE->getPointerBase(AccessFunction));
 
   assert(BasePointer && "Could not find base pointer");
   AccessFunction = SE->getMinusSCEV(AccessFunction, BasePointer);
 
void ScopInfo::buildMemoryAccess(
 
   for (LoadInst *LInst : AccessILS)
     if (!ScopRIL.count(LInst))
       IsAffine = false;
 
   // FIXME: Size of the number of bytes of an array element, not the number of
   // elements as probably intended here.
   const SCEV *SizeSCEV =
-      SE->getConstant(TD->getIntPtrType(Inst->getContext()), Size);
+      SE->getConstant(TD->getIntPtrType(Inst.getContext()), Size);
 
   if (!IsAffine && Type == MemoryAccess::MUST_WRITE)
     Type = MemoryAccess::MAY_WRITE;
 
   addArrayAccess(Inst, Type, BasePointer->getValue(), Size, IsAffine,
                  ArrayRef<const SCEV *>(AccessFunction),
                  ArrayRef<const SCEV *>(SizeSCEV), Val);
 }
for (BasicBlock::iterator I = BB.begin(), E = --BB.end(); I != E; ++I) {
     if (!PHI && IsExitBlock)
       break;
 
     // TODO: At this point we only know that elements of ScopRIL have to be
     //       invariant and will be hoisted for the SCoP to be processed. Though,
     //       there might be other invariant accesses that will be hoisted and
     //       that would allow to make a non-affine access affine.
     if (isa<LoadInst>(Inst) || isa<StoreInst>(Inst))
-      buildMemoryAccess(Inst, L, &R, BoxedLoops, ScopRIL);
+      buildMemoryAccess(MemAccInst(Inst), L, &R, BoxedLoops, ScopRIL);
 
     if (isIgnoredIntrinsic(Inst))
       continue;
 
     // Do not build scalar dependences for required invariant loads as we will
     // hoist them later on anyway or drop the SCoP if we cannot.
     if (ScopRIL.count(dyn_cast<LoadInst>(Inst)))
       continue;
if (!isKnownMustAccess && Type == MemoryAccess::MUST_WRITE)
     Type = MemoryAccess::MAY_WRITE;
 
   AccList.emplace_back(Stmt, Inst, Type, BaseAddress, ElemBytes, Affine,
                        Subscripts, Sizes, AccessValue, Kind, BaseName);
   Stmt->addAccess(&AccList.back());
   return &AccList.back();
 }
 
-void ScopInfo::addArrayAccess(Instruction *MemAccInst,
+void ScopInfo::addArrayAccess(MemAccInst 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,
+  assert(MemAccInst.isLoad() == (Type == MemoryAccess::READ));
+  addMemoryAccess(MemAccInst.getParent(), MemAccInst, Type, BaseAddress,
                   ElemBytes, IsAffine, AccessValue, Subscripts, Sizes,
                   ScopArrayInfo::MK_Array);
 }
 void ScopInfo::ensureValueWrite(Instruction *Value) {
   ScopStmt *Stmt = scop->getStmtForBasicBlock(Value->getParent());
 
   // Value not defined within this SCoP.
   if (!Stmt)

lib/CodeGen/BlockGenerators.cpp

void BlockGenerator::copyInstScalar(ScopStmt &Stmt, Instruction *Inst,
 
   Builder.Insert(NewInst);
   BBMap[Inst] = NewInst;
 
   if (!NewInst->getType()->isVoidTy())
     NewInst->setName("p_" + Inst->getName());
 }
 
-Value *BlockGenerator::generateLocationAccessed(
-    ScopStmt &Stmt, const Instruction *Inst, Value *Pointer, ValueMapT &BBMap,
-    LoopToScevMapT &LTS, isl_id_to_ast_expr *NewAccesses) {
+Value *
+BlockGenerator::generateLocationAccessed(ScopStmt &Stmt, MemAccInst Inst,
+                                         ValueMapT &BBMap, LoopToScevMapT &LTS,
+                                         isl_id_to_ast_expr *NewAccesses) {
   const MemoryAccess &MA = Stmt.getArrayAccessFor(Inst);
 
   isl_ast_expr *AccessExpr = isl_id_to_ast_expr_get(NewAccesses, MA.getId());
 
   if (AccessExpr) {
     AccessExpr = isl_ast_expr_address_of(AccessExpr);
     auto Address = ExprBuilder->create(AccessExpr);
 
if (OldPtrTy != NewPtrTy) {
       assert(OldPtrTy->getPointerElementType()->getPrimitiveSizeInBits() ==
                  NewPtrTy->getPointerElementType()->getPrimitiveSizeInBits() &&
              "Pointer types to elements with different size found");
       Address = Builder.CreateBitOrPointerCast(Address, OldPtrTy);
     }
     return Address;
   }
 
-  return getNewValue(Stmt, Pointer, BBMap, LTS, getLoopForInst(Inst));
+  return getNewValue(Stmt, Inst.getPointerOperand(), BBMap, LTS,
+                     getLoopForInst(Inst));
 }
 
 Loop *BlockGenerator::getLoopForInst(const llvm::Instruction *Inst) {
   return LI.getLoopFor(Inst->getParent());
 }
 
 Value *BlockGenerator::generateScalarLoad(ScopStmt &Stmt, LoadInst *Load,
                                           ValueMapT &BBMap, LoopToScevMapT &LTS,
                                           isl_id_to_ast_expr *NewAccesses) {
   if (Value *PreloadLoad = GlobalMap.lookup(Load))
     return PreloadLoad;
 
-  auto *Pointer = Load->getPointerOperand();
   Value *NewPointer =
-      generateLocationAccessed(Stmt, Load, Pointer, BBMap, LTS, NewAccesses);
+      generateLocationAccessed(Stmt, Load, BBMap, LTS, NewAccesses);
   Value *ScalarLoad = Builder.CreateAlignedLoad(
       NewPointer, Load->getAlignment(), Load->getName() + "_p_scalar_");
 
   if (DebugPrinting)
     RuntimeDebugBuilder::createCPUPrinter(Builder, "Load from ", NewPointer,
                                           ": ", ScalarLoad, "\n");
 
   return ScalarLoad;
 }
 
 void BlockGenerator::generateScalarStore(ScopStmt &Stmt, StoreInst *Store,
                                          ValueMapT &BBMap, LoopToScevMapT &LTS,
                                          isl_id_to_ast_expr *NewAccesses) {
-  auto *Pointer = Store->getPointerOperand();
   Value *NewPointer =
-      generateLocationAccessed(Stmt, Store, Pointer, BBMap, LTS, NewAccesses);
+      generateLocationAccessed(Stmt, Store, BBMap, LTS, NewAccesses);
   Value *ValueOperand = getNewValue(Stmt, Store->getValueOperand(), BBMap, LTS,
                                     getLoopForInst(Store));
 
   if (DebugPrinting)
     RuntimeDebugBuilder::createCPUPrinter(Builder, "Store to  ", NewPointer,
                                           ": ", ValueOperand, "\n");
 
   Builder.CreateAlignedStore(ValueOperand, NewPointer, Store->getAlignment());
Value *VectorBlockGenerator::generateStrideOneLoad(
     ScopStmt &Stmt, LoadInst *Load, VectorValueMapT &ScalarMaps,
     __isl_keep isl_id_to_ast_expr *NewAccesses, bool NegativeStride = false) {
   unsigned VectorWidth = getVectorWidth();
   auto *Pointer = Load->getPointerOperand();
   Type *VectorPtrType = getVectorPtrTy(Pointer, VectorWidth);
   unsigned Offset = NegativeStride ? VectorWidth - 1 : 0;
 
   Value *NewPointer = nullptr;
-  NewPointer = generateLocationAccessed(Stmt, Load, Pointer, ScalarMaps[Offset],
+  NewPointer = generateLocationAccessed(Stmt, Load, ScalarMaps[Offset],
                                         VLTS[Offset], NewAccesses);
   Value *VectorPtr =
       Builder.CreateBitCast(NewPointer, VectorPtrType, "vector_ptr");
   LoadInst *VecLoad =
       Builder.CreateLoad(VectorPtr, Load->getName() + "_p_vec_full");
   if (!Aligned)
     VecLoad->setAlignment(8);
 
Value *VectorBlockGenerator::generateStrideOneLoad(
   return VecLoad;
 }
 
 Value *VectorBlockGenerator::generateStrideZeroLoad(
     ScopStmt &Stmt, LoadInst *Load, ValueMapT &BBMap,
     __isl_keep isl_id_to_ast_expr *NewAccesses) {
   auto *Pointer = Load->getPointerOperand();
   Type *VectorPtrType = getVectorPtrTy(Pointer, 1);
-  Value *NewPointer = generateLocationAccessed(Stmt, Load, Pointer, BBMap,
-                                               VLTS[0], NewAccesses);
+  Value *NewPointer =
+      generateLocationAccessed(Stmt, Load, BBMap, VLTS[0], NewAccesses);
   Value *VectorPtr = Builder.CreateBitCast(NewPointer, VectorPtrType,
                                            Load->getName() + "_p_vec_p");
   LoadInst *ScalarLoad =
       Builder.CreateLoad(VectorPtr, Load->getName() + "_p_splat_one");
 
   if (!Aligned)
     ScalarLoad->setAlignment(8);
 
Value *VectorBlockGenerator::generateUnknownStrideLoad(
   int VectorWidth = getVectorWidth();
   auto *Pointer = Load->getPointerOperand();
   VectorType *VectorType = VectorType::get(
       dyn_cast<PointerType>(Pointer->getType())->getElementType(), VectorWidth);
 
   Value *Vector = UndefValue::get(VectorType);
 
   for (int i = 0; i < VectorWidth; i++) {
-    Value *NewPointer = generateLocationAccessed(
-        Stmt, Load, Pointer, ScalarMaps[i], VLTS[i], NewAccesses);
+    Value *NewPointer = generateLocationAccessed(Stmt, Load, ScalarMaps[i],
+                                                 VLTS[i], NewAccesses);
     Value *ScalarLoad =
         Builder.CreateLoad(NewPointer, Load->getName() + "_p_scalar_");
     Vector = Builder.CreateInsertElement(
         Vector, ScalarLoad, Builder.getInt32(i), Load->getName() + "_p_vec_");
   }
 
   return Vector;
 }
Value *Vector = getVectorValue(Stmt, Store->getValueOperand(), VectorMap,
                                  ScalarMaps, getLoopForInst(Store));
 
   // Make sure we have scalar values available to access the pointer to
   // the data location.
   extractScalarValues(Store, VectorMap, ScalarMaps);
 
   if (Access.isStrideOne(isl_map_copy(Schedule))) {
     Type *VectorPtrType = getVectorPtrTy(Pointer, getVectorWidth());
-    Value *NewPointer = generateLocationAccessed(
-        Stmt, Store, Pointer, ScalarMaps[0], VLTS[0], NewAccesses);
+    Value *NewPointer = generateLocationAccessed(Stmt, Store, ScalarMaps[0],
+                                                 VLTS[0], NewAccesses);
 
     Value *VectorPtr =
         Builder.CreateBitCast(NewPointer, VectorPtrType, "vector_ptr");
     StoreInst *Store = Builder.CreateStore(Vector, VectorPtr);
 
     if (!Aligned)
       Store->setAlignment(8);
   } else {
     for (unsigned i = 0; i < ScalarMaps.size(); i++) {
       Value *Scalar = Builder.CreateExtractElement(Vector, Builder.getInt32(i));
-      Value *NewPointer = generateLocationAccessed(
-          Stmt, Store, Pointer, ScalarMaps[i], VLTS[i], NewAccesses);
+      Value *NewPointer = generateLocationAccessed(Stmt, Store, ScalarMaps[i],
+                                                   VLTS[i], NewAccesses);
       Builder.CreateStore(Scalar, NewPointer);
     }
   }
 }
 
 bool VectorBlockGenerator::hasVectorOperands(const Instruction *Inst,
                                              ValueMapT &VectorMap) {
   for (Value *Operand : Inst->operands())