[LTO] Parse global inline assembly with +all target flags

[asavonic]

ModuleSymbolTable extracts symbol names from global inline assembly by running the target asm parser with a generic CPU and no target flags.

This used to cause problems for top-level inline assembly where instructions require a target feature. For example, in global-inline-asm-flags.c we have PACIB and RETAB instructions that need a +pauth target flag. This test used to fail with a diagnostic:

    <inline asm>:4:1: error: instruction requires: pauth
        4 | pacib     x30, x27
    <inline asm>:5:1: error: instruction requires: pauth
        5 | retab

ModuleSymbolTable does not always have a TargetMachine, so I don't see how we can get a CPU and target flags that are set in command line. Instead, the patch changes initialization routine to pass +all target flags, assuming that flags do not change symbols, and we don't use this asm parser for code generation.

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-llvm-binary-utilities

Author: Andrew Savonichev (asavonic)

Changes

ModuleSymbolTable extracts symbol names from global inline assembly by running the target asm parser with a generic CPU and no target flags.

This used to cause problems for top-level inline assembly where instructions require a target feature. For example, in global-inline-asm-flags.c we have PACIB and RETAB instructions that need a +pauth target flag. This test used to fail with a diagnostic:

    <inline asm>:4:1: error: instruction requires: pauth
        4 | pacib     x30, x27
    <inline asm>:5:1: error: instruction requires: pauth
        5 | retab

ModuleSymbolTable does not always have a TargetMachine, so I don't see how we can get a CPU and target flags that are set in command line. Instead, the patch changes initialization routine to pass +all target flags, assuming that flags do not change symbols, and we don't use this asm parser for code generation.

---

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

2 Files Affected:

• (added) clang/test/CodeGen/AArch64/global-inline-asm-flags.c (+20)
• (modified) llvm/lib/Object/ModuleSymbolTable.cpp (+2-1)

diff --git a/clang/test/CodeGen/AArch64/global-inline-asm-flags.c b/clang/test/CodeGen/AArch64/global-inline-asm-flags.c
new file mode 100644
index 0000000000000..0900d0cc44f68
--- /dev/null
+++ b/clang/test/CodeGen/AArch64/global-inline-asm-flags.c
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple aarch64-linux-gnu -target-feature +pauth -flto=thin -emit-llvm -o - %s | FileCheck %s
+// REQUIRES: aarch64-registered-target
+
+// Check that +pauth target flag is enabled for global inline assembler.
+
+// CHECK: module asm ".text"
+// CHECK: module asm ".balign 16"
+// CHECK: module asm ".globl foo"
+// CHECK: module asm "pacib     x30, x27"
+// CHECK: module asm "retab"
+// CHECK: module asm ".previous"
+
+asm (
+    ".text" "\n"
+    ".balign 16" "\n"
+    ".globl foo\n"
+    "pacib     x30, x27" "\n"
+    "retab" "\n"
+    ".previous" "\n"
+);
diff --git a/llvm/lib/Object/ModuleSymbolTable.cpp b/llvm/lib/Object/ModuleSymbolTable.cpp
index 9442becdb7d33..cf7159a41e714 100644
--- a/llvm/lib/Object/ModuleSymbolTable.cpp
+++ b/llvm/lib/Object/ModuleSymbolTable.cpp
@@ -90,7 +90,8 @@ initializeRecordStreamer(const Module &M,
   if (!MAI)
     return;
 
-  std::unique_ptr<MCSubtargetInfo> STI(T->createMCSubtargetInfo(TT, "", ""));
+  std::unique_ptr<MCSubtargetInfo> STI(
+      T->createMCSubtargetInfo(TT, "", "+all"));
   if (!STI)
     return;
 

[arsenm]

This isn't some universal thing that you can apply to all targets. I assume +all won't parse on every target, and even if it does, you cannot assume all features can just be enabled t

[asavonic]

This isn't some universal thing that you can apply to all targets. I assume +all won't parse on every target, and even if it does, you cannot assume all features can just be enabled t

Thanks for the quick reply Matt!
We can instead propagate a CPU and target flags from all tools that use ModuleSymbolTable. Would this be a correct fix?

[arsenm]

I'm not sure what the right answer is for this; we don't really have one. We have a lot of legacy issues in this area. Subtarget features / subtargets have been used for a variety of module level concepts that they should not have been. Some kind of module level directive should probably be represented as an IR module flag, in general (or should be implied by the sub-arch, not subtarget)

[asavonic]

We can instead propagate a CPU and target flags from all tools that use ModuleSymbolTable

I checked this briefly, and there are quite a few places where target features are not really known, and only an IR module is available. So I think we have to keep this information in a Module.

Some kind of module level directive should probably be represented as an IR module flag

RISC-V seems to have this already #80760. The discussion there went in favor of implementing a mechanism for module-level target features. Perhaps that is the way to fix the problem.

We have target-features function attribute that is a comma separated list of features (string attribute). If we add a similar module-level flag, we can read it in ModuleSymbolTable and initialize the parser correctly.