Skip to content
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

Windows: Secp256k1 tests assembly test frozen #448

Open
mratsim opened this issue Jul 31, 2024 · 0 comments
Open

Windows: Secp256k1 tests assembly test frozen #448

mratsim opened this issue Jul 31, 2024 · 0 comments
Labels
bug 🪲 Something isn't working correctness 🛂

Comments

@mratsim
Copy link
Owner

mratsim commented Jul 31, 2024

Since #444 the tests on Windows are frozen with Secp256k1 assembly:

image

This is blocking #445 as it changes the assembly.

The big difference for secp256k1 is that due to using the full 256-bit the final substraction needs to handle an extra underflow condition here:

proc finalSubMayOverflowImpl*(
ctx: var Assembler_x86,
r: Operand or OperandArray,
a, M, scratch: OperandArray,
a_in_scratch = false,
scratchReg: Operand or Register or OperandReuse = rax) =
## Reduce `a` into `r` modulo `M`
## To be used when the final substraction can
## also overflow the limbs (a 2^256 order of magnitude modulus stored in n words of total max size 2^256)
##
## r, a, scratch are mutated
## M is read-only
## This clobbers RAX
let N = M.len
ctx.comment "Final substraction (may carry)"
# Mask: scratchReg contains 0xFFFF or 0x0000
ctx.sbb scratchReg, scratchReg
# Now substract the modulus, and test a < p with the last borrow
if not a_in_scratch:
ctx.mov scratch[0], a[0]
ctx.sub scratch[0], M[0]
for i in 1 ..< N:
if not a_in_scratch:
ctx.mov scratch[i], a[i]
ctx.sbb scratch[i], M[i]
# If it overflows here, it means that it was
# smaller than the modulus and we don't need `scratch`
ctx.sbb scratchReg, 0
# If we borrowed it means that we were smaller than
# the modulus and we don't need "scratch"
for i in 0 ..< N:
ctx.cmovnc a[i], scratch[i]
ctx.mov r[i], a[i]

@mratsim mratsim added bug 🪲 Something isn't working correctness 🛂 labels Jul 31, 2024
mratsim added a commit that referenced this issue Jul 31, 2024
mratsim added a commit that referenced this issue Aug 1, 2024
…th assembly (#449)

* workaround #448: deactivated secp256k1 tests due to bug on Windows with assembly

* workaround #448: missed one
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🪲 Something isn't working correctness 🛂
Projects
None yet
Development

No branches or pull requests

1 participant