[NFC][Alignment] Remove max functions between Align and MaybeAlign

[gchatelet]

Summary

llvm::max(Align, MaybeAlign) and llvm::max(MaybeAlign, Align) are
not used often enough to be required. They also make the code more opaque.

[courbet]

llvm/lib/Transforms/IPO/AttributorAttributes.cpp

6252

Why is this OK ? I'm seeing:

if (!SimpleV.hasValue())
      return APInt(64, 0);

in getAPIInt

[gchatelet]

address comments and rebase

llvm/lib/Transforms/IPO/AttributorAttributes.cpp

6252

The assert above expects a value for alignment API and the value cannot be 0 for a valid alignment (should be at least 1).
I'll tweak the assert to make sure this is true and use assumeAligned as an extra protection.

llvm/include/llvm/Support/Alignment.h

 
 inline Align operator/(Align Lhs, uint64_t Divisor) {
   assert(llvm::isPowerOf2_64(Divisor) &&
          "Divisor must be positive and a power of 2");
   assert(Lhs != 1 && "Can't halve byte alignment");
   return Align(Lhs.value() / Divisor);
 }
 
-inline Align max(MaybeAlign Lhs, Align Rhs) {
-  return Lhs && *Lhs > Rhs ? *Lhs : Rhs;
-}
-
-inline Align max(Align Lhs, MaybeAlign Rhs) {
-  return Rhs && *Rhs > Lhs ? *Rhs : Lhs;
-}
-
 #ifndef NDEBUG
 // For usage in LLVM_DEBUG macros.
 inline std::string DebugStr(const Align &A) {
   return std::to_string(A.value());
 }
 // For usage in LLVM_DEBUG macros.
 inline std::string DebugStr(const MaybeAlign &MA) {
   if (MA)

llvm/lib/LTO/LTO.cpp

for (const InputFile::Symbol &Sym : Syms) {
     // Common resolution: collect the maximum size/alignment over all commons.
     // We also record if we see an instance of a common as prevailing, so that
     // if none is prevailing we can ignore it later.
     if (Sym.isCommon()) {
       // FIXME: We should figure out what to do about commons defined by asm.
       // For now they aren't reported correctly by ModuleSymbolTable.
       auto &CommonRes = RegularLTO.Commons[std::string(Sym.getIRName())];
       CommonRes.Size = std::max(CommonRes.Size, Sym.getCommonSize());
-      MaybeAlign SymAlign(Sym.getCommonAlignment());
-      if (SymAlign)
-        CommonRes.Align = max(*SymAlign, CommonRes.Align);
+      if (uint32_t SymAlignValue = Sym.getCommonAlignment()) {
+        const Align SymAlign(SymAlignValue);
+        CommonRes.Align = std::max(SymAlign, CommonRes.Align.valueOrOne());
+      }
       CommonRes.Prevailing |= Res.Prevailing;
     }
   }
 
   if (!M.getComdatSymbolTable().empty())
     for (GlobalValue &GV : M.global_values())
       handleNonPrevailingComdat(GV, NonPrevailingComdats);
 

llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp

void AMDGPUTargetLowering::analyzeFormalArgumentsCompute(
   const DataLayout &DL = Fn.getParent()->getDataLayout();
 
   unsigned InIndex = 0;
 
   for (const Argument &Arg : Fn.args()) {
     const bool IsByRef = Arg.hasByRefAttr();
     Type *BaseArgTy = Arg.getType();
     Type *MemArgTy = IsByRef ? Arg.getParamByRefType() : BaseArgTy;
-    MaybeAlign Alignment = IsByRef ? Arg.getParamAlign() : None;
-    if (!Alignment)
-      Alignment = DL.getABITypeAlign(MemArgTy);
-    MaxAlign = max(Alignment, MaxAlign);
+    Align Alignment = DL.getValueOrABITypeAlignment(
+        IsByRef ? Arg.getParamAlign() : None, MemArgTy);
+    MaxAlign = std::max(Alignment, MaxAlign);
     uint64_t AllocSize = DL.getTypeAllocSize(MemArgTy);
 
     uint64_t ArgOffset = alignTo(ExplicitArgOffset, Alignment) + ExplicitOffset;
     ExplicitArgOffset = alignTo(ExplicitArgOffset, Alignment) + AllocSize;
 
     // We're basically throwing away everything passed into us and starting over
     // to get accurate in-memory offsets. The "PartOffset" is completely useless
     // to us as computed in Ins.

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp

uint64_t AMDGPUSubtarget::getExplicitKernArgSize(const Function &F,
 
   const DataLayout &DL = F.getParent()->getDataLayout();
   uint64_t ExplicitArgBytes = 0;
   MaxAlign = Align(1);
 
   for (const Argument &Arg : F.args()) {
     const bool IsByRef = Arg.hasByRefAttr();
     Type *ArgTy = IsByRef ? Arg.getParamByRefType() : Arg.getType();
-    MaybeAlign Alignment = IsByRef ? Arg.getParamAlign() : None;
-    if (!Alignment)
-      Alignment = DL.getABITypeAlign(ArgTy);
-
+    Align Alignment = DL.getValueOrABITypeAlignment(
+        IsByRef ? Arg.getParamAlign() : None, ArgTy);
     uint64_t AllocSize = DL.getTypeAllocSize(ArgTy);
     ExplicitArgBytes = alignTo(ExplicitArgBytes, Alignment) + AllocSize;
-    MaxAlign = max(MaxAlign, Alignment);
+    MaxAlign = std::max(MaxAlign, Alignment);
   }
 
   return ExplicitArgBytes;
 }
 
 unsigned AMDGPUSubtarget::getKernArgSegmentSize(const Function &F,
                                                 Align &MaxAlign) const {
   uint64_t ExplicitArgBytes = getExplicitKernArgSize(F, MaxAlign);

llvm/lib/Target/Hexagon/HexagonVExtract.cpp

 bool HexagonVExtract::runOnMachineFunction(MachineFunction &MF) {
   HST = &MF.getSubtarget<HexagonSubtarget>();
   HII = HST->getInstrInfo();
   const auto &HRI = *HST->getRegisterInfo();
   MachineRegisterInfo &MRI = MF.getRegInfo();
   MachineFrameInfo &MFI = MF.getFrameInfo();
   Register AR =
       MF.getInfo<HexagonMachineFunctionInfo>()->getStackAlignBaseVReg();
-  std::map<unsigned, SmallVector<MachineInstr*,4>> VExtractMap;
-  MaybeAlign MaxAlign;
+  std::map<unsigned, SmallVector<MachineInstr *, 4>> VExtractMap;
   bool Changed = false;
 
   for (MachineBasicBlock &MBB : MF) {
     for (MachineInstr &MI : MBB) {
       unsigned Opc = MI.getOpcode();
       if (Opc != Hexagon::V6_extractw)
         continue;
       Register VecR = MI.getOperand(1).getReg();
       VExtractMap[VecR].push_back(&MI);
     }
   }
 
   auto EmitAddr = [&] (MachineBasicBlock &BB, MachineBasicBlock::iterator At,
                        DebugLoc dl, int FI, unsigned Offset) {
     Register AddrR = MRI.createVirtualRegister(&Hexagon::IntRegsRegClass);
     unsigned FiOpc = AR != 0 ? Hexagon::PS_fia : Hexagon::PS_fi;
     auto MIB = BuildMI(BB, At, dl, HII->get(FiOpc), AddrR);
     if (AR)
       MIB.addReg(AR);
     MIB.addFrameIndex(FI).addImm(Offset);
     return AddrR;
   };
 
+  MaybeAlign MaxAlign;
   for (auto &P : VExtractMap) {
     unsigned VecR = P.first;
     if (P.second.size() <= VExtractThreshold)
       continue;
 
     const auto &VecRC = *MRI.getRegClass(VecR);
     Align Alignment = HRI.getSpillAlign(VecRC);
-    MaxAlign = max(MaxAlign, Alignment);
+    MaxAlign = std::max(MaxAlign.valueOrOne(), Alignment);
     // Make sure this is not a spill slot: spill slots cannot be aligned
     // if there are variable-sized objects on the stack. They must be
     // accessible via FP (which is not aligned), because SP is unknown,
     // and AP may not be available at the location of the load/store.
     int FI = MFI.CreateStackObject(HRI.getSpillSize(VecRC), Alignment,
                                    /*isSpillSlot*/ false);
 
     MachineInstr *DefI = MRI.getVRegDef(VecR);

llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp

if (isKernelFunction(*F)) {
         continue;
       }
     }
 
     auto getOptimalAlignForParam = [TLI, &DL, &PAL, F,
                                     paramIndex](Type *Ty) -> Align {
       Align TypeAlign = TLI->getFunctionParamOptimizedAlign(F, Ty, DL);
       MaybeAlign ParamAlign = PAL.getParamAlignment(paramIndex);
-      return max(TypeAlign, ParamAlign);
+      return std::max(TypeAlign, ParamAlign.valueOrOne());
     };
 
     if (!PAL.hasParamAttr(paramIndex, Attribute::ByVal)) {
       if (Ty->isAggregateType() || Ty->isVectorTy() || Ty->isIntegerTy(128)) {
         // Just print .param .align <a> .b8 .param[size];
         // <a>  = optimal alignment for the element type; always multiple of
         //        PAL.getParamAlignment
         // size = typeallocsize of element type

llvm/lib/Transforms/IPO/AttributorAttributes.cpp

for (auto &It : AllocationInfos) {
         SizeOffsetEvalType SizeOffsetPair = Eval.compute(AI.CB);
         assert(SizeOffsetPair != ObjectSizeOffsetEvaluator::unknown() &&
                cast<ConstantInt>(SizeOffsetPair.second)->isZero());
         Size = SizeOffsetPair.first;
       }
 
       Align Alignment(1);
       if (MaybeAlign RetAlign = AI.CB->getRetAlign())
-        Alignment = max(Alignment, RetAlign);
+        Alignment = std::max(Alignment, *RetAlign);
       if (Value *Align = getAllocAlignment(AI.CB, TLI)) {
         Optional<APInt> AlignmentAPI = getAPInt(A, *this, *Align);
         assert(AlignmentAPI.hasValue() &&
+               AlignmentAPI.getValue().getZExtValue() > 0 &&
                "Expected an alignment during manifest!");
-        Alignment =
-            max(Alignment, MaybeAlign(AlignmentAPI.getValue().getZExtValue()));
+        Alignment = std::max(
+            Alignment, assumeAligned(AlignmentAPI.getValue().getZExtValue()));
       }
 
       // TODO: Hoist the alloca towards the function entry.
       unsigned AS = DL.getAllocaAddrSpace();
       Instruction *Alloca = new AllocaInst(Type::getInt8Ty(F->getContext()), AS,
                                            Size, Alignment, "", AI.CB);
 
       if (Alloca->getType() != AI.CB->getType())

llvm/lib/Transforms/Utils/InlineFunction.cpp

static Value *HandleByValArgument(Type *ByValType, Value *Arg,
   }
 
   // Create the alloca.  If we have DataLayout, use nice alignment.
   Align Alignment(DL.getPrefTypeAlignment(ByValType));
 
   // If the byval had an alignment specified, we *must* use at least that
   // alignment, as it is required by the byval argument (and uses of the
   // pointer inside the callee).
-  Alignment = max(Alignment, MaybeAlign(ByValAlignment));
+  if (ByValAlignment > 0)
+    Alignment = std::max(Alignment, Align(ByValAlignment));
 
   Value *NewAlloca =
       new AllocaInst(ByValType, DL.getAllocaAddrSpace(), nullptr, Alignment,
                      Arg->getName(), &*Caller->begin()->begin());
   IFI.StaticAllocas.push_back(cast<AllocaInst>(NewAlloca));
 
   // Uses of the argument in the function should use our new alloca
   // instead.

llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp

 }
 
 uint64_t getAllocaSizeInBytes(const AllocaInst &AI) {
   auto DL = AI.getModule()->getDataLayout();
   return AI.getAllocationSizeInBits(DL).getValue() / 8;
 }
 
 void alignAndPadAlloca(memtag::AllocaInfo &Info, llvm::Align Alignment) {
-  const Align NewAlignment = max(MaybeAlign(Info.AI->getAlign()), Alignment);
+  const Align NewAlignment = std::max(Info.AI->getAlign(), Alignment);
   Info.AI->setAlignment(NewAlignment);
   auto &Ctx = Info.AI->getFunction()->getContext();
 
   uint64_t Size = getAllocaSizeInBytes(*Info.AI);
   uint64_t AlignedSize = alignTo(Size, Alignment);
   if (Size == AlignedSize)
     return;
 

llvm/unittests/Support/AlignmentTest.cpp

for (size_t I = 1; I < ValidAlignments.size(); ++I) {
     EXPECT_EQ(MA, MA ? (*MA).value() : 0);
     EXPECT_NE(MA, MB ? (*MB).value() : 0);
 
     EXPECT_EQ(std::max(A, B), B);
     EXPECT_EQ(std::min(A, B), A);
   }
 }
 
-TEST(AlignmentTest, Max) {
-  // We introduce std::max here to test ADL.
-  using std::max;
-
-  // Uses llvm::max.
-  EXPECT_EQ(max(MaybeAlign(), Align(2)), Align(2));
-  EXPECT_EQ(max(Align(2), MaybeAlign()), Align(2));
-
-  EXPECT_EQ(max(MaybeAlign(1), Align(2)), Align(2));
-  EXPECT_EQ(max(Align(2), MaybeAlign(1)), Align(2));
-
-  EXPECT_EQ(max(MaybeAlign(2), Align(2)), Align(2));
-  EXPECT_EQ(max(Align(2), MaybeAlign(2)), Align(2));
-
-  EXPECT_EQ(max(MaybeAlign(4), Align(2)), Align(4));
-  EXPECT_EQ(max(Align(2), MaybeAlign(4)), Align(4));
-
-  // Uses std::max.
-  EXPECT_EQ(max(Align(2), Align(4)), Align(4));
-}
-
 TEST(AlignmentTest, AssumeAligned) {
   EXPECT_EQ(assumeAligned(0), Align(1));
   EXPECT_EQ(assumeAligned(0), Align());
   EXPECT_EQ(assumeAligned(1), Align(1));
   EXPECT_EQ(assumeAligned(1), Align());
 }
 
 // Death tests reply on assert which is disabled in release mode.