Closed Bug 538538 Opened 10 years ago Closed 10 years ago

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


(Core Graveyard :: Nanojit, defect)

Not set


(Not tracked)



(Reporter: njn, Assigned: njn)



(Whiteboard: fixed-in-nanojit, fixed-in-tracemonkey)


(2 files)

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

  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.
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?
Attached patch NJ patchSplinter Review
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

- Pipes the result of the random tests to /dev/null so they don't clutter 
  the output.
Attachment #421584 - Flags: review?(graydon)
Attachment #421584 - Flags: review?(stejohns)
Attached patch TM patchSplinter Review
Attachment #421585 - Flags: review?(graydon)
Attachment #421584 - Attachment description: patch → NJ patch
Attachment #421584 - Flags: review?(graydon) → review+
Attachment #421585 - Flags: review?(graydon) → review+
Blocks: 539629
Nb: no need for a TR patch as TR doesn't use nanojit::compile().
Attachment #421584 - Flags: review?(stejohns) → review+
I totally botched that commit.  WinNT bustage fix:
Follow-up to add random-opt.{in,out}, which were in the patch but somehow didn't land:
Closed: 10 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.