Closed
Bug 451788
Opened 16 years ago
Closed 13 years ago
TM: Multiplying, dividing, modulo two integers promotes to double
Categories
(Core :: JavaScript Engine, enhancement, P5)
Tracking
()
RESOLVED
FIXED
mozilla1.9.1
People
(Reporter: bzbarsky, Assigned: gal)
References
Details
Attachments
(1 file)
1.86 KB,
patch
|
brendan
:
review+
|
Details | Diff | Splinter Review |
The minimal shell testcase in bug 451673 has this line: limit = i * 1; Here 'i' is an integer counter. As far as I can tell, the tracer treats 'i' as an integer, and of course 1 is an integer, but the product is promoted to double. It might be worth it to bounds-check and not promote if we won't overflow.
Assignee | ||
Comment 1•16 years ago
|
||
I intentionally didn't demote this since multiplications often overflow (they do in one of the sunspider benchmarks). But yes, we should give this another try.
Updated•16 years ago
|
Severity: normal → enhancement
Priority: -- → P5
Target Milestone: --- → mozilla1.9.1
Assignee | ||
Updated•16 years ago
|
Summary: TM: Multiplying two integers promotes to double → TM: Multiplying and dividing two integers promotes to double
Assignee | ||
Comment 2•16 years ago
|
||
Changed title to include division as well (which implies modulo too).
Assignee | ||
Updated•16 years ago
|
Summary: TM: Multiplying and dividing two integers promotes to double → TM: Multiplying, dividing, modulo two integers promotes to double
Assignee | ||
Comment 3•16 years ago
|
||
Modulo by a non-zero constant is easy and helps 3d-raytrace: http://hg.mozilla.org/tracemonkey/rev/87fe68f51647
Assignee | ||
Comment 4•16 years ago
|
||
Backed out the previous patch again until its fixed. http://hg.mozilla.org/tracemonkey/rev/61b9209c186f
Assignee | ||
Comment 5•16 years ago
|
||
Assignee: general → gal
Attachment #338850 -
Flags: review?(brendan)
Updated•16 years ago
|
Attachment #338850 -
Flags: review?(brendan) → review+
Comment 6•16 years ago
|
||
Comment on attachment 338850 [details] [diff] [review] Patch, this time hopefully working. >+/* >+ * Note: Caller is responsible for ensuring that b is not 0, or really bad things are going to >+ * happen. This would look nicer if wrapped before column 80 -- most comments do. >+ case F_dmod: >+ JS_ASSERT(s0->isQuad() && args[1]->isQuad()); >+ if (args[1]->isconstq() && args[1]->constvalq() && isPromote(s0)) { >+ LIns* args2[] = { demote(out, s0), demote(out, args[1]) }; >+ return out->ins1(LIR_i2f, out->insCall(F_imod, args2)); Could you comment with a FIXME: bug NNNNNN where the bug number is the one about adding LIR_imod? /be
Assignee | ||
Comment 7•16 years ago
|
||
http://hg.mozilla.org/tracemonkey/rev/b8f1727c3c5b
Comment 8•16 years ago
|
||
Not a blocker as it's an enhancement.
Updated•13 years ago
|
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•