-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[AMDGPU] Remove calling conv check on entry function #162080
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-amdgpu Author: None (jofrn) ChangesIt is undefined behavior to call a function with a mismatched calling convention. Rather than crash on this behavior, it should compile. Full diff: https://github.com/llvm/llvm-project/pull/162080.diff 1 Files Affected:
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp b/llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp
index 0ea9add891111..b03d50f2d451d 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp
@@ -261,13 +261,6 @@ AMDGPUResourceUsageAnalysisImpl::analyzeResourceUsage(
const Function *Callee = getCalleeFunction(*CalleeOp);
- // Avoid crashing on undefined behavior with an illegal call to a
- // kernel. If a callsite's calling convention doesn't match the
- // function's, it's undefined behavior. If the callsite calling
- // convention does match, that would have errored earlier.
- if (Callee && AMDGPU::isEntryFunctionCC(Callee->getCallingConv()))
- report_fatal_error("invalid call to entry function");
-
auto isSameFunction = [](const MachineFunction &MF, const Function *F) {
return F == &MF.getFunction();
};
|
arsenm
left a comment
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.
Needs test
|
@CatherineMoore for awareness |
988f6de to
2f3d478
Compare
shiltian
left a comment
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.
Is there anything less fatal that will not crash the backend? I think the comment wanted it to behave like that, but the description of the PR seems to say other way.
The original comment is contradictory: it says to avoid crashing on undefined behavior but then crashes with report_fatal_error(). While crashing is still undefined behavior, it's less fatal to compile it (even if the result is incorrect) than to crash the backend. We have a lint check to catch the mismatched cc. |
It is undefined behavior to call a function with a mismatched calling convention. Rather than crash on this behavior, it should compile.
7886359 to
03c691d
Compare
It is undefined behavior to call a function with a mismatched calling convention. Rather than crash on this behavior, it should compile.
This LLVM defect was identified via the AMD Fuzzing project.