Last Comment Bug 747271 - IonMonkey: Assertion failure: opd->type() == phi->type() on test_webgl_conformance.html
: IonMonkey: Assertion failure: opd->type() == phi->type() on test_webgl_confor...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: David Anderson [:dvander]
:
Mentors:
: 737299 (view as bug list)
Depends on:
Blocks: IonGreen
  Show dependency treegraph
 
Reported: 2012-04-19 18:05 PDT by David Anderson [:dvander]
Modified: 2012-05-01 16:10 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
test case (636 bytes, application/javascript)
2012-04-19 18:16 PDT, David Anderson [:dvander]
no flags Details
fix (1.70 KB, patch)
2012-04-20 10:52 PDT, David Anderson [:dvander]
nicolas.b.pierron: review+
Details | Diff | Review

Description David Anderson [:dvander] 2012-04-19 18:05:41 PDT
Encountered this while trying to reproduce a separate bug.
Comment 1 David Anderson [:dvander] 2012-04-19 18:16:41 PDT
Created attachment 616834 [details]
test case
Comment 2 David Anderson [:dvander] 2012-04-20 10:52:55 PDT
Created attachment 617036 [details] [diff] [review]
fix

One of the phi respecialization cases was just missing a reflow.
Comment 3 Nicolas B. Pierron [:nbp] 2012-04-20 15:41:35 PDT
Comment on attachment 617036 [details] [diff] [review]
fix

Review of attachment 617036 [details] [diff] [review]:
-----------------------------------------------------------------

Good, small refactoring nits & small perf issue.

::: js/src/ion/IonAnalysis.cpp
@@ +274,5 @@
>              // Specialize phis with int32 and double operands as double.
>              if (IsNumberType(use->type()) && IsNumberType(phi->type())) {
>                  use->specialize(MIRType_Double);
> +                if (!addPhiToWorklist(use))
> +                    return false;

It seems like you can factor addPhiToWorklist at the end of the loop by filtering the cases which do not need updates.

        // Same type, no need to update.
        if (use->type() == phi->type())
            continue;

        // Propagate the most specialized type which fit both the use and the phi. 
        MIRType specialized = phi->type();
        if (use->type() != MIRType_None) {
            // Specialize phis with int32 and double operands as double.
            if (IsNumberType(use->type()) && IsNumberType(phi->type()))
                specialized = MIRType_Double;
            else
                specialized = MIRType_Value;
        }

        use->specialize(specialize);
        if (!addPhiToWorklist(use))
            return false;

There is one case which troubles me.  If the use type is already a Double, and the Phi is an Int32, then you re-specialize it and cause one extra useless iteration on the work list.  I suggest to check that the specialization is not current type.

        if (use->type() == specialize)
            continue;
        use->specialize(specialize);
        if (!addPhiToWorklist(use))
            return false;
Comment 4 David Anderson [:dvander] 2012-04-26 09:28:48 PDT
pushed w/ nits fixed: http://hg.mozilla.org/projects/ionmonkey/rev/7a4cbdcfac2d
Comment 5 David Anderson [:dvander] 2012-05-01 16:10:23 PDT
*** Bug 737299 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.