Closed Bug 755163 Opened 9 years ago Closed 9 years ago

Some jit tests fail with compiler optimizations disabled

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla15

People

(Reporter: Benjamin, Assigned: Benjamin)

Details

Attachments

(1 file, 1 obsolete file)

No description provided.
Specifically, configure with --disable-optimize and run jit tests to get

FAILURES:
    -m /home/benjamin/dev/repos/y/js/src/jit-test/tests/basic/bug529130.js
    -m -n /home/benjamin/dev/repos/y/js/src/jit-test/tests/basic/bug529130.js
TIMEOUTS:
OS: Mac OS X → All
It passes for me on 32-bit Windows and MacOS 10.6. What version of GCC are you using?
gcc 4.5.3 on Linux.
OS: All → Linux
The output looks like

|    0.0s1,2,3,4,5,6,7,8,9
1,2,3,4,5,6,7,8,9
1,2,3,4,5,6,7,8,9
1,2,3,4,5,6,7,8,9
1,2,3,4,5,6,7,8,9
1,2,3,4,5,6,7,8,9
1,2,3,4,5,6,7,8,9
1,2,3,4,5,6,7,8,9
1,2,3,4,5,6,7,8,9
1,2,3,4,5,6,7,8,9
1,2,3,4,5,6,7,8,9
1,2,3,4,5,6,7,8,9
1,2,3,4,5,6,7,8,9
1,2,3,4,5,6,7,8,9
1,2,3,4,5,6,7,8,9
1,2,3,4,5,6,7,8,9
1,2,3,4,5,6,7,8,9
1,2,3,4,5,6,7,8,9
1,2,3,4,5,6,7,8,9
1,2,3,4,5,6,7,8,9
1,2,3,4,5,6,7,8,9
1,2,3,4,5,6,7,8,9
1,2,3,4,5,6,7,8,9
/home/benjamin/dev/repos/y/js/src/jit-test/tests/basic/bug529130.js:6: InternalError: too much recursion
Exit code: 3
That's probably because the test recurs a lot, and usable recursion depth depends on the stack space consumed by the compiler.  I consider this test flaky and ignore failures from it, which quite arguably I shouldn't do.  Maybe we should actually fix the test to not recur so close to the overall limit that compiler-dependent stack sizes makes it fail?
Is this test even very useful? The code for the bug that it fixes is gone with tracing.
(In reply to Benjamin Peterson from comment #6)
> Is this test even very useful? The code for the bug that it fixes is gone
> with tracing.

Kill it with fire!
Attached patch kills the offending test (obsolete) — Splinter Review
Attachment #624252 - Attachment is obsolete: true
Attachment #624258 - Flags: review?(dmandelin)
Attachment #624258 - Flags: review?(dmandelin) → review?(jwalden+bmo)
Attachment #624258 - Flags: review?(jwalden+bmo) → review+
Is this the only test which exercises code which is no longer in use?  Should there be some audit activities to find out?
Keywords: checkin-needed
(In reply to Paul Wright from comment #10)
> Is this the only test which exercises code which is no longer in use? 
> Should there be some audit activities to find out?

I'm not sure what you mean. The code which this test tested is gone.
w00t!  Always did find that test annoying.
Assignee: general → bpeterson
Status: NEW → ASSIGNED
Keywords: checkin-needed
OS: Linux → All
Hardware: x86_64 → All
Target Milestone: --- → mozilla15
(In reply to Benjamin Peterson from comment #11)
> (In reply to Paul Wright from comment #10)
> > Is this the only test which exercises code which is no longer in use? 
> > Should there be some audit activities to find out?
> 
> I'm not sure what you mean. The code which this test tested is gone.

Exactly.  This test tested code which is now gone.  Are there any other tests which also test code which no longer exists?  Is there a way to audit the test suites to find out / clean them up?
https://hg.mozilla.org/mozilla-central/rev/e1406f8b5d54

Leaving open as it's not clear whether more work is planned for this bug or not. Please resolve it if there isn't.
Flags: in-testsuite-
No tests failure anymore, so this bug specifically is done.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.