Implement Montgomery multiplication assembly optimization for RV64GC by fengpengboa · Pull Request #28012 · openssl/openssl

fengpengboa · 2025-07-10T09:35:19Z

This PR introduces the RV64GC-optimized assembly implementation for Montgomery multiplication, and significantly accelerating BN modular exponentiation and RSA operations.

Performance Improvement

openssl speed rsa2048

benchmark results run on SG2042:

Test Item	Baseline (C)	ASM Optimized	Improvement
sign/s	101.6	209.0	105.71%
verify/s	4004.0	7270.1	81.57%
encr./s	3804.2	6682.1	75.65%
decr./s	100.7	205.3	103.87%

Test Verification

All relevant tests pass in the default build configuration:

make test

All tests successful.
Files=345, Tests=4491, 2186 wallclock secs (32.44 usr  3.42 sys + 1719.95 cusr 772.27 csys = 2528.08 CPU)
Result: PASS
make[2]: Leaving directory '/home/mnt/fpb/submit/openssl-master'
make[1]: Leaving directory '/home/mnt/fpb/submit/openssl-master'

Checklist

documentation is added or updated
tests are added or updated

fengpengboa · 2025-07-25T01:07:19Z

Hi @t8m @paulidale @kroeckx @nhorman ! The duplicate PR #[28067] was closed in favor of this one.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!

t8m · 2025-07-25T07:41:57Z

@fengpengboa Could you please get some independent review from somebody from the RISC-V community? We do not have RISC-V assembly experts among committers.

t8m · 2025-07-25T07:45:08Z

@ZenithalHourlyRate @HeliC829 @JerryShih previously reviewed RISC-V assembly code.

ZenithalHourlyRate

I have not reviewed all the logic, only covering some control logics here; the arithmetic part needs further reviewing; but I think some questions should be answered as they affect the arithmetic logic (i.e. the num >= 2 assumption)

The andi -16 is confusing for me. If it is a typo for addi, how would make test pass?

Also, please add comments before each asm block (like when you separate them by an empty line so it is more readable; the variable names are a bit confusing, like lo0,hi0,alo,ahi,lo1,hi1,aj,nj,i,j, need some time to distinguish what 0/1 means and what i/j means.

ZenithalHourlyRate · 2025-07-28T04:12:14Z

+
+$code .= <<___;
+
+    mv s0, sp


If you want to temporarily store sp in s0 while keeping s0's content (or $fp, since it is callee-saved), you should use my ($sp0) = use_regs(8); and mv $sp0, sp to make the style more consistent, namely in perlasm use all use_regs allocated reg and names instead of directly using reg names.

In the initial section, register mapping was already completed using my ($ra, $fp) = use_regs(1, 8). Here, I did not synchronize the variable names. I will modify this part.

ZenithalHourlyRate · 2025-07-28T04:19:13Z

+    addi    $ap, $ap, 16        
+    slli    $num, $num, 3       
+    ld      $n0, 0($n0)           
+    andi    $tp, $tp, -16       


Should not this be addi $tp, $tp, -16?

As previously mentioned, this instruction performs memory address alignment, rather than an addioperation.

ZenithalHourlyRate · 2025-07-28T04:22:20Z

+
+$code .= <<___;
+
+.Lmul_mont:		


If we do not branch back here, please do not add a label here; I read such label as if we were going to branch back here later.

I might consider adding a branch here subsequently. If it's unnecessary now, I can delete it.

ZenithalHourlyRate · 2025-07-28T04:54:14Z

+.Lmul_mont:		
+    ld      $m0, 0($bp)         
+    addi    $bp, $bp, 8         
+    slli    $temp, $num, 3         


This line can be merged with slli $num, $num, 3 below

I will modify this part

ZenithalHourlyRate · 2025-07-28T04:58:33Z

+    addi    $np, $np, 16        
+
+    mul     $lo0, $hi0, $m0         
+    addi    $j, $num, -16        


I understand that you want to process 2 ULONG at once, but are we guaranteed that num >= 2 (or 8 * num >= 16) ? what will happen when num = 1?

Before executing this assembly code, conditional checks are performed to ensure num >= 2.

#if defined(OPENSSL_BN_ASM_MONT) && defined(MONT_WORD) if (num > 1 && num <= BN_SOFT_LIMIT && a->top == num && b->top == num) { if (bn_wexpand(r, num) == NULL) return 0; if (bn_mul_mont(r->d, a->d, b->d, mont->N.d, mont->n0, num)) { r->neg = a->neg ^ b->neg; r->top = num; r->flags |= BN_FLG_FIXED_TOP; return 1; } } #endif

ZenithalHourlyRate · 2025-07-28T05:02:00Z

+    mulhu   $ahi, $aj, $m0       
+
+    mul     $m1, $lo0, $n0        
+    mv      sp, $tp               


if tp is (num + 2) * ULONG, I suggest allocaing tp at once instead of putting all these three lines apart

slli $temp, $num, 3 sub $tp, sp, $temp andi(maybe addi) $tp, $tp, -16 mv sp, $tp

into

slli $num, $num, 3 sub sp, sp, $num sub sp, sp, 16 mv $tp, sp

I will make the modifications as suggested.

ZenithalHourlyRate · 2025-07-28T05:17:14Z

+    addi    $tp, $tp, 8             
+    bnez    $j, .L1st           
+
+.L1st_skip:


I would name such section in a style like 1st_fix_tp_carry instead of an informationless skip; same for inner_skip below.

I will make the modifications as suggested.

ZenithalHourlyRate · 2025-07-28T05:33:07Z

+.Lcond_copy:
+    addi    $num,$num, -8         
+    bnez $carry1,.set_aj1 
+    mv   $nj,$tj
+    j    .end1
+.set_aj1:
+    mv   $nj,$aj         


These bnez/j are not constant-time. You can use snez to implement a constant time selection logic.

I will make the modifications as suggested

fengpengboa · 2025-07-29T02:04:40Z

@fengpengboa Could you please get some independent review from somebody from the RISC-V community? We do not have RISC-V assembly experts among committers.

Thank you for the recommendation. Should the reviewer’s availability be limited, we will proactively identify qualified RISC-V experts for an independent review.

fengpengboa · 2025-07-29T09:04:59Z

I have not reviewed all the logic, only covering some control logics here; the arithmetic part needs further reviewing; but I think some questions should be answered as they affect the arithmetic logic (i.e. the num >= 2 assumption)

The andi -16 is confusing for me. If it is a typo for addi, how would make test pass?

Also, please add comments before each asm block (like when you separate them by an empty line so it is more readable; the variable names are a bit confusing, like lo0,hi0,alo,ahi,lo1,hi1,aj,nj,i,j, need some time to distinguish what 0/1 means and what i/j means.

The instruction andi $tp, $tp, -16 is used for memory address alignment, rather than an addi operation.In addition, I will supplement the relevant code comments, including the meaning of the variable names.

fengpengboa · 2025-08-04T08:33:30Z

@t8m Hi, I've updated the code per review comments. Could you please approve the CI workflow? Thanks!

fengpengboa · 2025-08-05T01:56:45Z

@ZenithalHourlyRate Hello! I have now addressed all your previous review comments. The latest changes have been pushed. Would you please take a look when you have a moment? If you have any further suggestions, feel free to let me know. Thank you for your guidance!

ZenithalHourlyRate

Generally LGTM! Just some questions and nitpicking comments.

fengpengboa · 2025-08-14T06:52:49Z

Hi @t8m I've implemented all review suggestions in the latest code changes. Would you be kind enough to re-approve the CI workflow when you have a moment? Really appreciate your guidance on this!

fengpengboa · 2025-08-14T06:53:20Z

@ZenithalHourlyRate @paulidale I wanted to sincerely thank you all for taking the time to review the earlier version and provide your valuable insights. Following your guidance, I've now addressed all review comments in the latest code update. I'd be truly grateful if you could spare some time to examine the revised changes when your schedule permits.

fengpengboa · 2025-08-18T02:29:42Z

@t8m @ZenithalHourlyRate @paulidale Thank you for your thorough review and valuable suggestions! Could you please confirm if anything else needs adjustment? If not, would you be able to assist with the merge?Really appreciate your time and expertise!

ZenithalHourlyRate

From my side I think it is ok.

fengpengboa · 2025-08-20T01:33:46Z

From my side I think it is ok.

Thank you for your review! I really appreciate the confirmation from your side.

fengpengboa · 2025-08-25T01:42:29Z

@ZenithalHourlyRate Could you please add the formal approval checkmark when you have a moment?

fengpengboa · 2025-08-25T01:48:49Z

@t8m @nhorman Could you please help move this PR forward for merging? We have one approval and one verbal LGTM. Is there anything else required from our side?

fengpengboa · 2025-08-29T07:08:58Z

@HeliC829 Hi, I would be grateful if you could find the time to review this PR. Thank you!

HeliC829

LGTM.

fengpengboa · 2025-09-04T08:14:56Z

LGTM.

Thank you for the review!

fengpengboa · 2025-09-04T08:15:45Z

Hi @slontis , could you please review this PR when you have time? Thank you!

fengpengboa · 2025-09-05T02:47:59Z

Hi @InfoHunter , could you please review this PR when you have time? Thank you!

fengpengboa · 2025-09-15T01:41:53Z

@t8m @nhorman Hi ,This PR has been pending for some time. Could you please help to move it forward? A review and merge would be greatly appreciated.

openssl-machine · 2025-09-16T13:00:45Z

This pull request is ready to merge

paulidale · 2025-09-17T01:26:33Z

Merged after removing end of line whitespace. Thanks for the contribution.

Reviewed-by: Neil Horman <[email protected]> Reviewed-by: Paul Dale <[email protected]> (Merged from #28012)

…plement Montgomery multiplication assembly optimization for RV64GC" (openssl#28012). The key improvements include:code formatting unification、instruction scheduling optimization、register allocation strategy update、carry-chain subtraction implementation upgrade. These enhancements aim to improve both the code maintainability and its execution performance, particularly on out-of-order RISC-V cores.

… from the ARMv8 __bn_sqr8x_mont algorithm, complementing the earlier multiplication optimization (openssl#28012).

…plement Montgomery multiplication assembly optimization for RV64GC" (openssl#28012). The key improvements include:code formatting unification、instruction scheduling optimization、register allocation strategy update、updates to addition/subtraction carry and borrow flag handling. These enhancements aim to improve both the code maintainability and its execution performance, particularly on out-of-order RISC-V cores.

…ged "Implement Montgomery multiplication assembly optimization for RV64GC" (openssl#28012). The key improvements include:code formatting unification、instruction scheduling optimization、register allocation strategy update、updates to addition/subtraction carry and borrow flag handling. These enhancements aim to improve both the code maintainability and its execution performance, particularly on out-of-order RISC-V cores.

… from the ARMv8 __bn_sqr8x_mont algorithm, complementing the earlier multiplication optimization (openssl#28012).

…plement Montgomery multiplication assembly optimization for RV64GC" (#28012). The key improvements include:code formatting unification、instruction scheduling optimization、register allocation strategy update、updates to addition/subtraction carry and borrow flag handling. These enhancements aim to improve both the code maintainability and its execution performance, particularly on out-of-order RISC-V cores. Reviewed-by: Neil Horman <[email protected]> Reviewed-by: Paul Dale <[email protected]> (Merged from #29438)

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 % |

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 Jul 10, 2025

kroeckx mentioned this pull request Jul 24, 2025

Implement Montgomery multiplication assembly optimization for RV64GC #28067

Closed

2 tasks

ZenithalHourlyRate reviewed Jul 28, 2025

View reviewed changes

fengpengboa force-pushed the rv64-mont-asm branch from 584934f to 13312fe Compare August 4, 2025 08:18

ZenithalHourlyRate reviewed Aug 8, 2025

View reviewed changes

paulidale reviewed Aug 11, 2025

View reviewed changes

Comment thread crypto/bn/asm/riscv64-mont.pl Resolved

Implement Montgomery multiplication assembly optimization for RV64GC

5fd89cd

fengpengboa force-pushed the rv64-mont-asm branch from 13312fe to 5fd89cd Compare August 14, 2025 06:34

ZenithalHourlyRate reviewed Aug 18, 2025

View reviewed changes

paulidale approved these changes Aug 22, 2025

View reviewed changes

HeliC829 reviewed Aug 29, 2025

View reviewed changes

nhorman approved these changes Sep 15, 2025

View reviewed changes

nhorman 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 Sep 15, 2025

openssl-machine added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Sep 16, 2025

paulidale closed this Sep 17, 2025

openssl-machine pushed a commit that referenced this pull request Sep 17, 2025

Implement Montgomery multiplication assembly optimization for RV64GC

f60c677

Reviewed-by: Neil Horman <[email protected]> Reviewed-by: Paul Dale <[email protected]> (Merged from #28012)

ZenithalHourlyRate mentioned this pull request Oct 7, 2025

[bn] [riscv] Implement Montgomery multiplication assembly optimization for RV64GC #27926

Closed

fengpengboa mentioned this pull request Dec 18, 2025

Optimize code format, instruction order and flag usage in RV64 Montgomery multiplication (RISC-V) #29438

Closed

fengpengboa mentioned this pull request Dec 18, 2025

Add optimized Montgomery squaring for RV64GC (RISC-V) #29440

Closed

esyr mentioned this pull request Apr 14, 2026

[4.0] CHANGES.md: updates for 4.0.0 final release #30817

Closed

JamieMagee mentioned this pull request Apr 29, 2026

deps: enable OpenSSL asm support for riscv64 nodejs/node#62606

Merged


		$code .= <<___;

		.Lmul_mont:

Conversation

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

Uh oh!

Performance Improvement

Test Verification

Checklist

Uh oh!

fengpengboa commented Jul 25, 2025

Uh oh!

t8m commented Jul 25, 2025

Uh oh!

t8m commented Jul 25, 2025

Uh oh!

ZenithalHourlyRate left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fengpengboa commented Jul 29, 2025

Uh oh!

fengpengboa commented Jul 29, 2025

Uh oh!

fengpengboa commented Aug 4, 2025

Uh oh!

fengpengboa commented Aug 5, 2025

Uh oh!

ZenithalHourlyRate left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fengpengboa commented Aug 14, 2025

Uh oh!

fengpengboa commented Aug 14, 2025

Uh oh!

fengpengboa commented Aug 18, 2025

Uh oh!

ZenithalHourlyRate left a comment

Choose a reason for hiding this comment

Uh oh!

fengpengboa commented Aug 20, 2025

Uh oh!

fengpengboa commented Aug 25, 2025

fengpengboa commented Jul 10, 2025 •

edited

Loading