Add TLI.allowsMisalignedMemoryAccesses to LoadStoreVectorizer

[asbirlea]

Summary

Extend TTI to access TLI.allowsMisalignedMemoryAccesses(). Check condition when vectorizing load and store chains.

[arsenm]

include/llvm/Analysis/TargetTransformInfo.h

392 ↗

(On Diff #62532)

This is missing a lot of parameters compared to the TLI version. It at least needs and address space and the alignment value

392 ↗

(On Diff #62532)

IsFast would be helpful too

[asbirlea]

Adding AddressSpace and Alignment.

[asbirlea]

I'm not clear if the address space argument is the right one.

Looking into where to get the isFast info from.

[asbirlea]

Added Fast parameter.
Updated check on allowsMisaligned.
Updated test that now vectorizes.

[jlebar]

include/llvm/Analysis/TargetTransformInfo.h

391 ↗

(On Diff #62984)

Maybe a better comment would be

Determine whether we can read BitWidth bits from memory with the given alignment (in bytes).

That is, this isn't strictly for determining whether *misaligned* memory accesses are allowed -- if Alignment == BitWidth, the call is still allowed (right?).

lib/Transforms/Vectorize/LoadStoreVectorizer.cpp

703 ↗

(On Diff #62984)

Might as well set this to false, to guard against low-quality TTI overrides.

704 ↗

(On Diff #62984)

No == true.

705 ↗

(On Diff #62984)

I'm not sure why this is the right condition. "If the store is going to be misaligned, don't vectorize it." This is now, essentially, "if the store is going to be misaligned and misaligned stores of this witdh and alignment are not fast, don't vectorize it," right? So shouldn't the condition be

`(Alignment % SzInBytes) != 0 && (!allowsMisaligned(...) || !Fast)`

?

In fact, maybe it makes sense to centralize this logic, rename allowsMisaligned, and just do

`memoryAccessIsAllowedAndFast(SzInBytes, AS, Alignment)`

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

505 ↗

(On Diff #62984)

I am surprised we can vectorize this...are you sure it's not a bug?

[asbirlea]

Update after formatting.

[jlebar]

include/llvm/Analysis/TargetTransformInfo.h

391 ↗

(On Diff #63130)

(This suggestion may be wrong -- the callee may assume that the memory access is misaligned, even if Alignment would indicate it's not. I dunno.)

[asbirlea]

lib/Transforms/Vectorize/LoadStoreVectorizer.cpp

700 ↗

(On Diff #63130)

Updated this with new condition. Now there are other tests that vectorize, the ones with "natural" alignment that I had updated previously. I'm looking into whether this is correct behavior.

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

505 ↗

(On Diff #63130)

This is no longer vectorized with the above changes. I will update it with the other tests.

[arsenm]

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

505 ↗

(On Diff #63130)

I recently enabled misaligned access depending on the triple, so this should probably be vectorized?

[arsenm]

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

505 ↗

(On Diff #63130)

Never mind, I misread which test this is

[asbirlea]

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

505 ↗

(On Diff #63130)

I believe due to what you enabled, the tests ending in "_natural_align" are now vectorizing. They don't vectorize without the triple argument.
I'm still not clear why/if the vectorization is correct. Would you mind taking a look to confirm?
I can update the (4) tests in that case.

[arsenm]

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

505 ↗

(On Diff #63130)

I don't see the changes in the diff, but that should be fine

[asbirlea]

Updating tests.

[arsenm]

lib/Transforms/Vectorize/LoadStoreVectorizer.cpp

830 ↗

(On Diff #63146)

! instead of == false

[asbirlea]

Replace ==false with !

[asbirlea]

clang-format.

[asbirlea]

include/llvm/Analysis/TargetTransformInfo.h

391 ↗

(On Diff #63146)

The definition should be the same as the one in TLI. It reads:

/// \brief Determine if the target supports unaligned memory accesses.
///
/// This function returns true if the target allows unaligned memory accesses
/// of the specified type in the given address space. If true, it also returns
/// whether the unaligned memory access is "fast" in the last argument by
/// reference. This is used, for example, in situations where an array
/// copy/move/set is converted to a sequence of store operations. Its use
/// helps to ensure that such replacements don't generate code that causes an
/// alignment error (trap) on the target machine.

The difference in the API here is not having a type, but a size in bits. The brief description still applies. Perhaps use just that?

[jlebar]

include/llvm/Analysis/TargetTransformInfo.h

391 ↗

(On Diff #63155)

sgtm

lib/Transforms/Vectorize/LoadStoreVectorizer.cpp

702 ↗

(On Diff #63155)

I still think it might make sense to move the (Alignment % SzInBytes) != 0 && (Alignment % TargetBaseAlign) != 0 checks into the helper function (currently called allowsMisalignedAndIsFast, would need a new name). But, up to you.

[asbirlea]

Address comments.

[asbirlea]

include/llvm/Analysis/TargetTransformInfo.h

391 ↗

(On Diff #63155)

done.

lib/Transforms/Vectorize/LoadStoreVectorizer.cpp

702 ↗

(On Diff #63155)

I debated a bit whether to do this. Ideally I would have liked a single point of usage (and future removal) for TargetBaseAlign. Still, the condition is cleaner and I can remove a nesting level.

[asbirlea]

Format.

llvm/trunk/include/llvm/Analysis/TargetTransformInfo.h

public:
   /// floating-point operations because the semantics of vector and scalar
   /// floating-point semantics may differ. For example, ARM NEON v7 SIMD math
   /// does not support IEEE-754 denormal numbers, while depending on the
   /// platform, scalar floating-point math does.
   /// This applies to floating-point math operations and calls, not memory
   /// operations, shuffles, or casts.
   bool isFPVectorizationPotentiallyUnsafe() const;
 
+  /// \brief Determine if the target supports unaligned memory accesses.
+  bool allowsMisalignedMemoryAccesses(unsigned BitWidth, unsigned AddressSpace = 0,
+                                      unsigned Alignment = 1,
+                                      bool *Fast = nullptr) const;
+
   /// \brief Return hardware support for population count.
   PopcntSupportKind getPopcntSupport(unsigned IntTyWidthInBit) const;
 
   /// \brief Return true if the hardware has a fast square-root instruction.
   bool haveFastSqrt(Type *Ty) const;
 
   /// \brief Return the expected cost of supporting the floating point operation
   /// of the specified type.
public:
   virtual bool isProfitableToHoist(Instruction *I) = 0;
   virtual bool isTypeLegal(Type *Ty) = 0;
   virtual unsigned getJumpBufAlignment() = 0;
   virtual unsigned getJumpBufSize() = 0;
   virtual bool shouldBuildLookupTables() = 0;
   virtual bool enableAggressiveInterleaving(bool LoopHasReductions) = 0;
   virtual bool enableInterleavedAccessVectorization() = 0;
   virtual bool isFPVectorizationPotentiallyUnsafe() = 0;
+  virtual bool allowsMisalignedMemoryAccesses(unsigned BitWidth,
+                                              unsigned AddressSpace,
+                                              unsigned Alignment,
+                                              bool *Fast) = 0;
   virtual PopcntSupportKind getPopcntSupport(unsigned IntTyWidthInBit) = 0;
   virtual bool haveFastSqrt(Type *Ty) = 0;
   virtual int getFPOpCost(Type *Ty) = 0;
   virtual int getIntImmCost(const APInt &Imm, Type *Ty) = 0;
   virtual int getIntImmCost(unsigned Opc, unsigned Idx, const APInt &Imm,
                             Type *Ty) = 0;
   virtual int getIntImmCost(Intrinsic::ID IID, unsigned Idx, const APInt &Imm,
                             Type *Ty) = 0;
bool enableAggressiveInterleaving(bool LoopHasReductions) override {
     return Impl.enableAggressiveInterleaving(LoopHasReductions);
   }
   bool enableInterleavedAccessVectorization() override {
     return Impl.enableInterleavedAccessVectorization();
   }
   bool isFPVectorizationPotentiallyUnsafe() override {
     return Impl.isFPVectorizationPotentiallyUnsafe();
   }
+  bool allowsMisalignedMemoryAccesses(unsigned BitWidth, unsigned AddressSpace,
+                                      unsigned Alignment, bool *Fast) override {
+    return Impl.allowsMisalignedMemoryAccesses(BitWidth, AddressSpace,
+                                               Alignment, Fast);
+  }
   PopcntSupportKind getPopcntSupport(unsigned IntTyWidthInBit) override {
     return Impl.getPopcntSupport(IntTyWidthInBit);
   }
   bool haveFastSqrt(Type *Ty) override { return Impl.haveFastSqrt(Ty); }
 
   int getFPOpCost(Type *Ty) override { return Impl.getFPOpCost(Ty); }
 
   int getIntImmCost(const APInt &Imm, Type *Ty) override {

llvm/trunk/include/llvm/Analysis/TargetTransformInfoImpl.h

public:
   bool shouldBuildLookupTables() { return true; }
 
   bool enableAggressiveInterleaving(bool LoopHasReductions) { return false; }
 
   bool enableInterleavedAccessVectorization() { return false; }
 
   bool isFPVectorizationPotentiallyUnsafe() { return false; }
 
+  bool allowsMisalignedMemoryAccesses(unsigned BitWidth,
+                                      unsigned AddressSpace,
+                                      unsigned Alignment,
+                                      bool *Fast) { return false; }
+
   TTI::PopcntSupportKind getPopcntSupport(unsigned IntTyWidthInBit) {
     return TTI::PSK_Software;
   }
 
   bool haveFastSqrt(Type *Ty) { return false; }
 
   unsigned getFPOpCost(Type *Ty) { return TargetTransformInfo::TCC_Basic; }
 

llvm/trunk/include/llvm/CodeGen/BasicTTIImpl.h

public:
   // Provide value semantics. MSVC requires that we spell all of these out.
   BasicTTIImplBase(const BasicTTIImplBase &Arg)
       : BaseT(static_cast<const BaseT &>(Arg)) {}
   BasicTTIImplBase(BasicTTIImplBase &&Arg)
       : BaseT(std::move(static_cast<BaseT &>(Arg))) {}
 
   /// \name Scalar TTI Implementations
   /// @{
+  bool allowsMisalignedMemoryAccesses(unsigned BitWidth, unsigned AddressSpace,
+                                      unsigned Alignment, bool *Fast) const {
+    MVT M = MVT::getIntegerVT(BitWidth);
+    return getTLI()->allowsMisalignedMemoryAccesses(M, AddressSpace, Alignment, Fast);
+  }
 
   bool hasBranchDivergence() { return false; }
 
   bool isSourceOfDivergence(const Value *V) { return false; }
 
   bool isLegalAddImmediate(int64_t imm) {
     return getTLI()->isLegalAddImmediate(imm);
   }

llvm/trunk/lib/Analysis/TargetTransformInfo.cpp

 bool TargetTransformInfo::enableInterleavedAccessVectorization() const {
   return TTIImpl->enableInterleavedAccessVectorization();
 }
 
 bool TargetTransformInfo::isFPVectorizationPotentiallyUnsafe() const {
   return TTIImpl->isFPVectorizationPotentiallyUnsafe();
 }
 
+bool TargetTransformInfo::allowsMisalignedMemoryAccesses(unsigned BitWidth,
+                                                         unsigned AddressSpace,
+                                                         unsigned Alignment,
+                                                         bool *Fast) const {
+  return TTIImpl->allowsMisalignedMemoryAccesses(BitWidth, AddressSpace,
+                                                 Alignment, Fast);
+}
+
 TargetTransformInfo::PopcntSupportKind
 TargetTransformInfo::getPopcntSupport(unsigned IntTyWidthInBit) const {
   return TTIImpl->getPopcntSupport(IntTyWidthInBit);
 }
 
 bool TargetTransformInfo::haveFastSqrt(Type *Ty) const {
   return TTIImpl->haveFastSqrt(Ty);
 }

llvm/trunk/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp

private:
   /// Finds the load/stores to consecutive memory addresses and vectorizes them.
   bool vectorizeInstructions(ArrayRef<Value *> Instrs);
 
   /// Vectorizes the load instructions in Chain.
   bool vectorizeLoadChain(ArrayRef<Value *> Chain);
 
   /// Vectorizes the store instructions in Chain.
   bool vectorizeStoreChain(ArrayRef<Value *> Chain);
+
+  /// Check if this load/store access is misaligned accesses
+  bool accessIsMisaligned(unsigned SzInBytes, unsigned AddressSpace,
+                          unsigned Alignment);
 };
 
 class LoadStoreVectorizer : public FunctionPass {
 public:
   static char ID;
 
   LoadStoreVectorizer() : FunctionPass(ID) {
     initializeLoadStoreVectorizerPass(*PassRegistry::getPassRegistry());
DEBUG({
     for (Value *V : Chain)
       V->dump();
   });
 
   // Check alignment restrictions.
   unsigned Alignment = getAlignment(S0);
 
   // If the store is going to be misaligned, don't vectorize it.
-  // TODO: Check TLI.allowsMisalignedMemoryAccess
-  if ((Alignment % SzInBytes) != 0 && (Alignment % TargetBaseAlign) != 0) {
-    if (S0->getPointerAddressSpace() == 0) {
-      // If we're storing to an object on the stack, we control its alignment,
-      // so we can cheat and change it!
-      Value *V = GetUnderlyingObject(S0->getPointerOperand(), DL);
-      if (AllocaInst *AI = dyn_cast_or_null<AllocaInst>(V)) {
-        AI->setAlignment(TargetBaseAlign);
-        Alignment = TargetBaseAlign;
-      } else {
-        return false;
-      }
-    } else {
-      return false;
-    }
+  if (accessIsMisaligned(SzInBytes, AS, Alignment)) {
+    if (S0->getPointerAddressSpace() != 0)
+      return false;
+
   }
 
   BasicBlock::iterator First, Last;
   std::tie(First, Last) = getBoundaryInstrs(Chain);
 
   if (!isVectorizable(Chain, First, Last))
     return false;
 
if (ChainSize > VF) {
     return vectorizeLoadChain(Chain.slice(0, VF)) |
            vectorizeLoadChain(Chain.slice(VF));
   }
 
   // Check alignment restrictions.
   unsigned Alignment = getAlignment(L0);
 
   // If the load is going to be misaligned, don't vectorize it.
-  // TODO: Check TLI.allowsMisalignedMemoryAccess and remove TargetBaseAlign.
-  if ((Alignment % SzInBytes) != 0 && (Alignment % TargetBaseAlign) != 0) {
-    if (L0->getPointerAddressSpace() == 0) {
-      // If we're loading from an object on the stack, we control its alignment,
-      // so we can cheat and change it!
-      Value *V = GetUnderlyingObject(L0->getPointerOperand(), DL);
-      if (AllocaInst *AI = dyn_cast_or_null<AllocaInst>(V)) {
-        AI->setAlignment(TargetBaseAlign);
-        Alignment = TargetBaseAlign;
-      } else {
-        return false;
-      }
-    } else {
-      return false;
-    }
+  if (accessIsMisaligned(SzInBytes, AS, Alignment)) {
+    if (L0->getPointerAddressSpace() != 0)
+      return false;
+
   }
 
   DEBUG({
     dbgs() << "LSV: Loads to vectorize:\n";
     for (Value *V : Chain)
       V->dump();
   });
 
bool Vectorizer::vectorizeLoadChain(ArrayRef<Value *> Chain) {
   }
 
   eraseInstructions(Chain);
 
   ++NumVectorInstructions;
   NumScalarsVectorized += Chain.size();
   return true;
 }
+
+bool Vectorizer::accessIsMisaligned(unsigned SzInBytes, unsigned AddressSpace,
+                                    unsigned Alignment) {
+  bool Fast = false;
+  bool Allows = TTI.allowsMisalignedMemoryAccesses(SzInBytes * 8, AddressSpace,
+                                                   Alignment, &Fast);
+  // TODO: Remove TargetBaseAlign
+  return !(Allows && Fast) && (Alignment % SzInBytes) != 0 &&
+         (Alignment % TargetBaseAlign) != 0;
+}

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

define void @merge_global_store_2_constants_i8(i8 addrspace(1)* %out) #0 {
   %out.gep.1 = getelementptr i8, i8 addrspace(1)* %out, i32 1
 
   store i8 123, i8 addrspace(1)* %out.gep.1
   store i8 456, i8 addrspace(1)* %out, align 2
   ret void
 }
 
 ; CHECK-LABEL: @merge_global_store_2_constants_i8_natural_align
-; CHECK: store i8
-; CHECK: store i8
+; CHECK: store <2 x i8>
 define void @merge_global_store_2_constants_i8_natural_align(i8 addrspace(1)* %out) #0 {
   %out.gep.1 = getelementptr i8, i8 addrspace(1)* %out, i32 1
 
   store i8 123, i8 addrspace(1)* %out.gep.1
   store i8 456, i8 addrspace(1)* %out
   ret void
 }
 
define void @merge_global_store_2_constants_0_i16(i16 addrspace(1)* %out) #0 {
   %out.gep.1 = getelementptr i16, i16 addrspace(1)* %out, i32 1
 
   store i16 0, i16 addrspace(1)* %out.gep.1
   store i16 0, i16 addrspace(1)* %out, align 4
   ret void
 }
 
 ; CHECK-LABEL: @merge_global_store_2_constants_i16_natural_align
-; CHECK: store i16
-; CHECK: store i16
+; CHECK: store <2 x i16>
 define void @merge_global_store_2_constants_i16_natural_align(i16 addrspace(1)* %out) #0 {
   %out.gep.1 = getelementptr i16, i16 addrspace(1)* %out, i32 1
 
   store i16 123, i16 addrspace(1)* %out.gep.1
   store i16 456, i16 addrspace(1)* %out
   ret void
 }
 
 ; CHECK-LABEL: @merge_global_store_2_constants_half_natural_align
-; CHECK: store half
-; CHECK: store half
+; CHECK: store <2 x half>
 define void @merge_global_store_2_constants_half_natural_align(half addrspace(1)* %out) #0 {
   %out.gep.1 = getelementptr half, half addrspace(1)* %out, i32 1
 
   store half 2.0, half addrspace(1)* %out.gep.1
   store half 1.0, half addrspace(1)* %out
   ret void
 }
 
define void @merge_global_store_4_adjacent_loads_i8(i8 addrspace(1)* %out, i8 addrspace(1)* %in) #0 {
   store i8 %x, i8 addrspace(1)* %out, align 4
   store i8 %y, i8 addrspace(1)* %out.gep.1
   store i8 %z, i8 addrspace(1)* %out.gep.2
   store i8 %w, i8 addrspace(1)* %out.gep.3
   ret void
 }
 
 ; CHECK-LABEL: @merge_global_store_4_adjacent_loads_i8_natural_align
-; CHECK: load i8
-; CHECK: load i8
-; CHECK: load i8
-; CHECK: load i8
-; CHECK: store i8
-; CHECK: store i8
-; CHECK: store i8
-; CHECK: store i8
+; CHECK: load <4 x i8>
+; CHECK: store <4 x i8>
 define void @merge_global_store_4_adjacent_loads_i8_natural_align(i8 addrspace(1)* %out, i8 addrspace(1)* %in) #0 {
   %out.gep.1 = getelementptr i8, i8 addrspace(1)* %out, i8 1
   %out.gep.2 = getelementptr i8, i8 addrspace(1)* %out, i8 2
   %out.gep.3 = getelementptr i8, i8 addrspace(1)* %out, i8 3
   %in.gep.1 = getelementptr i8, i8 addrspace(1)* %in, i8 1
   %in.gep.2 = getelementptr i8, i8 addrspace(1)* %in, i8 2
   %in.gep.3 = getelementptr i8, i8 addrspace(1)* %in, i8 3