Skip to content

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.

@ghost
Copy link

ghost 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.

2 participants