Closed Bug 1091793 Opened 7 years ago Closed 7 years ago

IonMonkey: Remove useless NOPs

Categories

(Core :: JavaScript Engine: JIT, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: nbp, Assigned: nbp)

Details

Attachments

(1 file)

Currently we are producing a lot of Nop, and as GVN is folding constants, many of these Nops end-up in a sequence.  Nop instructions were added in order to reduce the pressure on registers.  Unfortunately, by keeping sequences of Nops, we might in fact slightly keep some pressure on our register allocator.

Ideally, we should remove any Nop which is not dominating any bailing instruction before the end of the block or before the next Nop.
This patch improves Kraken by 1.6% on x64 (860ms -> 846ms).
Attachment #8514465 - Flags: review?(sunfish)
Comment on attachment 8514465 [details] [diff] [review]
IonMonkey: Remove useless NOPs.

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

::: js/src/jit/ValueNumbering.cpp
@@ +713,5 @@
> +        // resume point of the basic block by the one from the resume point.
> +        if (iter == block->rend()) {
> +            nop->moveResumePointAsEntry();
> +            block->discard(nop);
> +            return true;

Please add a JitSpew line here mentioning that a nop's resume point is being moved to the entry.

@@ +719,5 @@
> +
> +        // The previous instruction is also a Nop, no need to keep it anymore.
> +        MInstruction *prev = *iter;
> +        if (prev->isNop()) {
> +            block->discard(prev);

and a JitSpew here mentioning that a nop is being discarded.
Attachment #8514465 - Flags: review?(sunfish) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/8cecdc9616a4

We only produce MNop with resume points, these test are not generating any resume points, which is an assumption of the previous patch. This patch fix the tests by removing the Nops.
https://hg.mozilla.org/mozilla-central/rev/92fef8eaceea
https://hg.mozilla.org/mozilla-central/rev/8cecdc9616a4
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.