Page MenuHomePhabricator

[PowerPC][FP128] Fix the incorrect signature for math library call
ClosedPublic

Authored by steven.zhang on Nov 17 2020, 6:38 PM.

Details

Summary

The runtime library has two family library implementation for ppc_fp128 and fp128. For IBM Long double(ppc_fp128), it is suffixed with 'l', i.e(sqrtl). For IEEE Long double(fp128), it is suffixed with "ieee128" or "f128". We miss to map several libcall for IEEE Long double.

Diff Detail

Event Timeline

steven.zhang created this revision.Nov 17 2020, 6:38 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 17 2020, 6:38 PM
steven.zhang requested review of this revision.Nov 17 2020, 6:38 PM
qiucf added inline comments.Nov 19 2020, 11:28 PM
llvm/lib/CodeGen/TargetLoweringBase.cpp
150

I see in llvm/include/llvm/IR/RuntimeLibcalls.def:

HANDLE_LIBCALL(FPTOSINT_F128_I128, "__fixtfti")
HANDLE_LIBCALL(FPTOSINT_PPCF128_I128, "__fixtfti")

So only PPC uses KF instead of TF for IEEE f128?

And it seems compiler-rt doesn't have impl for __fixkfti now.

llvm/lib/Target/PowerPC/PPCISelLowering.cpp
1312

Do we need to unify those names - *f128 or __*ieee128?

1318

Should be __llroundieee128

1321

Also, __llrintieee128.

steven.zhang added inline comments.Nov 26 2020, 6:04 PM
llvm/lib/CodeGen/TargetLoweringBase.cpp
150

Yes. See the comments in this code.

// For IEEE quad-precision libcall names, PPC uses "kf" instead of "tf".

Though the compiler-rt didn't have the implementation now, we still need to fix it.

llvm/lib/Target/PowerPC/PPCISelLowering.cpp
1312

Good suggestion. Though the ieee128 version works, we should use f128 to align with old name convention.

1318

oops, typo.

Address comments.

qiucf accepted this revision as: qiucf.Dec 6 2020, 6:27 PM

LGTM. Thanks for the fix.

This revision is now accepted and ready to land.Dec 6 2020, 6:27 PM