[llvm] 5518a02 - llc/MIR: Fix setFunctionAttributes for MIR functions

Matt Arsenault via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 6 14:22:00 PST 2020


Author: Matt Arsenault
Date: 2020-01-06T17:21:51-05:00
New Revision: 5518a02a83e855edeff7d8b4db685ec5d1b4144e

URL: https://github.com/llvm/llvm-project/commit/5518a02a83e855edeff7d8b4db685ec5d1b4144e
DIFF: https://github.com/llvm/llvm-project/commit/5518a02a83e855edeff7d8b4db685ec5d1b4144e.diff

LOG: 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.

Added: 
    llvm/test/CodeGen/MIR/AMDGPU/llc-target-cpu-attr-from-cmdline-ir.mir
    llvm/test/CodeGen/MIR/AMDGPU/llc-target-cpu-attr-from-cmdline.mir

Modified: 
    llvm/include/llvm/CodeGen/CommandFlags.inc
    llvm/include/llvm/CodeGen/MIRParser/MIRParser.h
    llvm/lib/CodeGen/MIRParser/MIRParser.cpp
    llvm/tools/llc/llc.cpp

Removed: 
    


################################################################################
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<std::string> 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<CallInst>(&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<CallInst>(&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 @@ class MIRParser {
 /// \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<MIRParser> 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<MIRParser> createMIRParserFromFile(
+    StringRef Filename, SMDiagnostic &Error, LLVMContext &Context,
+    std::function<void(Function &)> ProcessIRFunction = nullptr);
 
 /// This function is another interface to the MIR serialization format parser.
 ///
@@ -73,7 +76,8 @@ std::unique_ptr<MIRParser> 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<MIRParser>
-createMIRParser(std::unique_ptr<MemoryBuffer> Contents, LLVMContext &Context);
+createMIRParser(std::unique_ptr<MemoryBuffer> Contents, LLVMContext &Context,
+                std::function<void(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<void(Function &)> ProcessIRFunction;
+
 public:
-  MIRParserImpl(std::unique_ptr<MemoryBuffer> Contents,
-                StringRef Filename, LLVMContext &Context);
+  MIRParserImpl(std::unique_ptr<MemoryBuffer> Contents, StringRef Filename,
+                LLVMContext &Context,
+                std::function<void(Function &)> ProcessIRFunction);
 
   void reportDiagnostic(const SMDiagnostic &Diag);
 
@@ -92,6 +95,9 @@ class MIRParserImpl {
   /// Return null if an error occurred.
   std::unique_ptr<Module> 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<MemoryBuffer> Contents,
-                             StringRef Filename, LLVMContext &Context)
+                             StringRef Filename, LLVMContext &Context,
+                             std::function<void(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<MIRParser> llvm::createMIRParserFromFile(StringRef Filename,
-                                                         SMDiagnostic &Error,
-                                                         LLVMContext &Context) {
+std::unique_ptr<MIRParser> llvm::createMIRParserFromFile(
+    StringRef Filename, SMDiagnostic &Error, LLVMContext &Context,
+    std::function<void(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<MIRParser>
 llvm::createMIRParser(std::unique_ptr<MemoryBuffer> Contents,
-                      LLVMContext &Context) {
+                      LLVMContext &Context,
+                      std::function<void(Function &)> ProcessIRFunction) {
   auto Filename = Contents->getBufferIdentifier();
   if (Context.shouldDiscardValueNames()) {
     Context.diagnose(DiagnosticInfoMIRParser(
@@ -949,6 +960,6 @@ llvm::createMIRParser(std::unique_ptr<MemoryBuffer> Contents,
             "Can't read MIR with a Context that discards named Values")));
     return nullptr;
   }
-  return std::make_unique<MIRParser>(
-      std::make_unique<MIRParserImpl>(std::move(Contents), Filename, Context));
+  return std::make_unique<MIRParser>(std::make_unique<MIRParserImpl>(
+      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<Module> M;
   std::unique_ptr<MIRParser> 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:


        


More information about the llvm-commits mailing list