Skip to content

ConstraintAnalysis: Fix mixed signed/unsigned comparisons - #9045

Merged
kripken merged 33 commits into
WebAssembly:mainfrom
kripken:c.sun
Aug 26, 2026
Merged

ConstraintAnalysis: Fix mixed signed/unsigned comparisons#9045
kripken merged 33 commits into
WebAssembly:mainfrom
kripken:c.sun

Conversation

@kripken

@kripken kripken commented Aug 26, 2026

Copy link
Copy Markdown
Member

The IU64 idea was nice in theory - a single representation that includes both
signed and unsigned values - and it makes both of those comparisons work
in a simple and intuitive way, but it fails on mixed comparisons. When you have
x > C signed, and x < D unsigned, IU64 represents e.g. the number -1
twice, once signed and once unsigned. But what matters is the bit-pattern, in
the end, so we really need to represent it once.

To handle that, bite the bullet and implement Spans, a set of two Span
instances. An unsigned range is then a single Span, and a signed one,
fitted into the unsigned space, sometimes needs two. For example, x <= 100
signed turns into [0..100] and also [0x80000000, 0xffffffff], that is, small
positive numbers and also all negative numbers (and negative numbers, in
the unsigned space, are all those with the top bit set).

  • Remove IU64 and add Spans.
  • Use Spans where IU64 was used.
  • Update tests. That part is unfortunately tedious, sorry about that - I'd
    understand if you skim it (I did go over it carefully + fuzzing + AI).

@kripken
kripken requested a review from tlively August 26, 2026 15:41
@kripken
kripken requested a review from a team as a code owner August 26, 2026 15:41

@tlively tlively left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM % very optional nit

Comment thread src/support/span.h Outdated
}

// Initialize with pairs of coordinates.
Spans(std::initializer_list<T> init) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Using nested initializer lists might be more readable, e.g. {{a, b}, {x, y}}. Then this could probably be std::initializer_list<Span>.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good idea, and actually it turns out this "just works", c++ infers {a, b} must be a Span already. So we can just delete this constructor and rewrite the code. Done.

@kripken
kripken enabled auto-merge (squash) August 26, 2026 21:56
@kripken
kripken merged commit 65e9a83 into WebAssembly:main Aug 26, 2026
16 checks passed
@kripken
kripken deleted the c.sun branch August 26, 2026 23:44
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