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

Don't redefine and misuse ETHASH_MIX_BYTES #35

Merged
merged 1 commit into from
Jun 10, 2019

Conversation

solardiz
Copy link
Contributor

@solardiz solardiz commented Apr 4, 2019

The ProgPoW source tree changed the value of ETHASH_MIX_BYTES from 128 to 256 and made use of it for ProgPoW specifics while at the same time assuming that Ethash DAG is computed ignoring this macro and using the old value of 128. That was weird. This PR tries to fix that and it also paves the way for experiments with adjusting the value of PROGPOW_DAG_LOADS.

I tested that this compiles (both CUDA and OpenCL) and reports the same speeds as before. I didn't test that it actually computes the same hash values as before - is there a debugging mode included to test that?

@solardiz solardiz mentioned this pull request Apr 4, 2019
@lookfirst
Copy link

lookfirst commented Apr 5, 2019

Andrea added the benchmark mode to his ethminer fork, but it is no longer available. The last version of it is here: https://github.com/miscellaneousbits/ethminer

@solardiz
Copy link
Contributor Author

solardiz commented Apr 5, 2019

@lookfirst Thanks. There's a benchmark mode right in this ProgPoW tree, but apparently no debug mode that would print test vectors, etc.

@solardiz
Copy link
Contributor Author

solardiz commented Jun 8, 2019

I've just tested this PR's changes on top of those in #44 and #45 using the debugging output patch I posted in #25 (comment) and using block numbers 30000 and 10000000, but without any other changes (thus, computing ProgPoW 0.9.2). I got the expected correct digests (same as prior to this PR, and same as c-progpow's) for both blocks on both CUDA and OpenCL (and the latter using both AMD and NVIDIA cards).

I think this PR is ready to merge, along with my other 3 PRs that are currently open. I'm aware of good reasons to merge them into this tree, and unaware of any reasons not to.

@ifdefelse ifdefelse merged commit 60e94aa into ifdefelse:master Jun 10, 2019
@solardiz solardiz mentioned this pull request Mar 10, 2020
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.

3 participants