Closed
Bug 538538
Opened 14 years ago
Closed 14 years ago
lirasm: don't run the optimizers, except when using --random
Categories
(Core Graveyard :: Nanojit, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: n.nethercote, Assigned: n.nethercote)
References
Details
(Whiteboard: fixed-in-nanojit, fixed-in-tracemonkey)
Attachments
(2 files)
17.01 KB,
patch
|
graydon
:
review+
stejohns
:
review+
|
Details | Diff | Splinter Review |
1.80 KB,
patch
|
graydon
:
review+
|
Details | Diff | Splinter Review |
Currently lirasm runs ExprFilter and CseFilter in the writer pipeline, and StackFilter in the reader pipeline. I think it shouldn't run any of them -- it's an odd sort of assembler that optimizes your code for you. Furthermore, I have a specific example where this optimization is causing problems. For bug 521161 Jacob introduced several lirasm tests that test overflow and multiplication. Here's some code from mul_xxx.in: big = int 46340 res = mul big big check = ov res ExprFilter statically evaluates the multiplication so it ends up as this: 2147395600 = int 2147395600 ov1 = ov 2147395600 You can see this by running lirasm --verbose. There are two problems here. First, this code is bogus -- 'ov' should only be applied to neg/add/sub/mul -- so the value assigned to 'check' is unpredictable. Second, this means the test isn't actually testing what it's supposed to be testing. (FWIW, I found this problem with the revamped LIR checker, see bug 463137.) Turning off ExprFilter and CseFilter is easy. Turning off StackFilter is harder because it's set-up within Nanojit itself, not within lirasm. Maybe we could pass a boolean argument to ::compile(). A further complication is that for --random mode we want the optimizers on, because it's meant to stress the code like the real compilers do. This won't be too hard to do.
Comment 1•14 years ago
|
||
IMHO it would be useful to be able to run --random both with and without the optimizers. Perhaps optimization could be added as a separate command-line flag?
Assignee | ||
Comment 2•14 years ago
|
||
This patch: - Adds --optimize and --no-optimize options to lirasm. --no-optimize is the default. This required threading an argument through various places. - Makes the building of reader pipelines more consistent. - Adds a '--random 1000000 --optimize' invocation with 'make check' (required in two spots, because we currently do that in the Makefile and in testlirc.sh). - Pipes the result of the random tests to /dev/null so they don't clutter the output.
Attachment #421584 -
Flags: review?(graydon)
Assignee | ||
Updated•14 years ago
|
Attachment #421584 -
Flags: review?(stejohns)
Assignee | ||
Comment 3•14 years ago
|
||
Attachment #421585 -
Flags: review?(graydon)
Assignee | ||
Updated•14 years ago
|
Attachment #421584 -
Attachment description: patch → NJ patch
Updated•14 years ago
|
Attachment #421584 -
Flags: review?(graydon) → review+
Updated•14 years ago
|
Attachment #421585 -
Flags: review?(graydon) → review+
Assignee | ||
Comment 4•14 years ago
|
||
Nb: no need for a TR patch as TR doesn't use nanojit::compile().
Updated•14 years ago
|
Attachment #421584 -
Flags: review?(stejohns) → review+
Assignee | ||
Comment 5•14 years ago
|
||
http://hg.mozilla.org/projects/nanojit-central/rev/7ab1e0842f7d
Whiteboard: fixed-in-nanojit
Assignee | ||
Comment 6•14 years ago
|
||
I totally botched that commit. WinNT bustage fix: http://hg.mozilla.org/projects/nanojit-central/rev/f2f5796b620e
Assignee | ||
Comment 7•14 years ago
|
||
http://hg.mozilla.org/tracemonkey/rev/b6a90ee08524 http://hg.mozilla.org/tracemonkey/rev/67967239b556 http://hg.mozilla.org/tracemonkey/rev/2914418e48da
Whiteboard: fixed-in-nanojit → fixed-in-nanojit, fixed-in-tracemonkey
Assignee | ||
Comment 8•14 years ago
|
||
Follow-up to add random-opt.{in,out}, which were in the patch but somehow didn't land: http://hg.mozilla.org/projects/nanojit-central/rev/874d96dc4481
Assignee | ||
Comment 9•14 years ago
|
||
http://hg.mozilla.org/tracemonkey/rev/a583c659d138
Comment 10•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/b6a90ee08524
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Updated•10 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•