Skip to content
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

[amdgpu] Using fp128 fails with unsupported libcalls #121122

Open
Flakebi opened this issue Dec 26, 2024 · 6 comments
Open

[amdgpu] Using fp128 fails with unsupported libcalls #121122

Flakebi opened this issue Dec 26, 2024 · 6 comments

Comments

@Flakebi
Copy link
Member

Flakebi commented Dec 26, 2024

fp128 seems to work fine in arguments and return values, but trying to do computations like fmul or fdiv fails:

target triple = "amdgcn-amd-amdhsa"

define fp128 @fp128(fp128 %a, fp128 %b) {
  %res = fmul fp128 %a, %b
  ret fp128 %res
}

results in

LLVM ERROR: unsupported libcall legalization
Stack dump:
…
#10 llvm::SITargetLowering::LowerCall(llvm::TargetLowering::CallLoweringInfo&, llvm::SmallVectorImpl<llvm::SDValue>&) const llvm-project/llvm/lib/Target/AMDGPU/SIISelLowering.cpp:3877:34

I assume this is expected.
I encountered this while trying to compile Rust code, see also rust-lang/compiler-builtins#737.

@llvmbot
Copy link
Member

llvmbot commented Dec 26, 2024

@llvm/issue-subscribers-backend-amdgpu

Author: Sebastian Neubauer (Flakebi)

`fp128` seems to work fine in arguments and return values, but trying to do computations like `fmul` or `fdiv` fails: ```llvm target triple = "amdgcn-amd-amdhsa"

define fp128 @fp128(fp128 %a, fp128 %b) {
%res = fmul fp128 %a, %b
ret fp128 %res
}

results in

LLVM ERROR: unsupported libcall legalization
Stack dump:

#10 llvm::SITargetLowering::LowerCall(llvm::TargetLowering::CallLoweringInfo&, llvm::SmallVectorImpl<llvm::SDValue>&) const llvm-project/llvm/lib/Target/AMDGPU/SIISelLowering.cpp:3877:34


I assume this is expected.
I encountered this while trying to compile Rust code, see also https://github.com/rust-lang/compiler-builtins/pull/737.
</details>

@arsenm
Copy link
Contributor

arsenm commented Dec 26, 2024

This is expected. We do not have any support for fp128 operations in any form. There's no inline soft float expansion, and we don't have the infrastructure to use compiler-rt

@tgross35
Copy link

This is expected. We do not have any support for fp128 operations in any form. There's no inline soft float expansion, and we don't have the infrastructure to use compiler-rt

This is fp128-specific, correct? I.e. there isn't a technical reason that AMDGPU is unable to make use of any libcalls.

@arsenm
Copy link
Contributor

arsenm commented Dec 26, 2024

This is fp128-specific, correct? I.e. there isn't a technical reason that AMDGPU is unable to make use of any libcalls.

Oh there certainly are reasons we cannot use any libcalls. We do not have the infrastructure to link compiler-rt, and do not support object linking

@Flakebi
Copy link
Member Author

Flakebi commented Dec 29, 2024

Oh there certainly are reasons we cannot use any libcalls. We do not have the infrastructure to link compiler-rt, and do not support object linking

This seems to be frontend-specific. We may not have such infrastructure for clang yet, but rustc has it, it tries to link in the Rust compiler-builtins lib that provides implementations. (This linking can be done with LTO, so it doesn’t need object linking.)

I was able to compile builtins with Rust with 2 changes:

  1. Search for builtin implementations in the current module:
diff --git a/llvm/lib/Target/AMDGPU/SIISelLowering.cpp b/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
index 7f95442401db..4bf0f5c32d94 100644
--- a/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
+++ b/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
@@ -3667,8 +3667,27 @@ SDValue SITargetLowering::LowerCall(CallLoweringInfo &CLI,
                               "unsupported call to variadic function ");
   }
 
-  if (!CLI.CB)
-    report_fatal_error("unsupported libcall legalization");
+  if (!CLI.CB) {
+    // Is a libcall. If the function is defined in the current module, use it,
+    // otherwise abort.
+    bool FoundImpl = false;
+    if (ExternalSymbolSDNode *S = dyn_cast<ExternalSymbolSDNode>(Callee)) {
+      const char *SymName = S->getSymbol();
+      const Module *Mod = DAG.getMachineFunction().getFunction().getParent();
+      if (const Function *F =
+              dyn_cast_or_null<Function>(Mod->getNamedValue(SymName))) {
+        const TargetLowering &TLI = DAG.getTargetLoweringInfo();
+        EVT VT = TLI.getValueType(DAG.getDataLayout(), F->getType(), true);
+        Callee = DAG.getGlobalAddress(F, DL, VT);
+        FoundImpl = true;
+      } else if (SymName) {
+        report_fatal_error(Twine("unsupported libcall legalization, did not find '") + SymName + "'");
+      }
+    }
+
+    if (!FoundImpl)
+      report_fatal_error("unsupported libcall legalization");
+  }
 
   if (IsTailCall && MF.getTarget().Options.GuaranteedTailCallOpt) {
     return lowerUnhandledCall(CLI, InVals,

This is from PowerPC (slightly adjusted).

  1. Currently, amdgpu claims most libcalls as unsupported by setting the name to nullptr:
    // Disable most libcalls on AMDGPU.
    if (TT.isAMDGPU()) {
    for (int I = 0; I < RTLIB::UNKNOWN_LIBCALL; ++I) {
    if (I < RTLIB::ATOMIC_LOAD || I > RTLIB::ATOMIC_FETCH_NAND_16)
    setLibcallName(static_cast<RTLIB::Libcall>(I), nullptr);
    }
    }

As a result, the name we get at (1) is just nullptr.

It seems to me that the decision of which libcalls are available may be better in the frontend instead of the backend, though this is probably rather difficult to change.
Maybe we can keep marking the libcalls as unsupported, but preserve their name, so that frontends that link in a runtime lib can support them?

@arsenm
Copy link
Contributor

arsenm commented Dec 29, 2024

It seems to me that the decision of which libcalls are available may be better in the frontend instead of the backend,

It's the exact opposite. The fact that we try to treat "libcalls" as a front-end/language property is a significant fraction of the legacy issues we experience. We need to define the platform for what calls are available.

Maybe we can keep marking the libcalls as unsupported, but preserve their name, so that frontends that link in a runtime lib can support them?

Where the libcalls are linked in is significant. Without object linking, you would have to force linking in every possible libcall as used and codegen in it in 100% of situations just in case it ends up getting used, which is a significant compile time burden. We really need to put in the work to support compiler-rt in a normal-ish way

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants