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

fiat-constify: Fix implementation for fiat-crypto >= 0.0.21 #992

Merged
merged 5 commits into from
Dec 20, 2023
Merged

fiat-constify: Fix implementation for fiat-crypto >= 0.0.21 #992

merged 5 commits into from
Dec 20, 2023

Conversation

MasterAwesome
Copy link
Contributor

fiat-crypto introduced new types instead of just type aliases, this causes the generated code to no longer compile. This is a breaking change since this will no longer compile code generated by fiat-crypto version < 0.0.21

Tests:

  • All fiat-crypto rust crate can be compiled using rustc

Codegen

Multiple output args

Before
#[inline]
pub fn fiat_p224_divstep(out1: &mut u64, out2: &mut [u64; 5], out3: &mut [u64; 5], out4: &mut [u64; 4], out5: &mut [u64; 4], arg1: u64, arg2: &[u64; 5], arg3: &[u64; 5], arg4: &[u64; 4], arg5: &[u64; 4]) {
  // <snipped>
  *out1 = x112;
  out2[0] = x7;
  out2[1] = x8;
  out2[2] = x9;
  out2[3] = x10;
  out2[4] = x11;
  out3[0] = x114;
  out3[1] = x115;
  out3[2] = x116;
  out3[3] = x117;
  out3[4] = x118;
  out4[0] = x119;
  out4[1] = x120;
  out4[2] = x121;
  out4[3] = x122;
  out5[0] = x123;
  out5[1] = x124;
  out5[2] = x125;
  out5[3] = x126;
}
After
#[inline]
pub const fn fiat_p224_divstep(
    arg1: u64,
    arg2: &[u64; 5],
    arg3: &[u64; 5],
    arg4: &[u64; 4],
    arg5: &[u64; 4],
) -> (u64, [u64; 5], [u64; 5], [u64; 4], [u64; 4]) {
    // <snipped> 
    (
        x112,
        [x7, x8, x9, x10, x11],
        [x114, x115, x116, x117, x118],
        [x119, x120, x121, x122],
        [x123, x124, x125, x126],
    )
}

Newtyped const output/input

Before
pub fn fiat_p521_relax(out1: &mut fiat_p521_loose_field_element, arg1: &fiat_p521_tight_field_element) {
  let x1: u64 = (arg1[0]);
  let x2: u64 = (arg1[1]);
  let x3: u64 = (arg1[2]);
  let x4: u64 = (arg1[3]);
  let x5: u64 = (arg1[4]);
  let x6: u64 = (arg1[5]);
  let x7: u64 = (arg1[6]);
  let x8: u64 = (arg1[7]);
  let x9: u64 = (arg1[8]);
  out1[0] = x1;
  out1[1] = x2;
  out1[2] = x3;
  out1[3] = x4;
  out1[4] = x5;
  out1[5] = x6;
  out1[6] = x7;
  out1[7] = x8;
  out1[8] = x9;
}
After
pub const fn fiat_p521_relax(
    arg1: &fiat_p521_tight_field_element,
) -> fiat_p521_loose_field_element {
    let arg1 = &arg1.0;
    let x1: u64 = (arg1[0]);
    let x2: u64 = (arg1[1]);
    let x3: u64 = (arg1[2]);
    let x4: u64 = (arg1[3]);
    let x5: u64 = (arg1[4]);
    let x6: u64 = (arg1[5]);
    let x7: u64 = (arg1[6]);
    let x8: u64 = (arg1[7]);
    let x9: u64 = (arg1[8]);
    (fiat_p521_loose_field_element([x1, x2, x3, x4, x5, x6, x7, x8, x9]))
}

Cloned from #978
CC @tarcieri

fiat-crypto introduced new types instead of just type aliases, this
causes the generated code to no longer compile. This is a breaking
change since this will no longer compile code generated by fiat-crypto
version < 0.0.21

Tests:
 * All `fiat-crypto` rust crate can be compiled using rustc

Signed-off-by: Arvind Mukund <armu30@gmail.com>
Signed-off-by: Arvind Mukund <armu30@gmail.com>
Signed-off-by: Arvind Mukund <armu30@gmail.com>
Signed-off-by: Arvind Mukund <armu30@gmail.com>
Signed-off-by: Arvind Mukund <armu30@gmail.com>
Copy link
Member

@tarcieri tarcieri left a comment

Choose a reason for hiding this comment

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

With RustCrypto/elliptic-curves#1003 passing I'd say this is good to merge if you're finished

@MasterAwesome
Copy link
Contributor Author

Yep I'm done with this change. Are there existing benchmarks for p521 I can check to ensure no regressions?

@tarcieri
Copy link
Member

Hmm, doesn't look like it, but you can probably just copy the ones from e.g. p384 over

@tarcieri tarcieri merged commit cc77203 into RustCrypto:master Dec 20, 2023
3 checks passed
tarcieri pushed a commit to RustCrypto/elliptic-curves that referenced this pull request Jan 9, 2024
Implementation for the fiat-constify update @ RustCrypto/utils#992
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants