-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[win][arm64ec] Fix duplicate errors with the dontcall attribute #152810
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-x86 @llvm/pr-subscribers-backend-aarch64 Author: Daniel Paoliello (dpaoliello) ChangesSince the This change moves the checking for Full diff: https://github.com/llvm/llvm-project/pull/152810.diff 2 Files Affected:
diff --git a/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp b/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
index fb9eff942a464..5bc158a660080 100644
--- a/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
@@ -1148,9 +1148,11 @@ bool FastISel::lowerCall(const CallInst *CI) {
CLI.setCallee(RetTy, FuncTy, CI->getCalledOperand(), std::move(Args), *CI)
.setTailCall(IsTailCall);
- diagnoseDontCall(*CI);
-
- return lowerCallTo(CLI);
+ if (lowerCallTo(CLI)) {
+ diagnoseDontCall(*CI);
+ return true;
+ } else
+ return false;
}
bool FastISel::selectCall(const User *I) {
diff --git a/llvm/test/CodeGen/AArch64/arm64ec-dont-call.ll b/llvm/test/CodeGen/AArch64/arm64ec-dont-call.ll
new file mode 100644
index 0000000000000..febb8997f3e37
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/arm64ec-dont-call.ll
@@ -0,0 +1,21 @@
+; RUN: not llc -mtriple arm64ec-windows-msvc -o - %s 2>&1 | FileCheck %s
+
+define void @baz() #0 {
+ call void @foo()
+ ret void
+}
+
+define void @foo() #1 {
+ ret void
+}
+
+attributes #0 = { noinline optnone }
+attributes #1 = { "dontcall-error"="oh no foo" }
+
+; Regression test for `dontcall-error` for Arm64EC. Since this attribute is
+; checked both by FastISel and SelectionDAGBuilder, and FastISel was bailing for
+; Arm64EC AFTER doing the check, we ended up with duplicate copies of this
+; error.
+
+; CHECK: error: call to #foo marked "dontcall-error": oh no foo
+; CHECK-NOT: error:
|
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.
| ; RUN: not llc -mtriple arm64ec-windows-msvc -o - %s 2>&1 | FileCheck %s | |
| ; RUN: not llc -mtriple=arm64ec-windows-msvc -filetype=null %s 2>&1 | FileCheck %s |
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.
| } else | |
| return false; | |
| } | |
| return false; |
No else after return
551006c to
634dead
Compare
634dead to
c9f311e
Compare
c9f311e to
f946fcc
Compare
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.
| bool Result = translateCallBase(CI, MIRBuilder); | |
| if (Result) { | |
| if (translateCallBase(CI, MIRBuilder)) { |
f946fcc to
d7c6397
Compare
|
Possible build break: https://lab.llvm.org/buildbot/#/builders/154/builds/20134 Not sure why this change would mean NOT seeing the errors at all - is |
Fixed: #153302 |
…astSelectInstruction (#153302) Recently my change to avoid duplicate `dontcall` attribute errors (#152810) caused the Clang `Frontend/backend-attribute-error-warning.c` test to fail on Arm32: <https://lab.llvm.org/buildbot/#/builders/154/builds/20134> The root cause is that, if the default `IFastSel` path bails, then targets are given the opportunity to lower instructions via `fastSelectInstruction`. That's the path taken by Arm32 and since its implementation of `selectCall` didn't call `diagnoseDontCall` no error was emitted. I've checked the other implementations of `fastSelectInstruction` and the only other one that lowers call instructions in WebAssembly, so I've fixed that too.
|
Thanks for the fix, I was just getting around to looking at this. Our slower builders may continue to complain for a bit while they catch up but I can confirm the one you linked is green again. |
Since the
dontcall-*attributes are checked both byFastISel/GlobalISelandSelectionDAGBuilder, and bothFastISelandGlobalISelbail for calls on Arm64EC for AFTER doing the check, we ended up emitting duplicate copies of this error.This change moves the checking for
dontcall-*inFastISelandGlobalISelto after it has been successfully lowered.