Remove TargetBaseAlign. Keep alignment for stack adjustments.

[asbirlea]

Summary

TargetBaseAlign is no longer required since LSV checks if target allows misaligned accesses.
A constant defining a base alignment is still needed for stack accesses where alignment can be adjusted.

[jlebar]

lib/Transforms/Vectorize/LoadStoreVectorizer.cpp

43 ↗

(On Diff #65992)

I think we still want a comment here, because we're basically assuming that 4 bytes is good enough for any type.

1033 ↗

(On Diff #65992)

Can we rewrite this function as

if (Alignment % SzInBytes == 0)
  return true;
bool Fast = false;
bool Allows = ...;
return !Allows || !Fast;

That would make it a lot more obvious to me.

[arsenm]

lib/Transforms/Vectorize/LoadStoreVectorizer.cpp

43 ↗

(On Diff #65992)

I think this makes an easier TTI hook now that it isn't related to unaligned access

[asbirlea]

Address comments.

[asbirlea]

lib/Transforms/Vectorize/LoadStoreVectorizer.cpp

1036 ↗

(On Diff #66001)

I think you meant return false on the first condition. Updated.

[jlebar]

lib/Transforms/Vectorize/LoadStoreVectorizer.cpp

1036 ↗

(On Diff #66001)

Yes, "not misaligned". Sigh.

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

 using namespace llvm;
 
 #define DEBUG_TYPE "load-store-vectorizer"
 STATISTIC(NumVectorInstructions, "Number of vector accesses generated");
 STATISTIC(NumScalarsVectorized, "Number of scalar accesses vectorized");
 
 namespace {
 
-// TODO: Remove this
-static const unsigned TargetBaseAlign = 4;
-
+// FIXME: Assuming stack alignment of 4 is always good enough
+static const unsigned StackAdjustedAlignment = 4;
 typedef SmallVector<Instruction *, 8> InstrList;
 typedef MapVector<Value *, InstrList> InstrListMap;
 
 class Vectorizer {
   Function &F;
   AliasAnalysis &AA;
   DominatorTree &DT;
   ScalarEvolution &SE;
bool Vectorizer::vectorizeStoreChain(
   if (accessIsMisaligned(SzInBytes, AS, Alignment)) {
     if (S0->getPointerAddressSpace() != 0)
       return false;
 
     // 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;
+      AI->setAlignment(StackAdjustedAlignment);
+      Alignment = StackAdjustedAlignment;
     } else {
       return false;
     }
   }
 
   BasicBlock::iterator First, Last;
   std::tie(First, Last) = getBoundaryInstrs(Chain);
   Builder.SetInsertPoint(&*Last);
bool Vectorizer::vectorizeLoadChain(
   if (accessIsMisaligned(SzInBytes, AS, Alignment)) {
     if (L0->getPointerAddressSpace() != 0)
       return false;
 
     // 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;
+      AI->setAlignment(StackAdjustedAlignment);
+      Alignment = StackAdjustedAlignment;
     } else {
       return false;
     }
   }
 
   DEBUG({
     dbgs() << "LSV: Loads to vectorize:\n";
     for (Instruction *I : Chain)
bool Vectorizer::vectorizeLoadChain(
 
   ++NumVectorInstructions;
   NumScalarsVectorized += Chain.size();
   return true;
 }
 
 bool Vectorizer::accessIsMisaligned(unsigned SzInBytes, unsigned AddressSpace,
                                     unsigned Alignment) {
+  if (Alignment % SzInBytes == 0)
+    return false;
   bool Fast = false;
   bool Allows = TTI.allowsMisalignedMemoryAccesses(SzInBytes * 8, AddressSpace,
                                                    Alignment, &Fast);
-  // TODO: Remove TargetBaseAlign
-  return !(Allows && Fast) && (Alignment % SzInBytes) != 0 &&
-         (Alignment % TargetBaseAlign) != 0;
+  return !Allows || !Fast;
 }