Add optimized Montgomery squaring for RV64GC (RISC-V) by fengpengboa · Pull Request #29440 · openssl/openssl

fengpengboa · 2025-12-18T11:34:23Z

This PR adds an RV64GC-optimized Montgomery squaring assembly, ported from the ARMv8 __bn_sqr8x_mont algorithm, complementing the earlier multiplication optimization (#28012).

Performance Improvement

openssl speed  -seconds 1 rsa2048

Test Item	Baseline (C)	ASM Optimized	Improvement
sign/s	208	220	5.77%
verify/s	7190	7956.6	10.66%
encr./s	6638	7156.0	7.8%
decr./s	203	216	6.4 %

fengpengboa · 2025-12-18T11:41:52Z

Hi @t8m @paulidale @nhorman @ZenithalHourlyRate @HeliC829 @JerryShih !! My submitted PR is currently in Open status. When convenient, could the maintainer team please help move the review forward? I'm happy to address any needed changes or provide clarifications.Thank you for your time and consideration!

ZenithalHourlyRate

Please add RISC-V in the title so the more RISC-V CI can cover them. The detailed criteria is

openssl/.github/workflows/riscv-more-cross-compiles.yml

Lines 22 to 29 in da8f098

    
           cross-compilation-riscv: 
        
             # pull request title contains 'riscv' 
        
             # pull request title contains 'RISC-V' 
        
             # pull request body contains '[riscv ci]' 
        
             # push event commit message contains '[riscv ci]' 
        
             # cron job 
        
             # manual dispatch 
        
             if: contains(github.event.pull_request.title, 'riscv') || contains(github.event.pull_request.title, 'RISC-V') || contains(github.event.pull_request.body, '[riscv ci]') || contains(github.event.head_commit.message, '[riscv ci]') || (github.event_name == 'schedule' && github.repository == 'openssl/openssl') || github.event_name == 'workflow_dispatch'

It will take me some time to review 1k5 lines of assembly.

fengpengboa · 2025-12-22T01:57:50Z

Please add RISC-V in the title so the more RISC-V CI can cover them. The detailed criteria is

openssl/.github/workflows/riscv-more-cross-compiles.yml

Lines 22 to 29 in da8f098

cross-compilation-riscv:

# pull request title contains 'riscv'

# pull request title contains 'RISC-V'

# pull request body contains '[riscv ci]'

# push event commit message contains '[riscv ci]'

# cron job

# manual dispatch

if: contains(github.event.pull_request.title, 'riscv') || contains(github.event.pull_request.title, 'RISC-V') || contains(github.event.pull_request.body, '[riscv ci]') || contains(github.event.head_commit.message, '[riscv ci]') || (github.event_name == 'schedule' && github.repository == 'openssl/openssl') || github.event_name == 'workflow_dispatch'

It will take me some time to review 1k5 lines of assembly.

I've updated the PR title.
Thank you for taking the time to review. I'm happy to provide any clarification or additional information whenever you're ready.

ZenithalHourlyRate

I've roughly gone through it and it looks convincing. Though definitely needs careful checking...

I am busy in the next one or two months and may only get some Sunday night for you to do the review...sorry for the long process.

ZenithalHourlyRate · 2025-12-28T12:47:21Z

 .balign 32
 .globl bn_mul_mont
 .type   bn_mul_mont,\@function
 bn_mul_mont:


# Take the bn_sqr8x_mont branch when num is a multiply of 8 and num <= 32

The branch to Lscalar_impl here might be unnecessary. The armv8-mont code contains the branch because it has another neon branch but we do not have them.

# Take the bn_sqr8x_mont branch when num is a multiply of 8 and num <= 32

The branch to Lscalar_impl here might be unnecessary. The armv8-mont code contains the branch because it has another neon branch but we do not have them.

Thank you for pointing this out. You are right — since our current implementation does not include the vector branch, this conditional branch is indeed unnecessary. I will remove it.

fengpengboa · 2025-12-29T01:32:54Z

I've roughly gone through it and it looks convincing. Though definitely needs careful checking...

I am busy in the next one or two months and may only get some Sunday night for you to do the review...sorry for the long process.

Thank you for taking the time to review this. I really appreciate your initial feedback.

fengpengboa · 2026-02-04T06:44:36Z

Hi @t8m @paulidale @nhorman @ZenithalHourlyRate @HeliC829 @JerryShih !! My submitted PR is currently in Open status. When convenient, could the maintainer team please help move the review forward? I'm happy to address any needed changes or provide clarifications.Thank you for your time and consideration!

fengpengboa · 2026-02-26T08:23:28Z

Hi @t8m @paulidale @nhorman @ZenithalHourlyRate @HeliC829 @JerryShih, this PR has been open since Dec 18, 2025When you have a moment, could someone please take a look? I'm happy to make any changes or provide clarifications. Thanks!

ZenithalHourlyRate · 2026-02-26T15:11:30Z

Sorry for the late response. I've been busy with work in recent months and is expected to be busy throughout the Spring. Reviewing it line-by-line is out of my bandwidth now and the current state might be the best I can do.

My opinion is that given it is a line-by-line transcription of existing code, and the tests also pass, I am OK with the code. (The previous review comments have been addressed in the new commit.)

nhorman

I don't know much about riscv assembly, but the code passes on my riscv vm, so I think thats about the best I can do here.

HeliC829 · 2026-02-26T16:53:59Z

+    sd      x8,88(sp)
+    sd      x9,80(sp)
+    sd      x18,72(sp)
+    sd      x19,64(sp)
+    sd      x20,56(sp)
+    sd      x21,48(sp)
+    sd      x22,40(sp)
+    sd      x23,32(sp)
+    sd      x24,24(sp)
+    sd      x25,16(sp)
+    sd      x27,8(sp)
+    sd      x26,0(sp)


Can you use ABI name here instead of register?

Can you use ABI name here instead of register?

Sure, I will make the modification accordingly.

HeliC829 · 2026-02-26T16:55:02Z

+    ld      x8,88(sp)
+    ld      x9,80(sp)
+    ld      x18,72(sp)
+    ld      x19,64(sp)
+    ld      x20,56(sp)
+    ld      x21,48(sp)
+    ld      x22,40(sp)
+    ld      x23,32(sp)
+    ld      x24,24(sp)
+    ld      x25,16(sp)
+    ld      x27,8(sp)
+    ld      x26,0(sp)


Same as above.

Same as above.

I will make the modification accordingly.

HeliC829 · 2026-02-26T17:01:39Z

+# Return address and Frame pointer
+#      RISC-V    ABI
+# $ra   x1       ra
+# $fp   x8       s0
+my ($ra,$fp) = use_regs(1,8);
+
+# Temporary variable allocation
+#      RISC-V    ABI
+# $a0   x5     t0
+# $a1   x6     t1
+# $a2   x7     t2
+# $a3   x28    t3
+# $a4   x9     s1
+# $a5   x18    s2
+# $a6   x19    s3
+# $a7   x20    s4
+my ($a0,$a1,$a2,$a3,$a4,$a5,$a6,$a7) = use_regs(5,6,7,28,9,18,19,20);


ABI name has been define in riscv.pm line 51, so we don't need to redefine here again. Assembly can use ABI name directly without '$'

ABI name has been define in riscv.pm line 51, so we don't need to redefine here again. Assembly can use ABI name directly without '$'

Here, I am merely mapping these variables to the corresponding register numbers, allowing subsequent code to refer to registers by variable names. This enhances code readability and maintainability and does not constitute a redefinition of the ABI. I have tried removing the '$', and the code fails to compile at this point.

fengpengboa · 2026-02-27T02:46:15Z

Sorry for the late response. I've been busy with work in recent months and is expected to be busy throughout the Spring. Reviewing it line-by-line is out of my bandwidth now and the current state might be the best I can do.

My opinion is that given it is a line-by-line transcription of existing code, and the tests also pass, I am OK with the code. (The previous review comments have been addressed in the new commit.)

Thank you for your response and recognition! I completely understand that you've been busy with work recently, and I truly appreciate you taking the time to review this.

fengpengboa · 2026-02-27T02:49:11Z

I don't know much about riscv assembly, but the code passes on my riscv vm, so I think thats about the best I can do here.

Thank you for your testing and confirmation.

This PR adds an RV64GC-optimized Montgomery squaring assembly, ported from the ARMv8 __bn_sqr8x_mont algorithm, complementing the earlier multiplication optimization (openssl#28012). Performance Improvement | Test Item | Baseline (C) | ASM Optimized | Improvement | | ---------- | ------------- | -------------- | ------------ | | sign/s | 208 | 220 | 5.77% | | verify/s | 7190 | 7956.6 | 10.66% | | encr./s | 6638 | 7156.0 | 7.8% | | decr./s | 203 | 216 | 6.4 % |

fengpengboa · 2026-02-27T03:31:13Z

@HeliC829 , I have made the modifications as per your suggestion and pushed the updated code. Could you please review it again? Thanks!

HeliC829

LGTM

fengpengboa · 2026-02-28T02:02:39Z

@paulidale Could you please review this pull request? Thank you!

openssl-machine · 2026-03-03T06:00:55Z

This pull request is ready to merge

t8m · 2026-03-13T17:22:11Z

Merged to the master branch with trimmed whitespace at EOL. Thank you for your contribution.

This PR adds an RV64GC-optimized Montgomery squaring assembly, ported from the ARMv8 __bn_sqr8x_mont algorithm, complementing the earlier multiplication optimization (#28012). Performance Improvement | Test Item | Baseline (C) | ASM Optimized | Improvement | | ---------- | ------------- | -------------- | ------------ | | sign/s | 208 | 220 | 5.77% | | verify/s | 7190 | 7956.6 | 10.66% | | encr./s | 6638 | 7156.0 | 7.8% | | decr./s | 203 | 216 | 6.4 % | Reviewed-by: Neil Horman <[email protected]> Reviewed-by: Paul Dale <[email protected]> MergeDate: Fri Mar 13 17:21:14 2026 (Merged from #29440)

This PR adds an RV64GC-optimized Montgomery squaring assembly, ported from the ARMv8 __bn_sqr8x_mont algorithm, complementing the earlier multiplication optimization (openssl#28012). Performance Improvement | Test Item | Baseline (C) | ASM Optimized | Improvement | | ---------- | ------------- | -------------- | ------------ | | sign/s | 208 | 220 | 5.77% | | verify/s | 7190 | 7956.6 | 10.66% | | encr./s | 6638 | 7156.0 | 7.8% | | decr./s | 203 | 216 | 6.4 % | Reviewed-by: Neil Horman <[email protected]> Reviewed-by: Paul Dale <[email protected]> MergeDate: Fri Mar 13 17:21:14 2026 (Merged from openssl#29440)

github-actions Bot added the severity: fips change The pull request changes FIPS provider sources label Dec 18, 2025

ZenithalHourlyRate reviewed Dec 21, 2025

View reviewed changes

fengpengboa changed the title ~~Add optimized Montgomery squaring for RV64GC~~ Add optimized Montgomery squaring for RV64GC (RISC-V) Dec 22, 2025

ZenithalHourlyRate reviewed Dec 28, 2025

View reviewed changes

fengpengboa force-pushed the rv64-mont-sqr-asm branch from aec4cfe to 7d72f7d Compare December 30, 2025 02:33

loci-dev mentioned this pull request Dec 30, 2025

UPSTREAM PR #29440: Add optimized Montgomery squaring for RV64GC (RISC-V) auroralabs-loci/openssl#290

Open

nhorman previously approved these changes Feb 26, 2026

View reviewed changes

HeliC829 reviewed Feb 26, 2026

View reviewed changes

loci-dev mentioned this pull request Feb 27, 2026

UPSTREAM PR #29440: Add optimized Montgomery squaring for RV64GC (RISC-V) auroralabs-loci/openssl#551

Open

fengpengboa dismissed nhorman’s stale review via cbd20d6 February 27, 2026 03:23

fengpengboa force-pushed the rv64-mont-sqr-asm branch from 7d72f7d to cbd20d6 Compare February 27, 2026 03:23

nhorman approved these changes Feb 27, 2026

View reviewed changes

HeliC829 reviewed Feb 27, 2026

View reviewed changes

paulidale added branch: master Applies to master branch approval: review pending This pull request needs review by a committer labels Mar 2, 2026

paulidale approved these changes Mar 2, 2026

View reviewed changes

paulidale added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Mar 2, 2026

openssl-machine removed the approval: done This pull request has the required number of approvals label Mar 3, 2026

openssl-machine added the approval: ready to merge The 24 hour grace period has passed, ready to merge label Mar 3, 2026

t8m added this to the 4.1.0 milestone Mar 3, 2026

t8m added the hold: merge after release This pull request must not be merged until after the release from the base / merge branch is done label Mar 3, 2026

t8m closed this Mar 13, 2026

esyr mentioned this pull request Sep 9, 2026

[master] CHANGES.md, NEWS.md: update for 4.1.0-alpha1 #32758

Closed

	cross-compilation-riscv:
	# pull request title contains 'riscv'
	# pull request title contains 'RISC-V'
	# pull request body contains '[riscv ci]'
	# push event commit message contains '[riscv ci]'
	# cron job
	# manual dispatch
	if: contains(github.event.pull_request.title, 'riscv') \|\| contains(github.event.pull_request.title, 'RISC-V') \|\| contains(github.event.pull_request.body, '[riscv ci]') \|\| contains(github.event.head_commit.message, '[riscv ci]') \|\| (github.event_name == 'schedule' && github.repository == 'openssl/openssl') \|\| github.event_name == 'workflow_dispatch'

Conversation

fengpengboa commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fengpengboa commented Dec 18, 2025

Uh oh!

ZenithalHourlyRate left a comment

Choose a reason for hiding this comment

Uh oh!

fengpengboa commented Dec 22, 2025

Uh oh!

ZenithalHourlyRate left a comment

Choose a reason for hiding this comment

Uh oh!

ZenithalHourlyRate Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

fengpengboa Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

fengpengboa commented Dec 29, 2025

Uh oh!

fengpengboa commented Feb 4, 2026

Uh oh!

fengpengboa commented Feb 26, 2026

Uh oh!

ZenithalHourlyRate commented Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nhorman left a comment

Choose a reason for hiding this comment

Uh oh!

HeliC829 Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

fengpengboa Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

HeliC829 Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

fengpengboa Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

HeliC829 Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fengpengboa Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

fengpengboa commented Feb 27, 2026

Uh oh!

fengpengboa commented Feb 27, 2026

Uh oh!

fengpengboa commented Feb 27, 2026

Uh oh!

HeliC829 left a comment

Choose a reason for hiding this comment

Uh oh!

fengpengboa commented Feb 28, 2026

Uh oh!

openssl-machine commented Mar 3, 2026

Uh oh!

t8m commented Mar 13, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

fengpengboa commented Dec 18, 2025 •

edited

Loading

ZenithalHourlyRate commented Feb 26, 2026 •

edited

Loading

HeliC829 Feb 26, 2026 •

edited

Loading