Closed Bug 643653 Opened 9 years ago Closed 9 years ago

TI: Incorrect results with compiled fasta benchmark

Categories

(Core :: JavaScript Engine, defect)

Other Branch
x86
Linux
defect
Not set

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: azakai, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(5 files)

Attached file fasta benchmark
The attachment is the fasta benchmark compiled to JavaScript. Running
it on jaegermonkey with -m and parameter |20| gives incorrect results when using the method JIT. The output without -m is valid.

This is similar and perhaps related to bug 643635.
Reduced:
--
function f(x) {
    var a;
    var b = [1].length;
    var c = x;
    var d = b + 1; 
    AA = x;
    a = d;
    assertEq(b + d, 3);
}
f();
--
This fails with -m -a:
test.js:8: Error: Assertion failed: got NaN, expected 3
Bug in FrameState::storeTop where we would allocate a register for the data in a frame entry and then immediately evict it.  I can swear this exact problem has been reported and fixed before, maybe I accidentally rebroke it and the tests didn't catch it.

Anyways, I get the same output now with no flags, -m -a, and -m -n.  -m -n -a is still broken.

Jan, how are you reducing these testcases?

http://hg.mozilla.org/projects/jaegermonkey/rev/13323fd7e400
(In reply to comment #2)
> Jan, how are you reducing these testcases?

I use Lithium (written by Jesse) and Delta. Lithium is a bit more intuitive and I use it to reduce crashes or hangs. I use Delta when the test case consists of multiple files or when I want to compare the output of two different engines. (jorendorff is working on a syntax-based reducer, it should give better results but in the meantime these tools are very useful)

To debug the remaining problem in this bug (different output with -m -a -n):

1) Paste the file in jsbeautifier.org (use "Braces on own line" and uncheck "Preserve empty lines"). This makes it easier to remove loops etc. (jsbeautifier.org is not bug-free though, sometimes the resulting file has a SyntaxError)

2) Use Delta (http://delta.tigris.org/) to reduce the file (I will attach delta-output.py):
  
$ delta/delta -test=delta-output.py -in_place test.js

It's important to repeat this step until Delta is unable to remove more lines. This reduces fasta from 1360 to less than 200 lines. With -m -n -a the file prints an error about an undefined variable, with -m -a we get no error.

3) Track down the source of the error - with -n print($pct_addr_i) prints 0 instead of NaN

4) Remove some blocks, inline some functions, run Delta again, etc.
Attached file Delta test script
Python script for delta - runs file in two engines and compare stdout and stderr. You have to replace PATH_TO_JS_SHELL with the path to the JS shell you want to test. If it's a long-running script you also have to tweak the TIMEOUT variable, it's set to 1 second to make the reduction faster.
Attached file Reduced test case
Reduced test case, prints 0 with -m -n -a instead of NaN.
Similar issue to the first fix, in storeTop if there was already a type register allocated for the entry then we could evict it while allocating the data entry.  I get the same output now with any combination of flags.

http://hg.mozilla.org/projects/jaegermonkey/rev/87cbe5b2742a
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Attached a build with slightly different options, where I get different behavior with |-m| vs |-m -n| (the latter is incorrect).
Same benchmark, different compilation options. |-m| is fine, while |-m -n| segfaults here.

I tested running this and the previous attachment with 10 as an argument.
Reopening this because of failures with |-n|, see last two comments.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Works for me at revision a58525f1f4be with |-m -n| and |-m -n -a| with argument 10.
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → WORKSFORME
The first and third fasta scripts attached here crash with |-m -n| and argument 1000, latest JM (--enable-optimize --disable-debug).

First prints some stuff out before crashing, third crashes very quickly. I can't get a useful stack trace in either.

The reduced testcase works ok. Second attached fasta script also works ok.

|-m -n -a| works ok, I only see a problem with |-m -n|.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
All attachments work for me at revision 728a27bccb86 with |-m -n| and |-m -n -a| in release and debug builds. Please reopen if this still crashes for you.
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.