AMDGPU: Avoid selecting ds_{read,write}2_b32 on SI

[nhaehnle]

Summary

To workaround a hardware issue in the (base + offset) calculation
when base is negative. The impact on code quality should be limited
since SILoadStoreOptimizer still runs afterwards and is able to
combine loads/stores based on known sign information.

This fixes visible corruption in Hitman on SI (easily reproducible
by running benchmark mode).

Change-Id: Ia178d207a5e2ac38ae7cd98b532ea2ae74704e5f
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99923

[arsenm]

lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp

296

This seems like an expensive check for this. Is this so important?

[arsenm]

lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp

296

I mean I don't understand why this would really matter that much. If we ignore this problem and let it vectorize, the resulting code shouldn't be that different when selection fixes it. The advantage is just making the IR closer to the final hardware instructions, which has minor cost analysis benefits?

[arsenm]

This still doesn't solve the real issue? We still have the selection. The vectorized load could appear in the original source program

[nhaehnle]

In D53160#1263197, @arsenm wrote:

This still doesn't solve the real issue? We still have the selection. The vectorized load could appear in the original source program

Well, an argument could be made that the corruption issue really only occurred because the shader was doing something that would be undefined behavior in the usual C semantics, namely accessing out-of-bounds data. What kind of semantics we want to support for such accesses is kind of up to us. Do we want to support vectorized loads that are partially out-of-bounds? GLSL says that out-of-bounds loads should be handled gracefully, but Mesa will always create scalar loads, so this particular patch is fine.

That said, I suppose we could say that we want partially out-of-bounds vectorized loads to be handled gracefully as well (with per-element out-of-bounds checks), and scalarize those vectorized loads again during selection. I haven't looked into that yet.

[nhaehnle]

@arsenm How about this approach?

[nhaehnle]

Update the summary

[arsenm]

Yes, something like this. I still expected to see some change in SelectDS64Bit4ByteAligned that has the fixme for this bug? I suppose the check for the sign bit would need to be here in the lowering since it would be more difficult to split during selection

lib/Target/AMDGPU/SIISelLowering.cpp

6711 ↗

(On Diff #169860)

NumElements == 2 is redundant and possibly wrong?

[nhaehnle]

Right, I looked at SelectDS64Bit4ByteAligned, but doing the split there
requires more code and also seems wrong since it'd mean inserting
additional nodes fairly late without giving them a chance to be combined
(not that they'd be combined very often, but still).

In a sense, splitting the vectorized load is a form of legalization.

Anyway, removing the FIXME comment.

[nhaehnle]

lib/Target/AMDGPU/SIISelLowering.cpp

6711 ↗

(On Diff #169860)

I don't know. We shouldn't have unaligned i64 loads at this point, I guess, but the check does ensure that we're really dealing with a vector load. And NumElements > 2 is dealt with above.

[nhaehnle]

lib/Target/AMDGPU/SIISelLowering.cpp

6711 ↗

(On Diff #169860)

And what's more, even if we did have unaligned i64 load/store at this point, it doesn't really make sense to try to fix them. The SI bug only affects the case where the 8 bytes straddle the lower bound of LDS (i.e., vaddr == -4). Trying to load an i64 from there is wrong anyway.

[arsenm]

LGTM. Is this only actually a problem with the UB because we don't bother trying to set m0 to the allocated size?

[nhaehnle]

• actually disable the pattern to select ds_{read,write}2_b32 to catch cases where we'd regress this fix

[nhaehnle]

In D53160#1266994, @arsenm wrote:

LGTM. Is this only actually a problem with the UB because we don't bother trying to set m0 to the allocated size?

Thanks. Not quite, the issue is that the shader actually does an out-of-bounds load. That would be UB in a normal language, except that GLSL actually says it's okay in this case. Of course the shader doesn't use the resulting value (there's a loop, and the shader loads two adjacent i32 values in each loop iteration, from indices n-1 and n; the n-1 part ends up not contributing to the calculation during the first iteration).

include/llvm/Analysis/TargetTransformInfo.h

public:
 
   /// \returns True if the load instruction is legal to vectorize.
   bool isLegalToVectorizeLoad(LoadInst *LI) const;
 
   /// \returns True if the store instruction is legal to vectorize.
   bool isLegalToVectorizeStore(StoreInst *SI) const;
 
   /// \returns True if it is legal to vectorize the given load chain.
-  bool isLegalToVectorizeLoadChain(unsigned ChainSizeInBytes,
+  bool isLegalToVectorizeLoadChain(Value *BasePtr, unsigned ChainSizeInBytes,
                                    unsigned Alignment,
-                                   unsigned AddrSpace) const;
+                                   const DataLayout &DL) const;
 
   /// \returns True if it is legal to vectorize the given store chain.
-  bool isLegalToVectorizeStoreChain(unsigned ChainSizeInBytes,
+  bool isLegalToVectorizeStoreChain(Value *BasePtr, unsigned ChainSizeInBytes,
                                     unsigned Alignment,
-                                    unsigned AddrSpace) const;
+                                    const DataLayout &DL) const;
 
   /// \returns The new vector factor value if the target doesn't support \p
   /// SizeInBytes loads or has a better vector factor.
   unsigned getLoadVectorFactor(unsigned VF, unsigned LoadSize,
                                unsigned ChainSizeInBytes,
                                VectorType *VecTy) const;
 
   /// \returns The new vector factor value if the target doesn't support \p
virtual void getMemcpyLoopResidualLoweringType(
       unsigned RemainingBytes, unsigned SrcAlign, unsigned DestAlign) const = 0;
   virtual bool areInlineCompatible(const Function *Caller,
                                    const Function *Callee) const = 0;
   virtual bool isIndexedLoadLegal(MemIndexedMode Mode, Type *Ty) const = 0;
   virtual bool isIndexedStoreLegal(MemIndexedMode Mode,Type *Ty) const = 0;
   virtual unsigned getLoadStoreVecRegBitWidth(unsigned AddrSpace) const = 0;
   virtual bool isLegalToVectorizeLoad(LoadInst *LI) const = 0;
   virtual bool isLegalToVectorizeStore(StoreInst *SI) const = 0;
-  virtual bool isLegalToVectorizeLoadChain(unsigned ChainSizeInBytes,
+  virtual bool isLegalToVectorizeLoadChain(Value *BasePtr,
+                                           unsigned ChainSizeInBytes,
                                            unsigned Alignment,
-                                           unsigned AddrSpace) const = 0;
-  virtual bool isLegalToVectorizeStoreChain(unsigned ChainSizeInBytes,
+                                           const DataLayout &DL) const = 0;
+  virtual bool isLegalToVectorizeStoreChain(Value *BasePtr,
+                                            unsigned ChainSizeInBytes,
                                             unsigned Alignment,
-                                            unsigned AddrSpace) const = 0;
+                                            const DataLayout &DL) const = 0;
   virtual unsigned getLoadVectorFactor(unsigned VF, unsigned LoadSize,
                                        unsigned ChainSizeInBytes,
                                        VectorType *VecTy) const = 0;
   virtual unsigned getStoreVectorFactor(unsigned VF, unsigned StoreSize,
                                         unsigned ChainSizeInBytes,
                                         VectorType *VecTy) const = 0;
   virtual bool useReductionIntrinsic(unsigned Opcode, Type *Ty,
                                      ReductionFlags) const = 0;
unsigned getLoadStoreVecRegBitWidth(unsigned AddrSpace) const override {
     return Impl.getLoadStoreVecRegBitWidth(AddrSpace);
   }
   bool isLegalToVectorizeLoad(LoadInst *LI) const override {
     return Impl.isLegalToVectorizeLoad(LI);
   }
   bool isLegalToVectorizeStore(StoreInst *SI) const override {
     return Impl.isLegalToVectorizeStore(SI);
   }
-  bool isLegalToVectorizeLoadChain(unsigned ChainSizeInBytes,
+  bool isLegalToVectorizeLoadChain(Value *BasePtr, unsigned ChainSizeInBytes,
                                    unsigned Alignment,
-                                   unsigned AddrSpace) const override {
-    return Impl.isLegalToVectorizeLoadChain(ChainSizeInBytes, Alignment,
-                                            AddrSpace);
+                                   const DataLayout &DL) const override {
+    return Impl.isLegalToVectorizeLoadChain(BasePtr, ChainSizeInBytes,
+                                            Alignment, DL);
   }
-  bool isLegalToVectorizeStoreChain(unsigned ChainSizeInBytes,
+  bool isLegalToVectorizeStoreChain(Value *BasePtr, unsigned ChainSizeInBytes,
                                     unsigned Alignment,
-                                    unsigned AddrSpace) const override {
-    return Impl.isLegalToVectorizeStoreChain(ChainSizeInBytes, Alignment,
-                                             AddrSpace);
+                                    const DataLayout &DL) const override {
+    return Impl.isLegalToVectorizeStoreChain(BasePtr, ChainSizeInBytes,
+                                             Alignment, DL);
   }
   unsigned getLoadVectorFactor(unsigned VF, unsigned LoadSize,
                                unsigned ChainSizeInBytes,
                                VectorType *VecTy) const override {
     return Impl.getLoadVectorFactor(VF, LoadSize, ChainSizeInBytes, VecTy);
   }
   unsigned getStoreVectorFactor(unsigned VF, unsigned StoreSize,
                                 unsigned ChainSizeInBytes,

include/llvm/Analysis/TargetTransformInfoImpl.h

public:
   }
 
   unsigned getLoadStoreVecRegBitWidth(unsigned AddrSpace) const { return 128; }
 
   bool isLegalToVectorizeLoad(LoadInst *LI) const { return true; }
 
   bool isLegalToVectorizeStore(StoreInst *SI) const { return true; }
 
-  bool isLegalToVectorizeLoadChain(unsigned ChainSizeInBytes,
+  bool isLegalToVectorizeLoadChain(Value *BasePtr, unsigned ChainSizeInBytes,
                                    unsigned Alignment,
-                                   unsigned AddrSpace) const {
+                                   const DataLayout &DL) const {
     return true;
   }
 
-  bool isLegalToVectorizeStoreChain(unsigned ChainSizeInBytes,
+  bool isLegalToVectorizeStoreChain(Value *BasePtr, unsigned ChainSizeInBytes,
                                     unsigned Alignment,
-                                    unsigned AddrSpace) const {
+                                    const DataLayout &DL) const {
     return true;
   }
 
   unsigned getLoadVectorFactor(unsigned VF, unsigned LoadSize,
                                unsigned ChainSizeInBytes,
                                VectorType *VecTy) const {
     return VF;
   }

lib/Analysis/TargetTransformInfo.cpp

bool TargetTransformInfo::isLegalToVectorizeLoad(LoadInst *LI) const {
   return TTIImpl->isLegalToVectorizeLoad(LI);
 }
 
 bool TargetTransformInfo::isLegalToVectorizeStore(StoreInst *SI) const {
   return TTIImpl->isLegalToVectorizeStore(SI);
 }
 
 bool TargetTransformInfo::isLegalToVectorizeLoadChain(
-    unsigned ChainSizeInBytes, unsigned Alignment, unsigned AddrSpace) const {
-  return TTIImpl->isLegalToVectorizeLoadChain(ChainSizeInBytes, Alignment,
-                                              AddrSpace);
+    Value *BasePtr, unsigned ChainSizeInBytes, unsigned Alignment,
+    const DataLayout &DL) const {
+  return TTIImpl->isLegalToVectorizeLoadChain(BasePtr, ChainSizeInBytes,
+                                              Alignment, DL);
 }
 
 bool TargetTransformInfo::isLegalToVectorizeStoreChain(
-    unsigned ChainSizeInBytes, unsigned Alignment, unsigned AddrSpace) const {
-  return TTIImpl->isLegalToVectorizeStoreChain(ChainSizeInBytes, Alignment,
-                                               AddrSpace);
+    Value *BasePtr, unsigned ChainSizeInBytes, unsigned Alignment,
+    const DataLayout &DL) const {
+  return TTIImpl->isLegalToVectorizeStoreChain(BasePtr, ChainSizeInBytes,
+                                               Alignment, DL);
 }
 
 unsigned TargetTransformInfo::getLoadVectorFactor(unsigned VF,
                                                   unsigned LoadSize,
                                                   unsigned ChainSizeInBytes,
                                                   VectorType *VecTy) const {
   return TTIImpl->getLoadVectorFactor(VF, LoadSize, ChainSizeInBytes, VecTy);
 }

lib/Target/AMDGPU/AMDGPUTargetTransformInfo.h

public:
   unsigned getLoadVectorFactor(unsigned VF, unsigned LoadSize,
                                unsigned ChainSizeInBytes,
                                VectorType *VecTy) const;
   unsigned getStoreVectorFactor(unsigned VF, unsigned StoreSize,
                                 unsigned ChainSizeInBytes,
                                 VectorType *VecTy) const;
   unsigned getLoadStoreVecRegBitWidth(unsigned AddrSpace) const;
 
-  bool isLegalToVectorizeMemChain(unsigned ChainSizeInBytes,
+  bool isLegalToVectorizeMemChain(Value *BasePtr, unsigned ChainSizeInBytes,
                                   unsigned Alignment,
-                                  unsigned AddrSpace) const;
-  bool isLegalToVectorizeLoadChain(unsigned ChainSizeInBytes,
+                                  const DataLayout &DL) const;
+  bool isLegalToVectorizeLoadChain(Value *BasePtr, unsigned ChainSizeInBytes,
                                    unsigned Alignment,
-                                   unsigned AddrSpace) const;
-  bool isLegalToVectorizeStoreChain(unsigned ChainSizeInBytes,
+                                   const DataLayout &DL) const;
+  bool isLegalToVectorizeStoreChain(Value *BasePtr, unsigned ChainSizeInBytes,
                                     unsigned Alignment,
-                                    unsigned AddrSpace) const;
+                                    const DataLayout &DL) const;
 
   unsigned getMaxInterleaveFactor(unsigned VF);
 
   bool getTgtMemIntrinsic(IntrinsicInst *Inst, MemIntrinsicInfo &Info) const;
 
   int getArithmeticInstrCost(
     unsigned Opcode, Type *Ty,
     TTI::OperandValueKind Opd1Info = TTI::OK_AnyValue,
public:
 
   void getUnrollingPreferences(Loop *L, ScalarEvolution &SE,
                                TTI::UnrollingPreferences &UP);
   unsigned getHardwareNumberOfRegisters(bool Vec) const;
   unsigned getNumberOfRegisters(bool Vec) const;
   unsigned getRegisterBitWidth(bool Vector) const;
   unsigned getMinVectorRegisterBitWidth() const;
   unsigned getLoadStoreVecRegBitWidth(unsigned AddrSpace) const;
-  bool isLegalToVectorizeMemChain(unsigned ChainSizeInBytes, unsigned Alignment,
-                                  unsigned AddrSpace) const;
-  bool isLegalToVectorizeLoadChain(unsigned ChainSizeInBytes,
-                       unsigned Alignment,
-                                   unsigned AddrSpace) const;
-  bool isLegalToVectorizeStoreChain(unsigned ChainSizeInBytes,
-                                    unsigned Alignment,
-                                    unsigned AddrSpace) const;
+  bool isLegalToVectorizeMemChain(Value *BasePtr, unsigned ChainSizeInBytes,
+                                  unsigned Alignment,
+                                  const DataLayout &DL) const;
+  bool isLegalToVectorizeLoadChain(Value *BasePtr, unsigned ChainSizeInBytes,
+                                   const DataLayout &DL) const;
+  bool isLegalToVectorizeStoreChain(Value *BasePtr, unsigned ChainSizeInBytes,
+                                    unsigned Alignment,
+                                    const DataLayout &DL) const;
   unsigned getMaxInterleaveFactor(unsigned VF);
   unsigned getCFInstrCost(unsigned Opcode);
   int getVectorInstrCost(unsigned Opcode, Type *ValTy, unsigned Index);
 };
 
 } // end namespace llvm
 
 #endif // LLVM_LIB_TARGET_AMDGPU_AMDGPUTARGETTRANSFORMINFO_H

lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp

if (AddrSpace == AMDGPUAS::FLAT_ADDRESS ||
     return 128;
 
   if (AddrSpace == AMDGPUAS::PRIVATE_ADDRESS)
     return 8 * ST->getMaxPrivateElementSize();
 
   llvm_unreachable("unhandled address space");
 }
 
-bool GCNTTIImpl::isLegalToVectorizeMemChain(unsigned ChainSizeInBytes,
-                                               unsigned Alignment,
-                                               unsigned AddrSpace) const {
+bool GCNTTIImpl::isLegalToVectorizeMemChain(Value *BasePtr,
+                                            unsigned ChainSizeInBytes,
+                                            const DataLayout &DL) const {
+  auto Ty = cast<PointerType>(BasePtr->getType());
+  unsigned AS = Ty->getAddressSpace();
+
   // We allow vectorization of flat stores, even though we may need to decompose
   // them later if they may access private memory. We don't have enough context
   // here, and legalization can handle it.
-  if (AddrSpace == AMDGPUAS::PRIVATE_ADDRESS) {
+  if (AS == AMDGPUAS::PRIVATE_ADDRESS) {
     return (Alignment >= 4 || ST->hasUnalignedScratchAccess()) &&
       ChainSizeInBytes <= ST->getMaxPrivateElementSize();
   }
+
+  // SI has a hardware bug in the LDS / GDS bounds checking: if the base address
+  // is negative, then the instruction is incorrectly treated as out-of-bounds
+  // even if base + offset is in bounds. This affects the high words of a
+  // vectorized load / store.
+  if (ST->getGeneration() == AMDGPUSubtarget::SOUTHERN_ISLANDS &&
+      (AS == AMDGPUAS::LOCAL_ADDRESS || AS == AMDGPUAS::REGION_ADDRESS)) {
+    if (ChainSizeInBytes <= Alignment)
       return true;
+
+    KnownBits Bits = computeKnownBits(BasePtr, DL);
+    return Bits.isNonNegative();
   }
 
-bool GCNTTIImpl::isLegalToVectorizeLoadChain(unsigned ChainSizeInBytes,
-                                                unsigned Alignment,
-                                                unsigned AddrSpace) const {
-  return isLegalToVectorizeMemChain(ChainSizeInBytes, Alignment, AddrSpace);
+  return true;
+}
+
+bool GCNTTIImpl::isLegalToVectorizeLoadChain(Value *BasePtr,
+                                             unsigned ChainSizeInBytes,
+                                             const DataLayout &DL) const {
+  return isLegalToVectorizeMemChain(BasePtr, ChainSizeInBytes, Alignment, DL);
 }
 
-bool GCNTTIImpl::isLegalToVectorizeStoreChain(unsigned ChainSizeInBytes,
-                                                 unsigned Alignment,
-                                                 unsigned AddrSpace) const {
-  return isLegalToVectorizeMemChain(ChainSizeInBytes, Alignment, AddrSpace);
+bool GCNTTIImpl::isLegalToVectorizeStoreChain(Value *BasePtr,
+                                              unsigned ChainSizeInBytes,
+                                              const DataLayout &DL) const {
+  return isLegalToVectorizeMemChain(BasePtr, ChainSizeInBytes, Alignment, DL);
 }
 
 unsigned GCNTTIImpl::getMaxInterleaveFactor(unsigned VF) {
   // Disable unrolling if the loop is not vectorized.
   // TODO: Enable this again.
   if (VF == 1)
     return 1;
 
unsigned R600TTIImpl::getLoadStoreVecRegBitWidth(unsigned AddrSpace) const {
   if ((AddrSpace == AMDGPUAS::PARAM_D_ADDRESS ||
       AddrSpace == AMDGPUAS::PARAM_I_ADDRESS ||
       (AddrSpace >= AMDGPUAS::CONSTANT_BUFFER_0 &&
       AddrSpace <= AMDGPUAS::CONSTANT_BUFFER_15)))
     return 128;
   llvm_unreachable("unhandled address space");
 }
 
-bool R600TTIImpl::isLegalToVectorizeMemChain(unsigned ChainSizeInBytes,
+bool R600TTIImpl::isLegalToVectorizeMemChain(Value *BasePtr,
+                                             unsigned ChainSizeInBytes,
                                              unsigned Alignment,
-                                             unsigned AddrSpace) const {
+                                             const DataLayout &DL) const {
   // We allow vectorization of flat stores, even though we may need to decompose
   // them later if they may access private memory. We don't have enough context
   // here, and legalization can handle it.
-  return (AddrSpace != AMDGPUAS::PRIVATE_ADDRESS);
+  unsigned AS = cast<PointerType>(BasePtr->getType())->getAddressSpace();
+  return AS != AMDGPUAS::PRIVATE_ADDRESS;
 }
 
-bool R600TTIImpl::isLegalToVectorizeLoadChain(unsigned ChainSizeInBytes,
+bool R600TTIImpl::isLegalToVectorizeLoadChain(Value *BasePtr,
+                                              unsigned ChainSizeInBytes,
                                               unsigned Alignment,
-                                              unsigned AddrSpace) const {
-  return isLegalToVectorizeMemChain(ChainSizeInBytes, Alignment, AddrSpace);
+                                              const DataLayout &DL) const {
+  return isLegalToVectorizeMemChain(BasePtr, ChainSizeInBytes, Alignment, DL);
 }
 
-bool R600TTIImpl::isLegalToVectorizeStoreChain(unsigned ChainSizeInBytes,
+bool R600TTIImpl::isLegalToVectorizeStoreChain(Value *BasePtr,
+                                               unsigned ChainSizeInBytes,
                                                unsigned Alignment,
-                                               unsigned AddrSpace) const {
-  return isLegalToVectorizeMemChain(ChainSizeInBytes, Alignment, AddrSpace);
+                                               const DataLayout &DL) const {
+  return isLegalToVectorizeMemChain(BasePtr, ChainSizeInBytes, Alignment, DL);
 }
 
 unsigned R600TTIImpl::getMaxInterleaveFactor(unsigned VF) {
   // Disable unrolling if the loop is not vectorized.
   // TODO: Enable this again.
   if (VF == 1)
     return 1;
 

lib/Target/NVPTX/NVPTXTargetTransformInfo.h

public:
   bool isSourceOfDivergence(const Value *V);
 
   unsigned getFlatAddressSpace() const {
     return AddressSpace::ADDRESS_SPACE_GENERIC;
   }
 
   // Loads and stores can be vectorized if the alignment is at least as big as
   // the load/store we want to vectorize.
-  bool isLegalToVectorizeLoadChain(unsigned ChainSizeInBytes,
+  bool isLegalToVectorizeLoadChain(Value *BasePtr, unsigned ChainSizeInBytes,
                                    unsigned Alignment,
-                                   unsigned AddrSpace) const {
+                                   const DataLayout &DL) const {
     return Alignment >= ChainSizeInBytes;
   }
-  bool isLegalToVectorizeStoreChain(unsigned ChainSizeInBytes,
+  bool isLegalToVectorizeStoreChain(Value *BasePtr, unsigned ChainSizeInBytes,
                                     unsigned Alignment,
-                                    unsigned AddrSpace) const {
-    return isLegalToVectorizeLoadChain(ChainSizeInBytes, Alignment, AddrSpace);
+                                    const DataLayout &DL) const {
+    return isLegalToVectorizeLoadChain(BasePtr, ChainSizeInBytes, Alignment,
+                                       DL);
   }
 
   // NVPTX has infinite registers of all kinds, but the actual machine doesn't.
   // We conservatively return 1 here which is just enough to enable the
   // vectorizers but disables heuristics based on the number of registers.
   // FIXME: Return a more reasonable number, while keeping an eye on
   // LoopVectorizer's unrolling heuristics.
   unsigned getNumberOfRegisters(bool Vector) const { return 1; }

lib/Transforms/Vectorize/LoadStoreVectorizer.cpp

if (accessIsMisaligned(SzInBytes, AS, Alignment)) {
 
     unsigned NewAlign = getOrEnforceKnownAlignment(S0->getPointerOperand(),
                                                    StackAdjustedAlignment,
                                                    DL, S0, nullptr, &DT);
     if (NewAlign != 0)
       Alignment = NewAlign;
   }
 
-  if (!TTI.isLegalToVectorizeStoreChain(SzInBytes, Alignment, AS)) {
+  if (!TTI.isLegalToVectorizeStoreChain(S0->getPointerOperand(), SzInBytes,
+                                        Alignment, DL)) {
     auto Chains = splitOddVectorElts(Chain, Sz);
     return vectorizeStoreChain(Chains.first, InstructionsProcessed) |
            vectorizeStoreChain(Chains.second, InstructionsProcessed);
   }
 
   BasicBlock::iterator First, Last;
   std::tie(First, Last) = getBoundaryInstrs(Chain);
   Builder.SetInsertPoint(&*Last);
unsigned NewAlign = getOrEnforceKnownAlignment(L0->getPointerOperand(),
                                                    StackAdjustedAlignment,
                                                    DL, L0, nullptr, &DT);
     if (NewAlign != 0)
       Alignment = NewAlign;
 
     Alignment = NewAlign;
   }
 
-  if (!TTI.isLegalToVectorizeLoadChain(SzInBytes, Alignment, AS)) {
+  if (!TTI.isLegalToVectorizeLoadChain(L0->getPointerOperand(), SzInBytes,
+                                       Alignment, DL)) {
     auto Chains = splitOddVectorElts(Chain, Sz);
     return vectorizeLoadChain(Chains.first, InstructionsProcessed) |
            vectorizeLoadChain(Chains.second, InstructionsProcessed);
   }
 
   LLVM_DEBUG({
     dbgs() << "LSV: Loads to vectorize:\n";
     for (Instruction *I : Chain)

test/Transforms/LoadStoreVectorizer/AMDGPU/ds-bounds.ll

+; RUN: opt -mtriple=amdgcn-mesa-mesa3d -mcpu=verde -load-store-vectorizer -S -o - %s | FileCheck -check-prefixes=ALL,SI %s
+; RUN: opt -mtriple=amdgcn-mesa-mesa3d -mcpu=bonaire -load-store-vectorizer -S -o - %s | FileCheck -check-prefixes=ALL,NONSI %s
+
+target datalayout = "e-p:64:64-p1:64:64-p2:32:32:32:32:16-p3:32:32:32:32:16-p4:64:64-p5:32:32-p6:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5"
+
+@compute_lds = external addrspace(3) global [512 x i32], align 16
+
+; ALL-LABEL: @store_aligned(
+; ALL: store <2 x i32> <i32 42, i32 43>, <2 x i32> addrspace(3)* %0, align 8
+define amdgpu_cs void @store_aligned(i32 addrspace(3)* %ptr) #0 {
+entry:
+  %ptr.gep.1 = getelementptr i32, i32 addrspace(3)* %ptr, i32 1
+
+  store i32 42, i32 addrspace(3)* %ptr, align 8
+  store i32 43, i32 addrspace(3)* %ptr.gep.1
+  ret void
+}
+
+
+; ALL-LABEL: @store_global_const_idx(
+;
+; TODO: Addresses are known-positive, this could be merged!
+; SI: store i32
+; SI: store i32
+;
+; NONSI: store <2 x i32> <i32 42, i32 43>, <2 x i32> addrspace(3)* %0, align 4
+define amdgpu_cs void @store_global_const_idx() #0 {
+entry:
+  %ptr.a = getelementptr [512 x i32], [512 x i32] addrspace(3)* @compute_lds, i32 0, i32 3
+  %ptr.b = getelementptr [512 x i32], [512 x i32] addrspace(3)* @compute_lds, i32 0, i32 4
+
+  store i32 42, i32 addrspace(3)* %ptr.a
+  store i32 43, i32 addrspace(3)* %ptr.b
+  ret void
+}
+
+
+; ALL-LABEL: @store_global_var_idx_case1(
+; SI: store i32
+; SI: store i32
+; NONSI: store <2 x i32> <i32 42, i32 43>, <2 x i32> addrspace(3)* %0, align 4
+define amdgpu_cs void @store_global_var_idx_case1(i32 %idx) #0 {
+entry:
+  %ptr.a = getelementptr [512 x i32], [512 x i32] addrspace(3)* @compute_lds, i32 0, i32 %idx
+  %ptr.b = getelementptr i32, i32 addrspace(3)* %ptr.a, i32 1
+
+  store i32 42, i32 addrspace(3)* %ptr.a
+  store i32 43, i32 addrspace(3)* %ptr.b
+  ret void
+}
+
+
+; ALL-LABEL: @store_global_var_idx_case2(
+;
+; TODO: Addresses are known-positive, this could be merged!
+; SI: store i32
+; SI: store i32
+;
+; NONSI: store <2 x i32> <i32 42, i32 43>, <2 x i32> addrspace(3)* %0, align 4
+define amdgpu_cs void @store_global_var_idx_case2(i32 %idx) #0 {
+entry:
+  %idx.and = and i32 %idx, 255
+  %ptr.a = getelementptr [512 x i32], [512 x i32] addrspace(3)* @compute_lds, i32 0, i32 %idx.and
+  %ptr.b = getelementptr i32, i32 addrspace(3)* %ptr.a, i32 1
+
+  store i32 42, i32 addrspace(3)* %ptr.a
+  store i32 43, i32 addrspace(3)* %ptr.b
+  ret void
+}
+
+
+attributes #0 = { nounwind }

test/Transforms/LoadStoreVectorizer/AMDGPU/merge-stores.ll

-; RUN: opt -mtriple=amdgcn-amd-amdhsa -load-store-vectorizer -S -o - %s | FileCheck %s
+; RUN: opt -mtriple=amdgcn-amd-amdhsa -mcpu=verde -load-store-vectorizer -S -o - %s | FileCheck --check-prefixes=CHECK,SI %s
+; RUN: opt -mtriple=amdgcn-amd-amdhsa -mcpu=bonaire -load-store-vectorizer -S -o - %s | FileCheck --check-prefixes=CHECK,NONSI %s
 ; Copy of test/CodeGen/AMDGPU/merge-stores.ll with some additions
 
 target datalayout = "e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5"
 
 ; TODO: Vector element tests
 ; TODO: Non-zero base offset for load and store combinations
 ; TODO: Same base addrspacecasted
 
define amdgpu_kernel void @merge_local_store_2_constants_i8(i8 addrspace(3)* %out) #0 {
   %out.gep.1 = getelementptr i8, i8 addrspace(3)* %out, i32 1
 
   store i8 123, i8 addrspace(3)* %out.gep.1
   store i8 456, i8 addrspace(3)* %out, align 2
   ret void
 }
 
 ; CHECK-LABEL: @merge_local_store_2_constants_i32
-; CHECK: store <2 x i32> <i32 456, i32 123>, <2 x i32> addrspace(3)* %{{[0-9]+}}, align 4
+; SI: store i32
+; SI: store i32
+; NONSI: store <2 x i32> <i32 456, i32 123>, <2 x i32> addrspace(3)* %{{[0-9]+}}, align 4
 define amdgpu_kernel void @merge_local_store_2_constants_i32(i32 addrspace(3)* %out) #0 {
   %out.gep.1 = getelementptr i32, i32 addrspace(3)* %out, i32 1
 
   store i32 123, i32 addrspace(3)* %out.gep.1
   store i32 456, i32 addrspace(3)* %out
   ret void
 }
 
 ; CHECK-LABEL: @merge_local_store_2_constants_i32_align_2
 ; CHECK: store i32
 ; CHECK: store i32
 define amdgpu_kernel void @merge_local_store_2_constants_i32_align_2(i32 addrspace(3)* %out) #0 {
   %out.gep.1 = getelementptr i32, i32 addrspace(3)* %out, i32 1
 
   store i32 123, i32 addrspace(3)* %out.gep.1, align 2
   store i32 456, i32 addrspace(3)* %out, align 2
   ret void
 }
 
 ; CHECK-LABEL: @merge_local_store_4_constants_i32
-; CHECK: store <4 x i32> <i32 1234, i32 123, i32 456, i32 333>, <4 x i32> addrspace(3)*
+; SI: store i32
+; SI: store i32
+; SI: store i32
+; SI: store i32
+; NONSI: store <4 x i32> <i32 1234, i32 123, i32 456, i32 333>, <4 x i32> addrspace(3)*
 define amdgpu_kernel void @merge_local_store_4_constants_i32(i32 addrspace(3)* %out) #0 {
   %out.gep.1 = getelementptr i32, i32 addrspace(3)* %out, i32 1
   %out.gep.2 = getelementptr i32, i32 addrspace(3)* %out, i32 2
   %out.gep.3 = getelementptr i32, i32 addrspace(3)* %out, i32 3
 
   store i32 123, i32 addrspace(3)* %out.gep.1
   store i32 456, i32 addrspace(3)* %out.gep.2
   store i32 333, i32 addrspace(3)* %out.gep.3

test/Transforms/LoadStoreVectorizer/AMDGPU/missing-alignment.ll

-; RUN: opt -mtriple=amdgcn-- -load-store-vectorizer -S -o - %s | FileCheck %s
+; RUN: opt -mtriple=amdgcn-- -mcpu=bonaire -load-store-vectorizer -S -o - %s | FileCheck %s
 
 target datalayout = "e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5"
 
 @lds = internal addrspace(3) global [512 x float] undef, align 4
 
 ; The original load has an implicit alignment of 4, and should not
 ; increase to an align 8 load.
 

test/Transforms/LoadStoreVectorizer/AMDGPU/multiple_tails.ll

-; RUN: opt -mtriple=amdgcn-amd-amdhsa -basicaa -load-store-vectorizer -S -o - %s | FileCheck %s
+; RUN: opt -mtriple=amdgcn-amd-amdhsa -mcpu=bonaire -basicaa -load-store-vectorizer -S -o - %s | FileCheck %s
 
 target datalayout = "e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5"
 
 ; Checks that there is no crash when there are multiple tails
 ; for a the same head starting a chain.
 @0 = internal addrspace(3) global [16384 x i32] undef
 
 ; CHECK-LABEL: @no_crash(

test/Transforms/LoadStoreVectorizer/AMDGPU/pointer-elements.ll

-; RUN: opt -mtriple=amdgcn-amd-amdhsa -basicaa -load-store-vectorizer -S -o - %s | FileCheck %s
+; RUN: opt -mtriple=amdgcn-amd-amdhsa -mcpu=bonaire -basicaa -load-store-vectorizer -S -o - %s | FileCheck %s
 
 target datalayout = "e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5"
 
 declare i32 @llvm.amdgcn.workitem.id.x() #1
 
 ; CHECK-LABEL: @merge_v2p1i8(
 ; CHECK: load <2 x i64>
 ; CHECK: inttoptr i64 %{{[^ ]+}} to i8 addrspace(1)*