538538 - lirasm: don't run the optimizers, except when using --random

Status: RESOLVED FIXED

(Core Graveyard :: Nanojit, defect)

Description

[Nicholas Nethercote [inactive]]

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

[Steven Johnson]

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?

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: NJ patch

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.

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: TM patch

Comment 4

[Nicholas Nethercote [inactive]]

Nb: no need for a TR patch as TR doesn't use nanojit::compile().

Comment 5

[Nicholas Nethercote [inactive]]

http://hg.mozilla.org/projects/nanojit-central/rev/7ab1e0842f7d

Comment 6

[Nicholas Nethercote [inactive]]

I totally botched that commit.  WinNT bustage fix:

http://hg.mozilla.org/projects/nanojit-central/rev/f2f5796b620e

Comment 7

[Nicholas Nethercote [inactive]]

http://hg.mozilla.org/tracemonkey/rev/b6a90ee08524
http://hg.mozilla.org/tracemonkey/rev/67967239b556
http://hg.mozilla.org/tracemonkey/rev/2914418e48da

Comment 8

[Nicholas Nethercote [inactive]]

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

Comment 9

[Nicholas Nethercote [inactive]]

http://hg.mozilla.org/tracemonkey/rev/a583c659d138

Comment 10

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/b6a90ee08524