Page MenuHomePhabricator

[X86][ISel] Lowering FROUND(f16) and FROUNDEVEN(f16)
ClosedPublic

Authored by FreddyYe on Thu, Sep 23, 12:03 AM.

Details

Summary

When AVX512FP16 is enabled, FROUND(f16) cannot be dealt with
TypeLegalize, and no libcall in libm is ready for fround(f16) now.
FROUNDEVEN(f16) has related instruction in AVX512FP16.

Diff Detail

Event Timeline

FreddyYe created this revision.Thu, Sep 23, 12:03 AM
FreddyYe requested review of this revision.Thu, Sep 23, 12:03 AM
Herald added a project: Restricted Project. · View Herald TranscriptThu, Sep 23, 12:03 AM
craig.topper added inline comments.Thu, Sep 23, 12:12 AM
llvm/test/CodeGen/X86/fp-roundeven.ll
45

Why can’t this just be vrndscalesh?

craig.topper added inline comments.Thu, Sep 23, 12:17 AM
llvm/lib/Target/X86/X86ISelLowering.cpp
1954

What about strict_fround?

Where are tests for strict_froundeven?

FreddyYe marked an inline comment as done.Thu, Sep 23, 1:38 AM
FreddyYe added inline comments.
llvm/test/CodeGen/X86/fp-roundeven.ll
45

Yes, It can. It was a mistake. I'll update, including the summary.

FreddyYe marked 2 inline comments as done.Sun, Sep 26, 12:29 AM
FreddyYe added inline comments.
llvm/lib/Target/X86/X86ISelLowering.cpp
1954

I used a report_fatal_error to deal with strict_fround.
Added.

FreddyYe updated this revision to Diff 375083.Sun, Sep 26, 12:30 AM
FreddyYe marked an inline comment as done.

Thanks for review! Addressed comments.

FreddyYe updated this revision to Diff 375085.Sun, Sep 26, 12:33 AM

nfc refine.

pengfei added inline comments.Sun, Sep 26, 12:41 AM
llvm/lib/Target/X86/X86ISelLowering.cpp
1954–1957

How about the vector type?

22503

Can we customize STRICT_FROUND here too? I think you just need to chain the FP nodes togerther.

FreddyYe added inline comments.Sun, Sep 26, 1:11 AM
llvm/lib/Target/X86/X86ISelLowering.cpp
1954–1957

For now, vector expands to vrndscalesh. While vrndscaleph can be emited. I'll refine, too.

22503

@craig.topper created this function originally. He mentions

Constrained intrinsics would use STRICT_FROUND which won't go through this code.

at https://reviews.llvm.org/D73607. Hi Craig, can we chain the FP nodes here?

pengfei added inline comments.Sun, Sep 26, 1:24 AM
llvm/lib/Target/X86/X86ISelLowering.cpp
22503

Oh, I see. Can we return SDValue() so that we can call lib function finally?

FreddyYe retitled this revision from [X86][ISel] setOperationAction for FROUND(f16) and FROUNDEVEN(f16) to [X86][ISel] Lowering FROUND(f16) and FROUNDEVEN(f16).Sun, Sep 26, 1:32 AM
FreddyYe edited the summary of this revision. (Show Details)
FreddyYe added inline comments.Sun, Sep 26, 1:34 AM
llvm/lib/Target/X86/X86ISelLowering.cpp
22503

No, that causes abort for now since round(fp16) is not handled yet in LLVM. I suppose libm hasn't supoprted yet?

pengfei added inline comments.Sun, Sep 26, 1:46 AM
llvm/lib/Target/X86/X86ISelLowering.cpp
22503

Right, we don't have lib functions for FP16. I think it's OK to report error for now.

This revision is now accepted and ready to land.Sun, Sep 26, 10:39 AM
FreddyYe updated this revision to Diff 375138.Sun, Sep 26, 7:34 PM

Refine tests.

This revision was landed with ongoing or failed builds.Sun, Sep 26, 10:44 PM
This revision was automatically updated to reflect the committed changes.