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

Switch to Montgomery representation #663

Open
wants to merge 62 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
62 commits
Select commit Hold shift + click to select a range
0c7df67
montgomery mult with correctness
HadarIngonyama Sep 20, 2024
5f29239
fix reduction condition
HadarIngonyama Sep 30, 2024
79cd3de
test performance
HadarIngonyama Oct 1, 2024
779babe
tmp
yshekel Nov 3, 2024
6a8793b
Merge branch 'hadar/montgomery_mult' into yuval/performance_mult
HadarIngonyama Nov 4, 2024
df45d69
merge with montgomery cpu
HadarIngonyama Nov 5, 2024
0f3275e
added mont const computation
HadarIngonyama Nov 12, 2024
9cdb4bd
field tests pass
HadarIngonyama Nov 13, 2024
21d97d4
ifdef barret
HadarIngonyama Nov 13, 2024
4dbbd72
montgomery SOS reduction added
Koren-Brand Nov 14, 2024
b2533cd
EC addition and MSM on CPU modified to work with montgomery represent…
Koren-Brand Nov 18, 2024
c979161
split device math from field
HadarIngonyama Nov 18, 2024
87ae5ff
Merge remote-tracking branch 'origin/koren/mont_mult_in_msm' into had…
HadarIngonyama Nov 18, 2024
2c8e2ad
field tests pass after merge
HadarIngonyama Nov 18, 2024
c53e3db
support 32 bit mont (babybear stiil fails)
HadarIngonyama Nov 19, 2024
4282114
SOS mont reduction now implemented for 32bits as well
Koren-Brand Nov 19, 2024
7f5345b
all c++ tests pass
HadarIngonyama Nov 20, 2024
a738a41
fix extention
HadarIngonyama Nov 20, 2024
7ae18a5
all tests pass
HadarIngonyama Nov 20, 2024
c923e6e
formatting
HadarIngonyama Nov 21, 2024
5745d8c
Merge remote-tracking branch 'origin/main' into hadar/switch-to-mont
HadarIngonyama Nov 21, 2024
258f0ef
formatting
HadarIngonyama Nov 21, 2024
01c5143
small fix
HadarIngonyama Nov 21, 2024
9d4394c
small fix to tests
HadarIngonyama Nov 21, 2024
1172cec
bug fix
HadarIngonyama Nov 21, 2024
95e92ff
CR fixes
HadarIngonyama Nov 24, 2024
620a4ae
Merge remote-tracking branch 'origin/main' into hadar/switch-to-mont
yshekel Nov 24, 2024
54abe47
temp commit for testing
mickeyasa Nov 25, 2024
30f93a3
fix const_mul
mickeyasa Nov 25, 2024
1f1c7c0
Merge remote-tracking branch 'refs/remotes/origin/hadar/switch-to-mon…
mickeyasa Nov 25, 2024
1d7e819
constexpre in const mul
mickeyasa Nov 25, 2024
208dfb6
const mul optimization
mickeyasa Nov 25, 2024
bc84c83
update arkworks example
yshekel Nov 25, 2024
61c65a5
small fix
mickeyasa Nov 26, 2024
09cca99
Merge remote-tracking branch 'refs/remotes/origin/hadar/switch-to-mon…
mickeyasa Nov 26, 2024
9cd3506
bug fix - b param, still need to fix g2
mickeyasa Nov 26, 2024
2b6889e
formattinggggggg
mickeyasa Nov 26, 2024
c4ff934
uroll loops improves EC-add by ~2-3X
yshekel Nov 26, 2024
eefa062
Merge remote-tracking branch 'origin/main' into hadar/switch-to-mont
yshekel Nov 26, 2024
507ee38
fix b multiplier for g2
mickeyasa Nov 26, 2024
3dc6154
Merge remote-tracking branch 'refs/remotes/origin/hadar/switch-to-mon…
mickeyasa Nov 26, 2024
58a2cda
bugfix 377 g2
mickeyasa Dec 2, 2024
2c7c379
g2 377 bugfix
HadarIngonyama Dec 2, 2024
2727e07
Merge remote-tracking branch 'origin/main' into hadar/switch-to-mont
HadarIngonyama Dec 2, 2024
997145f
fmatting
HadarIngonyama Dec 2, 2024
d978869
fix 254 g2
HadarIngonyama Dec 2, 2024
a345010
Uniting signature of gpu and cpu montgomery reduce
Koren-Brand Dec 2, 2024
ceb1f3e
fmat
HadarIngonyama Dec 2, 2024
a005854
adding sqr, not yet working
HadarIngonyama Dec 3, 2024
b2aaff6
add inv array
HadarIngonyama Dec 4, 2024
68f4326
fmt
HadarIngonyama Dec 4, 2024
ed7aef4
change inv log func
HadarIngonyama Dec 4, 2024
2d3075b
fmt
HadarIngonyama Dec 4, 2024
820ee9e
fix stupid bug
mickeyasa Dec 5, 2024
a880d5d
fmt
mickeyasa Dec 5, 2024
613b744
wok around
HadarIngonyama Dec 10, 2024
686d3cd
Merge remote-tracking branch 'origin/main' into hadar/switch-to-mont
HadarIngonyama Dec 10, 2024
a2a8372
ffffffff
HadarIngonyama Dec 10, 2024
963349d
small fix
HadarIngonyama Dec 10, 2024
740c3f4
i hate the formatter
HadarIngonyama Dec 10, 2024
f757986
update backend
HadarIngonyama Dec 10, 2024
1a4e6c1
revert ntt
HadarIngonyama Dec 11, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions icicle/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,11 @@ if (HASH)
setup_hash_target()
endif()

option(BARRET "Enable memory address sanitizer" OFF)
Copy link
Contributor

Choose a reason for hiding this comment

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

what the address sanitizer got to do with the BARRET?

if(BARRET)
add_compile_definitions(BARRET)
endif()

if (CPU_BACKEND)
add_subdirectory(backend/cpu)
endif()
Expand Down
21 changes: 18 additions & 3 deletions icicle/backend/cpu/src/curve/cpu_msm.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -451,9 +451,13 @@ void Msm<A, P>::phase1_bucket_accumulator(const scalar_t* scalars, const A* base
bool negate_p_and_s = scalar.get_scalar_digit(scalar_t::NBITS - 1, 1) > 0;
if (negate_p_and_s) { scalar = scalar_t::neg(scalar); }
for (int j = 0; j < m_precompute_factor; j++) {
// Handle required preprocess of base P
// Handle required preprocess of base P according to the version of Field/Ec adder (accepting Barret / Montgomery)
A base =
#ifdef BARRET
m_are_points_mont ? A::from_montgomery(bases[m_precompute_factor * i + j]) : bases[m_precompute_factor * i + j];
#else
m_are_points_mont ? bases[m_precompute_factor * i + j] : A::to_montgomery(bases[m_precompute_factor * i + j]);
#endif
if (base == A::zero()) { continue; }
if (negate_p_and_s) { base = A::neg(base); }

Expand Down Expand Up @@ -780,12 +784,23 @@ eIcicleError cpu_msm_precompute_bases(
const unsigned int shift = c * ((num_bms_no_precomp - 1) / precompute_factor + 1);
for (int i = 0; i < nof_bases; i++) {
output_bases[precompute_factor * i] = input_bases[i];
P point = P::from_affine(is_mont ? A::from_montgomery(input_bases[i]) : input_bases[i]);
// Handle required preprocess of base P according to the version of Field/Ec adder (accepting Barret / Montgomery)
P point =
#ifdef BARRET
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe the degredation in performance comes from the Z=1 when moving from affine to projectve?
1 is a complex number and that might affect the multiplication by Z at the EC adder

Copy link
Contributor

Choose a reason for hiding this comment

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

Degradation in the precompute?

P::from_affine(is_mont ? A::from_montgomery(input_bases[i]) : input_bases[i]);
#else
P::from_affine(is_mont ? input_bases[i] : A::to_montgomery(input_bases[i]));
#endif
for (int j = 1; j < precompute_factor; j++) {
for (int k = 0; k < shift; k++) {
point = P::dbl(point);
}
output_bases[precompute_factor * i + j] = is_mont ? A::to_montgomery(P::to_affine(point)) : P::to_affine(point);
output_bases[precompute_factor * i + j] =
#ifdef BARRET
is_mont ? A::to_montgomery(P::to_affine(point)) : P::to_affine(point);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
is_mont ? A::to_montgomery(P::to_affine(point)) : P::to_affine(point);
is_mont ? A::from_montgomery(P::to_affine(point)) : P::to_affine(point);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#else
is_mont ? P::to_affine(point) : A::from_montgomery(P::to_affine(point));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
is_mont ? P::to_affine(point) : A::from_montgomery(P::to_affine(point));
is_mont ? P::to_affine(point) : A::to_montgomery(P::to_affine(point));

#endif
}
}
return eIcicleError::SUCCESS;
Expand Down
11 changes: 11 additions & 0 deletions icicle/include/icicle/curves/projective.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,14 @@ class Projective
return {FF::from_montgomery(point.x), FF::from_montgomery(point.y), FF::from_montgomery(point.z)};
}

#ifdef BARRET
static HOST_DEVICE_INLINE Projective generator() { return {Gen::gen_x, Gen::gen_y, FF::one()}; }
#else
static HOST_DEVICE_INLINE Projective generator()
{
return {FF::to_montgomery(Gen::gen_x), FF::to_montgomery(Gen::gen_y), FF::one()};
Copy link
Contributor

Choose a reason for hiding this comment

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

don't we get gen_x, gen_y from the Field as constants?
Can't you switch from
gen_x, gen_y
to
gen_barret_x, gen_barret_y, gen_montgomery_x, gen_montgomery_y

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we can, it just means we need to change all the parameters in all the fields manually. This function is only called for testing so I thought this solution is good enough, but we can do it differently.

}
#endif

static HOST_DEVICE_INLINE Projective neg(const Projective& point) { return {point.x, FF::neg(point.y), point.z}; }

Expand Down Expand Up @@ -179,6 +186,10 @@ class Projective

friend HOST_DEVICE Projective operator*(SCALAR_FF scalar, const Projective& point)
{
#ifndef BARRET
scalar = SCALAR_FF::from_montgomery(scalar);
#endif
Comment on lines +199 to +201
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we want to convert from mont here if BARRET isn't defined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the double and add algorithm requires barret representation, same for msm


// Precompute points: P, 2P, ..., (2^window_size - 1)P
constexpr unsigned window_size =
4; // 4 seems fastest. Optimum is minimizing EC add and depends on the field size. for 256b it's 4.
Expand Down
9 changes: 3 additions & 6 deletions icicle/include/icicle/fields/complex_extension.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,12 @@ class ComplexExtensionField

static constexpr HOST_DEVICE_INLINE ComplexExtensionField to_montgomery(const ComplexExtensionField& xs)
{
return ComplexExtensionField{xs.real * FF{CONFIG::montgomery_r}, xs.imaginary * FF{CONFIG::montgomery_r}};
return ComplexExtensionField{FF::to_montgomery(xs.real), FF::to_montgomery(xs.imaginary)};
}

static constexpr HOST_DEVICE_INLINE ComplexExtensionField from_montgomery(const ComplexExtensionField& xs)
{
return ComplexExtensionField{xs.real * FF{CONFIG::montgomery_r_inv}, xs.imaginary * FF{CONFIG::montgomery_r_inv}};
return ComplexExtensionField{FF::from_montgomery(xs.real), FF::from_montgomery(xs.imaginary)};
}

static HOST_INLINE ComplexExtensionField rand_host()
Expand Down Expand Up @@ -128,17 +128,14 @@ class ComplexExtensionField
return ExtensionWide{FF::mul_wide(xs.real, ys), FF::mul_wide(xs.imaginary, ys)};
Copy link
Contributor

Choose a reason for hiding this comment

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

you can remove the template also here

}

template <unsigned MODULUS_MULTIPLE = 1>
static constexpr HOST_DEVICE_INLINE ExtensionWide mul_wide(const FF& xs, const ComplexExtensionField& ys)
{
return mul_wide(ys, xs);
}

template <unsigned MODULUS_MULTIPLE = 1>
static constexpr HOST_DEVICE_INLINE ComplexExtensionField reduce(const ExtensionWide& xs)
{
return ComplexExtensionField{
FF::template reduce<MODULUS_MULTIPLE>(xs.real), FF::template reduce<MODULUS_MULTIPLE>(xs.imaginary)};
return ComplexExtensionField{FF::reduce(xs.real), FF::reduce(xs.imaginary)};
}

template <class T1, class T2>
Expand Down
Loading
Loading