Closed Bug 451788 Opened 13 years ago Closed 10 years ago

TM: Multiplying, dividing, modulo two integers promotes to double

Categories

(Core :: JavaScript Engine, enhancement, P5)

x86
macOS
enhancement

Tracking

()

RESOLVED FIXED
mozilla1.9.1

People

(Reporter: bzbarsky, Assigned: gal)

References

Details

Attachments

(1 file)

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.
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.
Severity: normal → enhancement
Priority: -- → P5
Target Milestone: --- → mozilla1.9.1
Summary: TM: Multiplying two integers promotes to double → TM: Multiplying and dividing two integers promotes to double
Changed title to include division as well (which implies modulo too).
Summary: TM: Multiplying and dividing two integers promotes to double → TM: Multiplying, dividing, modulo two integers promotes to double
Modulo by a non-zero constant is easy and helps 3d-raytrace:

http://hg.mozilla.org/tracemonkey/rev/87fe68f51647
Backed out the previous patch again until its fixed. 

http://hg.mozilla.org/tracemonkey/rev/61b9209c186f
Assignee: general → gal
Attachment #338850 - Flags: review?(brendan)
Attachment #338850 - Flags: review?(brendan) → review+
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
Not a blocker as it's an enhancement.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.