Closed Bug 788476 Opened 12 years ago Closed 12 years ago

IonMonkey: Fold x & -1 to x

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: jandem, Assigned: jandem)

References

(Blocks 1 open bug)

Details

(Whiteboard: [ion:t])

Attachments

(1 file)

Attached patch PatchSplinter Review
As Alon said in bug 788166 comment 2, x & -1 is a common way to convert values to int32. This patch folds them just like we do with x|0.

(Note that there was already an unused foldIfNegOne method).
Attachment #658456 - Flags: review?(dvander)
My bad, I half-added this code many moons ago, but didn't actually add it, since it was just one more place to fail, and none of our benchmarks actually use &-1.
(In reply to Marty Rosenberg [:mjrosenb] from comment #1)
> My bad, I half-added this code many moons ago, but didn't actually add it,
> since it was just one more place to fail, and none of our benchmarks
> actually use &-1.

Do we know of anything that does? I have no objection to the optimization--I know Mandreel uses this pattern--but it would be fun to see the effect on something.
Mandreel used to use it everywhere, but it looks like they stopped, it isn't in the octane-mandreel benchmark. I'm not aware of another compiler or VM using this pattern (but I think it's nice to optimize it for consistency just so if some new project uses &-1 there isn't a surprising slowdown compared to |0).
The interpreter roc wrote (bug 608733) does something like this a lot:

    regs[xx] = (regs[xx] + regs[yy]) & 0xFFFFFFFF;

0xFFFFFFFF (double) is folded to -1 (int32).

If I change the call to testCompiler to testInterpreter in the shell testcase in bug 608733, the time drops from 1711 to 1560 ms with the patch. Strangely enough I see only a very small win on the (slightly modified) testcase I added to awfy-assorted, not sure why (iteration count is a bit lower but still high (50000000) and the numbers are comparable otherwise).
Cool. I learned something by asking a question, even if it wasn't what I expected. :-) And 8% is a big win on the interpreter for such a small thing.
Attachment #658456 - Flags: review?(dvander) → review+
https://hg.mozilla.org/projects/ionmonkey/rev/54711415fb53
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.