-
Notifications
You must be signed in to change notification settings - Fork 696
Types and linear memory #245
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Put all the type information together in one consice section, and all the linear memory information together in one section. The following changes are notable: - rename *local types* to *basic types*. - introduce *expression types* - add `void` as an expression type - make *memory types* an explicit superset of basic types - drop the `load_mem` and `store_mem` operators, which are obsolete now that we have the new typed load and store operators. - drop the `load_mem_with_offset` and `store_mem_with_offset` operators too, and make the typed load and store operators have offsets now. - define *natural alignment*, *misaligned* access, and *aligned* access. - define the *effective address* of an access
In the MVP, linear memory is not shared between threads. When | ||
[threads](PostMVP.md#threads) are added as a feature, regular load and store | ||
nodes will have the most relaxed semantics specified in the memory model and new | ||
memory-access nodes will be added with atomic and ordering guarantees. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regular loads and stores won't have relaxed semantics: they're instead bound by happens-before with atomics in the same thread of execution, which themselves synchronize-with atomics in other threads.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is text that was already present, and is just being moved in this patch. I can attempt to clarify this text though and avoid the ambiguous word "relaxed" in this context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ha, I didn't notice it the first time around. I can also do a separate PR following yours, if that's cleaner.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, if you don't mind, that'd be simpler.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do. Filed #248 to do this once this PR is checked in.
And clarify the out-of-bounds condition.
e.g. whether locals with type `int32` and `int64` must be contiguous and separate from | ||
others, etc. | ||
Each linear memory access operation also has an address operand and an immediate | ||
integer offset attribute. The infinite-precision sum of the address operand's |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say "immediate integer offset attribute, in bytes".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about "immediate integer byte offset attribute"? I updated the pull request to say that.
lgtm, assuming I do a follow-up PR and you do 2 :-) |
Ok, I submitted #253 which includes updates for both issues I said I'd fix above. Note that that PR depends on this one. |
Linear memory operations are annotated with a memory type and perform a | ||
conversion between that memory type and a basic type. | ||
|
||
Loads read data from linear memory, convert from their memory type to a basic |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've been using the term local type in other places.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I grepped the design repo and didn't see any other references. Do you prefer "local" over "basic" here? These types do get used outside of "local" contexts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I recall using "local type" in AstSemantics. Maybe those references have
been refactored away. "Local" is nice because it relates to local variables.
On Tue, Jul 7, 2015 at 2:52 PM, Dan Gohman [email protected] wrote:
In AstSemantics.md
#245 (comment):+
memory_size
bytes. The linear memory can be considered to be a untyped array
+of bytes. The linear memory is sandboxed; it does not alias the execution
+engine's internal data structures, the execution stack, local variables, global
+variables, or other process memory.
+
+In the MVP, linear memory is not shared between threads. When
+threads are added as a feature, regular load and store
+nodes will have the most relaxed semantics specified in the memory model and new
+memory-access nodes will be added with atomic and ordering guarantees.
+
+### Linear Memory Operations
+
+Linear memory operations are annotated with a memory type and perform a
+conversion between that memory type and a basic type.
+
+Loads read data from linear memory, convert from their memory type to a basicI grepped the design repo and didn't see any other references. Do you
prefer "local" over "basic" here? These types do get used outside of
"local" contexts.—
Reply to this email directly or view it on GitHub
https://github.com/WebAssembly/design/pull/245/files#r34035409.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I had found and replaced them all :-). But I don't feel strongly about this right now, so I've now added another patch to revert back to "local types".
I filed #255 to discuss whether 64-bit addressing is a mode or separate opcodes. And I've now updated the PR to avoid mentioning modes. |
I believe I've addressed all outstanding comments as of this comment. The text mentioning modes has been changed to be more vague. |
Can this land, so subsequent PRs are unblocked? |
As far as I know, all outstanding concerns have been addressed. |
lgtm On Wed, Jul 8, 2015 at 7:43 PM, Dan Gohman [email protected] wrote:
|
Many LGTMs, so merge is yes. |
The current AstSemantics sections on types and linear memory are a little disorganized at the moment. Linear memory access operations are described in the types section, and the linear memory section itself has an obsolete set of load and store opcodes.
This PR reorganizes and generally revises these sections, creating a simple section for just types, and putting all the linear memory content in the linear memory section. It doesn't make any semantic changes, though it does make a few terminology changes.