-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[ARM] Generate build-attributes more correctly in the presence of intrinsic declarations. #160749
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-backend-arm Author: David Green (davemgreen) ChangesThis code doesn't work very well, but this makes it work when intrinsic definitions are present. It now discounts functions declarations from the set of attributes it looks at. The code would have worked better before 0ab5b5b when module-level attributes could provide the information used to construct build-attributes. Full diff: https://github.com/llvm/llvm-project/pull/160749.diff 5 Files Affected:
diff --git a/llvm/lib/Target/ARM/ARMAsmPrinter.cpp b/llvm/lib/Target/ARM/ARMAsmPrinter.cpp
index 1c42f44765abf..f5faea631852c 100644
--- a/llvm/lib/Target/ARM/ARMAsmPrinter.cpp
+++ b/llvm/lib/Target/ARM/ARMAsmPrinter.cpp
@@ -610,20 +610,23 @@ void ARMAsmPrinter::emitEndOfAsmFile(Module &M) {
// to appear in the .ARM.attributes section in ELF.
// Instead of subclassing the MCELFStreamer, we do the work here.
- // Returns true if all functions have the same function attribute value.
- // It also returns true when the module has no functions.
+// Returns true if all functions have the same function attribute value.
+// It also returns true when the module has no functions.
static bool checkFunctionsAttributeConsistency(const Module &M, StringRef Attr,
StringRef Value) {
- return !any_of(M, [&](const Function &F) {
- return F.getFnAttribute(Attr).getValueAsString() != Value;
- });
+ return !any_of(M, [&](const Function &F) {
+ if (F.isDeclaration())
+ return false;
+ return F.getFnAttribute(Attr).getValueAsString() != Value;
+ });
}
// Returns true if all functions have the same denormal mode.
// It also returns true when the module has no functions.
-static bool checkDenormalAttributeConsistency(const Module &M,
- StringRef Attr,
+static bool checkDenormalAttributeConsistency(const Module &M, StringRef Attr,
DenormalMode Value) {
return !any_of(M, [&](const Function &F) {
+ if (F.isDeclaration())
+ return false;
StringRef AttrVal = F.getFnAttribute(Attr).getValueAsString();
return parseDenormalFPAttribute(AttrVal) != Value;
});
diff --git a/llvm/test/CodeGen/ARM/build-attributes-fn-attr3.ll b/llvm/test/CodeGen/ARM/build-attributes-fn-attr3.ll
index 7f70c44c78f9c..27d1dc20bd815 100644
--- a/llvm/test/CodeGen/ARM/build-attributes-fn-attr3.ll
+++ b/llvm/test/CodeGen/ARM/build-attributes-fn-attr3.ll
@@ -11,7 +11,10 @@
define i32 @foo() local_unnamed_addr #0 {
entry:
+ %a = call float @llvm.fma.f32(float 0.0, float 0.0, float 0.0)
ret i32 42
}
+declare float @llvm.fma.f32(float, float, float)
+
attributes #0 = { minsize norecurse nounwind optsize readnone "no-trapping-math"="true" "denormal-fp-math"="ieee"}
diff --git a/llvm/test/CodeGen/ARM/build-attributes-fn-attr4.ll b/llvm/test/CodeGen/ARM/build-attributes-fn-attr4.ll
index c99cb27adf155..9c8dd8d95c61c 100644
--- a/llvm/test/CodeGen/ARM/build-attributes-fn-attr4.ll
+++ b/llvm/test/CodeGen/ARM/build-attributes-fn-attr4.ll
@@ -10,7 +10,10 @@
define i32 @foo1() local_unnamed_addr #0 {
entry:
+ %a = call float @llvm.fma.f32(float 0.0, float 0.0, float 0.0)
ret i32 42
}
+declare float @llvm.fma.f32(float, float, float)
+
attributes #0 = { minsize norecurse nounwind optsize readnone "denormal-fp-math"="positive-zero,positive-zero" }
diff --git a/llvm/test/CodeGen/ARM/build-attributes-fn-attr5.ll b/llvm/test/CodeGen/ARM/build-attributes-fn-attr5.ll
index ba1e7d7ce55c1..cda3ea0fc6d18 100644
--- a/llvm/test/CodeGen/ARM/build-attributes-fn-attr5.ll
+++ b/llvm/test/CodeGen/ARM/build-attributes-fn-attr5.ll
@@ -10,7 +10,10 @@
define i32 @foo1() local_unnamed_addr #0 {
entry:
+ %a = call float @llvm.fma.f32(float 0.0, float 0.0, float 0.0)
ret i32 42
}
+declare float @llvm.fma.f32(float, float, float)
+
attributes #0 = { minsize norecurse nounwind optsize readnone "denormal-fp-math"="preserve-sign,preserve-sign"}
diff --git a/llvm/test/CodeGen/ARM/build-attributes-fn-attr6.ll b/llvm/test/CodeGen/ARM/build-attributes-fn-attr6.ll
index 1cd68aed1e051..59d0a40198392 100644
--- a/llvm/test/CodeGen/ARM/build-attributes-fn-attr6.ll
+++ b/llvm/test/CodeGen/ARM/build-attributes-fn-attr6.ll
@@ -11,6 +11,7 @@
define i32 @foo1() local_unnamed_addr #0 {
entry:
+ %a = call float @llvm.fma.f32(float 0.0, float 0.0, float 0.0)
ret i32 42
}
@@ -19,5 +20,7 @@ entry:
ret i32 42
}
+declare float @llvm.fma.f32(float, float, float)
+
attributes #0 = { minsize norecurse nounwind optsize readnone "denormal-fp-math"="preserve-sign,preserve-sign"}
attributes #1 = { minsize norecurse nounwind optsize readnone "denormal-fp-math"="positive-zero,positive-zero"}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I agree that we should skip over declarations when considering the build attributes for this module. The module defining the declared functions will define its build attributes.
@@ -610,20 +610,23 @@ void ARMAsmPrinter::emitEndOfAsmFile(Module &M) { | |||
// to appear in the .ARM.attributes section in ELF. | |||
// Instead of subclassing the MCELFStreamer, we do the work here. | |||
|
|||
// Returns true if all functions have the same function attribute value. | |||
// It also returns true when the module has no functions. | |||
// Returns true if all functions have the same function attribute value. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest a small comment update, also could be applied on line 623
Returns true if all function definitions have the same function attribute value.
…unction declarations. This code doesn't work very well, but this at least makes it work when intrinsic definitions are present. It now discounts functions declarations from the set of attributes it looks at. It would have worked better before 0ab5b5b when module-level attributes could provide the information used to construct build-attributes.
5eb129b
to
bac9b86
Compare
Thanks |
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/17/builds/11365 Here is the relevant piece of the build log for the reference
|
…rinsic declarations. (llvm#160749) This code doesn't work very well, but this makes it work when intrinsic definitions are present. It now discounts functions declarations from the set of attributes it looks at. The code would have worked better before 0ab5b5b when module-level attributes could provide the information used to construct build-attributes.
This code doesn't work very well, but this makes it work when intrinsic definitions are present. It now discounts functions declarations from the set of attributes it looks at.
The code would have worked better before 0ab5b5b when module-level attributes could provide the information used to construct build-attributes.