Page MenuHomePhabricator

[RISCV] Custom lowering of FLT_ROUNDS_
ClosedPublic

Authored by sepavloff on Nov 5 2020, 8:06 AM.

Diff Detail

Event Timeline

sepavloff created this revision.Nov 5 2020, 8:06 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 5 2020, 8:06 AM
sepavloff requested review of this revision.Nov 5 2020, 8:06 AM
sepavloff updated this revision to Diff 304877.Nov 12 2020, 9:47 AM

Small changes similar to those made in D91242

jrtc27 added inline comments.Nov 12 2020, 9:52 AM
llvm/lib/Target/RISCV/RISCVISelLowering.h
267–271

Fixed comments

  • IEEE-754 rounding direction names are replaced by llvm::RoundingMode enumerators,
  • Fixed comment for RDN.
lenary resigned from this revision.Jan 14 2021, 9:46 AM
sepavloff updated this revision to Diff 332680.Mar 23 2021, 8:12 AM

Adapted the patch for alternative CSR implementation

sepavloff updated this revision to Diff 338753.Apr 20 2021, 1:25 AM

Rebased patch

craig.topper added inline comments.Apr 20 2021, 12:46 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
377

You can use XLenVT instead of picking i64/i32.

4118

Rename IntTy to XLenVT to match what most of the RISCV back does.

4122

Use RISCVSysReg::lookupSysRegByName("FRM")->Encoding it's not common enough to justify having the encoding in 2 places.

4625

Is this code reachable? This would require setOperaction(ISD::FLT_ROUNDS_, MVT::i32, Custom) when Subtarget.is64Bit() is false.

It's also the default behavior for the target independent type legalizer.

llvm/lib/Target/RISCV/RISCVISelLowering.h
266

Can you use RISCVFPRndMode from lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h

craig.topper added inline comments.Apr 20 2021, 12:54 PM
llvm/test/CodeGen/RISCV/fpenv.ll
2

Use update_llc_test_checks.py

sepavloff updated this revision to Diff 339208.Apr 21 2021, 6:24 AM

Addressed reviewer's notes

sepavloff updated this revision to Diff 339214.Apr 21 2021, 6:36 AM

Added missed change

sepavloff marked 4 inline comments as done.Apr 21 2021, 7:47 AM
sepavloff added inline comments.
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
4625

Removed this check.

craig.topper added inline comments.Apr 21 2021, 8:58 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
15

This is already included in RISCV.h because it's very common file to need.

sepavloff updated this revision to Diff 339281.Apr 21 2021, 9:42 AM

Remove superfluous include

craig.topper accepted this revision.Apr 21 2021, 10:19 AM

LGTM.

llvm/lib/Target/RISCV/RISCVISelLowering.cpp
4627

I missed it before, but you can use Res.getValue(0) and Res.getValue(1) to shorten these lines if you want.

This revision is now accepted and ready to land.Apr 21 2021, 10:19 AM
sepavloff added inline comments.Apr 21 2021, 8:40 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
4627

Thank you for advice! I will incorporate these changes too.

This revision was landed with ongoing or failed builds.Apr 21 2021, 9:40 PM
This revision was automatically updated to reflect the committed changes.