Details

Reviewers

craig.topper
reames
jrtc27
asb
luismarques

Commits

rGd64d3c5a8f81: [RISCV] Add pass to remove W suffix from ADDIW and SLLIW to improve…

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

Repository: rG LLVM Github Monorepo

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

Herald added subscribers: sunshaoce, VincentWu, vkmr and 30 others. · View Herald Transcript

nitinjohnraj requested review of this revision.Dec 13 2022, 8:54 AM

Herald added a project: Restricted Project. · View Herald TranscriptDec 13 2022, 8:54 AM

Herald added subscribers: llvm-commits, pcwang-thead, eopXD, MaskRay. · View Herald Transcript

nitinjohnraj added a reviewer: craig.topper.Dec 13 2022, 8:57 AM

Herald added a subscriber: StephenFan. · View Herald TranscriptDec 13 2022, 8:57 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?

Harbormaster completed remote builds in B202871: Diff 482508.Dec 13 2022, 10:14 AM

Updated the test checks

Herald added subscribers: armkevincheng, sjarus, eric-k256. · View Herald TranscriptDec 13 2022, 12:19 PM

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?

In D139948#3992908, @jrtc27 wrote:

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

In D139948#3992911, @craig.topper wrote:

In D139948#3992908, @jrtc27 wrote:

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.

Harbormaster completed remote builds in B202926: Diff 482590.Dec 13 2022, 1:17 PM

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

Harbormaster completed remote builds in B202952: Diff 482630.Dec 13 2022, 2:39 PM

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

Harbormaster completed remote builds in B203227: Diff 483011.Dec 14 2022, 3:19 PM

craig.topper added reviewers: reames, jrtc27, asb, luismarques.Dec 14 2022, 3:23 PM

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

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?

Pulled from main

Added the VT_MASKC/N changes

LGTM

This revision is now accepted and ready to land.Dec 22 2022, 11:48 AM

Harbormaster completed remote builds in B204621: Diff 484895.Dec 22 2022, 12:04 PM

This revision was landed with ongoing or failed builds.Dec 22 2022, 2:20 PM

Closed by commit rGd64d3c5a8f81: [RISCV] Add pass to remove W suffix from ADDIW and SLLIW to improve… (authored by nitinjohnraj, committed by craig.topper). · Explain Why

This revision was automatically updated to reflect the committed changes.

craig.topper added a commit: rGd64d3c5a8f81: [RISCV] Add pass to remove W suffix from ADDIW and SLLIW to improve….

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

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 484963

llvm/lib/Target/RISCV/CMakeLists.txt

llvm/lib/Target/RISCV/RISCV.h

llvm/lib/Target/RISCV/RISCVInstrInfo.h

llvm/lib/Target/RISCV/RISCVInstrInfo.cpp

llvm/lib/Target/RISCV/RISCVSExtWRemoval.cpp

llvm/lib/Target/RISCV/RISCVStripWSuffix.cpp

llvm/lib/Target/RISCV/RISCVTargetMachine.cpp

llvm/test/CodeGen/RISCV/O3-pipeline.ll

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

llvm/test/CodeGen/RISCV/add-imm.ll

llvm/test/CodeGen/RISCV/atomic-cmpxchg-branch-on-result.ll

llvm/test/CodeGen/RISCV/atomic-cmpxchg.ll

llvm/test/CodeGen/RISCV/atomic-rmw.ll

llvm/test/CodeGen/RISCV/atomic-signext.ll

llvm/test/CodeGen/RISCV/bswap-bitreverse.ll

llvm/test/CodeGen/RISCV/calling-conv-lp64-lp64f-lp64d-common.ll

llvm/test/CodeGen/RISCV/ctlz-cttz-ctpop.ll

llvm/test/CodeGen/RISCV/div-by-constant.ll

llvm/test/CodeGen/RISCV/div-pow2.ll

llvm/test/CodeGen/RISCV/div.ll

llvm/test/CodeGen/RISCV/fpenv.ll

llvm/test/CodeGen/RISCV/machine-cse.ll

llvm/test/CodeGen/RISCV/mul.ll

llvm/test/CodeGen/RISCV/pr58511.ll

llvm/test/CodeGen/RISCV/rem.ll

llvm/test/CodeGen/RISCV/rv64i-demanded-bits.ll

llvm/test/CodeGen/RISCV/rv64i-exhaustive-w-insts.ll

llvm/test/CodeGen/RISCV/rv64i-w-insts-legalization.ll

llvm/test/CodeGen/RISCV/rv64zba.ll

llvm/test/CodeGen/RISCV/rv64zbb-zbkb.ll

llvm/test/CodeGen/RISCV/rv64zbb.ll

llvm/test/CodeGen/RISCV/rv64zbkb.ll

llvm/test/CodeGen/RISCV/rvv/fixed-vector-strided-load-store-asm.ll

llvm/test/CodeGen/RISCV/rvv/sink-splat-operands.ll

llvm/test/CodeGen/RISCV/sextw-removal.ll

llvm/test/CodeGen/RISCV/shadowcallstack.ll

llvm/test/CodeGen/RISCV/shift-masked-shamt.ll

llvm/test/CodeGen/RISCV/shlimm-addimm.ll

llvm/test/CodeGen/RISCV/srem-lkk.ll

llvm/test/CodeGen/RISCV/srem-seteq-illegal-types.ll

llvm/test/CodeGen/RISCV/srem-vector-lkk.ll

llvm/test/CodeGen/RISCV/unaligned-load-store.ll

llvm/test/CodeGen/RISCV/urem-seteq-illegal-types.ll

llvm/test/CodeGen/RISCV/urem-vector-lkk.ll

llvm/test/CodeGen/RISCV/vararg.ll

llvm/test/CodeGen/RISCV/vec3-setcc-crash.ll

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

Revision Contents
Changeset List