Assertion failure: i != graph.poEnd() (Reached the end of the graph while searching for the loop header), at jit/IonAnalysis.cpp:3181

RESOLVED FIXED in mozilla35

Status

()

Core
JavaScript Engine: JIT
--
critical
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: decoder, Assigned: sunfish)

Tracking

(Blocks: 1 bug, {assertion, testcase})

Trunk
mozilla35
x86_64
Linux
assertion, testcase
Points:
---

Firefox Tracking Flags

(firefox35 affected)

Details

(Whiteboard: [jsbugmon:update])

Attachments

(2 attachments)

(Reporter)

Description

4 years ago
The following testcase asserts on mozilla-central revision 5bd6e09f074e (run with --no-threads --fuzzing-safe --ion-eager):


var generics = {
  String: []
};
for (var c in generics) {
  try {
    throw new Error();
  } catch (e) {} 
  while (--noop != 0);
}
(Reporter)

Updated

4 years ago
status-firefox35: --- → affected
Whiteboard: [jsbugmon:update,bisect]
(Reporter)

Updated

4 years ago
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
(Reporter)

Comment 1

4 years ago
JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/8f27a48a25d5
user:        Dan Gohman
date:        Wed Sep 17 10:27:25 2014 -0700
summary:     Bug 1029830 - IonMonkey: GVN: Replace UCE with GVN r=nbp

This iteration took 341.162 seconds to run.
Dan, can you have a look? Bisect points to UCE/GVN unification. I know you fixed some similar issues in the last days. Can you confirm this is fixed by those? Or else have a look what is going wrong here?
Flags: needinfo?(sunfish)
(Assignee)

Comment 3

4 years ago
Created attachment 8496191 [details] [diff] [review]
broken-loops.patch

With GVN now folding MTest to MGoto, it has opened up the possibility that such folding could remove the only path from a loop header to its backedge. In such cases, the loop becomes degenerate, and optimizations like LICM are no longer useful. Detect such loops, and skip loop-oriented optimizations on them.
Assignee: nobody → sunfish
Attachment #8496191 - Flags: review?(nicolas.b.pierron)
Flags: needinfo?(sunfish)
Comment on attachment 8496191 [details] [diff] [review]
broken-loops.patch

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

::: js/src/jit/IonAnalysis.cpp
@@ +3239,5 @@
>      }
> +
> +    // If there's no path connecting the header to the backedge, then this isn't
> +    // actually a loop. This can happen when the code starts with a loop but GVN
> +    // folds some branches away.

I am not sure to understand what you mean by "fold - away"?
Comment on attachment 8496191 [details] [diff] [review]
broken-loops.patch

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

Ask again for review after answering the previous question.
Attachment #8496191 - Flags: review?(nicolas.b.pierron)
(Reporter)

Comment 6

4 years ago
Created attachment 8499437 [details]
[crash-signature] Machine-readable crash signature
(Assignee)

Comment 7

4 years ago
Comment on attachment 8496191 [details] [diff] [review]
broken-loops.patch

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

(In reply to Nicolas B. Pierron [:nbp] from comment #4)
> Comment on attachment 8496191 [details] [diff] [review]
> broken-loops.patch
> 
> Review of attachment 8496191 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/src/jit/IonAnalysis.cpp
> @@ +3239,5 @@
> >      }
> > +
> > +    // If there's no path connecting the header to the backedge, then this isn't
> > +    // actually a loop. This can happen when the code starts with a loop but GVN
> > +    // folds some branches away.
> 
> I am not sure to understand what you mean by "fold - away"?

I meant folding an MTest into an MGoto. In the testcase, there's an MTest with a constant condition. GVN folds it to an MGoto to one of the successors, one that happens to be outside the loop, so that there's no longer any edge to a block that's inside the loop.
Attachment #8496191 - Flags: review?(nicolas.b.pierron)
Attachment #8496191 - Flags: review?(nicolas.b.pierron) → review+
https://hg.mozilla.org/mozilla-central/rev/c3bb5abdd01e
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.