Add support for atomic.fence from the threads proposal#1231
Add support for atomic.fence from the threads proposal#1231binji merged 1 commit intoWebAssembly:masterfrom
Conversation
sbc100
left a comment
There was a problem hiding this comment.
lgtm, with a couple of comments
| WABT_TOKEN(Try, "try") | ||
| WABT_TOKEN(Unary, "UNARY") | ||
| WABT_TOKEN(Unreachable, "unreachable") | ||
| WABT_TOKEN_FIRST(Opcode, AtomicLoad) |
There was a problem hiding this comment.
Was AtomicLoad not supposed to be here at all?
There was a problem hiding this comment.
Mmm, I think this was a bad change that snuck in. I will make sure it's not there in the revised patch and regenerate the lexer. EDIT: Actually this patch is the right thing. WABT_TOKEN_FIRST specifies the first Opcode token. It was AtomicLoad, but I added AtomicFence before it, so here I replace the operand.
| return Result::Error; | ||
| } | ||
| return Result::Ok; | ||
| } |
There was a problem hiding this comment.
Is the type checker the right place for this? i.e. is this a more general validation thing than a type checking thing?
There was a problem hiding this comment.
Hooo, finally picking up this patch again. Rebasing and re-uploading, but yes you are right -- will include this check in the validator rather than the type checker.
wingo
left a comment
There was a problem hiding this comment.
Addressing comments before rebase
| return Result::Error; | ||
| } | ||
| return Result::Ok; | ||
| } |
There was a problem hiding this comment.
Hooo, finally picking up this patch again. Rebasing and re-uploading, but yes you are right -- will include this check in the validator rather than the type checker.
| WABT_TOKEN(Try, "try") | ||
| WABT_TOKEN(Unary, "UNARY") | ||
| WABT_TOKEN(Unreachable, "unreachable") | ||
| WABT_TOKEN_FIRST(Opcode, AtomicLoad) |
There was a problem hiding this comment.
Mmm, I think this was a bad change that snuck in. I will make sure it's not there in the revised patch and regenerate the lexer. EDIT: Actually this patch is the right thing. WABT_TOKEN_FIRST specifies the first Opcode token. It was AtomicLoad, but I added AtomicFence before it, so here I replace the operand.
See WebAssembly/threads#141 for the binary encoding. This patch does add a field to AtomicFenceExpr for the consistency model, though without a type for the time being.
See WebAssembly/threads#141 for the binary encoding. This patch does add a field to
AtomicFenceExprfor the consistency model, though without a type for the time being.