Skip to content

[RISC-V] Lowering multiplication by power of 2 #116783

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from

Conversation

namu-lee
Copy link
Contributor

@namu-lee namu-lee commented Jun 18, 2025

Lowering multiplication operations by power of 2, for optimization.
Added codegen for GT_MUL, which emits slli for GT_MUL with overflow handling.

@SkyShield
@credo-quia-absurdum

part of #84834, cc @dotnet/samsung

namu-lee added 2 commits June 18, 2025 19:19
* Lower multiplication operations by power of 2

* Add codegen for GT_MUL

* Emit slli for GT_MUL with overflow handling
@Copilot Copilot AI review requested due to automatic review settings June 18, 2025 11:19
@github-actions github-actions bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jun 18, 2025
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jun 18, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a new lowering optimization for multiplication by power‐of‑2 values by replacing the multiplication with a shift left instruction and adding overflow handling.

  • In lowerriscv64.cpp, an early exit is added for power-of‑2 constant operands to mark them as contained.
  • In codegenriscv64.cpp, new code generates optimized instructions for multiplication by power of 2, including separate handling for overflow and unsigned cases.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/coreclr/jit/lowerriscv64.cpp Adds an early exit for contained power-of‑2 multiplication operands before the standard check.
src/coreclr/jit/codegenriscv64.cpp Implements code generation for power-of‑2 multiplications with overflow handling and optimizations.
Comments suppressed due to low confidence (2)

src/coreclr/jit/lowerriscv64.cpp:129

  • Consider adding a comment explaining the early return when op2 is a power-of-2 constant to clarify that further containment checks are not needed.
            return mul->gtNext;

src/coreclr/jit/codegenriscv64.cpp:1086

  • [nitpick] Consider adding a comment to explain why additional slli and srli instructions are emitted for unsigned multiplication to improve clarity for future maintainers.
        if (isUnsigned)

Copy link
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

@risc-vv
Copy link

risc-vv commented Jun 18, 2025

RISC-V Release-CLR-QEMU: 9083 / 9113 (99.67%)
=======================
      passed: 9083
      failed: 2
     skipped: 599
      killed: 28
------------------------
 TOTAL tests: 9712
VIRTUAL time: 35h 26min 48s 902ms
   REAL time: 36min 12s 190ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst

RISC-V Release-CLR-VF2: 9083 / 9113 (99.67%)
=======================
      passed: 9083
      failed: 2
     skipped: 599
      killed: 28
------------------------
 TOTAL tests: 9712
VIRTUAL time: 11h 2min 15s 865ms
   REAL time: 45min 12s 740ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst

RISC-V Release-FX-VF2: 306949 / 308668 (99.44%)
=======================
      passed: 306949
      failed: 1710
     skipped: 39
      killed: 9
------------------------
 TOTAL tests: 308707
VIRTUAL time: 19h 21min 13s 790ms
   REAL time: 2h 9min 3s 926ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst

RISC-V Release-FX-QEMU: 283871 / 284946 (99.62%)
=======================
      passed: 283871
      failed: 1070
     skipped: 39
      killed: 5
------------------------
 TOTAL tests: 284985
VIRTUAL time: 30h 2min 5s 208ms
   REAL time: 1h 11min 50s 240ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst

Build information and commands

GIT: 51d50830e04b64d926f769f2e0a5180f78277cab
CI: 3679e38be10e43c36a92b0f56774af3dfa4facf2
REPO: dotnet/runtime
BRANCH: main
CONFIG: Release
LIB_CONFIG: Release

@namu-lee
Copy link
Contributor Author

@dotnet-policy-service agree

@am11 am11 added arch-riscv Related to the RISC-V architecture labels Jun 18, 2025
@clamp03
Copy link
Member

clamp03 commented Jun 18, 2025

CC @dotnet/samsung

Copy link
Member

@tomeksowi tomeksowi Jun 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could use some tests stressing RISC-V corner-cases (if they aren't already covered by existing ones).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your review! I'll add tests for any uncovered RISC-V corner cases shortly.

@risc-vv
Copy link

risc-vv commented Jun 23, 2025

RISC-V Release-CLR-VF2: 9086 / 9116 (99.67%)
=======================
      passed: 9086
      failed: 2
     skipped: 600
      killed: 28
------------------------
 TOTAL tests: 9716
VIRTUAL time: 10h 31min 48s 129ms
   REAL time: 43min 2s 678ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst

RISC-V Release-CLR-QEMU: 9085 / 9115 (99.67%)
=======================
      passed: 9085
      failed: 2
     skipped: 600
      killed: 28
------------------------
 TOTAL tests: 9715
VIRTUAL time: 35h 12min 59s 393ms
   REAL time: 36min 2s 944ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst

RISC-V Release-FX-QEMU: 283423 / 284499 (99.62%)
=======================
      passed: 283423
      failed: 1071
     skipped: 39
      killed: 5
------------------------
 TOTAL tests: 284538
VIRTUAL time: 29h 44min 8s 665ms
   REAL time: 1h 11min 20s 462ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst

RISC-V Release-FX-VF2: 303870 / 305621 (99.43%)
=======================
      passed: 303870
      failed: 1743
     skipped: 39
      killed: 8
------------------------
 TOTAL tests: 305660
VIRTUAL time: 20h 58min 38s 759ms
   REAL time: 2h 17min 53s 383ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst

Build information and commands

GIT: ee4035287c713aa50c790bb42552386615575aac
CI: c45cff50d122cfdb93fa1c9cc1b21408b5a6ac50
REPO: dotnet/runtime
BRANCH: main
CONFIG: Release
LIB_CONFIG: Release

@@ -1014,6 +1014,111 @@ void CodeGen::genCodeForIncSaturate(GenTree* tree)
genProduceReg(tree);
}

// Generate code for multiplications by power of 2.
void CodeGen::genCodeForMul(GenTreeOp* treeNode)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed instead of it hitting the standard morph optimization which is meant to do the same?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops. I think this is good for practicing newcomer.
I missed checking the opt. Sorry, it is my fault.
I close it.

@risc-vv
Copy link

risc-vv commented Jun 23, 2025

RISC-V Release-CLR-VF2: 9085 / 9115 (99.67%)
=======================
      passed: 9085
      failed: 2
     skipped: 600
      killed: 28
------------------------
 TOTAL tests: 9715
VIRTUAL time: 10h 25min 27s 701ms
   REAL time: 42min 34s 701ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst

RISC-V Release-CLR-QEMU: 9085 / 9115 (99.67%)
=======================
      passed: 9085
      failed: 2
     skipped: 600
      killed: 28
------------------------
 TOTAL tests: 9715
VIRTUAL time: 35h 19min 26s 604ms
   REAL time: 36min 6s 96ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst

RISC-V Release-FX-QEMU: 268237 / 269331 (99.59%)
=======================
      passed: 268237
      failed: 1088
     skipped: 39
      killed: 6
------------------------
 TOTAL tests: 269370
VIRTUAL time: 30h 2min 34s 490ms
   REAL time: 1h 1min 4s 453ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst

RISC-V Release-FX-VF2: 494814 / 496559 (99.65%)
=======================
      passed: 494814
      failed: 1737
     skipped: 39
      killed: 8
------------------------
 TOTAL tests: 496598
VIRTUAL time: 21h 46min 51s 208ms
   REAL time: 2h 15min 48s 697ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst

Build information and commands

GIT: ee4035287c713aa50c790bb42552386615575aac
CI: c45cff50d122cfdb93fa1c9cc1b21408b5a6ac50
REPO: dotnet/runtime
BRANCH: main
CONFIG: Release
LIB_CONFIG: Release

@clamp03 clamp03 closed this Jun 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-riscv Related to the RISC-V architecture area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants