Page MenuHomePhabricator

[RISCV] Support Zfhmin extension
ClosedPublic

Authored by achieveartificialintelligence on Oct 14 2021, 11:35 PM.

Details

Summary

According to RISC-V Unprivileged ISA 15.6.

Diff Detail

Event Timeline

achieveartificialintelligence requested review of this revision.Oct 14 2021, 11:35 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptOct 14 2021, 11:35 PM
asb added a comment.Oct 28 2021, 6:21 AM

From an initial look, I think this is almost ready to land - thank you.

One suggestion would be to review the various Subtarget.hasStdExtZfhmin() || Subtarget.hasStdExtZfh(). I'd first thought adding a new helper to RISCVSubtarget to be called instead, but actually it looks like a number of these could be replaced by just hasStdExtZfhmin due to Zfhmin implying Zfh.

llvm/lib/Target/RISCV/RISCVInstrInfoZfh.td
187

Unintended change?

achieveartificialintelligence marked an inline comment as done.Oct 28 2021, 8:19 AM
asb added inline comments.Tue, Nov 2, 8:02 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
1169

This could equivalently just be if (VT == MVT::f16 && !Subtarget.hasStdExtZfhmin()), right?

1192

This could just be !Subtarget.hasStdExtZfhmin()?

1203

This could just be Subtarget.hasStdExtZfhmin()?

1415

This could just be !Subtarget.hasStdExtZfhmin()?

asb accepted this revision.Fri, Nov 5, 4:21 AM

LGTM, modulo one tiny nit on a comment. Thanks!

llvm/lib/Target/RISCV/RISCVInstrInfoZfh.td
253

HasStdExtZfh=>HasStdExtZfhmin

This revision is now accepted and ready to land.Fri, Nov 5, 4:21 AM
achieveartificialintelligence marked an inline comment as done.Fri, Nov 5, 10:29 AM
This revision was landed with ongoing or failed builds.Fri, Nov 5, 10:41 AM
This revision was automatically updated to reflect the committed changes.

LGTM, modulo one tiny nit on a comment. Thanks!

Thank you for your detailed suggestions!