This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Add pass to remove W suffix from ADDIW and SLLIW to improve compressibility
ClosedPublic

Authored by nitinjohnraj on Dec 13 2022, 8:54 AM.

Details

Summary

SLLI and ADD are more compressible than SLLIW and ADDW. SLLI/ADD both have a 5-bit register encoding. SLLIW/ADDW have a 3-bit register encoding. They both require the dest to also be one of the sources.

We aggressively form ADDW/SLLIW as it helps hasAllWBitUsers in RISCVISelDAGToDAG to not require recursion. So we need a pass to remove excessive -w suffixes.

Diff Detail

Event Timeline

nitinjohnraj created this revision.Dec 13 2022, 8:54 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 13 2022, 8:54 AM
nitinjohnraj requested review of this revision.Dec 13 2022, 8:54 AM
craig.topper added inline comments.Dec 13 2022, 8:58 AM
llvm/lib/Target/RISCV/RISCVISelDAGToDAG.h
79 ↗(On Diff #482508)

This appears to be the code from D139462. This should not be part of this diff.

llvm/test/CodeGen/RISCV/add-before-shl.ll
33

Something weird happened here. Why are these lines removed?

craig.topper added inline comments.Dec 13 2022, 8:59 AM
llvm/test/CodeGen/RISCV/rvv/fixed-vectors-vwmulu.ll
256 ↗(On Diff #482508)

This doesn't look correct. Why did the capitilzation of VLENB change?

Updated the test checks

I don't understand this, your patch subject says you're adding a pass to remove the W suffix from instructions, but the actual diff doesn't add a pass and the test changes all add W suffices

llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
2286 ↗(On Diff #482590)

That's what you've implemented?

I don't understand this, your patch subject says you're adding a pass to remove the W suffix from instructions, but the actual diff doesn't add a pass and the test changes all add W suffices

The last update looks like it should have gone to D139462

I don't understand this, your patch subject says you're adding a pass to remove the W suffix from instructions, but the actual diff doesn't add a pass and the test changes all add W suffices

The last update looks like it should have gone to D139462

Yes, sorry, that's my mistake. I can't seem to find a way to delete this revision, I seem to have messed this up a bit.

nitinjohnraj retitled this revision from [RISCV][WIP] Add pass to remove W suffix from ADDIW and SLLIW to improve compressibility to [RISCV] Add pass to remove W suffix from ADDIW and SLLIW to improve compressibility.Dec 13 2022, 2:28 PM
nitinjohnraj edited the summary of this revision. (Show Details)

Fixed previous erroneous revision, linked parent revision via commit message

craig.topper added inline comments.Dec 13 2022, 6:43 PM
llvm/lib/Target/RISCV/RISCVInstrInfo.h
253

Capitalize returns

253

dependen->depend

255

Move the TODO to the implementation

llvm/lib/Target/RISCV/RISCVStripWSuffix.cpp
12

Missing period at end of sentence.

I think you can describe what "whenever possible" means.

Fixed some documentation nits

craig.topper added inline comments.Dec 14 2022, 3:25 PM
llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
2286 ↗(On Diff #483011)

This was in D139462

nitinjohnraj marked 5 inline comments as done.

Pulled from main

craig.topper added inline comments.Dec 22 2022, 9:56 AM
llvm/lib/Target/RISCV/RISCVSExtWRemoval.cpp
224–231

I added VT_MASKC and VT_MASKCN here 2 days ago. When did you pull from main?

nitinjohnraj marked an inline comment as done.

Pulled from main

Added the VT_MASKC/N changes

This revision is now accepted and ready to land.Dec 22 2022, 11:48 AM
This revision was landed with ongoing or failed builds.Dec 22 2022, 2:20 PM
This revision was automatically updated to reflect the committed changes.