747271 - IonMonkey: Assertion failure: opd->type() == phi->type() on test_webgl_conformance.html

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[David Anderson [:dvander] - inactive, e-mail if emergency]

Encountered this while trying to reproduce a separate bug.

Comment 1

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: test case

Comment 2

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: fix

One of the phi respecialization cases was just missing a reflow.

Comment 3

[Nicolas B. Pierron [:nbp]]

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] - inactive, e-mail if emergency]

pushed w/ nits fixed: http://hg.mozilla.org/projects/ionmonkey/rev/7a4cbdcfac2d