This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Teach RISCVMatInt to prefer li+slli over lui+addi(w) for compressibility.
ClosedPublic

Authored by craig.topper on Dec 1 2022, 12:08 PM.

Details

Summary

With C extension, li with a 6 bit immediate followed by slli is 4 bytes.
The lui+addi(w) sequence is at least 6 bytes.

The two sequences probably have similar execution latency. The exception
being if the target supports lui+addi(w) macrofusion.

Since the execution latency is probably the same I didn't restrict
this to C extension.

Diff Detail

Event Timeline

craig.topper created this revision.Dec 1 2022, 12:08 PM
craig.topper requested review of this revision.Dec 1 2022, 12:08 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 1 2022, 12:08 PM
jrtc27 added inline comments.Dec 1 2022, 12:33 PM
llvm/lib/Target/RISCV/MCTargetDesc/RISCVMatInt.cpp
192

This is confusing, LI is a pseudo implemented using this function but here you mean it to be what C.LI expands to. Though it doesn’t help there’s the LI alias that’s deemed canonical (which I don’t like but came from binutils).

reames added inline comments.Dec 1 2022, 12:51 PM
llvm/lib/Target/RISCV/MCTargetDesc/RISCVMatInt.cpp
197

I think we could phrase this comparison in terms of getInstSeqCost, and abstract away the details of the compression cost here. It would use a different default for when compression isn't enabled, but not sure we care about that point?

Address @jrtc27's comment

I didn't use the getInstSeqCost because I think it would make the code harder to reason about.

reames accepted this revision.Dec 6 2022, 10:25 AM

LGTM

JFYI, Craig and I talked about this one a bit offline. My prior proposal (to use the cost metric) doesn't simplify this code - I tried locally - and Craig has landed some cleanups to the surrounding code which makes it easier to read anyways.

This revision is now accepted and ready to land.Dec 6 2022, 10:25 AM
This revision was landed with ongoing or failed builds.Dec 6 2022, 10:39 AM
This revision was automatically updated to reflect the committed changes.