This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Optimize multiplication with constant
ClosedPublic

Authored by benshi001 on Dec 21 2020, 12:38 AM.

Diff Detail

Event Timeline

benshi001 created this revision.Dec 21 2020, 12:38 AM
benshi001 requested review of this revision.Dec 21 2020, 12:38 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 21 2020, 12:38 AM
benshi001 updated this revision to Diff 313050.Dec 21 2020, 2:13 AM

i64 mul on riscv32 is omitted, since 6-10 extra instruction are generated, not sure if it is a win against calling __muldi3.

craig.topper added inline comments.Dec 21 2020, 9:50 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
3924

What about 128-bit on riscv64?

3929

Can we just use countTrailingZeros here?

benshi001 updated this revision to Diff 313230.Dec 21 2020, 7:21 PM
benshi001 updated this revision to Diff 313231.Dec 21 2020, 7:23 PM
benshi001 marked 2 inline comments as done.Dec 21 2020, 7:31 PM

This is the best form I can tune now, the best trade among int128, int64 and int32. It generated optimized code for most of them,

except some cases are left not covered,

  1. rv32 with M extension: not sure if is a real win since some HW multiplier has quite low lantency, even to 2 cycles.
  2. i128 mul on rv32: the generated code is too large, not sure if it is better than calling __multi3
  3. -(1<<m)-(1<<n): more NEGS are generated than other (1<<m)±(1<<n), so it may not be optimal than quick HW multipliers.

Shall we commit current form first? Then I will go on with the hard cases in the future.

benshi001 updated this revision to Diff 313235.Dec 21 2020, 7:53 PM
benshi001 updated this revision to Diff 313236.Dec 21 2020, 7:57 PM
craig.topper added inline comments.Dec 21 2020, 8:32 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
3924

Is the first part of this just !Imm.isSignedIntN(12)? And 0 != (Imm & High) is !Imm.isIntN(11)?

benshi001 updated this revision to Diff 313240.Dec 21 2020, 9:18 PM
benshi001 marked an inline comment as done.
craig.topper added inline comments.Jan 4 2021, 7:11 PM
llvm/test/CodeGen/RISCV/mul.ll
964

Please pre-commit these test cases and rebase this patch.

benshi001 marked an inline comment as done.Jan 4 2021, 10:55 PM
benshi001 added inline comments.
llvm/test/CodeGen/RISCV/mul.ll
964

The test cases are commited to

https://reviews.llvm.org/D94062

jrtc27 added inline comments.Jan 4 2021, 10:57 PM
llvm/test/CodeGen/RISCV/mul.ll
964

Please rebase and mark that as a parent of this revision.

benshi001 updated this revision to Diff 315045.Jan 6 2021, 9:04 PM
benshi001 marked an inline comment as done.
benshi001 marked an inline comment as done.
benshi001 updated this revision to Diff 315046.Jan 6 2021, 9:09 PM
benshi001 updated this revision to Diff 315047.Jan 6 2021, 9:21 PM
jrtc27 added inline comments.Jan 6 2021, 10:00 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
3907–3914

Please refrain from using unicode

benshi001 updated this revision to Diff 315051.Jan 6 2021, 10:57 PM
benshi001 marked an inline comment as done.
craig.topper accepted this revision.Jan 8 2021, 3:09 PM

LGTM. Please write a description of the changes in the the commit message when you commit this.

This revision is now accepted and ready to land.Jan 8 2021, 3:09 PM
This revision was automatically updated to reflect the committed changes.