817515 - BaselineCompiler: Compile NEG and BITNOT

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jan de Mooij [:jandem]]

Need these for SS, eg nsieve-bits.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Part 1: Add NegOperation

Patch for m-c to move JSOP_NEG code into NegOperation.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Part 2: Add IC

Comment 3

[Jan de Mooij [:jandem]]

Part 1:

https://hg.mozilla.org/integration/mozilla-inbound/rev/f8312a05c1a3

Comment 4

[Jan de Mooij [:jandem]]

https://hg.mozilla.org/projects/ionmonkey/rev/38c1e6c4ddfe

Comment 5

[Brendan Eich [:brendan]]

Comment on attachment 687689 [details] [diff] [review]
Part 1: Add NegOperation

>-    /*
>-     * When the operand is int jsval, INT32_FITS_IN_JSVAL(i) implies
>-     * INT32_FITS_IN_JSVAL(-i) unless i is 0 or INT32_MIN when the
>-     * results, -0.0 or INT32_MAX + 1, are double values.
>-     */
>-    Value ref = regs.sp[-1];
>-    int32_t i;
>-    if (ref.isInt32() && (i = ref.toInt32()) != 0 && i != INT32_MIN) {
>-        i = -i;
>-        regs.sp[-1].setInt32(i);
>-    } else {
>-        double d;
>-        if (!ToNumber(cx, regs.sp[-1], &d))
>-            goto error;
>-        d = -d;
>-        if (!regs.sp[-1].setNumber(d) && !ref.isDouble())
>-            TypeScript::MonitorOverflow(cx, script, regs.pc);
>-    }
>+    RootedValue &val = rootValue0;

Why this ref to and use of rootValue0? There should be no need for a copy of the value to negate.

Check code-gen, did this really equate to the previous code (which should uave used a Value& ref but never mind)? I may well be picking on the previous version too!

/be

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f8312a05c1a3

Comment 7

[Jan de Mooij [:jandem]]

(In reply to Brendan Eich [:brendan] from comment #5)
> 
> Why this ref to and use of rootValue0? There should be no need for a copy of
> the value to negate.
> 
> Check code-gen, did this really equate to the previous code (which should
> uave used a Value& ref but never mind)? I may well be picking on the
> previous version too!
> 
> /be

It couldn't use a reference, it has to keep the original value around after setting the result, to check if it's an int32 overflow:

if (!regs.sp[-1].setNumber(d) && !ref.isDouble())

ToNumber can trigger a GC, so this value has to be exactly rooted. Looking at other ops (e.g. SUB/MUL/DIV), the convention is to use rootValue0.