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

fix: change compile option maxvx512f to mavx2 for goldilocks #376

Merged
merged 1 commit into from
Apr 5, 2024

Conversation

chokobole
Copy link
Contributor

Description

This is to resolve a runtime error of Goldilocks when building with g++-12 and optimization mode.

Plus, if we want to build with avx512, we need to add defines = ["__AVX512__"], too.

See
https://github.com/0xPolygonHermez/zkevm-prover/blob/93c36f120c3201bfaf12b8f6f2796f53e72193ff/Makefile#L31-L37 for more details.

This is to resolve a runtime error of Goldilocks when building with g++-12 and
optimization mode.

Plus, if we want to build with avx512, we need to add
`defines = ["__AVX512__"]`, too.

See
https://github.com/0xPolygonHermez/zkevm-prover/blob/93c36f120c3201bfaf12b8f6f2796f53e72193ff/Makefile#L31-L37
for more details.

Related: #370
@chokobole chokobole force-pushed the fix/change-compiler-option-to-mavx2-for-goldilocks branch from df1ed87 to 6ec463b Compare April 5, 2024 07:00
@ashjeong
Copy link
Contributor

ashjeong commented Apr 5, 2024

Fixes the following tests that fail:
//tachyon/crypto/commitments/fri:fri_unittests,
//tachyon/math/finite_fields/goldilocks:goldilocks_unittests,
//tachyon/math/finite_fields:finite_fields_unittests

Copy link
Contributor

@ashjeong ashjeong left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@Insun35 Insun35 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@dongchangYoo dongchangYoo left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@TomTaehoonKim TomTaehoonKim left a comment

Choose a reason for hiding this comment

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

LGTM

@chokobole chokobole merged commit 8fb4935 into main Apr 5, 2024
3 checks passed
@chokobole chokobole deleted the fix/change-compiler-option-to-mavx2-for-goldilocks branch April 5, 2024 15:03
chokobole added a commit that referenced this pull request Apr 20, 2024
Insun35 pushed a commit that referenced this pull request Apr 23, 2024
Insun35 pushed a commit that referenced this pull request May 7, 2024
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.

None yet

5 participants