478301 - TM: housekeeping the Assembler and LIR classes

Status: RESOLVED WONTFIX

(Core :: JavaScript Engine, defect)

Description

[Nicholas Nethercote [inactive]]

This patch does some simple housekeeping, mostly in the Assembler and LIR classes. It's a small step towards making the use of the _err field in Assembler less pathologically convoluted, which was my original aim but I found too difficult for now.

- Moves compile() from LIR.cpp into the Assembler class, a much better spot for
  it.  This allows various public Assembler methods (beginAssembly(), etc) to be
  made private.  Various other Assembler members were made private too
  (codeBytes(), handoverPages(), hasLoop, etc).

- Moved Assembler::findVictim from RegAlloc.cpp (an odd place for it) into
  Assembler.cpp (the obvious place for it).

- Removed dead code:
  - Assembler::_functions and LirNameMap::_functions
  - Assembler::nFrameRestore
  - Assembler::setCallTable
  - A couple of unnecessary declarations

- In pageValidate() and resourceConsistencyCheck(), abort on prior error,
  rather than skipping;  this gives more information, because it establishes
  an invariant (and prior error should be impossible due to the calling
  context).

- In LIR.cpp, a lot of small functions were duplicated, with one global
  version which took an LOpcode as an arg, and one version within class
  LIns.  Mostly the global versions were only called from the class version,
  so I inlined them.

This is all mechanical stuff, there is (should be) no change in
functionality.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: housekeeping patch

Here's the patch.

Comment 2

[Edwin Smith]

Comment on attachment 362140 [details] [diff] [review]
housekeeping patch

patch is fine as is

as for future direction

- we need an attribute table for instructions to clean up all the various category predicates (isCond, isFloat, isQuad, etc).

- tamarin wasn't able to use compile() as-is, so we call beginAssembly, etc, directly.  no suggestions for now, but one thing i think we both need is for Assembler to to be factored so its just the last stage in the pipeline, and for the bottom-up pipeline to be more customizeable.

Comment 3

[Nicholas Nethercote [inactive]]

This will clash with the widen-LIR patch (#488775) which is more important.  So I'm marking this as dependent on #488775, it can be rebased once that has landed.

Comment 4

[Nicholas Nethercote [inactive]]

This patch is pretty wretched, combining a bunch of not-very-related things into one.  I'm going to close it and open multiple, more focussed bugs.