Closed Bug 478301 Opened 12 years ago Closed 12 years ago
TM: housekeeping the Assembler and LIR classes
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.
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.