[ARM] Recognize abi tag module flags

[paperchalice]

Recognize abi tag hints from frontend rather than from architecture and options.
Frontend part: #161106.

[llvmbot]

@llvm/pr-subscribers-backend-arm

Author: None (paperchalice)

Changes

Recognize abi tag hints from frontend rather than from architecture and options.

---

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

2 Files Affected:

• (modified) llvm/lib/Target/ARM/ARMAsmPrinter.cpp (+22-6)
• (added) llvm/test/CodeGen/ARM/build-attributes-module.ll (+14)

diff --git a/llvm/lib/Target/ARM/ARMAsmPrinter.cpp b/llvm/lib/Target/ARM/ARMAsmPrinter.cpp
index 1f773e2a7e0fc..125c002682012 100644
--- a/llvm/lib/Target/ARM/ARMAsmPrinter.cpp
+++ b/llvm/lib/Target/ARM/ARMAsmPrinter.cpp
@@ -701,9 +701,21 @@ void ARMAsmPrinter::emitAttributes() {
                       ARMBuildAttrs::AddressDirect);
   }
 
+  Metadata *MDEx = nullptr;
+  Metadata *MDDM = nullptr;
+  Metadata *MDNM = nullptr;
+  if (const Module *M = MMI->getModule()) {
+    MDEx = M->getModuleFlag("arm-eabi-fp-exceptions");
+    MDDM = M->getModuleFlag("arm-eabi-fp-denormal");
+    MDNM = M->getModuleFlag("arm-eabi-fp-number-model");
+  }
+
   // Set FP Denormals.
-  if (checkDenormalAttributeConsistency(*MMI->getModule(), "denormal-fp-math",
-                                        DenormalMode::getPreserveSign()))
+  if (auto *DM = mdconst::extract_or_null<ConstantInt>(MDDM))
+    ATS.emitAttribute(ARMBuildAttrs::ABI_FP_denormal, DM->getZExtValue());
+  else if (checkDenormalAttributeConsistency(*MMI->getModule(),
+                                             "denormal-fp-math",
+                                             DenormalMode::getPreserveSign()))
     ATS.emitAttribute(ARMBuildAttrs::ABI_FP_denormal,
                       ARMBuildAttrs::PreserveFPSign);
   else if (checkDenormalAttributeConsistency(*MMI->getModule(),
@@ -743,9 +755,11 @@ void ARMAsmPrinter::emitAttributes() {
   }
 
   // Set FP exceptions and rounding
-  if (checkFunctionsAttributeConsistency(*MMI->getModule(),
-                                         "no-trapping-math", "true") ||
-      TM.Options.NoTrappingFPMath)
+  if (auto *Ex = mdconst::extract_or_null<ConstantInt>(MDEx))
+    ATS.emitAttribute(ARMBuildAttrs::ABI_FP_exceptions, Ex->getZExtValue());
+  else if (checkFunctionsAttributeConsistency(*MMI->getModule(),
+                                              "no-trapping-math", "true") ||
+           TM.Options.NoTrappingFPMath)
     ATS.emitAttribute(ARMBuildAttrs::ABI_FP_exceptions,
                       ARMBuildAttrs::Not_Allowed);
   else {
@@ -759,7 +773,9 @@ void ARMAsmPrinter::emitAttributes() {
 
   // TM.Options.NoInfsFPMath && TM.Options.NoNaNsFPMath is the
   // equivalent of GCC's -ffinite-math-only flag.
-  if (TM.Options.NoInfsFPMath && TM.Options.NoNaNsFPMath)
+  if (auto *NM = mdconst::extract_or_null<ConstantInt>(MDNM))
+    ATS.emitAttribute(ARMBuildAttrs::ABI_FP_number_model, NM->getZExtValue());
+  else if (TM.Options.NoInfsFPMath && TM.Options.NoNaNsFPMath)
     ATS.emitAttribute(ARMBuildAttrs::ABI_FP_number_model,
                       ARMBuildAttrs::Allowed);
   else
diff --git a/llvm/test/CodeGen/ARM/build-attributes-module.ll b/llvm/test/CodeGen/ARM/build-attributes-module.ll
new file mode 100644
index 0000000000000..d7ac8e7df376f
--- /dev/null
+++ b/llvm/test/CodeGen/ARM/build-attributes-module.ll
@@ -0,0 +1,14 @@
+; RUN: llc %s -mtriple=arm-none-none-eabi -o - | FileCheck %s
+
+define void @f() {
+    ret void
+}
+
+!llvm.module.flags = !{!0, !1, !2}
+
+; CHECK: .eabi_attribute 20, 0
+!0 = !{i32 2, !"arm-eabi-fp-denormal", i32 0}
+; CHECK: .eabi_attribute 21, 1
+!1 = !{i32 2, !"arm-eabi-fp-exceptions", i32 1}
+; CHECK: .eabi_attribute 23, 1
+!2 = !{i32 2, !"arm-eabi-fp-number-model", i32 1}

[smithp35]

Just to make sure I understand. If there are no module flags for arm-eabi-fp-exceptions, arm-eabi-fp-denormal and arm-eabi-fp-number-model then the mdconst::extract_or_null<ConstantInt>(MD... ) will return nullptr and we'll fall back to checking function attribute consistency.

How are the module flags expected to be set by the front-end? In particular are the module flags always set (with default values) if the user does not express a preference.

I'd like to make sure that the LTO case where the user doesn't put the floating point options on the command line for the final link step (with just objects) still works as it does today.

[smithp35]

Is it ever possible for MMI->getModule() to be nullptr when writing build attributes? In the existing code for checkDenormalConsistency it is universally dereferenced?

If it isn't possible then I think it would be worth moving the M->getModuleFlag() calls closer to where they are used. I find it difficult to tell MDDM and MDNM apart with my eyesight.

For example (using SourceModule instead of M, as SourceModule is used on line 801).

const Module *SourceModule = MMI->getModule();
// Set FP Denormals
Metadata *MDDM = SourceModule->getModuleFlag("arm-eabi-fp-denormal");
if (auto *DM = mdconst::extract_or_null<ConstantInt>(MDDM))
  ATS.emitAttribute(ARMBuildAttrs::ABI_FP_denormal, DM->getZExtValue());
else if (checkDenormalAttributeConsistency(*SourceModule,
                                             "denormal-fp-math",
                                             DenormalMode::getPreserveSign()))

If we do have to preserve the if (const Module *M = MMI->getModule()) can you expand the MDDM and MDNM a bit? perhaps MDDenorm and MDNumModel .

[paperchalice]

How are the module flags expected to be set by the front-end? In particular are the module flags always set (with default values) if the user does not express a preference.

In #161106, arm-eabi-fp-denormal is 0 when -ffp-exception-behavior=ignore, arm-eabi-fp-number-model is controlled by -ffinite-math-only(quiet nan is not supported now)

I'd like to make sure that the LTO case where the user doesn't put the floating point options on the command line for the final link step (with just objects) still works as it does today.

Currently in pull request #161106 the behavior is warning, so there might be some extra warnings when these attributes are disagreed.

[smithp35]

The code changes look good to me for the proposed change. I'll be away for a couple of weeks so I'll tag my colleague that reviewed the clang patch.

I'm still a bit unsure about what to do in the LTO case.

What I'm concerned about is something like:
clang -flto (non-default options one or more of these flags) src1.c
clang -flto (non-default options one or more of these flags) src2.c
clang -flto src1.o src2.o -o output.exe (module flags get defaults)

We've been asked to fix problems like this with architecture flags as this does occur in projects that separate their compile and link flags.

We used to pick up the build attributes from the functions if they were consistent. Now it looks like we'll overwrite them with the default module flags.

In the linked PR #161106 you mention that UnsafeFPMath and related flags were disappearing. I wasn't sure what that referred to. If the per function attributes are disappearing and the module flags are all we have then there's not much more we can do, perhaps making sure we release note it so that users can make sure they add the flags to their link line.

Alternatively it may be worth looking at all the functions, and if they are all consistent, but different from the module flags then use the function flags.

[paperchalice]

I'm still a bit unsure about what to do in the LTO case.

What I'm concerned about is something like:
clang -flto (non-default options one or more of these flags) src1.c
clang -flto (non-default options one or more of these flags) src2.c
clang -flto src1.o src2.o -o output.exe (module flags get defaults)

We've been asked to fix problems like this with architecture flags as this does occur in projects that separate their compile and > link flags.

Test locally with llvm-lto/llvm-link, but I think it should also be true for clang...
It will output something like

linking module flags 'arm-eabi-fp-number-model': IDs have conflicting values ('i32 2' from src2.bc with 'i32 1' from ld-temp.o)

The final tag value comes from the first input moudle.

In the linked PR #161106 you mention that UnsafeFPMath and related flags were disappearing. I wasn't sure what that referred to. If the per function attributes are disappearing and the module flags are all we have then there's not much more we can do, perhaps making sure we release note it so that users can make sure they add the flags to their link line.

Sorry for the unclear explaination, I mean fast-math related function attributes like "unsafe-fp-math", and the --enable-unsafe-fp-math/UnsafeFPMath option in llvm/lib/CodeGen.

Alternatively it may be worth looking at all the functions, and if they are all consistent, but different from the module flags then use the function flags.

This is the implementation before #151275, but was is already broken during iteration...

We can remove these function attributes support directly because they are undocumented, but should we document these new module flags?