Skip to content
This repository was archived by the owner on Jan 25, 2022. It is now read-only.

No quantum garbage! #51

Closed
lars-t-hansen opened this issue Jan 27, 2016 · 12 comments
Closed

No quantum garbage! #51

lars-t-hansen opened this issue Jan 27, 2016 · 12 comments

Comments

@lars-t-hansen
Copy link
Collaborator

Suppose ta is a TypedArray mapping shared memory. Consider this code:

let x = ta[n]
if (x)
  print(x)

If the initial value of ta[n] is 1, and there is a race on that access that writes 0, and the JIT duplicates the load to re-read ta[n] in the print statement (a legal optimization in unshared memory), then the program would print 0. This is considered absurd by much of the committee.

The load introduction might be advantageous if the "then" branch is more elaborate with more code before the print call that does not affect the value of x, for example, and register pressure requires x to be saved and reloaded across that code.

Fixing this involves prohibiting load introduction on shared memory unless the compiler can prove that the access is not racy (generally very hard). The optimization is still legal in unshared memory and existing code need not be impacted, if the JIT can use type information properly to tell shared from unshared memory (data needed, and it would be useful to have it here).

Part of the work here is to consider whether there are other cases that might be similar.

@lars-t-hansen
Copy link
Collaborator Author

Fixing this is a hard requirement for stage 3.

@lars-t-hansen
Copy link
Collaborator Author

The following transformation ought to be valid still. Original code:

let x = ta[n]
if (y) {
   // lots of stuff that does not affect ta[n]
   print(x)
} else {
  // lots of stuff that does not affect ta[n]
  print(x)
}

Rewritten code:

if (y) {
   // lots of stuff that does not affect ta[n]
   print(ta[n])
} else {
  // lots of stuff that does not affect ta[n]
  print(ta[n])
}

In other words, simplistic spec language that makes read from shared memory volatilish is not desirable here. It should be possible to move reads and duplicate reads and so on, just not in certain ways that makes it possible to observe two different values for what is the same read.

@lars-t-hansen
Copy link
Collaborator Author

One wonders idly how this interacts with the bailout/recover/deopt logic in ion. In nonshared memory it is OK for a non-effecting operation to bailout and retry, even if it reads memory. In shared memory the read can only happen once, in one of the execution engines.

@taisel
Copy link

taisel commented Jan 30, 2016

edit: moved to #56

@lars-t-hansen
Copy link
Collaborator Author

edit: this comment pertained to previous comment, which was moved.

@jfbastien
Copy link
Contributor

Also discussed at TC39: what's the cost of "fixing" this issue?

@lars-t-hansen
Copy link
Collaborator Author

The cost of curtailing rematerialization (assuming that's the issue you were referring to) is under investigation. Input from the compiler teams would be helpful, by and by. Firefox does not currently do rematerialization of TypedArray data, but I'm guessing it's an optimization that might benefit asm.js and wasm more than straight JS. Clearly there will be no impact on asm.js and wasm programs that don't use shared memory, since whether the memory is shared or not is known at compile time (and checked at link time), and non-shared-memory programs don't have to worry about the restriction.

(The cost is actually a moot point, I think - TC39 is, for better or worse, not usually swayed by how hard it is to implement something efficiently. But of course it would be nice to know.)

@taisel
Copy link

taisel commented Jan 30, 2016

edit: comment pertained to previous comments that were moved to issue #56.

@sunfishcode
Copy link
Member

@lars-t-hansen Concerning your example here of moving a load in a non-observable way: the optimization in the example there would be valid even with volatileish semantics. If there's no way to observe the difference, does the spec need explicit language to accommodate it?

Concerning the cost of rematerialization: As two data points, GCC and LLVM are both examples of aggressively optimizing compilers, and neither currently implements rematerialization of loads from user data unless it's provably immutable. So while there theoretically is some cost to disallowing such rematerializations, it's at least not prohibitive in practice.

@lars-t-hansen
Copy link
Collaborator Author

@sunfishcode, if it's not observable we should not need language to prevent it. Thanks for pointing out my abuse of "volatilish" :)

Also thanks for the data point on rematerialization, if the benefits of allowing it are elusive then it becomes more palatable to prohibit it (in possibly-shared memory), if we're able to spec that properly without preventing a lot of desirable optimizations at the same time.

@lars-t-hansen
Copy link
Collaborator Author

In the context of the two rules I just proposed in Issue #71, the problem in the present bug might be addressed by strengthening or augmenting the second of those rules: in a race, the value in memory may change unpredictably over the course of the race (which by Issue #22 may last a long time), that follows also from the "rules we should have" section later in my comment on Issue #71, so let's make sure that's expressed well. In light of that I think it would break the existing semantics of the language to rematerialize a value from shared memory, since the same semantic instance of GetValueFromBuffer could produce multiple values and that could be observed. It might thus be sufficient to point this issue out in a Note.

Those rules would not prohibit collapsing reads (including hoisting reads out of loops) or propagating a write to a read.

@lars-t-hansen
Copy link
Collaborator Author

I believe today's (forthcoming) draft addresses this adequately by providing a foundation on which to stand (most optimizations that make sense in single-threaded executions are allowable) and then stating that observing (in the sense of the semantics) more than one value for what is a single read is not allowable, this in a context of a set of general restrictions on transformations of reads and writes.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants