Closed Bug 478301 Opened 12 years ago Closed 12 years ago

TM: housekeeping the Assembler and LIR classes

Categories

(Core :: JavaScript Engine, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED WONTFIX

People

(Reporter: n.nethercote, Unassigned)

References

Details

Attachments

(1 file)

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.
Here's the patch.
Attachment #362140 - Flags: review?(edwsmith)
Attachment #362140 - Flags: review?(edwsmith) → review+
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.
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.
Status: NEW → ASSIGNED
Depends on: 488775
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.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.