[Alignment][NFC] Value::getPointerAlignment returns MaybeAlign

[gchatelet]

Summary

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

[jdoerfert]

llvm/lib/Analysis/Loads.cpp

48

While I'm all for making the "getAlign" function explicit eventually, I think it would be good to keep it as is for this patch as there doesn't seem to be a reason to do this here.

fwiw: I'm behind but eventually I was going to refactor this: D66618

[gchatelet]

llvm/lib/Analysis/Loads.cpp

48

Which function are you talking about exactly? I don't see a getAlign function.

[gchatelet]

llvm/lib/Analysis/Loads.cpp

48

@jdoerfert

Ha I suppose you're talking about getBaseAlign.
I was looking at D66618 and I believe the code will be easier to modify once it is moved to llvm::Align (I'm biased obviously).
Also the getPointerAlignment function has already been massaged in D67918 to prevent reuse of the Align variable and make logic more local rather than spanning the whole function.
I believe you'll have to rebase D66618 and you'll have conflicts there already.

On my side I'd need to move forward with the introduction of llvm::Align, I can make progress on other fronts but I'll have to update Value and Load eventually.
Please let me know what are your plans and schedule with D66618 so I can plan accordingly. Thx!

[gchatelet]

A gentle ping @jdoerfert

[gchatelet]

• rebase

[gchatelet]

I don't see any progress on D66618 so I'll proceed with the review of this patch and submit if you don't mind @jdoerfert

[courbet]

llvm/lib/Analysis/Loads.cpp

35

just None.

llvm/lib/IR/Value.cpp

687

remove one MaybeAlign

713

ditto

721

ditto

732

just None.

[gchatelet]

• Address comments

llvm/include/llvm/IR/Value.h

 
 #ifndef LLVM_IR_VALUE_H
 #define LLVM_IR_VALUE_H
 
 #include "llvm-c/Types.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/IR/Use.h"
+#include "llvm/Support/Alignment.h"
 #include "llvm/Support/CBindingWrapping.h"
 #include "llvm/Support/Casting.h"
 #include <cassert>
 #include <iterator>
 #include <memory>
 
 namespace llvm {
 
#include "llvm/IR/Value.def"
   /// dereferenceable up to the returned number of bytes.
   uint64_t getPointerDereferenceableBytes(const DataLayout &DL,
                                           bool &CanBeNull) const;
 
   /// Returns an alignment of the pointer value.
   ///
   /// Returns an alignment which is either specified explicitly, e.g. via
   /// align attribute of a function argument, or guaranteed by DataLayout.
-  unsigned getPointerAlignment(const DataLayout &DL) const;
+  MaybeAlign getPointerAlignment(const DataLayout &DL) const;
 
   /// Translate PHI node to its predecessor from the given basic block.
   ///
   /// If this value is a PHI node with CurBB as its parent, return the value in
   /// the PHI node corresponding to PredBB.  If not, return ourself.  This is
   /// useful if you want to know the value something has in a predecessor
   /// block.
   const Value *DoPHITranslation(const BasicBlock *CurBB,

llvm/lib/Analysis/Loads.cpp

 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/Operator.h"
 #include "llvm/IR/Statepoint.h"
 
 using namespace llvm;
 
-static bool isAligned(const Value *Base, const APInt &Offset, unsigned Align,
-                      const DataLayout &DL) {
-  APInt BaseAlign(Offset.getBitWidth(), Base->getPointerAlignment(DL));
-
-  if (!BaseAlign) {
-    Type *Ty = Base->getType()->getPointerElementType();
-    if (!Ty->isSized())
-      return false;
-    BaseAlign = DL.getABITypeAlignment(Ty);
-  }
+static MaybeAlign getBaseAlign(const Value *Base, const DataLayout &DL) {
+  if (const MaybeAlign PA = Base->getPointerAlignment(DL))
+    return *PA;
+  Type *const Ty = Base->getType()->getPointerElementType();
+    return llvm::None;
+  return Align(DL.getABITypeAlignment(Ty));
 
-  APInt Alignment(Offset.getBitWidth(), Align);
-
-  assert(Alignment.isPowerOf2() && "must be a power of 2!");
-  return BaseAlign.uge(Alignment) && !(Offset & (Alignment-1));
+static bool isAligned(const Value *Base, const APInt &Offset, Align Alignment,
+                      const DataLayout &DL) {
+  if (MaybeAlign BA = getBaseAlign(Base, DL)) {
+    const APInt APBaseAlign(Offset.getBitWidth(), BA->value());
+    const APInt APAlign(Offset.getBitWidth(), Alignment.value());
+    assert(APAlign.isPowerOf2() && "must be a power of 2!");
+    return APBaseAlign.uge(APAlign) && !(Offset & (APAlign - 1));
+  }
+  return false;
 }
 
 /// Test if V is always a pointer to allocated and suitably aligned memory for
 /// a simple load or store.
 static bool isDereferenceableAndAlignedPointer(
     const Value *V, unsigned Align, const APInt &Size, const DataLayout &DL,
     const Instruction *CtxI, const DominatorTree *DT,
     SmallPtrSetImpl<const Value *> &Visited) {
   // Already visited?  Bail out, we've likely hit unreachable code.
static bool isDereferenceableAndAlignedPointer(
   if (KnownDerefBytes.getBoolValue() && KnownDerefBytes.uge(Size))
     if (!CheckForNonNull || isKnownNonZero(V, DL, 0, nullptr, CtxI, DT)) {
       // As we recursed through GEPs to get here, we've incrementally checked
       // that each step advanced by a multiple of the alignment. If our base is
       // properly aligned, then the original offset accessed must also be.  
       Type *Ty = V->getType();
       assert(Ty->isSized() && "must be sized");
       APInt Offset(DL.getTypeStoreSizeInBits(Ty), 0);
-      return isAligned(V, Offset, Align, DL);
+      return isAligned(V, Offset, llvm::Align(Align), DL);
     }
 
   // For GEPs, determine if the indexing lands within the allocated object.
   if (const GEPOperator *GEP = dyn_cast<GEPOperator>(V)) {
     const Value *Base = GEP->getPointerOperand();
 
     APInt Offset(DL.getIndexTypeSizeInBits(GEP->getType()), 0);
     if (!GEP->accumulateConstantOffset(DL, Offset) || Offset.isNegative() ||

llvm/lib/Analysis/ValueTracking.cpp

if (const GlobalAlias *GA = dyn_cast<GlobalAlias>(V)) {
     return;
   }
 
   if (const Operator *I = dyn_cast<Operator>(V))
     computeKnownBitsFromOperator(I, Known, Depth, Q);
 
   // Aligned pointers have trailing zeros - refine Known.Zero set
   if (V->getType()->isPointerTy()) {
-    unsigned Align = V->getPointerAlignment(Q.DL);
+    const MaybeAlign Align = V->getPointerAlignment(Q.DL);
     if (Align)
-      Known.Zero.setLowBits(countTrailingZeros(Align));
+      Known.Zero.setLowBits(countTrailingZeros(Align->value()));
   }
 
   // computeKnownBitsFromAssume strictly refines Known.
   // Therefore, we run them after computeKnownBitsFromOperator.
 
   // Check whether a nearby assume intrinsic can determine some known bits.
   computeKnownBitsFromAssume(V, Known, Depth, Q);
 

llvm/lib/IR/ConstantFold.cpp

case Instruction::And:
             return C1;
         }
 
         // If and'ing the address of a global with a constant, fold it.
         if (CE1->getOpcode() == Instruction::PtrToInt &&
             isa<GlobalValue>(CE1->getOperand(0))) {
           GlobalValue *GV = cast<GlobalValue>(CE1->getOperand(0));
 
-          unsigned GVAlign;
+          MaybeAlign GVAlign;
 
           if (Module *TheModule = GV->getParent()) {
             GVAlign = GV->getPointerAlignment(TheModule->getDataLayout());
 
             // If the function alignment is not specified then assume that it
             // is 4.
             // This is dangerous; on x86, the alignment of the pointer
             // corresponds to the alignment of the function, but might be less
             // than 4 if it isn't explicitly specified.
             // However, a fix for this behaviour was reverted because it
             // increased code size (see https://reviews.llvm.org/D55115)
             // FIXME: This code should be deleted once existing targets have
             // appropriate defaults
-            if (GVAlign == 0U && isa<Function>(GV))
-              GVAlign = 4U;
+            if (!GVAlign && isa<Function>(GV))
+              GVAlign = Align(4);
           } else if (isa<Function>(GV)) {
             // Without a datalayout we have to assume the worst case: that the
             // function pointer isn't aligned at all.
-            GVAlign = 0U;
+            GVAlign = llvm::None;
           } else {
-            GVAlign = GV->getAlignment();
+            GVAlign = MaybeAlign(GV->getAlignment());
           }
 
-          if (GVAlign > 1) {
+          if (GVAlign && *GVAlign > 1) {
             unsigned DstWidth = CI2->getType()->getBitWidth();
-            unsigned SrcWidth = std::min(DstWidth, Log2_32(GVAlign));
+            unsigned SrcWidth = std::min(DstWidth, Log2(*GVAlign));
             APInt BitsNotSet(APInt::getLowBitsSet(DstWidth, SrcWidth));
 
             // If checking bits we know are clear, return zero.
             if ((CI2->getValue() & BitsNotSet) == CI2->getValue())
               return Constant::getNullValue(CI2->getType());
           }
         }
       }

llvm/lib/IR/Value.cpp

if (GV->getValueType()->isSized() && !GV->hasExternalWeakLinkage()) {
       // CanBeNull flag.
       DerefBytes = DL.getTypeStoreSize(GV->getValueType());
       CanBeNull = false;
     }
   }
   return DerefBytes;
 }
 
-unsigned Value::getPointerAlignment(const DataLayout &DL) const {
+MaybeAlign Value::getPointerAlignment(const DataLayout &DL) const {
   assert(getType()->isPointerTy() && "must be pointer");
   if (auto *GO = dyn_cast<GlobalObject>(this)) {
     if (isa<Function>(GO)) {
       const MaybeAlign FunctionPtrAlign = DL.getFunctionPtrAlign();
-      const unsigned Align = FunctionPtrAlign ? FunctionPtrAlign->value() : 0;
       switch (DL.getFunctionPtrAlignType()) {
       case DataLayout::FunctionPtrAlignType::Independent:
-        return Align;
+        return FunctionPtrAlign;
       case DataLayout::FunctionPtrAlignType::MultipleOfFunctionAlign:
-        return std::max(Align, GO->getAlignment());
+        return std::max(FunctionPtrAlign, MaybeAlign(GO->getAlignment()));
       }
       llvm_unreachable("Unhandled FunctionPtrAlignType");
     }
-    const unsigned Align = GO->getAlignment();
-    if (!Align) {
+    const MaybeAlign Alignment = MaybeAlign(GO->getAlignment());
+    if (!Alignment) {
       if (auto *GVar = dyn_cast<GlobalVariable>(GO)) {
         Type *ObjectType = GVar->getValueType();
         if (ObjectType->isSized()) {
           // If the object is defined in the current Module, we'll be giving
           // it the preferred alignment. Otherwise, we have to assume that it
           // may only have the minimum ABI alignment.
           if (GVar->isStrongDefinitionForLinker())
-            return DL.getPreferredAlignment(GVar);
+            return MaybeAlign(DL.getPreferredAlignment(GVar));
           else
-            return DL.getABITypeAlignment(ObjectType);
+            return Align(DL.getABITypeAlignment(ObjectType));
         }
       }
     }
-    return Align;
+    return Alignment;
   } else if (const Argument *A = dyn_cast<Argument>(this)) {
-    const unsigned Align = A->getParamAlignment();
-    if (!Align && A->hasStructRetAttr()) {
+    const MaybeAlign Alignment = MaybeAlign(A->getParamAlignment());
+    if (!Alignment && A->hasStructRetAttr()) {
       // An sret parameter has at least the ABI alignment of the return type.
       Type *EltTy = cast<PointerType>(A->getType())->getElementType();
       if (EltTy->isSized())
-        return DL.getABITypeAlignment(EltTy);
+        return Align(DL.getABITypeAlignment(EltTy));
     }
-    return Align;
+    return Alignment;
   } else if (const AllocaInst *AI = dyn_cast<AllocaInst>(this)) {
-    const unsigned Align = AI->getAlignment();
-    if (!Align) {
+    const MaybeAlign Alignment = MaybeAlign(AI->getAlignment());
+    if (!Alignment) {
       Type *AllocatedType = AI->getAllocatedType();
       if (AllocatedType->isSized())
-        return DL.getPrefTypeAlignment(AllocatedType);
+        return MaybeAlign(DL.getPrefTypeAlignment(AllocatedType));
     }
-    return Align;
+    return Alignment;
   } else if (const auto *Call = dyn_cast<CallBase>(this)) {
-    const unsigned Align = Call->getRetAlignment();
-    if (!Align && Call->getCalledFunction())
-      return Call->getCalledFunction()->getAttributes().getRetAlignment();
-    return Align;
+    const MaybeAlign Alignment = MaybeAlign(Call->getRetAlignment());
+    if (!Alignment && Call->getCalledFunction())
+      return MaybeAlign(
+          Call->getCalledFunction()->getAttributes().getRetAlignment());
+    return Alignment;
   } else if (const LoadInst *LI = dyn_cast<LoadInst>(this)) {
     if (MDNode *MD = LI->getMetadata(LLVMContext::MD_align)) {
       ConstantInt *CI = mdconst::extract<ConstantInt>(MD->getOperand(0));
-      return CI->getLimitedValue();
+      return MaybeAlign(CI->getLimitedValue());
     }
   }
-  return 0;
+  return llvm::None;
 }
 
 const Value *Value::DoPHITranslation(const BasicBlock *CurBB,
                                      const BasicBlock *PredBB) const {
   auto *PN = dyn_cast<PHINode>(this);
   if (PN && PN->getParent() == CurBB)
     return PN->getIncomingValueForBlock(PredBB);
   return this;

llvm/lib/Target/AArch64/AArch64InstrInfo.td

 def : InstAlias<"prfm $Rt, [$Rn]", (PRFMui prfop:$Rt, GPR64sp:$Rn, 0)>;
 
 //---
 // (literal)
 
 def alignedglobal : PatLeaf<(iPTR iPTR:$label), [{
   if (auto *G = dyn_cast<GlobalAddressSDNode>(N)) {
     const DataLayout &DL = MF->getDataLayout();
-    unsigned Align = G->getGlobal()->getPointerAlignment(DL);
-    return Align >= 4 && G->getOffset() % 4 == 0;
+    MaybeAlign Align = G->getGlobal()->getPointerAlignment(DL);
+    return Align && *Align >= 4 && G->getOffset() % 4 == 0;
   }
   if (auto *C = dyn_cast<ConstantPoolSDNode>(N))
     return C->getAlignment() >= 4 && C->getOffset() % 4 == 0;
   return false;
 }]>;
 
 def LDRWl : LoadLiteral<0b00, 0, GPR32z, "ldr",
   [(set GPR32z:$Rt, (load (AArch64adr alignedglobal:$label)))]>;

llvm/lib/Transforms/IPO/Attributor.cpp

struct AAAlignFloating : AAAlignImpl {
   ChangeStatus updateImpl(Attributor &A) override {
     const DataLayout &DL = A.getDataLayout();
 
     auto VisitValueCB = [&](Value &V, AAAlign::StateType &T,
                             bool Stripped) -> bool {
       const auto &AA = A.getAAFor<AAAlign>(*this, IRPosition::value(V));
       if (!Stripped && this == &AA) {
         // Use only IR information if we did not strip anything.
-        T.takeKnownMaximum(V.getPointerAlignment(DL));
+        const MaybeAlign PA = V.getPointerAlignment(DL);
+        T.takeKnownMaximum(PA ? PA->value() : 0);
         T.indicatePessimisticFixpoint();
       } else {
         // Use abstract attribute information.
         const AAAlign::StateType &DS =
             static_cast<const AAAlign::StateType &>(AA.getState());
         T ^= DS;
       }
       return T.isValidState();

llvm/unittests/IR/FunctionTest.cpp

 }
 
 TEST(FunctionTest, GetPointerAlignment) {
   LLVMContext Context;
   Type *VoidType(Type::getVoidTy(Context));
   FunctionType *FuncType(FunctionType::get(VoidType, false));
   std::unique_ptr<Function> Func(Function::Create(
       FuncType, GlobalValue::ExternalLinkage));
-  EXPECT_EQ(0U, Func->getPointerAlignment(DataLayout("")));
-  EXPECT_EQ(1U, Func->getPointerAlignment(DataLayout("Fi8")));
-  EXPECT_EQ(1U, Func->getPointerAlignment(DataLayout("Fn8")));
-  EXPECT_EQ(2U, Func->getPointerAlignment(DataLayout("Fi16")));
-  EXPECT_EQ(2U, Func->getPointerAlignment(DataLayout("Fn16")));
-  EXPECT_EQ(4U, Func->getPointerAlignment(DataLayout("Fi32")));
-  EXPECT_EQ(4U, Func->getPointerAlignment(DataLayout("Fn32")));
+  EXPECT_EQ(MaybeAlign(), Func->getPointerAlignment(DataLayout("")));
+  EXPECT_EQ(Align(1), Func->getPointerAlignment(DataLayout("Fi8")));
+  EXPECT_EQ(Align(1), Func->getPointerAlignment(DataLayout("Fn8")));
+  EXPECT_EQ(Align(2), Func->getPointerAlignment(DataLayout("Fi16")));
+  EXPECT_EQ(Align(2), Func->getPointerAlignment(DataLayout("Fn16")));
+  EXPECT_EQ(Align(4), Func->getPointerAlignment(DataLayout("Fi32")));
+  EXPECT_EQ(Align(4), Func->getPointerAlignment(DataLayout("Fn32")));
 
   Func->setAlignment(Align(4));
 
-  EXPECT_EQ(0U, Func->getPointerAlignment(DataLayout("")));
-  EXPECT_EQ(1U, Func->getPointerAlignment(DataLayout("Fi8")));
-  EXPECT_EQ(4U, Func->getPointerAlignment(DataLayout("Fn8")));
-  EXPECT_EQ(2U, Func->getPointerAlignment(DataLayout("Fi16")));
-  EXPECT_EQ(4U, Func->getPointerAlignment(DataLayout("Fn16")));
-  EXPECT_EQ(4U, Func->getPointerAlignment(DataLayout("Fi32")));
-  EXPECT_EQ(4U, Func->getPointerAlignment(DataLayout("Fn32")));
+  EXPECT_EQ(MaybeAlign(), Func->getPointerAlignment(DataLayout("")));
+  EXPECT_EQ(Align(1), Func->getPointerAlignment(DataLayout("Fi8")));
+  EXPECT_EQ(Align(4), Func->getPointerAlignment(DataLayout("Fn8")));
+  EXPECT_EQ(Align(2), Func->getPointerAlignment(DataLayout("Fi16")));
+  EXPECT_EQ(Align(4), Func->getPointerAlignment(DataLayout("Fn16")));
+  EXPECT_EQ(Align(4), Func->getPointerAlignment(DataLayout("Fi32")));
+  EXPECT_EQ(Align(4), Func->getPointerAlignment(DataLayout("Fn32")));
 }
 
 } // end namespace