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(ci): enforce basic CPU instruction set to prevent execution problems #1759

Merged
merged 1 commit into from
May 25, 2023

Conversation

vpavlin
Copy link
Member

@vpavlin vpavlin commented May 25, 2023

Description

It seems Github Action runners started using new machines with new fancy CPUs which include AVX512 instructions. These are not widely available in non-server / older HW (it seems), which result in unusable binaries built in GH Actions.

We can pass explicit architecture / CPU instruction set to build for to C compiler through Nim using --passC:"-march=..."

The list of supported architectures: https://gcc.gnu.org/onlinedocs/gcc/x86-Options.html

I am choosing basic instruction set x86-64 for this PR. This may potentially influence speed of some crypto algorithms (which would otherwise leverage AVX instructions), so we should consider doing some benchmarking in the future to asses the impact and consider some newer architecture(s), but to have current builds working and as general as possible, x86-64 seems to be a reasonable choice.

A variable ARCHITECTURE is added for cases where operators would prefer to rebuild with newer architecture or native option (which leverages all available instructions on the build machine) to benefit from AVX and other "fancy" instructions.

Changes

  • added -march option to Makefile to make binaries use limited set of CPU instructions

Copy link
Contributor

@jm-clius jm-clius left a comment

Choose a reason for hiding this comment

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

Thanks!

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