Closed Bug 747271 Opened 12 years ago Closed 12 years ago

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

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: dvander, Assigned: dvander)

References

Details

Attachments

(2 files)

Encountered this while trying to reproduce a separate bug.
Attached patch fixSplinter Review
One of the phi respecialization cases was just missing a reflow.
Attachment #617036 - Flags: review?(nicolas.b.pierron)
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;
Attachment #617036 - Flags: review?(nicolas.b.pierron) → review+
pushed w/ nits fixed: http://hg.mozilla.org/projects/ionmonkey/rev/7a4cbdcfac2d
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.