Page MenuHomePhabricator

[LoongArch] Add codegen support for handling floating point immediates
ClosedPublic

Authored by gonglingqin on Jun 30 2022, 2:42 AM.

Diff Detail

Unit TestsFailed

TimeTest
60,120 msx64 debian > BOLT.runtime/X86::exceptions-pic.test
Script: -- : 'RUN: at line 5'; /usr/bin/clang --driver-mode=g++ -fpic -Wl,-q /var/lib/buildkite-agent/builds/llvm-project/bolt/test/runtime/X86/Inputs/exception4.cpp -o /var/lib/buildkite-agent/builds/llvm-project/build/tools/bolt/test/runtime/X86/Output/exceptions-pic.test.tmp.pic

Event Timeline

gonglingqin created this revision.Jun 30 2022, 2:42 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 30 2022, 2:42 AM
gonglingqin requested review of this revision.Jun 30 2022, 2:42 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 30 2022, 2:42 AM

Hello everyone. @gonglingqin is another engineer from Loongson. Please help to review her patches. Thanks.

xen0n added inline comments.Jul 1 2022, 10:38 AM
llvm/lib/Target/LoongArch/LoongArchFloat64InstrInfo.td
211–212

Are we better off handling this by just hard-coding the bit patterns then movgr2fr.w and movgr2frh.w? I don't know the exact latencies for fcvt.d.s but plain moves should be a bit faster.

llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp
121

nit: "will be added later"

gonglingqin added inline comments.Jul 1 2022, 11:01 PM
llvm/lib/Target/LoongArch/LoongArchFloat64InstrInfo.td
211–212

Thanks for the suggestion, I will change that.

llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp
121

Thanks. I will change that.

Address @xen0n's comments.

xen0n accepted this revision.Jul 2 2022, 12:47 AM

LGTM but ideally someone else could take a look at this too.

This revision is now accepted and ready to land.Jul 2 2022, 12:47 AM
SixWeining added inline comments.Jul 3 2022, 6:25 PM
llvm/lib/Target/LoongArch/LoongArchInstrInfo.td
167–174

Suggest to remove useless blank lines and indent.

gonglingqin added inline comments.Jul 3 2022, 8:41 PM
llvm/lib/Target/LoongArch/LoongArchInstrInfo.td
167–174

Thanks. I will change that.

Address @SixWeining's comments.

SixWeining accepted this revision.Jul 6 2022, 2:59 AM
SixWeining edited the summary of this revision. (Show Details)

need to rebase before landing

xry111 added inline comments.Jul 6 2022, 3:06 AM
llvm/test/CodeGen/LoongArch/double-imm.ll
86

I'd suggest:

lu52i.d $a0, $zero, 0x3ff
movgr2fr.d $fa0, $a0

to reduce one instruction. The combination of lu52i.d and movgr2fr.d can always load $2^k$ as a f64 for all integral k in $[0, 1023]$.

But as it's already approved it can be done in a later revision.

xen0n accepted this revision.Jul 6 2022, 5:35 AM
xen0n added inline comments.
llvm/test/CodeGen/LoongArch/double-imm.ll
86

Wow that's some serious simplification. I don't think I've seen anything like this recently. Agreed this optimization is better done in a new patch, as it's more of a peephole kind, not deeply related to the generic handling done here.

SixWeining added inline comments.Jul 6 2022, 5:51 AM
llvm/test/CodeGen/LoongArch/double-imm.ll
86

Quite nice. Thanks for the suggestion. Let’s implement it in later patch. :)

This revision was landed with ongoing or failed builds.Jul 6 2022, 5:14 PM
This revision was automatically updated to reflect the committed changes.