Page MenuHomePhabricator

[2/N][RISCV] Model vxrm in LLVM intrinsics and machine instructions for RVV fixed-point instructions
Needs ReviewPublic

Authored by eopXD on Wed, May 24, 7:10 PM.

Details

Summary

Depends on D151395.

This is the 2nd patch of the patch-set. For the cover letter of the
patch-set, please checkout D151395. This patch originates from
D121376.

This commit models vxrm by adding an immediate operand into intrinsics
and machine instructions of RVV fixed-point instruction vaadd,
vaaddu, vasub, and vasubu. This commit only covers intrinsics of
the four instructions, the proceeding patches of the patch-set will do
the same to other RVV fixed-point instructions.

The current naiive approach is to have a write to vxrm inserted before
every fixed-point instruction. This is done by the new added pass
RISCVInsertReadWriteCSR. The reason to name the pass in a more general
term is because we will also model rounding mode for the RVV floating-
point instructions. The approach will be improved in the future,
implementing partial redundancy elimination algorithms to it.

The original LLVM intrinsics and machine instructions, take vaadd as
an example, does not model the rounding mode is not removed in this
patch. That is, int.riscv.vaadd.* co-exists with
int.riscv.vaadd.rm.* after this patch. The next patch will add C
intrinsics of vaadd with an additional operand that models the control
of the rounding mode, in this patch, int.riscv.vaadd.rm.* will
replace int.riscv.vaadd.*.

Authored-by: ShihPo Hung <shihpo.hung@sifive.com>
Co-Authored-by: eop Chen <eop.chen@sifive.com>

Diff Detail

Event Timeline

eopXD created this revision.Wed, May 24, 7:10 PM
Herald added a project: Restricted Project. · View Herald TranscriptWed, May 24, 7:10 PM
eopXD requested review of this revision.Wed, May 24, 7:10 PM
eopXD updated this revision to Diff 525412.Wed, May 24, 7:46 PM

Bump CI.

craig.topper added inline comments.Wed, May 24, 8:19 PM
llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
118

Extra blank line

llvm/lib/Target/RISCV/RISCVInsertReadWriteCSR.cpp
2

Incorrect file name and description

12

naive*

"insertion a write" -> "insertion of a write"?

84

Use operator* instead of optional::value. See many patches like 1792821c8308755593c114c3b7ae8ce33bdb08e9

llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
2037

80 characters

2038

This isn't indented far enought to line up with RetClass.

6713

I would kind of prefer you do this 4 instruction group together.

eopXD edited the summary of this revision. (Show Details)Wed, May 24, 10:27 PM
eopXD updated this revision to Diff 525441.Wed, May 24, 10:28 PM
eopXD marked 7 inline comments as done.

Address comments from Craig and resolve other test failures.

eopXD edited the summary of this revision. (Show Details)Wed, May 24, 11:56 PM
eopXD edited the summary of this revision. (Show Details)Thu, May 25, 12:21 AM
craig.topper added inline comments.Thu, May 25, 9:35 PM
llvm/lib/Target/RISCV/RISCVInsertReadWriteCSR.cpp
23

RISCV -> RISC-V

87

We need to add an implicit use of VXRM to the vector instruction. That's how we do it in the InsertVSETVLI pass. See

MI.addOperand(MachineOperand::CreateReg(RISCV::VL, /*isDef*/ false,      
                                        /*isImp*/ true));

from RISCVInsertVSETVLI::emitVSETVLIs

llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
6043–6044

The Pseudos don't use VXRM until the VXRM write instruction is inserted.

6045

Line up the colons

llvm/test/CodeGen/RISCV/O0-pipeline.ll
44

RISCV -> RISC-V

llvm/test/CodeGen/RISCV/rvv/vxrm.mir
27–28

Should we delete the existing WriteVXRMImm instruction from this test?

eopXD updated this revision to Diff 525956.Thu, May 25, 11:28 PM
eopXD marked 6 inline comments as done.

Address comments from Craig.

craig.topper added inline comments.Fri, May 26, 12:00 PM
llvm/test/CodeGen/RISCV/rvv/vxrm.mir
17

I would expect there to be two $vxrm operands since you started with one and the pass added one.

28

Drop the implicit $vxrm here. It shouldn't exist until the pass runs.

The implicit $vl, implicit $vtype operands shouldn't exist until the InsertVSETVLI pass runs.