This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Introduce RISCVISD::VWMACC(U/SU)_VL opcode
ClosedPublic

Authored by nitinjohnraj on Jun 15 2023, 11:14 AM.

Diff Detail

Event Timeline

nitinjohnraj created this revision.Jun 15 2023, 11:14 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 15 2023, 11:14 AM
nitinjohnraj requested review of this revision.Jun 15 2023, 11:14 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 15 2023, 11:14 AM
nitinjohnraj planned changes to this revision.Jun 15 2023, 11:16 AM
nitinjohnraj added a reviewer: craig.topper.
craig.topper added inline comments.Jun 15 2023, 11:22 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
12379

No else after return

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

fourth -> fifth

llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td
404

riscv_vwmaccsu_vl isn't commutative

1873

Shoudln't the VPatWidenMultiplyAddVL_VV_VX be deleted?

nitinjohnraj marked an inline comment as done.

Check that VWMUL merge operand is undef.

nitinjohnraj planned changes to this revision.Jun 15 2023, 12:29 PM
nitinjohnraj marked 2 inline comments as done.

Addressing comments

nitinjohnraj marked an inline comment as done.

vwmaccsu is not commutative. Delete the old multiclass used to associate patterns to vwmacc(u/su). Fix documentation.

craig.topper added inline comments.Jun 15 2023, 2:22 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
11964

Please add static_asserts to verify this is safe.

15510

This doesn't apply cleanly.

craig.topper added inline comments.Jun 15 2023, 3:13 PM
llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td
1599

vti.Mask:$mask should be (vti.Mask V0) everywhere it appears.

craig.topper requested changes to this revision.Jun 15 2023, 3:14 PM
This revision now requires changes to proceed.Jun 15 2023, 3:14 PM
nitinjohnraj updated this revision to Diff 531959.EditedJun 15 2023, 5:51 PM
nitinjohnraj marked 3 inline comments as done.

Rebased, added static asserts and fixed documentation

craig.topper added inline comments.Jun 15 2023, 7:01 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
15509

I think these case statements are in nearly the same order as the RISCVISelLowering.h enum. Please use the same order.

Changed order of case statements to match the enum constructor order

nitinjohnraj marked an inline comment as done.Jun 16 2023, 10:54 AM
This revision is now accepted and ready to land.Jun 16 2023, 11:17 AM
This revision was automatically updated to reflect the committed changes.