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

CN: Consistency cleanup #2793

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

Spudz76
Copy link
Contributor

@Spudz76 Spudz76 commented Dec 5, 2021

  • Remove props.isHeavy() logic within the _gr_ functions as they are no longer called unless already CN_GR_[0-5]
  • Add IS_CN_HEAVY_XHV to match existing IS_CN_HEAVY_TUBE and use it everywhere the long-form test was used
  • Rename IS_HEAVY to IS_CN_HEAVY to better match the IS_CN_HEAVY_(TUBE|XHV)
  • Use IS_CN_HEAVY the same everywhere props.isHeavy() was called separately
  • Ensure IS_CN_HEAVY is also defined everywhere CN_STEP4 macro was used, as its props.isHeavy() was changed
  • Flip ordering of cn_vaes_enabled tests for slightly improved readability, and allows to not validate the other bools unless VAES is available (now in roughly likelihood-to-be-false and importance order)

Tested compile, and valid accepts for cn-heavy/xhv and ghostrider but I do not have any VAES capable rigs, so it still needs a diligence proof test on something that does have it. Also don't have any pools to test the cn-heavy/0 or cn-heavy/tube but I think the cn-heavy/xhv test proves those by association.

Should be a tiny bit nicer since all these are validated once into constexpr-bool's the same way everywhere.

src/crypto/cn/CryptoNight_x86.h Outdated Show resolved Hide resolved
src/crypto/cn/CryptoNight_x86.h Outdated Show resolved Hide resolved
src/crypto/cn/CryptoNight_x86.h Outdated Show resolved Hide resolved
src/crypto/cn/CryptoNight_x86.h Outdated Show resolved Hide resolved
src/crypto/cn/CryptoNight_x86.h Outdated Show resolved Hide resolved
src/crypto/cn/CryptoNight_x86.h Outdated Show resolved Hide resolved
src/crypto/cn/CryptoNight_x86.h Outdated Show resolved Hide resolved
src/crypto/cn/CryptoNight_x86.h Outdated Show resolved Hide resolved
@SChernykh
Copy link
Contributor

Flip ordering of cn_vaes_enabled tests for slightly improved readability, and allows to not validate the other bools unless VAES is available (now in roughly likelihood-to-be-false and importance order)

Do you know how compile-time checks are optimized by the compiler? They are essentially zero-cost.

@SChernykh
Copy link
Contributor

Same goes for all pros.isHeavy() replacements - these are already compile time and your change doesn't change anything in the generated x64 code.

@Spudz76
Copy link
Contributor Author

Spudz76 commented Dec 5, 2021

OK, following those rules, I've gone the other direction then.

Consistency is when everything is done the same way, whichever way that is. The latest commit pushes everything back the other direction instead (use props.isWhatever() for everything, drop all unneeded local constexprs shadowing the props).

@Spudz76 Spudz76 force-pushed the dev-removeExtraCnHeavyChecks branch from f6ef5af to d414525 Compare March 7, 2023 21:42
@Spudz76 Spudz76 force-pushed the dev-removeExtraCnHeavyChecks branch from d414525 to 6252422 Compare April 1, 2023 21:40
@Spudz76 Spudz76 force-pushed the dev-removeExtraCnHeavyChecks branch 2 times, most recently from 0d46d7f to 735e6e9 Compare May 23, 2023 22:49
@Spudz76 Spudz76 force-pushed the dev-removeExtraCnHeavyChecks branch from 735e6e9 to 862280f Compare July 12, 2023 08:31
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

2 participants