[Alignment][NFC] Assume AlignmentFromAssumptions::getNewAlignment is always set.

[gchatelet]

Summary

In D77454 we explain that LoadInst and StoreInst always have their alignment defined.
This allows to work backward here and to infer that getNewAlignment does not need to return 0 in case of failure.
Returning 1 also works since it needs to be greater than the Load/Store alignment which is a least 1.

This is patch is part of a series to introduce an Alignment type.
See this thread for context: http://lists.llvm.org/pipermail/llvm-dev/2019-July/133851.html
See this patch for the introduction of the type: https://reviews.llvm.org/D64790

[courbet]

why not change the single return 0 in getNewAlignment() with return Align(1), and modify that type ?

[gchatelet]

In D77538#1963575, @courbet wrote:

why not change the single return 0 in getNewAlignment() with return Align(1), and modify that type ?

Sure, I wanted to do it in several steps but sounds good to me.

[gchatelet]

• Address comments

[gchatelet]

llvm/lib/Transforms/Scalar/AlignmentFromAssumptions.cpp

182

I could remove these checks since we know both NewAlignment and NewIncAlignment are powers of two (and so are multiples of each other)

[courbet]

llvm/lib/Transforms/Scalar/AlignmentFromAssumptions.cpp

172

And now getNewAlignmentDiff() can return MaybeAlign, so we do not need that assert.

[gchatelet]

• Address comments

[efriedma]

llvm/lib/Transforms/Scalar/AlignmentFromAssumptions.cpp

329

Until D77454 lands, you have to use something like "DL.getValueOrABITypeAlignment(LI->getAlign(), LI->getType())", not "*LI->getAlign()". Similar for stores. Yes, it's sort of messy.

Not sure what MemIntrinsic does at the moment. I briefly tried to trace it, I think it crashes if the MemIntrinsic doesn't have an align attribute (which must be non-zero). So I guess that's fine, but we should probably fix the API.

llvm/lib/Transforms/Scalar/AlignmentFromAssumptions.cpp

 }
 
 // Given an expression for the (constant) alignment, AlignSCEV, and an
 // expression for the displacement between a pointer and the aligned address,
 // DiffSCEV, compute the alignment of the displaced pointer if it can be reduced
 // to a constant. Using SCEV to compute alignment handles the case where
 // DiffSCEV is a recurrence with constant start such that the aligned offset
 // is constant. e.g. {16,+,32} % 32 -> 16.
-static unsigned getNewAlignmentDiff(const SCEV *DiffSCEV,
+static MaybeAlign getNewAlignmentDiff(const SCEV *DiffSCEV,
                                       const SCEV *AlignSCEV,
                                       ScalarEvolution *SE) {
   // DiffUnits = Diff % int64_t(Alignment)
   const SCEV *DiffUnitsSCEV = SE->getURemExpr(DiffSCEV, AlignSCEV);
 
   LLVM_DEBUG(dbgs() << "\talignment relative to " << *AlignSCEV << " is "
                     << *DiffUnitsSCEV << " (diff: " << *DiffSCEV << ")\n");
 
   if (const SCEVConstant *ConstDUSCEV =
       dyn_cast<SCEVConstant>(DiffUnitsSCEV)) {
     int64_t DiffUnits = ConstDUSCEV->getValue()->getSExtValue();
 
     // If the displacement is an exact multiple of the alignment, then the
     // displaced pointer has the same alignment as the aligned pointer, so
     // return the alignment value.
     if (!DiffUnits)
-      return (unsigned)
-        cast<SCEVConstant>(AlignSCEV)->getValue()->getSExtValue();
+      return cast<SCEVConstant>(AlignSCEV)->getValue()->getAlignValue();
 
     // If the displacement is not an exact multiple, but the remainder is a
     // constant, then return this remainder (but only if it is a power of 2).
     uint64_t DiffUnitsAbs = std::abs(DiffUnits);
     if (isPowerOf2_64(DiffUnitsAbs))
-      return (unsigned) DiffUnitsAbs;
+      return Align(DiffUnitsAbs);
   }
 
-  return 0;
+  return None;
 }
 
 // There is an address given by an offset OffSCEV from AASCEV which has an
 // alignment AlignSCEV. Use that information, if possible, to compute a new
 // alignment for Ptr.
-static unsigned getNewAlignment(const SCEV *AASCEV, const SCEV *AlignSCEV,
-                                const SCEV *OffSCEV, Value *Ptr,
-                                ScalarEvolution *SE) {
+static Align getNewAlignment(const SCEV *AASCEV, const SCEV *AlignSCEV,
   const SCEV *PtrSCEV = SE->getSCEV(Ptr);
   // On a platform with 32-bit allocas, but 64-bit flat/global pointer sizes
   // (*cough* AMDGPU), the effective SCEV type of AASCEV and PtrSCEV
   // may disagree. Trunc/extend so they agree.
   PtrSCEV = SE->getTruncateOrZeroExtend(
       PtrSCEV, SE->getEffectiveSCEVType(AASCEV->getType()));
   const SCEV *DiffSCEV = SE->getMinusSCEV(PtrSCEV, AASCEV);
 
   // On 32-bit platforms, DiffSCEV might now have type i32 -- we've always
   // sign-extended OffSCEV to i64, so make sure they agree again.
   DiffSCEV = SE->getNoopOrSignExtend(DiffSCEV, OffSCEV->getType());
 
   // What we really want to know is the overall offset to the aligned
   // address. This address is displaced by the provided offset.
   DiffSCEV = SE->getMinusSCEV(DiffSCEV, OffSCEV);
 
   LLVM_DEBUG(dbgs() << "AFI: alignment of " << *Ptr << " relative to "
                     << *AlignSCEV << " and offset " << *OffSCEV
                     << " using diff " << *DiffSCEV << "\n");
 
-  unsigned NewAlignment = getNewAlignmentDiff(DiffSCEV, AlignSCEV, SE);
-  LLVM_DEBUG(dbgs() << "\tnew alignment: " << NewAlignment << "\n");
+  if (MaybeAlign NewAlignment = getNewAlignmentDiff(DiffSCEV, AlignSCEV, SE)) {
+    LLVM_DEBUG(dbgs() << "\tnew alignment: " << DebugStr(NewAlignment) << "\n");
+    return *NewAlignment;
+  }
 
-  if (NewAlignment) {
-    return NewAlignment;
-  } else if (const SCEVAddRecExpr *DiffARSCEV =
-             dyn_cast<SCEVAddRecExpr>(DiffSCEV)) {
+  if (const SCEVAddRecExpr *DiffARSCEV = dyn_cast<SCEVAddRecExpr>(DiffSCEV)) {
     // The relative offset to the alignment assumption did not yield a constant,
     // but we should try harder: if we assume that a is 32-byte aligned, then in
     // for (i = 0; i < 1024; i += 4) r += a[i]; not all of the loads from a are
     // 32-byte aligned, but instead alternate between 32 and 16-byte alignment.
     // As a result, the new alignment will not be a constant, but can still
     // be improved over the default (of 4) to 16.
 
     const SCEV *DiffStartSCEV = DiffARSCEV->getStart();
     const SCEV *DiffIncSCEV = DiffARSCEV->getStepRecurrence(*SE);
 
     LLVM_DEBUG(dbgs() << "\ttrying start/inc alignment using start "
                       << *DiffStartSCEV << " and inc " << *DiffIncSCEV << "\n");
 
     // Now compute the new alignment using the displacement to the value in the
     // first iteration, and also the alignment using the per-iteration delta.
     // If these are the same, then use that answer. Otherwise, use the smaller
     // one, but only if it divides the larger one.
-    NewAlignment = getNewAlignmentDiff(DiffStartSCEV, AlignSCEV, SE);
-    unsigned NewIncAlignment = getNewAlignmentDiff(DiffIncSCEV, AlignSCEV, SE);
-
-    LLVM_DEBUG(dbgs() << "\tnew start alignment: " << NewAlignment << "\n");
-    LLVM_DEBUG(dbgs() << "\tnew inc alignment: " << NewIncAlignment << "\n");
+    MaybeAlign NewAlignment = getNewAlignmentDiff(DiffStartSCEV, AlignSCEV, SE);
+    MaybeAlign NewIncAlignment =
+        getNewAlignmentDiff(DiffIncSCEV, AlignSCEV, SE);
 
-    if (!NewAlignment || !NewIncAlignment) {
-      return 0;
-    } else if (NewAlignment > NewIncAlignment) {
-      if (NewAlignment % NewIncAlignment == 0) {
-        LLVM_DEBUG(dbgs() << "\tnew start/inc alignment: " << NewIncAlignment
-                          << "\n");
-        return NewIncAlignment;
-      }
-    } else if (NewIncAlignment > NewAlignment) {
-      if (NewIncAlignment % NewAlignment == 0) {
-        LLVM_DEBUG(dbgs() << "\tnew start/inc alignment: " << NewAlignment
-                          << "\n");
-        return NewAlignment;
-      }
-    } else if (NewIncAlignment == NewAlignment) {
-      LLVM_DEBUG(dbgs() << "\tnew start/inc alignment: " << NewAlignment
+    LLVM_DEBUG(dbgs() << "\tnew start alignment: " << DebugStr(NewAlignment)
+    LLVM_DEBUG(dbgs() << "\tnew inc alignment: " << DebugStr(NewIncAlignment)
+
+    if (!NewAlignment || !NewIncAlignment)
+      return Align(1);
+
+    const Align NewAlign = *NewAlignment;
+    const Align NewIncAlign = *NewIncAlignment;
+    if (NewAlign > NewIncAlign) {
+      LLVM_DEBUG(dbgs() << "\tnew start/inc alignment: "
+                        << DebugStr(NewIncAlign) << "\n");
+      return NewIncAlign;
+    if (NewIncAlign > NewAlign) {
+      LLVM_DEBUG(dbgs() << "\tnew start/inc alignment: " << DebugStr(NewAlign)
                         << "\n");
-      return NewAlignment;
+      return NewAlign;
     }
+    assert(NewIncAlign == NewAlign);
+    LLVM_DEBUG(dbgs() << "\tnew start/inc alignment: " << DebugStr(NewAlign)
+                      << "\n");
+    return NewAlign;
   }
 
-  return 0;
+  return Align(1);
 }
 
 bool AlignmentFromAssumptionsPass::extractAlignmentInfo(CallInst *I,
                                                         Value *&AAPtr,
                                                         const SCEV *&AlignSCEV,
                                                         const SCEV *&OffSCEV) {
   // An alignment assume must be a statement about the least-significant
   // bits of the pointer being zero, possibly with some offset.
for (User *J : AAPtr->users()) {
 
     if (Instruction *K = dyn_cast<Instruction>(J))
       if (isValidAssumeForContext(ACall, K, DT))
         WorkList.push_back(K);
   }
 
   while (!WorkList.empty()) {
     Instruction *J = WorkList.pop_back_val();
-
     if (LoadInst *LI = dyn_cast<LoadInst>(J)) {
-      unsigned NewAlignment = getNewAlignment(AASCEV, AlignSCEV, OffSCEV,
+      Align NewAlignment = getNewAlignment(AASCEV, AlignSCEV, OffSCEV,
                                            LI->getPointerOperand(), SE);
 
-      if (NewAlignment > LI->getAlignment()) {
-        LI->setAlignment(MaybeAlign(NewAlignment));
+      if (NewAlignment > *LI->getAlign()) {
+        LI->setAlignment(NewAlignment);
         ++NumLoadAlignChanged;
       }
     } else if (StoreInst *SI = dyn_cast<StoreInst>(J)) {
-      unsigned NewAlignment = getNewAlignment(AASCEV, AlignSCEV, OffSCEV,
+      Align NewAlignment = getNewAlignment(AASCEV, AlignSCEV, OffSCEV,
                                            SI->getPointerOperand(), SE);
 
-      if (NewAlignment > SI->getAlignment()) {
-        SI->setAlignment(MaybeAlign(NewAlignment));
+      if (NewAlignment > *SI->getAlign()) {
+        SI->setAlignment(NewAlignment);
         ++NumStoreAlignChanged;
       }
     } else if (MemIntrinsic *MI = dyn_cast<MemIntrinsic>(J)) {
-      unsigned NewDestAlignment = getNewAlignment(AASCEV, AlignSCEV, OffSCEV,
-        MI->getDest(), SE);
+      Align NewDestAlignment =
+          getNewAlignment(AASCEV, AlignSCEV, OffSCEV, MI->getDest(), SE);
 
-      LLVM_DEBUG(dbgs() << "\tmem inst: " << NewDestAlignment << "\n";);
-      if (NewDestAlignment > MI->getDestAlignment()) {
+      LLVM_DEBUG(dbgs() << "\tmem inst: " << DebugStr(NewDestAlignment)
+                        << "\n";);
+      if (NewDestAlignment > *MI->getDestAlign()) {
         MI->setDestAlignment(NewDestAlignment);
         ++NumMemIntAlignChanged;
       }
 
       // For memory transfers, there is also a source alignment that
       // can be set.
       if (MemTransferInst *MTI = dyn_cast<MemTransferInst>(MI)) {
-        unsigned NewSrcAlignment = getNewAlignment(AASCEV, AlignSCEV, OffSCEV,
-          MTI->getSource(), SE);
+        Align NewSrcAlignment =
+            getNewAlignment(AASCEV, AlignSCEV, OffSCEV, MTI->getSource(), SE);
 
-        LLVM_DEBUG(dbgs() << "\tmem trans: " << NewSrcAlignment << "\n";);
+        LLVM_DEBUG(dbgs() << "\tmem trans: " << DebugStr(NewSrcAlignment)
+                          << "\n";);
 
-        if (NewSrcAlignment > MTI->getSourceAlignment()) {
+        if (NewSrcAlignment > *MTI->getSourceAlign()) {
           MTI->setSourceAlignment(NewSrcAlignment);
           ++NumMemIntAlignChanged;
         }
       }
     }
 
     // Now that we've updated that use of the pointer, look for other uses of
     // the pointer to update.