Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Memory model document. #75790
Memory model document. #75790
Changes from 32 commits
b1dc5ee
1fa2ecc
c4b4765
b872aba
aae251b
f13605d
a8a1003
b15a921
d554b05
38f0585
fe0a65e
e8861ec
5a25cab
be72e2e
75adccb
eec3b96
aaeadfc
4cdac19
05694e3
804942f
f1e13ed
d7c179c
615c434
20b6b52
5579343
31431ed
8b23a88
e02a5df
280014d
608f45d
ed89b04
136e637
34a074d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 implications are not clear to me. Add more explanation as to why?
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.
These are basically the rules that we define as a matter of codifying the model that existed for many years.
I think that is important to have precise but not overly verbose rules.
I wonder how this can be clearer besides saying "we can`t introduce or remove observable sideeffects", which is kind of circular. Perhaps I can add some explanation what these rules try to achieve:
A read cannot be re-done, since it could fetch a different value and introduce a race that the program did not have.
On the other hand, if the value was not used, it is the same as not doing the read at all (same sideeffects), so unused reads can be removed.
Coalescing of adjacent ordinary memory accesses to the same location is ok, since it is generally ok to remove races.
We can't allow speculative writes as we consider changing the value to be generally observable, thus effects of a speculative write may not be possible to undo.
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.
One danger with data dependency is that values might be the same but come from different sources. Imagine something like
and a compiler that deduces that
local1
andlocal3
are the same but doesn't rewrite that static read for whatever reason. This change would need to be blocked:According to this doc (and @AndyAyersMS comment and your reply), this optimization would really be
but optimizations might not be considering it this way.
Furthermore, I believe that a rewrite to
would also be invalid because it would lose specifying the data dependency to the processor (unless I misunderstand how that typically works?). It's a little more subtle, but I guess trying to write out the steps makes it clear:
Optimization to
is ok:
Maybe Andy will tell me that this is all handled already!
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.
The way specifying the data dependency in th ehardware:
if a value from location X was used to compute address Y, reading from Y cannot happen earlier than reading from X; i.e. we cannot get a cached value of a field that is older than the value of the instance. Note - it is ok if both were cached long time ago, as long as value of Y is not older than X.
Compiler will typically not violate this, since it would not know how to read Y without reading X. However with some prefetching, or profile guided optimization, it might in theory be possible to "know" the location of Y and read it directly (not via X), so we forbid such optimizations in this doc.
I think we need to forbid compiler optimizations that violate such guarantee. Otherwise there is no point to instruct the hardware to do the ordering.
the following emit should be invalid
Just saying "nonlocal writes cannot reorder" would prevent reordering of unrelated writes, which seems a bit too strict.
should be, in theory, ok to emit as:
In a case of complex aliasing, compiler could just behave conservatively and keep the program order of writes, if the writes are not local and not known to be "unrelated".
I am just not sure how to nicely define "related" - reachable through an indirection, through a dependent read?
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.
Not sure about all this. I suspect the jit will not do store reordering today, but I can't say for sure.
Dead stores may also be a problem, say we have
and the jit can prove
nonlocal
andx
don't alias, I think it may remove the first store as dead. But maybe this only happens whenx
is known to be a stack location.In the "transitive case"
presumably
x
has already been "exposed" in some way?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.
Removing the first write should be illegal in this case. If
x.y
starts asnull
andnonlocal = x
is the first publish, the reader might expect thatnonlocal?.y.ToString()
will not throw NRE.The following seems ok:
If publishing is a release, it cannot allow preceding memory operations to be delayed after it, but it would be ok for the operations after it to move earlier.
If we instruct the hardware do a release, I think the compiler should follow the same rules.
Perhaps we can just document it as something like - "for the purpose of compiler optimizations nonlocal object writes should be treated as releases".
We may not always emit the assignment as an actual release. For example when assigning a frozen/preallocated string literal, we do not need a release in hardware, since the string is immutable and is guaranteed to be completely formed, but we still must preserve the order of writes in compiler optimizations.
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.
Is this essentially the opposite of data-dependent reads, or is it more subtle?
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.
Is this -any- write to the object's fields?
is clearly covered, but what about
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.
Is this code guaranteed to commit all of the updates at the final store?
(Is "reachable through the instance reference" direct or transitive?)
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, it is essentially the opposite of data-dependent reads. Object writes must be emitted as releases, so that one could, for example, traverse a published linked list.
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.
Indirectly it means that compiler cannot reorder data-dependent writes.
I.E. we cannot emit
I do not think JIT violates this, but I wonder if this already follows from existing rules or we need to add some constraint on reordering of dependent writes?
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.
Interesting, when I wrote the comment I was just trying to clarify the language, but now I realize that I was expecting direct dependencies only, unless this is intended to be very restrictive.
The writes
static = obj1
andobj2.field = value
have no relationship other thanobj1
happens to point toobj2
. The preceding line could be in a function or otherwise hidden. However, the compiler would need to prove thatobj1
doesn't point toobj2
, so in order to do the reordering it actually needs to prove that all fields ofobj1
point to "something else", and if that is indirect/transitive, then it would be quite difficult.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.
The given links lead to HTTP 404.
Is the resulting url.
Edit: created #79785 to fix this.