Skip to content

Conversation

@arsenm
Copy link
Contributor

@arsenm arsenm commented Jun 21, 2025

No description provided.

Copy link
Contributor Author

arsenm commented Jun 21, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@arsenm arsenm requested review from rampitec, rovka and shiltian June 21, 2025 02:21
@arsenm arsenm marked this pull request as ready for review June 21, 2025 02:21
@llvmbot
Copy link
Member

llvmbot commented Jun 21, 2025

@llvm/pr-subscribers-backend-amdgpu

Author: Matt Arsenault (arsenm)

Changes

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

1 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/SIISelLowering.cpp (+1-1)
diff --git a/llvm/lib/Target/AMDGPU/SIISelLowering.cpp b/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
index 07d79d677104a..b752ccd6e14b1 100644
--- a/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
+++ b/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
@@ -3805,7 +3805,7 @@ SDValue SITargetLowering::LowerCall(CallLoweringInfo &CLI,
   }
 
   if (!CLI.CB)
-    report_fatal_error("unsupported libcall legalization");
+    return lowerUnhandledCall(CLI, InVals, "unsupported libcall legalization");
 
   if (IsTailCall && MF.getTarget().Options.GuaranteedTailCallOpt) {
     return lowerUnhandledCall(CLI, InVals,

@rampitec
Copy link
Collaborator

Then how the error will look now for an user?

@arsenm
Copy link
Contributor Author

arsenm commented Jun 21, 2025

Then how the error will look now for an user?

Should be
error: : ....

@shiltian
Copy link
Contributor

but there is no test for that?

@rampitec
Copy link
Collaborator

but there is no test for that?

Ideally we should have one... But there was not one before too.

@arsenm
Copy link
Contributor Author

arsenm commented Jun 22, 2025

but there is no test for that?

Ideally we should have one... But there was not one before too.

This isn't really supposed to happen in the first place so there isn't a reliable test. Would need to have a known broken operation

@shiltian
Copy link
Contributor

but if that should really not happen, existing report_fatal_error might be good?

@arsenm
Copy link
Contributor Author

arsenm commented Jun 22, 2025

but if that should really not happen, existing report_fatal_error might be good?

report_fatal_error is never good. Also it's highly likely to appear with the legalizer's bias towards libcalls by default

@arsenm arsenm merged commit 584a2c2 into main Jun 22, 2025
11 checks passed
@arsenm arsenm deleted the users/arsenm/amdgpu/avoid-report_fatal_error-libcall-legalization branch June 22, 2025 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants