Skip to content

Automatic detection of CPU capabilities#136

Merged
tevador merged 5 commits intomasterfrom
pr-flags
Oct 10, 2019
Merged

Automatic detection of CPU capabilities#136
tevador merged 5 commits intomasterfrom
pr-flags

Conversation

@tevador
Copy link
Owner

@tevador tevador commented Oct 9, 2019

Can autodetect AES, SSSE3 and AVX2 and sets the JIT flag if supported.

Usage:

randomx_flags flags = randomx_get_flags();
randomx_cache *myCache = randomx_alloc_cache(flags);
randomx_init_cache(myCache, &myKey, sizeof myKey);
randomx_vm *myMachine = randomx_create_vm(flags, myCache, NULL);

AES detection on ARM64 needs testing.

@tevador
Copy link
Owner Author

tevador commented Oct 9, 2019

BTW also includes --auto flag in the benchmark, so:

./randomx-benchmark --mine --jit --largePages --init 8 --avx2

can be simplified to

./randomx-benchmark --mine --auto --largePages

hyc
hyc approved these changes Oct 10, 2019
View reviewed changes
Copy link
Collaborator

@hyc hyc left a comment

Choose a reason for hiding this comment

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

clang is quiet now.

@tevador
Copy link
Owner Author

tevador commented Oct 10, 2019

@SChernykh please review

SChernykh
SChernykh reviewed Oct 10, 2019
View reviewed changes
src/randomx.cpp
randomx_cache *cache = nullptr;
auto impl = randomx::selectArgonImpl(flags);
if (impl == nullptr) {
return cache;
Copy link
Collaborator

@SChernykh SChernykh Oct 10, 2019

Choose a reason for hiding this comment

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

It's probably a bad idea to return uninitialized cache here even if impl can't be nullptr with current code. It should output some error and abort, or throw an exception.

Copy link
Owner Author

@tevador tevador Oct 10, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

  1. impl can be NULL if the caller selects an unsupported implementation (e.g. AVX2 on ARM).
  2. It's not returning an uninitialized cache but simply a NULL pointer. It's up to the caller to check for NULLs.
  3. This is a C-language API, so we can't throw exceptions unless we handle them internally.

Copy link
Collaborator

@SChernykh SChernykh Oct 10, 2019

Choose a reason for hiding this comment

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

Then it's ok, I somehow thought cache was already allocated at this point.

src/randomx.cpp
randomx_flags randomx_get_flags() {
randomx_flags flags = RANDOMX_HAVE_COMPILER ? RANDOMX_FLAG_JIT : RANDOMX_FLAG_DEFAULT;
randomx::Cpu cpu;
if (HAVE_AES && cpu.hasAes()) {
Copy link
Collaborator

@SChernykh SChernykh Oct 10, 2019

Choose a reason for hiding this comment

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

It's better to wrap it into #if HAVE_AES == 1 ... #endif, no need to make it compile-time when preprocessor can handle it.

Copy link
Owner Author

@tevador tevador Oct 10, 2019

Choose a reason for hiding this comment

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

If HAVE_AES == 0, it will be optimized out during compilation. I find the code more readable this way.

@tevador tevador merged commit 4296c35 into master Oct 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hyc hyc hyc approved these changes

@SChernykh SChernykh SChernykh approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tevador @hyc @SChernykh