[llvm] Fix the MCSubtargetInfo used for module-level assembly.

[chrisnc]

Provide both the default target CPU and default target features from the
module's context, rather than empty strings.

Fixes #61991.

[llvmbot]

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

Author: Chris Copeland (chrisnc)

Changes

Provide both the default target CPU and default target features from the
module's context, rather than empty strings.

Fixes #61991.

---

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

1 Files Affected:

• (modified) llvm/lib/Object/ModuleSymbolTable.cpp (+5-1)

diff --git a/llvm/lib/Object/ModuleSymbolTable.cpp b/llvm/lib/Object/ModuleSymbolTable.cpp
index d8f520ad02c2f..079c33c801f6e 100644
--- a/llvm/lib/Object/ModuleSymbolTable.cpp
+++ b/llvm/lib/Object/ModuleSymbolTable.cpp
@@ -22,6 +22,7 @@
 #include "llvm/IR/GlobalValue.h"
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/InlineAsm.h"
+#include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/MCContext.h"
@@ -92,8 +93,11 @@ initializeRecordStreamer(const Module &M,
   if (!MAI)
     return;
 
+  LLVMContext &Context = M.getContext();
+
   std::unique_ptr<MCSubtargetInfo> STI(
-      T->createMCSubtargetInfo(TT.str(), "", ""));
+      T->createMCSubtargetInfo(TT.str(), Context.getDefaultTargetCPU(),
+                               Context.getDefaultTargetFeatures()));
   if (!STI)
     return;
 

[chrisnc]

test.c

asm("vmsr fpexc, r0");

Without fix:

$ bin/clang -target arm-none-eabi test.c -c -flto=thin -mcpu=cortex-r4
<inline asm>:1:1: error: instruction requires: VFP2
    1 | vmsr fpexc, r0
      | ^
1 error generated.

$ bin/clang -target arm-none-eabi test.c -c -flto=thin -mcpu=cortex-r5
<inline asm>:1:1: error: instruction requires: VFP2
    1 | vmsr fpexc, r0
      | ^
1 error generated.

With fix, using -mcpu=cortex-r5 (which implies FPU support, while cortex-r4 does not), does not emit an error:

$ bin/clang -target arm-none-eabi test.c -c -flto=thin -mcpu=cortex-r4
<inline asm>:1:1: error: instruction requires: VFP2
    1 | vmsr fpexc, r0
      | ^
1 error generated.

$ bin/clang -target arm-none-eabi test.c -c -flto=thin -mcpu=cortex-r5

[chrisnc]

Credit to #61991 (comment) @Dirbaio for finding where the problem was.

Hopefully this is not a breaking change for anything and the fix is this simple...

[efriedma-quic]

The idea here makes sense, but the API in question is likely to change, so I'd rather not add new uses of that API until we've sorted that out. See https://discourse.llvm.org/t/functions-generated-by-function-createwithdefaultattr-should-respect-target-features/79838 .

[MaskRay]

Request changes since the ABI will change. My summary:

#96721 implemented a strategy by storing the target features in LLVMContext, which is not serialized.
Function::createWithDefaultAttr will load the target features from LLVMContext.

While 96721 works well, there is a drawback that the generate module from the LTO pre-link compilation does not contain sufficient information.
Running opt without the correct target features would not mimick Clang .
To address this drawback, we can use module flags metadata that is dropped during merging.

[chrisnc]

Oh! I did not notice that the interfaces I'm using only appeared a week ago. Thanks for the additional context.

use module flags metadata

Is this something I can do now at this call-site, or something that needs more iteration on what the correct way to store/retrieve this information from various phases should be?

[chrisnc]

Hi @MaskRay, is #98673 the confirmed direction for how this info will be handled? If so, I'll watch for that to merge and then re-visit this.

[MaskRay]

Hi @MaskRay, is #98673 the confirmed direction for how this info will be handled? If so, I'll watch for that to merge and then re-visit this.

I think so. Thanks for waiting :)