727480 - IonMonkey: Incorrect creation of def-use chains for the operands of the osrvalue and osrscopechain instructions

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Pericles Alves]

Attachment: func0-pass0-BuildSSA-mir.gv.png

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_7_2) AppleWebKit/534.52.7 (KHTML, like Gecko) Version/5.1.2 Safari/534.52.7

Steps to reproduce:

When running the following code, which is a simple division algorithm, IonMonkey produces the MIR graph that goes attached.
--
function div(a, b)                                                                                                                                     
{
        result = 0;

        if(b < a)
        {
            while(a >= b)
            {
                a -= b;
                result++;
            }
        }
        else return -1;

        return result;
}

div(100, 2);
--


Actual results:

Printing the number of uses of each instruction, we noticed that IonMonkey was not storing the uses of the osrentry (id=30) instruction.


Expected results:

As shown in the attached graph, the osrentry (id=30) instruction has four uses, one as an operand of the osrscopechain (id=32) instruction and other three as operands of the osrvalue (id=34, id=36 and id=38) instructions.

Further investigating this, we noticed a lack of a call to the initOperand function in the constructors of the MOsrValue and MOsrScopeChain classes, as done in other classes, like MStoreElement and others. 

When adding the missing calls, like in the following patch, it seems that things are now working just fine:


diff -r b860d2cd42f9 js/src/ion/MIR.h
--- a/js/src/ion/MIR.h	Mon Feb 06 15:21:47 2012 -0200
+++ b/js/src/ion/MIR.h	Mon Feb 06 15:30:06 2012 -0200
@@ -1957,6 +1957,7 @@
     MOsrValue(MOsrEntry *entry, ptrdiff_t frameOffset)
       : frameOffset_(frameOffset)
     {
+        initOperand(0,entry);
         setOperand(0, entry);
         setResultType(MIRType_Value);
     }
@@ -1987,6 +1988,7 @@
   private:
     MOsrScopeChain(MOsrEntry *entry)
     {
+        initOperand(0,entry);
         setOperand(0, entry);
         setResultType(MIRType_Object);
     }

Comment 1

[Sean Stangl [:sstangl]]

Attachment: Guarantee all Operands have set Uses.

Great catch. This fixes the OSR instructions by having them extend MUnaryInstruction, which calls initOperand() in the constructor.

To make sure that Use information isn't missed again, this patch also extends AssertGraphCoherency() and calls it after each MIR-affecting phase. This caught a bug in LICM: remove() was being called with the intention to reposition an MInstruction, but remove() wipes out the Use information. Fixed by implementing a dedicated move instruction.

Comment 2

[Sean Stangl [:sstangl]]

http://hg.mozilla.org/projects/ionmonkey/rev/ebd61b600176