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 Miri error with -Zmiri-tag-raw-pointers #67

Merged
merged 2 commits into from
May 1, 2022

Conversation

jturner314
Copy link
Contributor

Before this PR, running MIRIFLAGS="-Zmiri-tag-raw-pointers" cargo miri test caused Miri to report undefined behavior in the test_dgemm test. This PR fixes the underlying issue – Miri doesn't like us using a reference to an element to access other elements.

There may be other Miri errors. Miri takes forever to run, and I haven't waited for it to finish yet.

Before this PR, running `MIRIFLAGS="-Zmiri-tag-raw-pointers" cargo
miri test` caused Miri to report undefined behavior in the
`test_dgemm` test. This PR fixes the underlying issue – Miri doesn't
like us using a reference to an element to access other elements.
@bluss
Copy link
Owner

bluss commented Dec 23, 2021

I need to run this through the benchmark. I guess it's fine of course. Thanks a lot.

@5225225
Copy link

5225225 commented Dec 23, 2021

Miri takes forever to run, and I haven't waited for it to finish yet.

You could adjust the FAST_TEST in sgemm.rs to be

const FAST_TEST: bool = option_env!("MMTEST_FAST_TEST").is_some() || cfg!(miri);

which makes a full run take a minute on my machine

@bluss
Copy link
Owner

bluss commented Dec 23, 2021

miri ci already sets MMTEST_FAST_TEST FWIW, that's why it's there.

@jturner314
Copy link
Contributor Author

Oh, that's much better. MMTEST_FAST_TEST=1 MIRIFLAGS="-Zmiri-tag-raw-pointers -Zmiri-check-number-validity" RUSTFLAGS="-Zrandomize-layout" cargo +nightly miri test runs in less than 20 seconds and has no errors (with this PR).

@bluss Would you like me to add MIRIFLAGS="-Zmiri-tag-raw-pointers -Zmiri-check-number-validity" RUSTFLAGS="-Zrandomize-layout" to ci/miri.sh?

@bluss
Copy link
Owner

bluss commented Dec 25, 2021

yes why not

@bluss
Copy link
Owner

bluss commented May 1, 2022

Thanks!

@bluss bluss merged commit c2cb362 into bluss:master May 1, 2022
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