Page MenuHomePhabricator

[PowerPC] Add intrinsic to convert between ppc_fp128 and fp128
Needs ReviewPublic

Authored by qiucf on Sep 8 2021, 12:54 AM.

Details

Reviewers
nemanjai
uweigand
craig.topper
RKSimon
spatel
jsji
efriedma
shchenz
Group Reviewers
Restricted Project
Summary

ppc_fp128 and fp128 are both 128-bit floating point types. However, we can't do conversion between them now. (trunc/ext are not allowed for same-size fp types)

This patch adds two new intrinsics: llvm.ppc.convert.f128.to.ppcf128 and llvm.convert.ppcf128.to.f128, to support such conversion.

Diff Detail

Event Timeline

qiucf created this revision.Sep 8 2021, 12:54 AM
qiucf requested review of this revision.Sep 8 2021, 12:54 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 8 2021, 12:54 AM
qiucf added a reviewer: jsji.
lkail edited reviewers, added: efriedma; removed: eli.friedman.Sep 8 2021, 2:09 AM

This could be a powerpc-specific intrinsic, maybe? I guess that basically just shoving around the code a bit, but maybe clearer to readers. The name is also a little ambiguous: it's not clear from the name that the target type must be fp128.

Long-term, we might want to consider replacing fpext/fptrunc instructions with something like fpconv. We now have two pairs of floating-point types that have the same size, and if we ever add more floating-point types, the problems will just get more complicated.

llvm/lib/IR/Function.cpp
1031

Please define a new IITDescriptorKind for this.

qiucf updated this revision to Diff 372411.Sep 13 2021, 11:49 PM
qiucf marked an inline comment as done.
qiucf retitled this revision from [SDAG] Add intrinsic to convert between ppc_fp128 and fp128 to [PowerPC] Add intrinsic to convert between ppc_fp128 and fp128.
qiucf edited the summary of this revision. (Show Details)

Make it a PowerPC intrinsic.

Thanks for doing this. Looks reasonable to me. Some code format comments.

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

Lint warnings.

Do we need a local variable for LC?

11031

Can we put this also in LowerINTRINSIC_WO_CHAIN? and call LowerINTRINSIC_WO_CHAIN here? So we can have a concentrated place for the ppc128/f128 conversion?

11039

no need for local Op?