Wasmtime GitHub notifications bot (Mar 02 2026 at 11:58): Wasmtime GitHub notifications bot (Mar 02 2026 at 11:58):ahqsoftwares edited PR #12701.
Wasmtime GitHub notifications bot (Mar 02 2026 at 11:58):ahqsoftwares has marked PR #12701 as ready for review.
Wasmtime GitHub notifications bot (Mar 02 2026 at 11:58):ahqsoftwares requested alexcrichton for a review on PR #12701.
Wasmtime GitHub notifications bot (Mar 02 2026 at 11:58):ahqsoftwares requested wasmtime-compiler-reviewers for a review on PR #12701.
Wasmtime GitHub notifications bot (Mar 02 2026 at 14:49):cfallin commented on PR #12701:
This implementation currently increases the size of InstructionData to 24 bytes. I am fully aware this is a performance regression against the 16-byte target mentioned in the codebase.
Thanks for this PR! Unfortunately if it inflates
InstructionDatathen it's probably a nonstarter. As mentioned in my earlier comment I don't think that there is going to be much run-time performance win with this, even after adding lowerings; on the other hand, the compile-time hit of inflating a key IR data structure by 50% is going to be pretty large (more cache misses/memory traffic primarily).If you can find a way to keep
InstructionDataat 16 bytes, then this is more viable, I think. Which instruction format is pushing it over? Is itAtomicRmwOpor something else? Can we pack the ordering intoMemFlagsor the RMW op or...?
Wasmtime GitHub notifications bot (Mar 02 2026 at 15:37):alexcrichton requested cfallin for a review on PR #12701.
Wasmtime GitHub notifications bot (Mar 02 2026 at 15:37):alexcrichton unassigned alexcrichton from PR #12701 [Frontend IR] Add AtomicOrdering enum to Instruction Builder Frontend IR.
Wasmtime GitHub notifications bot (Mar 03 2026 at 04:30):ahqsoftwares commented on PR #12701:
Actually, I shall look into exactly what is causing it. If it's AtomicRmwOp, we probably can merge the two into AtomicFlags.
Wasmtime GitHub notifications bot (Mar 03 2026 at 04:31):ahqsoftwares edited a comment on PR #12701:
Actually, I shall look into exactly what is causing it. If it's AtomicRmwOp, we probably can merge the two into AtomicFlags.
And yes, i really think there are a few unused bytes.
Wasmtime GitHub notifications bot (Mar 03 2026 at 04:31):ahqsoftwares edited a comment on PR #12701:
Actually, I shall look into exactly what is causing it. If it's AtomicRmwOp, we probably can merge the two into AtomicFlags.
And yes, i really think there are a few unused bytes we can take advantage of.
Wasmtime GitHub notifications bot (Mar 03 2026 at 04:32):ahqsoftwares edited a comment on PR #12701:
Actually, I shall look into exactly what is causing it. If it's AtomicRmwOp, we probably can merge the two into AtomicFlags.
And yes, i really think there are a few unused bytes we can take advantage of.
As I saw, we probably cannot pack it in Memflags as it's fully occupied (and 3 bits cannot be shaved from there)
Wasmtime GitHub notifications bot (Mar 05 2026 at 03:10):ahqsoftwares edited PR #12701:
issue #12679
As i mentioned in https://github.com/bytecodealliance/wasmtime/issues/12679#issuecomment-3980345081
this PR only adds AtomicOrdering enum and sets the implementation to Default (SeqCst) for compatibility.This PR Also upgrades the test clif files, internal-cranelfit as well.
The Lowering is set to SeqCst by default.
Changes:
- Added the
AtomicOrderingenum to the instructions (atomic_load, atomic_store, atomic_rmw, atomic_cas, fence)- CDSL has been updated to reflect the changes
- The internal
wasmtime-cranelifthas to updated to exclusively use the default AtomicOrdering (SeqCst) to comply with webassembly expectations.- 45+ clif test files have been updated to reflect the change.
[!IMPORTANT]
This implementation currently increases the size of InstructionData to 24 bytes. I am fully aware this is a performance regression against the 16-byte target mentioned in the codebase.The last commit in this PR introduces a way to bitpack the AtomicCasMemFlags from 17-bits to 16-bits using mixed radix decimal expansion (180states, 8bits). But due to maintainability concerns, I am leaving the logic right here and this PR is currently being closed.
More information can be found about my approaches in ZulipChat: #cranelift > ISLE Lowering, Extractors, MemFlags
Wasmtime GitHub notifications bot (Mar 05 2026 at 03:10):ahqsoftwares edited PR #12701:
issue #12679
As i mentioned in https://github.com/bytecodealliance/wasmtime/issues/12679#issuecomment-3980345081
this PR only adds AtomicOrdering enum and sets the implementation to Default (SeqCst) for compatibility.This PR Also upgrades the test clif files, internal-cranelfit as well.
The Lowering is set to SeqCst by default.
Changes:
- Added the
AtomicOrderingenum to the instructions (atomic_load, atomic_store, atomic_rmw, atomic_cas, fence)- CDSL has been updated to reflect the changes
- The internal
wasmtime-cranelifthas to updated to exclusively use the default AtomicOrdering (SeqCst) to comply with webassembly expectations.- 45+ clif test files have been updated to reflect the change.
[!IMPORTANT]
This implementation currently increases the size of InstructionData to 24 bytes. I am fully aware this is a performance regression against the 16-byte target mentioned in the codebase.Experimental Insights
The last commit in this PR introduces a way to bitpack the AtomicCasMemFlags from 17-bits to 16-bits using mixed radix decimal expansion (180states, 8bits). But due to maintainability concerns, I am leaving the logic right here and this PR is currently being closed.
More information can be found about my approaches in ZulipChat: #cranelift > ISLE Lowering, Extractors, MemFlags
Wasmtime GitHub notifications bot (Mar 05 2026 at 03:11):ahqsoftwares updated PR #12701.
Wasmtime GitHub notifications bot (Mar 05 2026 at 03:11):ahqsoftwares closed without merge PR #12701.
Last updated: Mar 23 2026 at 16:19 UTC