[LLVM] only add exclude metadata to ELF and COFF only

[madanial0]

As per

llvm-project/llvm/docs/LangRef.rst

Line 6799 in 9a87c5d

option is only valid for global variables with an explicit section targeting ELF

, the exclude metadata only targets platforms with COFF and ELF object formats. Modifying flang embed test case to only check for the exclude metadata on windows and linux.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-flang-driver

Author: None (madanial0)

Changes

As per

llvm-project/llvm/docs/LangRef.rst

Line 6799 in 9a87c5d

option is only valid for global variables with an explicit section targeting ELF

, the exclude metadata only targets platforms with COFF and ELF object formats. Modifying flang embed test case to only check for the exclude metadata on windows and linux.

---

Full diff: https://github.com/llvm/llvm-project/pull/80722.diff

2 Files Affected:

• (modified) flang/test/Driver/embed.f90 (+8-3)
• (modified) llvm/lib/Transforms/Utils/ModuleUtils.cpp (+7-1)

diff --git a/flang/test/Driver/embed.f90 b/flang/test/Driver/embed.f90
index 60d532a8a94fd..7905b0918fd2b 100644
--- a/flang/test/Driver/embed.f90
+++ b/flang/test/Driver/embed.f90
@@ -3,12 +3,17 @@
 ! RUN lines
 !----------
 ! Embed something that can be easily checked
-! RUN: %flang_fc1 -emit-llvm -o - -fembed-offload-object=%S/Inputs/hello.f90 %s 2>&1 | FileCheck %s
+! RUN: %flang_fc1 -emit-llvm -o - -fembed-offload-object=%S/Inputs/hello.f90 %s 2>&1 | FileCheck %s \
+! RUN:            --check-prefixes=%if system-linux || system-windows %{CHECK-EXCLUDE%} \ 
+! RUN:            %else %{CHECK-NOEXC%}
 
 ! RUN: %flang_fc1 -emit-llvm-bc -o %t.bc %s 2>&1
-! RUN: %flang_fc1 -emit-llvm -o - -fembed-offload-object=%S/Inputs/hello.f90 %t.bc 2>&1 | FileCheck %s
+! RUN: %flang_fc1 -emit-llvm -o - -fembed-offload-object=%S/Inputs/hello.f90 %t.bc 2>&1 | FileCheck %s \
+! RUN:            --check-prefixes=%if system-linux || system-windows %{CHECK-EXCLUDE%} \
+! RUN:            %else %{CHECK-NOEXC%}
 
-! CHECK: @[[OBJECT_1:.+]] = private constant [61 x i8] c"program hello\0A  write(*,*), \22Hello world!\22\0Aend program hello\0A", section ".llvm.offloading", align 8, !exclude !0
+! CHECK-NOEXC: @[[OBJECT_1:.+]] = private constant [61 x i8] c"program hello\0A  write(*,*), \22Hello world!\22\0Aend program hello\0A", section ".llvm.offloading", align 8
+! CHECK-EXCLUDE: @[[OBJECT_1:.+]] = private constant [61 x i8] c"program hello\0A  write(*,*), \22Hello world!\22\0Aend program hello\0A", section ".llvm.offloading", align 8, !exclude !0
 ! CHECK: @llvm.compiler.used = appending global [1 x ptr] [ptr @[[OBJECT_1]]], section "llvm.metadata"
 
 
diff --git a/llvm/lib/Transforms/Utils/ModuleUtils.cpp b/llvm/lib/Transforms/Utils/ModuleUtils.cpp
index 209a6a34a3c9c..63d64e8add029 100644
--- a/llvm/lib/Transforms/Utils/ModuleUtils.cpp
+++ b/llvm/lib/Transforms/Utils/ModuleUtils.cpp
@@ -20,6 +20,7 @@
 #include "llvm/IR/Module.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/xxhash.h"
+#include "llvm/TargetParser/Triple.h"
 
 using namespace llvm;
 
@@ -346,7 +347,12 @@ void llvm::embedBufferInModule(Module &M, MemoryBufferRef Buf,
                         MDString::get(Ctx, SectionName)};
 
   MD->addOperand(llvm::MDNode::get(Ctx, MDVals));
-  GV->setMetadata(LLVMContext::MD_exclude, llvm::MDNode::get(Ctx, {}));
+
+  // The exclude metadata node is only supported by ELF and COFF
+  // object file formats.
+  Triple TargetTriple = Triple(M.getTargetTriple());
+  if (TargetTriple.isOSBinFormatELF() || TargetTriple.isOSBinFormatCOFF())
+    GV->setMetadata(LLVMContext::MD_exclude, llvm::MDNode::get(Ctx, {}));
 
   appendToCompilerUsed(M, GV);
 }

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[jhuber6]

The attribute is handled the same, it's just that the Kind.isExclude does nothing in TargetLoweringObjectFileMachO::SelectSectionForGlobal. It's valid to have in IR, it just doesn't do anything on MACH-O targets.

[madanial0]

Is there a specific advantage to adding the exclude metadata to the IR in all targets even though does nothing to targets like MACH-O and XCOFF?

[jhuber6]

Is there a specific advantage to adding the exclude metadata to the IR in all targets even though does nothing to targets like MACH-O and XCOFF?

The tests aren't divergent and if support ever gets added it will happen automatically. It's an attribute in LLVM-IR, so it's perfectly legal to have in LLVM-IR. I was mulling over whether or not it should get a warning or something if it's present on an unsupported platform when we do the lowering.

[madanial0]

Perfect, thanks for the quick review @jhuber6. I agree that keeping the tests from diverging is important!

[mandlebug]

It's an attribute in LLVM-IR, so it's perfectly legal to have in LLVM-IR.

I disagree with this statement. The documentation for the exclude metadata node explicitly states its only valid for ELF or COFF, likewise the SectionKind the metadata node causes the global to get mapped to is commented as only valid on ELF or COFF targets. For XCOFF we have no reliable way to implement this - there is no flag that guarantees a section gets discarded, and individual symbols may or may not be discarded but it varies based on compile/link options and the individual symbols properties, as well as the properties of the section it gets mapped to.

I was mulling over whether or not it should get a warning or something if it's present on an unsupported platform when we do the lowering.

I believe TargetLoweringObjectFileXCOFF will emit a fatal error when trying to codegen a symbol that has a section kind of 'Exclude' although it will just say unsupported section kind. It might be nice to have a more specific error for this case explaining why the node is not supported.