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

Make dedup guard optional #1215

Merged

Conversation

hkratz
Copy link
Contributor

@hkratz hkratz commented Sep 9, 2021

The dedup guard creates additional instructions in the test shims which cause various problems with testing intrinsics such as inadvertedly checking for dedup guard instructions, not useful instructions counts and inadvertedly crossing the inlining threshold.

This PR disables the dedup guard if the environment variable STDARCH_DISABLE_DEDUP_GUARD is set. It only works properly if the LLVM MergeFunctions pass is disabled, e.g. with -Z merge-functions=disabledin RUSTFLAGS.

It also disables the dedup guard for the ci/run.sh

TODO:

  • Fix arm expected instructions
  • Fix Windows x86/x86_64 missing functions
  • Fix aarch64 expected instructions
  • Fix x86 expected instructions
  • Fix x86_64 expected instructions
  • Fix s390x-unknown-linux-gnu warning
  • Try again with DEDUP

@rust-highfive
Copy link

r? @Amanieu

(rust-highfive has picked a reviewer for you, use r? to override)

@hkratz hkratz force-pushed the disable_merge_functions_and_dedup_guard branch 4 times, most recently from dbeaa5c to 301c8e0 Compare September 14, 2021 14:29
@hkratz hkratz changed the title Test: make dedup guard optional Make dedup guard optional Sep 14, 2021
@hkratz hkratz marked this pull request as ready for review September 14, 2021 15:00
This change also unconditionally enables `-D warnings`.
- vreinterpret* do not emit any instructions on arm.
- vst1* intrinsics actual emit vst1.* instructions.
- arm vext_s64/vext_u64 don't cause any instructions to be emitted.
@hkratz hkratz force-pushed the disable_merge_functions_and_dedup_guard branch from 301c8e0 to 19a920f Compare September 14, 2021 15:03
@Amanieu Amanieu merged commit 5fdbc47 into rust-lang:master Sep 20, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 27, 2021
Update stdarch submodule

This is mainly to fix the critical issue of aarch64 store intrinsics overwriting additional memory, see rust-lang/stdarch#1220

Changes:
* aarch64/armv7: additional vld1/vst1 intrinsics + perf fixes for existing ones
  * rust-lang/stdarch#1205
  * rust-lang/stdarch#1207
  * rust-lang/stdarch#1216
* armv7: Make FMA work with vfpv4 and optimize
  * rust-lang/stdarch#1219
* Non-visible changes to the testing framework
  * rust-lang/stdarch#1208
  * rust-lang/stdarch#1211
  * rust-lang/stdarch#1213
  * rust-lang/stdarch#1215
  * rust-lang/stdarch#1218
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