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

Define "asm.js integer literal" once and use the name throughout the spec #57

Closed
jruderman opened this Issue Jun 2, 2013 · 8 comments

Comments

Projects
None yet
4 participants
@jruderman

jruderman commented Jun 2, 2013

  • The "does not contain a . character" requirement is weird. Bytecode might not include this information. This choice could use some explanation.
  • The phrase "does not contain a . character" occurs many times in the spec.
  • Some places forget to require that it's an integer, e.g. http://asmjs.org/spec/latest/#caseclause seems to allow 1e-3 (no dots, but also not an integer).
@cscott

This comment has been minimized.

Show comment
Hide comment
@cscott

cscott Jun 5, 2013

I assumed 1e3 would be a valid way to write the integer 1000. Some minimizers use hacks like this to save space. But you're correct, integer literals should not be allowed to have negative exponents!

cscott commented Jun 5, 2013

I assumed 1e3 would be a valid way to write the integer 1000. Some minimizers use hacks like this to save space. But you're correct, integer literals should not be allowed to have negative exponents!

@curiousdannii

This comment has been minimized.

Show comment
Hide comment
@curiousdannii

curiousdannii commented Jun 5, 2013

See also #53

@cscott

This comment has been minimized.

Show comment
Hide comment
@cscott

cscott Jun 8, 2013

I verified that the spidermonkey parser accepts 1e3 as an integer literal. FWIW.

cscott commented Jun 8, 2013

I verified that the spidermonkey parser accepts 1e3 as an integer literal. FWIW.

@cscott

This comment has been minimized.

Show comment
Hide comment
@cscott

cscott Jun 12, 2013

In my asm.js validator I use the rule which spidermonkey seems to use, which is: "an asm.js integer numeric literal is a literal written without '.' in the source, whose value is an integer". This allows 1e3, 1e0, and 1e-0, while disallowing 1e-1.

Note issue #67 bears on this as well, if you want to allow negative integer literals.

@jruderman's first point is a good one, though: asm.js is requiring the tokenizer to preserve the "dotness" of integer literals. The only (partial) way around this (that I can think of) is to distinguish double 0 from int 0 by writing the former as +0. Carried to its logical end, though, negative double literals would be written as +(-5), which is a little odd.... and there's no particular reason to believe that the leading unary operators won't get constant folded away in a bytecode representation anyway. It might be worth a sentence in the spec to describe the tokenizer change.

cscott commented Jun 12, 2013

In my asm.js validator I use the rule which spidermonkey seems to use, which is: "an asm.js integer numeric literal is a literal written without '.' in the source, whose value is an integer". This allows 1e3, 1e0, and 1e-0, while disallowing 1e-1.

Note issue #67 bears on this as well, if you want to allow negative integer literals.

@jruderman's first point is a good one, though: asm.js is requiring the tokenizer to preserve the "dotness" of integer literals. The only (partial) way around this (that I can think of) is to distinguish double 0 from int 0 by writing the former as +0. Carried to its logical end, though, negative double literals would be written as +(-5), which is a little odd.... and there's no particular reason to believe that the leading unary operators won't get constant folded away in a bytecode representation anyway. It might be worth a sentence in the spec to describe the tokenizer change.

@cscott

This comment has been minimized.

Show comment
Hide comment
@cscott

cscott Jun 12, 2013

Note that the spidermonkey rule also allows 0e-3 as a valid integer literal.

cscott commented Jun 12, 2013

Note that the spidermonkey rule also allows 0e-3 as a valid integer literal.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Aug 12, 2014

You're right; the use of . to distinguish doubles from ints does mean the tokenizer has to remember dotness, but (1) that isn't hard, (2) it avoids a more complex definition that operates in terms of IEEE754 doubles. It is important to be razor-sharp on this definition and I think the '.' rule achieves this. Also, we can't use normal minifiers on asm.js anyway (they break all sorts of structure). Emscripten has its own minifier which takes advantage of the structure of asm.js and is, consequently, way way faster on large codes. I don't think there is a bug here, so tentatively closing.

ghost commented Aug 12, 2014

You're right; the use of . to distinguish doubles from ints does mean the tokenizer has to remember dotness, but (1) that isn't hard, (2) it avoids a more complex definition that operates in terms of IEEE754 doubles. It is important to be razor-sharp on this definition and I think the '.' rule achieves this. Also, we can't use normal minifiers on asm.js anyway (they break all sorts of structure). Emscripten has its own minifier which takes advantage of the structure of asm.js and is, consequently, way way faster on large codes. I don't think there is a bug here, so tentatively closing.

@ghost ghost closed this Aug 12, 2014

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Mar 22, 2016

On the face of it, this may seem like a minor issue, but it is actually quite pernicious. Observe that representing a numeric literal—whose source contains a negative exponent, but not a . character—by an integer may introduce a loss in precision. (May because there is no such loss for, e.g., 1e-0.) It follows that AOT compilation may affect the observational behaviour of JavaScript code, which is contrary to the intentions of asm.js. Consider, for example, the following valid module:

function M() {
  "use asm"
  function f() {
    return 1e-3
  }
  return f
}

Without AOT compilation, M()() yields 0.001, but with AOT compilation, it yields 0. Hence, a more sophisticated procedure for deciding how to represent a numeric literal is required. In particular, a procedure that satisfies the following conditions would avoid the aforementioned bug:

n resolves to an integer representation     ⇒     MV(n) is an integer
n resolves to a floating point representation     ⇐     the source of n contains a . character

where MV( · ) is defined in section 11.8.3.1 of ECMA-262/6.

ghost commented Mar 22, 2016

On the face of it, this may seem like a minor issue, but it is actually quite pernicious. Observe that representing a numeric literal—whose source contains a negative exponent, but not a . character—by an integer may introduce a loss in precision. (May because there is no such loss for, e.g., 1e-0.) It follows that AOT compilation may affect the observational behaviour of JavaScript code, which is contrary to the intentions of asm.js. Consider, for example, the following valid module:

function M() {
  "use asm"
  function f() {
    return 1e-3
  }
  return f
}

Without AOT compilation, M()() yields 0.001, but with AOT compilation, it yields 0. Hence, a more sophisticated procedure for deciding how to represent a numeric literal is required. In particular, a procedure that satisfies the following conditions would avoid the aforementioned bug:

n resolves to an integer representation     ⇒     MV(n) is an integer
n resolves to a floating point representation     ⇐     the source of n contains a . character

where MV( · ) is defined in section 11.8.3.1 of ECMA-262/6.

@kripken

This comment has been minimized.

Show comment
Hide comment
@kripken

kripken Mar 22, 2016

Collaborator

Nice find, that's a bug that went unnoticed I guess.

Collaborator

kripken commented Mar 22, 2016

Nice find, that's a bug that went unnoticed I guess.

This issue was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment