664034 - IonMonkey: definition iterator

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Ryan Pearl [:rpearl]]

There should be an iterator for all the definitions (that is, phi nodes and regular instructions) in a given basic block. 

This comes up several times in GVN, and probably in other places as well.

Comment 1

[Ryan Pearl [:rpearl]]

Attachment: implements iterator

Add the iterator and use it in the current case it appears in (it appears in more places in GVN).

Comment 2

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

Comment on attachment 539057 [details] [diff] [review]
implements iterator

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

r=me with those fixed

::: js/src/ion/IonAnalysis.cpp
@@ +365,5 @@
> +        while (itr.more()) {
> +            MInstruction *ins = *itr;
> +            if (ins->isPhi()) {
> +                specializePhi(ins->toPhi());
> +            }

This should be:

if (ins->isPhi() && !specializePhi(ins->toPhi()))
    continue;

To preserve the old code's semantics.

::: js/src/ion/MIRGraph.h
@@ +409,5 @@
> +{
> +  private:
> +    MBasicBlock *block_;
> +    size_t phiIndex_;
> +    MInstructionIterator itr_;

Oh, you can add the "e" :)

@@ +418,5 @@
> +            return block_->getPhi(phiIndex_);
> +        } else {
> +            return *itr_;
> +        }
> +    }

Style nit, no "else-after-return", i.e.:

    if (phiIndex_ < block_->numPhis())
        return block_->getPhi(phiIndex_);
    return *iter_;

@@ +431,5 @@
> +        if (phiIndex_ < block_->numPhis()) {
> +            phiIndex_++;
> +        } else {
> +            itr_++;
> +        }

If all the paths are one-liners, house style is to not use braces.

Comment 3

[Ryan Pearl [:rpearl]]

(In reply to comment #2)

> Review of attachment 539057 [details] [diff] [review] [review]:
> -----------------------------------------------------------------
> 
> r=me with those fixed
> 
http://hg.mozilla.org/users/danderson_mozilla.com/ionmonkey/rev/ea05c15fee01

> If all the paths are one-liners, house style is to not use braces.

Ewww...