Skip to content

[RISCV][GISel] Support fp128 arithmetic and conversion for RV64. #118707

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 5, 2024
Merged

[RISCV][GISel] Support fp128 arithmetic and conversion for RV64. #118707

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d072797
[RISCV][GISel] Support fp128 arithmetic and conversion for RV64.
topperc Dec 4, 2024
c508ec9
fixup! Disable G_FREM libcalls for rv32.
topperc Dec 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions llvm/include/llvm/CodeGen/GlobalISel/LegalizerInfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -778,6 +778,11 @@ class LegalizeRuleSet {
LegalizeRuleSet &libcallFor(std::initializer_list<LLT> Types) {
return actionFor(LegalizeAction::Libcall, Types);
}
LegalizeRuleSet &libcallFor(bool Pred, std::initializer_list<LLT> Types) {
if (!Pred)
return *this;
return actionFor(LegalizeAction::Libcall, Types);
}
LegalizeRuleSet &
libcallFor(std::initializer_list<std::pair<LLT, LLT>> Types) {
return actionFor(LegalizeAction::Libcall, Types);
Expand Down
25 changes: 16 additions & 9 deletions llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -491,21 +491,23 @@ RISCVLegalizerInfo::RISCVLegalizerInfo(const RISCVSubtarget &ST)

// FP Operations

// FIXME: Support s128 for rv32 when libcall handling is able to use sret.
getActionDefinitionsBuilder(
{G_FADD, G_FSUB, G_FMUL, G_FDIV, G_FMA, G_FSQRT, G_FMAXNUM, G_FMINNUM})
.legalFor(ST.hasStdExtF(), {s32})
.legalFor(ST.hasStdExtD(), {s64})
.legalFor(ST.hasStdExtZfh(), {s16})
.libcallFor({s32, s64});
.libcallFor({s32, s64})
.libcallFor(ST.is64Bit(), {s128});

getActionDefinitionsBuilder({G_FNEG, G_FABS})
.legalFor(ST.hasStdExtF(), {s32})
.legalFor(ST.hasStdExtD(), {s64})
.legalFor(ST.hasStdExtZfh(), {s16})
.lowerFor({s32, s64});
.lowerFor({s32, s64, s128});

getActionDefinitionsBuilder(G_FREM)
.libcallFor({s32, s64})
.libcallFor({s32, s64, s128})
Copy link
Member

@lenary lenary Dec 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why s128 is fine here, but not for G_FADD. Both fmodl and __addtf3 have the same signature. What am I missing?

Copy link
Collaborator Author

@topperc topperc Dec 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I messed up. It should check is64Bit.

.minScalar(0, s32)
.scalarize(0);

Expand All @@ -521,19 +523,22 @@ RISCVLegalizerInfo::RISCVLegalizerInfo(const RISCVSubtarget &ST)
.legalFor(ST.hasStdExtD(), {{s32, s64}})
.legalFor(ST.hasStdExtZfh(), {{s16, s32}})
.legalFor(ST.hasStdExtZfh() && ST.hasStdExtD(), {{s16, s64}})
.libcallFor({{s32, s64}});
.libcallFor({{s32, s64}})
.libcallFor(ST.is64Bit(), {{s32, s128}, {s64, s128}});
Copy link
Member

@lenary lenary Dec 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why aren't these ok on 32-bit? They return 32/64-bit values, rather than 128-bit, right?

Maybe you're waiting so the FPTRUNC/FPEXT definitions can be symmetric? If so, that's reasonable.

Copy link
Collaborator Author

@topperc topperc Dec 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't think too hard about it and I didn't make a rv32 test. I can split off the rv32 subset into a new test and fix this.

getActionDefinitionsBuilder(G_FPEXT)
.legalFor(ST.hasStdExtD(), {{s64, s32}})
.legalFor(ST.hasStdExtZfh(), {{s32, s16}})
.legalFor(ST.hasStdExtZfh() && ST.hasStdExtD(), {{s64, s16}})
.libcallFor({{s64, s32}});
.libcallFor({{s64, s32}})
.libcallFor(ST.is64Bit(), {{s128, s32}, {s128, s64}});

getActionDefinitionsBuilder(G_FCMP)
.legalFor(ST.hasStdExtF(), {{sXLen, s32}})
.legalFor(ST.hasStdExtD(), {{sXLen, s64}})
.legalFor(ST.hasStdExtZfh(), {{sXLen, s16}})
.clampScalar(0, sXLen, sXLen)
.libcallFor({{sXLen, s32}, {sXLen, s64}});
.libcallFor({{sXLen, s32}, {sXLen, s64}})
.libcallFor(ST.is64Bit(), {{sXLen, s128}});
Copy link
Member

@lenary lenary Dec 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be fine on rv32 too, right? the libcall is just returning an int, not something 128-bits

Copy link
Collaborator Author

@topperc topperc Dec 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I'll split off an rv32 test.


// TODO: Support vector version of G_IS_FPCLASS.
getActionDefinitionsBuilder(G_IS_FPCLASS)
Expand All @@ -546,7 +551,7 @@ RISCVLegalizerInfo::RISCVLegalizerInfo(const RISCVSubtarget &ST)
.legalFor(ST.hasStdExtF(), {s32})
.legalFor(ST.hasStdExtD(), {s64})
.legalFor(ST.hasStdExtZfh(), {s16})
.lowerFor({s32, s64});
.lowerFor({s32, s64, s128});

getActionDefinitionsBuilder({G_FPTOSI, G_FPTOUI})
.legalFor(ST.hasStdExtF(), {{sXLen, s32}})
Expand All @@ -558,7 +563,8 @@ RISCVLegalizerInfo::RISCVLegalizerInfo(const RISCVSubtarget &ST)
.widenScalarToNextPow2(0)
.minScalar(0, s32)
.libcallFor({{s32, s32}, {s64, s32}, {s32, s64}, {s64, s64}})
.libcallFor(ST.is64Bit(), {{s128, s32}, {s128, s64}});
.libcallFor(ST.is64Bit(), {{s32, s128}, {s64, s128}}) // FIXME RV32.
.libcallFor(ST.is64Bit(), {{s128, s32}, {s128, s64}, {s128, s128}});

getActionDefinitionsBuilder({G_SITOFP, G_UITOFP})
.legalFor(ST.hasStdExtF(), {{s32, sXLen}})
Expand All @@ -579,7 +585,8 @@ RISCVLegalizerInfo::RISCVLegalizerInfo(const RISCVSubtarget &ST)
// Otherwise only promote to s32 since we have si libcalls.
.minScalar(1, s32)
.libcallFor({{s32, s32}, {s64, s32}, {s32, s64}, {s64, s64}})
.libcallFor(ST.is64Bit(), {{s32, s128}, {s64, s128}});
.libcallFor(ST.is64Bit(), {{s128, s32}, {s128, s64}}) // FIXME RV32.
.libcallFor(ST.is64Bit(), {{s32, s128}, {s64, s128}, {s128, s128}});

// FIXME: We can do custom inline expansion like SelectionDAG.
// FIXME: Legal with Zfa.
Expand Down
Loading
Loading