Skip to content

assert_instr: add support for stricter tests - #2212

Open
RalfJung wants to merge 4 commits into
rust-lang:mainfrom
RalfJung:assert_instr_not
Open

assert_instr: add support for stricter tests#2212
RalfJung wants to merge 4 commits into
rust-lang:mainfrom
RalfJung:assert_instr_not

Conversation

@RalfJung

Copy link
Copy Markdown
Member

Motivated by #2208, this extends assert_instr with support for stricter tests:

  • The first commit adds clauses of the form not(instr); the macro will then ensure that the given instruction does not appear in the disassembly.
  • The second commit adds clauses of the form limit(n); the macro will ensure that overall there are no more than n instructions in the disassembly.

I verified that the problematic pack tests indeed fail before the revert of #2033. I am not sure if the not part is even worth it, which is why the 2nd commit removes its uses -- one has to have very specific instructions in mind to exclude them, a general limit on how many instructions total there are seems better? So unless you think I should keep it I plan to remove the first commit from this again.

r? @folkertdev

@RalfJung RalfJung changed the title assert_instr: add support for explicit exclusion of bad instructions assert_instr: add support for stricter tests Aug 19, 2026
@RalfJung

Copy link
Copy Markdown
Member Author

I am also confused by ci/run.sh... it seems to never even run the AVX(2) tests with assert_instr turned on...?

@RalfJung
RalfJung force-pushed the assert_instr_not branch 2 times, most recently from 53f3cf0 to 7e05b7e Compare August 19, 2026 21:17
@rustbot

This comment has been minimized.

@RalfJung
RalfJung force-pushed the assert_instr_not branch 2 times, most recently from e55b6c8 to 373a0de Compare August 19, 2026 21:41
@RalfJung

Copy link
Copy Markdown
Member Author

Ah... it's 5 instructions somewhere. That's annoying.

disassembly for stdarch_test_shim__mm256_packs_epi16_vpacksswb: 
	 0: pushq %rbp
	 1: movq %rsp, %rbp
	 2: vpacksswb %ymm1, %ymm0, %ymm0
	 3: popq %rbp
	 4: retq

Maybe it should ignore some of these as they have nothing to do with the actual intrinsic?

@RalfJung
RalfJung force-pushed the assert_instr_not branch 2 times, most recently from 993c8d9 to 38a6da0 Compare August 20, 2026 06:16
@sayantn

sayantn commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

ci/run.sh runs the instruction assertions with only SSE2 enabled, because otherwise sometimes LLVM generates different assembly (e.g. if AVX is enabled, some SSE intrinsics will generate AVX instructions instead)

@RalfJung

Copy link
Copy Markdown
Member Author

Yeah at some point I realized that this is about which instructions are enabled statically, but it'll still actually generate code for all intrinsics and even run assert_instr for all of them, even the ones my CPU could not execute.

@rustbot

rustbot commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

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.

4 participants