Skip to content

Conversation

@arsenm
Copy link
Contributor

@arsenm arsenm commented Jun 16, 2025

Whether the libcall implementation is supported should always
be checked before emission. Previously this would emit a call
to a non-existent function, and then since r600 doesn't support
calls it would error on the call. This switches it to a direct
legalization failure.

Also swap to use reportFatalInternalError. Probably should
be reportFatalUsageError; we don't want a backtrace and this will
never be fixed, but it is tested.

Whether the libcall implementation is supported should always
be checked before emission. Previously this would emit a call
to a non-existent function, and then since r600 doesn't support
calls it would error on the call. This switches it to a direct
legalization failure.

Also swap to use reportFatalInternalError. Probably should
be reportFatalUsageError; we don't want a backtrace and this will
never be fixed, but it is tested.
@arsenm arsenm added the llvm:SelectionDAG SelectionDAGISel as well label Jun 16, 2025 — with Graphite App
Copy link
Contributor Author

arsenm commented Jun 16, 2025

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

@arsenm arsenm requested review from RKSimon and topperc June 16, 2025 08:20
@llvmbot
Copy link
Member

llvmbot commented Jun 16, 2025

@llvm/pr-subscribers-backend-amdgpu

@llvm/pr-subscribers-llvm-selectiondag

Author: Matt Arsenault (arsenm)

Changes

Whether the libcall implementation is supported should always
be checked before emission. Previously this would emit a call
to a non-existent function, and then since r600 doesn't support
calls it would error on the call. This switches it to a direct
legalization failure.

Also swap to use reportFatalInternalError. Probably should
be reportFatalUsageError; we don't want a backtrace and this will
never be fixed, but it is tested.


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

2 Files Affected:

  • (modified) llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp (+6-4)
  • (modified) llvm/test/CodeGen/AMDGPU/fneg.ll (+3-1)
diff --git a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
index a0ffb4b6d5a4c..1be61e4b86af2 100644
--- a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
@@ -178,10 +178,12 @@ TargetLowering::makeLibCall(SelectionDAG &DAG, RTLIB::Libcall LC, EVT RetVT,
     Args.push_back(Entry);
   }
 
-  if (LC == RTLIB::UNKNOWN_LIBCALL)
-    report_fatal_error("Unsupported library call operation!");
-  SDValue Callee = DAG.getExternalSymbol(getLibcallName(LC),
-                                         getPointerTy(DAG.getDataLayout()));
+  const char *LibcallName = getLibcallName(LC);
+  if (LC == RTLIB::UNKNOWN_LIBCALL || !LibcallName)
+    reportFatalInternalError("unsupported library call operation");
+
+  SDValue Callee =
+      DAG.getExternalSymbol(LibcallName, getPointerTy(DAG.getDataLayout()));
 
   Type *RetTy = RetVT.getTypeForEVT(*DAG.getContext());
   TargetLowering::CallLoweringInfo CLI(DAG);
diff --git a/llvm/test/CodeGen/AMDGPU/fneg.ll b/llvm/test/CodeGen/AMDGPU/fneg.ll
index 7262724064918..c3f4ebe30152b 100644
--- a/llvm/test/CodeGen/AMDGPU/fneg.ll
+++ b/llvm/test/CodeGen/AMDGPU/fneg.ll
@@ -3,7 +3,9 @@
 ; RUN: llc -mtriple=amdgcn -mcpu=tonga < %s | FileCheck -enable-var-scope -check-prefixes=GCN,VI %s
 ; RUN: llc -mtriple=amdgcn -mcpu=gfx1100 -mattr=+real-true16 < %s | FileCheck -enable-var-scope -check-prefixes=GCN,GFX11,GFX11-TRUE16 %s
 ; RUN: llc -mtriple=amdgcn -mcpu=gfx1100 -mattr=-real-true16 < %s | FileCheck -enable-var-scope -check-prefixes=GCN,GFX11,GFX11-FAKE16 %s
-; RUN: not llc -mtriple=r600 -mcpu=redwood < %s
+; RUN: not --crash llc -mtriple=r600 -mcpu=redwood < %s 2>&1 | FileCheck -check-prefix=R600-ERR %s
+
+; R600-ERR: LLVM ERROR: unsupported library call operation
 
 define amdgpu_kernel void @s_fneg_f32(ptr addrspace(1) %out, float %in) {
 ; SI-LABEL: s_fneg_f32:

@arsenm arsenm marked this pull request as ready for review June 16, 2025 08:24
@arsenm
Copy link
Contributor Author

arsenm commented Jun 23, 2025

ping

1 similar comment
@arsenm
Copy link
Contributor Author

arsenm commented Jun 27, 2025

ping

Copy link
Collaborator

@RKSimon RKSimon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@arsenm arsenm merged commit 7255c3a into main Jun 27, 2025
12 checks passed
@arsenm arsenm deleted the users/arsenm/dag/check-libcall-before-emission branch June 27, 2025 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend:AMDGPU llvm:SelectionDAG SelectionDAGISel as well

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants