[amdgpu] Using fp128 fails with unsupported libcalls #121122

Activity

1. Flakebi

   added

   backend:AMDGPU

   on Dec 26, 2024
2. llvmbot commented on Dec 26, 2024

   

   llvmbot

   on Dec 26, 2024

   Member

   More actions

   @llvm/issue-subscribers-backend-amdgpu

   Author: Sebastian Neubauer (Flakebi)

   Details

   `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>
   
3. arsenm commented on Dec 26, 2024

   

   arsenm

   on Dec 26, 2024

   Contributor

   More actions

   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
4. tgross35 commented on Dec 26, 2024

   

   tgross35

   on Dec 26, 2024

   Contributor

   More actions

   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.
5. arsenm commented on Dec 26, 2024

   

   arsenm

   on Dec 26, 2024

   Last edited by arsenm

   Contributor

   More actions

   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

   👍React with 👍1Reacted by Trevor Gross
6. Flakebi commented on Dec 29, 2024

   

   Flakebi

   on Dec 29, 2024

   MemberAuthor

   More actions

   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).

   2. Currently, amdgpu claims most libcalls as unsupported by setting the name to nullptr:
      

      llvm-project/llvm/lib/IR/RuntimeLibcalls.cpp

      Lines 208 to 214 in 7144325

      	// 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?
7. arsenm commented on Dec 29, 2024

   

   arsenm

   on Dec 29, 2024

   Contributor

   More actions

   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