Closed
Bug 1016951
Opened 11 years ago
Closed 11 years ago
OdinMonkey: jit-tests/tests/asm.js/bug941877.js contains invalid asm.js code
Categories
(Core :: JavaScript Engine: JIT, defect)
Tracking
()
RESOLVED
FIXED
mozilla32
People
(Reporter: bbouvier, Assigned: bbouvier)
References
Details
Attachments
(1 file)
4.88 KB,
patch
|
sunfish
:
review+
|
Details | Diff | Splinter Review |
It looks rather correct with this patch but I want to make sure that I didn't change the tests semantics.
Attachment #8430020 -
Flags: review?(sunfish)
Comment 1•11 years ago
|
||
Comment on attachment 8430020 [details] [diff] [review]
fix-bug-941877-tests.patch
Review of attachment 8430020 [details] [diff] [review]:
-----------------------------------------------------------------
::: js/src/jit-test/tests/asm.js/bug941877.js
@@ +75,5 @@
> {
> x = x|0;
> y = y|0;
> var t = 0;
> + t = (((x >> 0) % (y >> 0))|0) > -2;
Is there a reason for using >>0 instead of |0 ?
@@ +97,5 @@
> return t|0;
> }
> return f;
> }
> +var compiled = asmCompile(USE_ASM + FunctionBody(test4));
What's the reason for using USE_ASM + ... instead of leaving the "use asm" in test4 itself?
@@ -93,5 @@
> {
> x = x|0;
> y = y|0;
> - var t = 0;
> - t = ((x>>>0) % (y>>>0)) * 1.01;
This used to work? Crazy.
Attachment #8430020 -
Flags: review?(sunfish) → review+
Assignee | ||
Comment 2•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/e808372ebbd4
(In reply to Dan Gohman [:sunfish] from comment #1)
> Comment on attachment 8430020 [details] [diff] [review]
> fix-bug-941877-tests.patch
>
> Review of attachment 8430020 [details] [diff] [review]:
> -----------------------------------------------------------------
> Is there a reason for using >>0 instead of |0 ?
No indeed, changed it to |0 as it's simpler.
>
> @@ +97,5 @@
> > return t|0;
> > }
> > return f;
> > }
> > +var compiled = asmCompile(USE_ASM + FunctionBody(test4));
>
> What's the reason for using USE_ASM + ... instead of leaving the "use asm"
> in test4 itself?
If we leave the "use asm" directive directly in the function body, the interpreter will try to compile it as an asm.js module directly, falling back to the interpreter if the module isn't valid, without any warning. asmCompile makes sure that the warning "Successfully compiled asm.js module" is caught. We could leave the "use asm" directive in the function body *and* use asmCompile, but it means that compilation would be done twice (once as soon as the function is parsed, a second time in the eval present in asmCompile).
Comment 3•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in
before you can comment on or make changes to this bug.
Description
•