Skip to content

Conversation

@cfallin
Copy link
Member

@cfallin cfallin commented Apr 7, 2025

This is a followup from #214. We have use-cases in Cranelift now where we generate constraints on call instructions that have:

  • A number of fixed-reg uses at "early";
  • A number of fixed-reg defs at "late";
  • A number of register clobbers, potentially overlapping with uses;
  • A number of "any"-constrained defs at "late", to add register pressure.

The existing fuzzing function generator would create at most 3 operands, and would add either fixed-reg constraints or clobbers, not both.

This PR adds a "callsite-ish constraints" branch as a subcase of the fixed-reg choice that also adds the "any" defs to add register pressure, and adds new clobbers that don't interfere with existing constraints. It also loosens the logic around constructing fixed-reg constraints: if a conflict is found with some arbitrarily-chosen constraint, it keeps going to the next operand and tries to make it fixed, rather than giving up on further operands.

@cfallin cfallin requested a review from fitzgen April 7, 2025 21:20
@cfallin
Copy link
Member Author

cfallin commented Apr 7, 2025

I'm fuzzing locally on top of a branch without #214 to see if this will find the bug. That bug was in the middle of an extremely large function body, so I'm not sure how long the search might take, but in theory the constraints on generated functions should be capable of covering the case that we hit there.

@cfallin cfallin force-pushed the callsite-ish-fuzzing branch from 9fafff1 to 7eced4b Compare April 7, 2025 21:24
Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Looking forward to seeing if this can independently find that bug

This is a followup from bytecodealliance#214. We have use-cases in Cranelift now where
we generate constraints on call instructions that have:

- A number of fixed-reg uses at "early";
- A number of fixed-reg defs at "late";
- A number of register clobbers, potentially overlapping with uses;
- A number of "any"-constrained defs at "late", to add register
  pressure.

The existing fuzzing function generator would create at most 3 operands,
and would add *either* fixed-reg constraints *or* clobbers, not both.

This PR adds a "callsite-ish constraints" branch as a subcase of the
fixed-reg choice that also adds the "any" defs to add register pressure,
and adds new clobbers that don't interfere with existing constraints. It
also loosens the logic around constructing fixed-reg constraints: if a
conflict is found with some arbitrarily-chosen constraint, it keeps
going to the next operand and tries to make it fixed, rather than giving
up on further operands.
@cfallin cfallin force-pushed the callsite-ish-fuzzing branch from 7eced4b to 3e8c9f0 Compare April 7, 2025 21:42
@cfallin
Copy link
Member Author

cfallin commented Apr 7, 2025

This has at least fuzzed for a few CPU-hours without finding any other issues, so I'll go ahead and merge. I can verify (from playing with the constraint generation and making it too loose) that the new case is being explored; it seems just not to have gotten the right combo of register pressure and liveranges to get a bundle-merge at the right point yet.

(Sometime in the further future it would be neat to hook the bundle-merging and other heuristic bits into a chaos control plane of sorts, like we have in Cranelift, to more easily explore edge cases like this)

@cfallin cfallin merged commit b995300 into bytecodealliance:main Apr 7, 2025
6 checks passed
@cfallin cfallin deleted the callsite-ish-fuzzing branch April 7, 2025 21:45
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