From 5518a02a83e855edeff7d8b4db685ec5d1b4144e Mon Sep 17 00:00:00 2001 From: Matt Arsenault Date: Sat, 7 Dec 2019 00:06:34 +0530 Subject: [PATCH] llc/MIR: Fix setFunctionAttributes for MIR functions A random set of attributes are implemented by llc/opt forcing the string attributes on the IR functions before processing anything. This would not happen for MIR functions, which have not yet been created at this point. Use a callback in the MIR parser, purely to avoid dealing with the ugliness that the command line flags are in a .inc file, and would require allowing access to these flags from multiple places (either from the MIR parser directly, or a new utility pass to implement these flags). It would probably be better to cleanup the flag handling into a separate library. This is in preparation for treating more command line flags with a corresponding function attribute in a more uniform way. The fast math flags in particular have a messy system where the command line flag sets the behavior from a function attribute if present, and otherwise the command line flag. This means if any other pass tries to inspect the function attributes directly, it will be inconsistent with the intended behavior. This is also inconsistent with the current behavior of -mcpu and -mattr, which overwrites any pre-existing function attributes. I would like to move this to consistenly have the command line flags not overwrite any pre-existing attributes, and to always ensure the command line flags are consistent with the function attributes. --- llvm/include/llvm/CodeGen/CommandFlags.inc | 82 ++++++++++--------- .../llvm/CodeGen/MIRParser/MIRParser.h | 14 ++-- llvm/lib/CodeGen/MIRParser/MIRParser.cpp | 45 ++++++---- .../llc-target-cpu-attr-from-cmdline-ir.mir | 55 +++++++++++++ .../llc-target-cpu-attr-from-cmdline.mir | 23 ++++++ llvm/tools/llc/llc.cpp | 11 ++- 6 files changed, 167 insertions(+), 63 deletions(-) create mode 100644 llvm/test/CodeGen/MIR/AMDGPU/llc-target-cpu-attr-from-cmdline-ir.mir create mode 100644 llvm/test/CodeGen/MIR/AMDGPU/llc-target-cpu-attr-from-cmdline.mir diff --git a/llvm/include/llvm/CodeGen/CommandFlags.inc b/llvm/include/llvm/CodeGen/CommandFlags.inc index 76071b38c413..225c44db9999 100644 --- a/llvm/include/llvm/CodeGen/CommandFlags.inc +++ b/llvm/include/llvm/CodeGen/CommandFlags.inc @@ -372,46 +372,52 @@ LLVM_ATTRIBUTE_UNUSED static std::vector getFeatureList() { return Features.getFeatures(); } +/// Set function attributes of function \p F based on CPU, Features, and command +/// line flags. +LLVM_ATTRIBUTE_UNUSED static void +setFunctionAttributes(StringRef CPU, StringRef Features, Function &F) { + auto &Ctx = F.getContext(); + AttributeList Attrs = F.getAttributes(); + AttrBuilder NewAttrs; + + if (!CPU.empty()) + NewAttrs.addAttribute("target-cpu", CPU); + if (!Features.empty()) + NewAttrs.addAttribute("target-features", Features); + if (FramePointerUsage.getNumOccurrences() > 0) { + if (FramePointerUsage == llvm::FramePointer::All) + NewAttrs.addAttribute("frame-pointer", "all"); + else if (FramePointerUsage == llvm::FramePointer::NonLeaf) + NewAttrs.addAttribute("frame-pointer", "non-leaf"); + else if (FramePointerUsage == llvm::FramePointer::None) + NewAttrs.addAttribute("frame-pointer", "none"); + } + if (DisableTailCalls.getNumOccurrences() > 0) + NewAttrs.addAttribute("disable-tail-calls", + toStringRef(DisableTailCalls)); + if (StackRealign) + NewAttrs.addAttribute("stackrealign"); + + if (TrapFuncName.getNumOccurrences() > 0) + for (auto &B : F) + for (auto &I : B) + if (auto *Call = dyn_cast(&I)) + if (const auto *F = Call->getCalledFunction()) + if (F->getIntrinsicID() == Intrinsic::debugtrap || + F->getIntrinsicID() == Intrinsic::trap) + Call->addAttribute( + llvm::AttributeList::FunctionIndex, + Attribute::get(Ctx, "trap-func-name", TrapFuncName)); + + // Let NewAttrs override Attrs. + F.setAttributes( + Attrs.addAttributes(Ctx, AttributeList::FunctionIndex, NewAttrs)); +} + /// Set function attributes of functions in Module M based on CPU, /// Features, and command line flags. LLVM_ATTRIBUTE_UNUSED static void setFunctionAttributes(StringRef CPU, StringRef Features, Module &M) { - for (auto &F : M) { - auto &Ctx = F.getContext(); - AttributeList Attrs = F.getAttributes(); - AttrBuilder NewAttrs; - - if (!CPU.empty()) - NewAttrs.addAttribute("target-cpu", CPU); - if (!Features.empty()) - NewAttrs.addAttribute("target-features", Features); - if (FramePointerUsage.getNumOccurrences() > 0) { - if (FramePointerUsage == llvm::FramePointer::All) - NewAttrs.addAttribute("frame-pointer", "all"); - else if (FramePointerUsage == llvm::FramePointer::NonLeaf) - NewAttrs.addAttribute("frame-pointer", "non-leaf"); - else if (FramePointerUsage == llvm::FramePointer::None) - NewAttrs.addAttribute("frame-pointer", "none"); - } - if (DisableTailCalls.getNumOccurrences() > 0) - NewAttrs.addAttribute("disable-tail-calls", - toStringRef(DisableTailCalls)); - if (StackRealign) - NewAttrs.addAttribute("stackrealign"); - - if (TrapFuncName.getNumOccurrences() > 0) - for (auto &B : F) - for (auto &I : B) - if (auto *Call = dyn_cast(&I)) - if (const auto *F = Call->getCalledFunction()) - if (F->getIntrinsicID() == Intrinsic::debugtrap || - F->getIntrinsicID() == Intrinsic::trap) - Call->addAttribute( - llvm::AttributeList::FunctionIndex, - Attribute::get(Ctx, "trap-func-name", TrapFuncName)); - - // Let NewAttrs override Attrs. - F.setAttributes( - Attrs.addAttributes(Ctx, AttributeList::FunctionIndex, NewAttrs)); - } + for (Function &F : M) + setFunctionAttributes(CPU, Features, F); } diff --git a/llvm/include/llvm/CodeGen/MIRParser/MIRParser.h b/llvm/include/llvm/CodeGen/MIRParser/MIRParser.h index 6a04e48e533c..385baea0446f 100644 --- a/llvm/include/llvm/CodeGen/MIRParser/MIRParser.h +++ b/llvm/include/llvm/CodeGen/MIRParser/MIRParser.h @@ -23,10 +23,11 @@ namespace llvm { -class StringRef; +class Function; class MIRParserImpl; class MachineModuleInfo; class SMDiagnostic; +class StringRef; /// This class initializes machine functions by applying the state loaded from /// a MIR file. @@ -60,9 +61,11 @@ public: /// \param Filename - The name of the file to parse. /// \param Error - Error result info. /// \param Context - Context which will be used for the parsed LLVM IR module. -std::unique_ptr createMIRParserFromFile(StringRef Filename, - SMDiagnostic &Error, - LLVMContext &Context); +/// \param ProcessIRFunction - function to run on every IR function or stub +/// loaded from the MIR file. +std::unique_ptr createMIRParserFromFile( + StringRef Filename, SMDiagnostic &Error, LLVMContext &Context, + std::function ProcessIRFunction = nullptr); /// This function is another interface to the MIR serialization format parser. /// @@ -73,7 +76,8 @@ std::unique_ptr createMIRParserFromFile(StringRef Filename, /// \param Contents - The MemoryBuffer containing the machine level IR. /// \param Context - Context which will be used for the parsed LLVM IR module. std::unique_ptr -createMIRParser(std::unique_ptr Contents, LLVMContext &Context); +createMIRParser(std::unique_ptr Contents, LLVMContext &Context, + std::function ProcessIRFunction = nullptr); } // end namespace llvm diff --git a/llvm/lib/CodeGen/MIRParser/MIRParser.cpp b/llvm/lib/CodeGen/MIRParser/MIRParser.cpp index 55fac93d8991..10157c746b46 100644 --- a/llvm/lib/CodeGen/MIRParser/MIRParser.cpp +++ b/llvm/lib/CodeGen/MIRParser/MIRParser.cpp @@ -64,9 +64,12 @@ class MIRParserImpl { /// parts. bool NoMIRDocuments = false; + std::function ProcessIRFunction; + public: - MIRParserImpl(std::unique_ptr Contents, - StringRef Filename, LLVMContext &Context); + MIRParserImpl(std::unique_ptr Contents, StringRef Filename, + LLVMContext &Context, + std::function ProcessIRFunction); void reportDiagnostic(const SMDiagnostic &Diag); @@ -92,6 +95,9 @@ public: /// Return null if an error occurred. std::unique_ptr parseIRModule(); + /// Create an empty function with the given name. + Function *createDummyFunction(StringRef Name, Module &M); + bool parseMachineFunctions(Module &M, MachineModuleInfo &MMI); /// Parse the machine function in the current YAML document. @@ -163,13 +169,13 @@ static void handleYAMLDiag(const SMDiagnostic &Diag, void *Context) { } MIRParserImpl::MIRParserImpl(std::unique_ptr Contents, - StringRef Filename, LLVMContext &Context) + StringRef Filename, LLVMContext &Context, + std::function Callback) : SM(), - In(SM.getMemoryBuffer( - SM.AddNewSourceBuffer(std::move(Contents), SMLoc()))->getBuffer(), - nullptr, handleYAMLDiag, this), - Filename(Filename), - Context(Context) { + In(SM.getMemoryBuffer(SM.AddNewSourceBuffer(std::move(Contents), SMLoc())) + ->getBuffer(), + nullptr, handleYAMLDiag, this), + Filename(Filename), Context(Context), ProcessIRFunction(Callback) { In.setContext(&In); } @@ -256,14 +262,17 @@ bool MIRParserImpl::parseMachineFunctions(Module &M, MachineModuleInfo &MMI) { return false; } -/// Create an empty function with the given name. -static Function *createDummyFunction(StringRef Name, Module &M) { +Function *MIRParserImpl::createDummyFunction(StringRef Name, Module &M) { auto &Context = M.getContext(); Function *F = Function::Create(FunctionType::get(Type::getVoidTy(Context), false), Function::ExternalLinkage, Name, M); BasicBlock *BB = BasicBlock::Create(Context, "entry", F); new UnreachableInst(Context, BB); + + if (ProcessIRFunction) + ProcessIRFunction(*F); + return F; } @@ -925,21 +934,23 @@ bool MIRParser::parseMachineFunctions(Module &M, MachineModuleInfo &MMI) { return Impl->parseMachineFunctions(M, MMI); } -std::unique_ptr llvm::createMIRParserFromFile(StringRef Filename, - SMDiagnostic &Error, - LLVMContext &Context) { +std::unique_ptr llvm::createMIRParserFromFile( + StringRef Filename, SMDiagnostic &Error, LLVMContext &Context, + std::function ProcessIRFunction) { auto FileOrErr = MemoryBuffer::getFileOrSTDIN(Filename); if (std::error_code EC = FileOrErr.getError()) { Error = SMDiagnostic(Filename, SourceMgr::DK_Error, "Could not open input file: " + EC.message()); return nullptr; } - return createMIRParser(std::move(FileOrErr.get()), Context); + return createMIRParser(std::move(FileOrErr.get()), Context, + ProcessIRFunction); } std::unique_ptr llvm::createMIRParser(std::unique_ptr Contents, - LLVMContext &Context) { + LLVMContext &Context, + std::function ProcessIRFunction) { auto Filename = Contents->getBufferIdentifier(); if (Context.shouldDiscardValueNames()) { Context.diagnose(DiagnosticInfoMIRParser( @@ -949,6 +960,6 @@ llvm::createMIRParser(std::unique_ptr Contents, "Can't read MIR with a Context that discards named Values"))); return nullptr; } - return std::make_unique( - std::make_unique(std::move(Contents), Filename, Context)); + return std::make_unique(std::make_unique( + std::move(Contents), Filename, Context, ProcessIRFunction)); } diff --git a/llvm/test/CodeGen/MIR/AMDGPU/llc-target-cpu-attr-from-cmdline-ir.mir b/llvm/test/CodeGen/MIR/AMDGPU/llc-target-cpu-attr-from-cmdline-ir.mir new file mode 100644 index 000000000000..588c9a5df786 --- /dev/null +++ b/llvm/test/CodeGen/MIR/AMDGPU/llc-target-cpu-attr-from-cmdline-ir.mir @@ -0,0 +1,55 @@ +# RUN: llc -march=amdgcn -mcpu=hawaii -run-pass=none -o - %s | FileCheck -check-prefix=MCPU %s +# RUN: llc -march=amdgcn -mattr=+unaligned-buffer-access -run-pass=none -o - %s | FileCheck -check-prefix=MATTR %s + +# FIXME: This overrides attributes that already are present. It should probably +# only touch functions without an existing attribute. + +# MCPU: attributes #0 = { "target-cpu"="hawaii" } +# MCPU-NOT: attributes #1 + +# MATTR: attributes #0 = { "target-cpu"="fiji" "target-features"="+unaligned-buffer-access" } +# MATTR: attributes #1 = { "target-features"="+unaligned-buffer-access" } + +--- | + define amdgpu_kernel void @with_cpu_attr() #0 { + ret void + } + + define amdgpu_kernel void @no_cpu_attr() { + ret void + } + + attributes #0 = { "target-cpu"="fiji" } +... + +--- +name: with_cpu_attr +legalized: true +regBankSelected: true +tracksRegLiveness: true + +body: | + bb.0: + liveins: $sgpr0, $sgpr1 + + %0:sgpr(s32) = COPY $sgpr0 + %1:sgpr(s32) = COPY $sgpr1 + %2:vgpr(s32) = G_OR %0, %1 + S_ENDPGM 0, implicit %2 +... + +--- +name: no_cpu_attr +legalized: true +regBankSelected: true +tracksRegLiveness: true + +body: | + bb.0: + liveins: $sgpr0, $sgpr1 + + %0:sgpr(s32) = COPY $sgpr0 + %1:sgpr(s32) = COPY $sgpr1 + %2:vgpr(s32) = G_OR %0, %1 + S_ENDPGM 0, implicit %2 +... diff --git a/llvm/test/CodeGen/MIR/AMDGPU/llc-target-cpu-attr-from-cmdline.mir b/llvm/test/CodeGen/MIR/AMDGPU/llc-target-cpu-attr-from-cmdline.mir new file mode 100644 index 000000000000..bd16888bf07e --- /dev/null +++ b/llvm/test/CodeGen/MIR/AMDGPU/llc-target-cpu-attr-from-cmdline.mir @@ -0,0 +1,23 @@ +# RUN: llc -march=amdgcn -mcpu=hawaii -run-pass=none -o - %s | FileCheck -check-prefix=MCPU %s +# RUN: llc -march=amdgcn -mattr=+unaligned-buffer-access -run-pass=none -o - %s | FileCheck -check-prefix=MATTR %s + +# The command line arguments for -mcpu and -mattr should manifest themselves by adding the corresponding attributes to the stub IR function. + +# MCPU: attributes #0 = { "target-cpu"="hawaii" } +# MATTR: attributes #0 = { "target-features"="+unaligned-buffer-access" } + +--- +name: no_ir +legalized: true +regBankSelected: true +tracksRegLiveness: true + +body: | + bb.0: + liveins: $sgpr0, $sgpr1 + + %0:sgpr(s32) = COPY $sgpr0 + %1:sgpr(s32) = COPY $sgpr1 + %2:vgpr(s32) = G_OR %0, %1 + S_ENDPGM 0, implicit %2 +... diff --git a/llvm/tools/llc/llc.cpp b/llvm/tools/llc/llc.cpp index b5503a4157b1..b35f8e853c30 100644 --- a/llvm/tools/llc/llc.cpp +++ b/llvm/tools/llc/llc.cpp @@ -395,6 +395,12 @@ static int compileModule(char **argv, LLVMContext &Context) { std::unique_ptr M; std::unique_ptr MIR; Triple TheTriple; + std::string CPUStr = getCPUStr(), FeaturesStr = getFeaturesStr(); + + // Set attributes on functions as loaded from MIR from command line arguments. + auto setMIRFunctionAttributes = [&CPUStr, &FeaturesStr](Function &F) { + setFunctionAttributes(CPUStr, FeaturesStr, F); + }; bool SkipModule = MCPU == "help" || (!MAttrs.empty() && MAttrs.front() == "help"); @@ -403,7 +409,8 @@ static int compileModule(char **argv, LLVMContext &Context) { if (!SkipModule) { if (InputLanguage == "mir" || (InputLanguage == "" && StringRef(InputFilename).endswith(".mir"))) { - MIR = createMIRParserFromFile(InputFilename, Err, Context); + MIR = createMIRParserFromFile(InputFilename, Err, Context, + setMIRFunctionAttributes); if (MIR) M = MIR->parseIRModule(); } else @@ -433,8 +440,6 @@ static int compileModule(char **argv, LLVMContext &Context) { return 1; } - std::string CPUStr = getCPUStr(), FeaturesStr = getFeaturesStr(); - CodeGenOpt::Level OLvl = CodeGenOpt::Default; switch (OptLevel) { default: