From f1255186c7c4a631c051e05ed12ea79ed091141d Mon Sep 17 00:00:00 2001 From: Guillaume Chatelet Date: Sat, 18 Jun 2022 14:34:11 +0000 Subject: [PATCH] [NFC][Alignment] Remove max functions between Align and MaybeAlign `llvm::max(Align, MaybeAlign)` and `llvm::max(MaybeAlign, Align)` are not used often enough to be required. They also make the code more opaque. Differential Revision: https://reviews.llvm.org/D128121 --- llvm/include/llvm/Support/Alignment.h | 8 ------- llvm/lib/LTO/LTO.cpp | 7 ++++--- llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp | 7 +++---- llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp | 8 +++---- llvm/lib/Target/Hexagon/HexagonVExtract.cpp | 6 +++--- llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp | 2 +- .../Transforms/IPO/AttributorAttributes.cpp | 7 ++++--- llvm/lib/Transforms/Utils/InlineFunction.cpp | 3 ++- .../Transforms/Utils/MemoryTaggingSupport.cpp | 2 +- llvm/unittests/Support/AlignmentTest.cpp | 21 ------------------- 10 files changed, 21 insertions(+), 50 deletions(-) diff --git a/llvm/include/llvm/Support/Alignment.h b/llvm/include/llvm/Support/Alignment.h index 6cfb9bf8e40f93..ede17ce0cbcadd 100644 --- a/llvm/include/llvm/Support/Alignment.h +++ b/llvm/include/llvm/Support/Alignment.h @@ -326,14 +326,6 @@ inline Align operator/(Align Lhs, uint64_t Divisor) { 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) { diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp index 69188a4977ce23..2cb832faa9c047 100644 --- a/llvm/lib/LTO/LTO.cpp +++ b/llvm/lib/LTO/LTO.cpp @@ -819,9 +819,10 @@ LTO::addRegularLTO(BitcodeModule BM, ArrayRef Syms, // 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; } } diff --git a/llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp b/llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp index ae2edbb57a5473..269d28c8a7ac3a 100644 --- a/llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp +++ b/llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp @@ -939,10 +939,9 @@ void AMDGPUTargetLowering::analyzeFormalArgumentsCompute( 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; diff --git a/llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp b/llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp index 40614f4d37a56e..77816a78363068 100644 --- a/llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp +++ b/llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp @@ -535,13 +535,11 @@ uint64_t AMDGPUSubtarget::getExplicitKernArgSize(const Function &F, 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; diff --git a/llvm/lib/Target/Hexagon/HexagonVExtract.cpp b/llvm/lib/Target/Hexagon/HexagonVExtract.cpp index d93f2352d6e08e..845fa1e49578cc 100644 --- a/llvm/lib/Target/Hexagon/HexagonVExtract.cpp +++ b/llvm/lib/Target/Hexagon/HexagonVExtract.cpp @@ -106,8 +106,7 @@ bool HexagonVExtract::runOnMachineFunction(MachineFunction &MF) { MachineFrameInfo &MFI = MF.getFrameInfo(); Register AR = MF.getInfo()->getStackAlignBaseVReg(); - std::map> VExtractMap; - MaybeAlign MaxAlign; + std::map> VExtractMap; bool Changed = false; for (MachineBasicBlock &MBB : MF) { @@ -131,6 +130,7 @@ bool HexagonVExtract::runOnMachineFunction(MachineFunction &MF) { return AddrR; }; + MaybeAlign MaxAlign; for (auto &P : VExtractMap) { unsigned VecR = P.first; if (P.second.size() <= VExtractThreshold) @@ -138,7 +138,7 @@ bool HexagonVExtract::runOnMachineFunction(MachineFunction &MF) { 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, diff --git a/llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp b/llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp index 3ebfa40df0e2a4..b1d8421220607a 100644 --- a/llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp +++ b/llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp @@ -1427,7 +1427,7 @@ void NVPTXAsmPrinter::emitFunctionParamList(const Function *F, raw_ostream &O) { 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)) { diff --git a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp index 19c9191b210b7c..cc72b1be76077b 100644 --- a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp +++ b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp @@ -6242,13 +6242,14 @@ struct AAHeapToStackFunction final : public AAHeapToStack { 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 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. diff --git a/llvm/lib/Transforms/Utils/InlineFunction.cpp b/llvm/lib/Transforms/Utils/InlineFunction.cpp index 8f88ca24708f5c..03cb9e2cc1365d 100644 --- a/llvm/lib/Transforms/Utils/InlineFunction.cpp +++ b/llvm/lib/Transforms/Utils/InlineFunction.cpp @@ -1421,7 +1421,8 @@ static Value *HandleByValArgument(Type *ByValType, Value *Arg, // 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, diff --git a/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp b/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp index f306abbda29979..071097706050e3 100644 --- a/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp +++ b/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp @@ -148,7 +148,7 @@ uint64_t getAllocaSizeInBytes(const AllocaInst &AI) { } 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(); diff --git a/llvm/unittests/Support/AlignmentTest.cpp b/llvm/unittests/Support/AlignmentTest.cpp index be95f584d7b16e..9841e0938a5a31 100644 --- a/llvm/unittests/Support/AlignmentTest.cpp +++ b/llvm/unittests/Support/AlignmentTest.cpp @@ -270,27 +270,6 @@ TEST(AlignmentTest, AlignComparisons) { } } -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());