Skip to content

Conversation

tlively
Copy link
Member

@tlively tlively commented Jan 31, 2025

In particular, clarify that the type of the expected value operand to
cmpxchg can be a supertype of the field type. This gives optimizers like
Binaryen more leeway to refine field types because they are constrained
by only the type of the replacement operand and not by the type of the
expected value operand.

In particular, clarify that the type of the expected value operand to
cmpxchg can be a supertype of the field type. This gives optimizers like
Binaryen more leeway to refine field types because they are constrained
by only the type of the replacement operand and not by the type of the
expected value operand.
C |- struct.atomic.rmw.add x y : [(ref null x) t] -> [t]
- C.types[x] = struct fields
- fields[i] = mut t
Copy link
Collaborator

Choose a reason for hiding this comment

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

where is i coming from here?

C |- struct.atomic.rmw.cmpxchg x y : [(ref null x) t1 t2] -> [t2]
- C.types[x] = struct fields
- fields[i] = mut t2
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, should be y. Will fix.

@conrad-watt
Copy link
Collaborator

In particular, clarify that the type of the expected value operand to
cmpxchg can be a supertype of the field type. This gives optimizers like
Binaryen more leeway to refine field types because they are constrained
by only the type of the replacement operand and not by the type of the
expected value operand.

Just to double check, this comes for free from subsumption and isn't a semantic change, right?

@tlively
Copy link
Member Author

tlively commented Jan 31, 2025

Just to double check, this comes for free from subsumption and isn't a semantic change, right?

This comes for free from subsumption given the described typing rule, but not from an alternative simpler rule where the "expected" operand also has to be a subtype of the field type, just like the "replacement" operand. With that rule, an optimizer could only refine the field type to be the LUB of the expected and replacement operand types.

Copy link
Member

@rossberg rossberg left a comment

Choose a reason for hiding this comment

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

LGTM modulo Conrad's comments.

@tlively tlively enabled auto-merge (squash) February 1, 2025 00:31
tlively added a commit to WebAssembly/binaryen that referenced this pull request Feb 1, 2025
Both passes use StructUtils::StructScanner to analyze struct operations.
Add support for RMW operations to this utility and update its users to
provide the new `noteRMW` hook. Test that GTO and TypeRefining
optimizations work as expected in the presence of RMW operations, but
leave a proper implementation in ConstantFieldPropagation to a later PR.

To allow TypeRefining to refine field types based only on the
"replacement" operand and not the "expected" operand of cmpxchg
operations, update validation to allow the "expected" field to be a
supertype of the accessed field type as long as it is still equality
comparable.
WebAssembly/shared-everything-threads#92
clarifies this intended typing in the upstream proposal.
@tlively tlively requested a review from conrad-watt February 3, 2025 18:48
tlively added a commit to WebAssembly/binaryen that referenced this pull request Feb 4, 2025
Both passes use StructUtils::StructScanner to analyze struct operations.
Add support for RMW operations to this utility and update its users to
provide the new `noteRMW` hook. Test that GTO and TypeRefining
optimizations work as expected in the presence of RMW operations, but
leave a proper implementation in ConstantFieldPropagation to a later PR.

To allow TypeRefining to refine field types based only on the
"replacement" operand and not the "expected" operand of cmpxchg
operations, update validation to allow the "expected" field to be a
supertype of the accessed field type as long as it is still equality
comparable.
WebAssembly/shared-everything-threads#92
clarifies this intended typing in the upstream proposal.
@tlively
Copy link
Member Author

tlively commented Feb 10, 2025

ping @conrad-watt

@tlively tlively disabled auto-merge February 12, 2025 07:13
@tlively tlively merged commit 6dd2c0c into main Feb 12, 2025
@tlively tlively deleted the cmpxchg-typing branch February 12, 2025 07:13
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