457038 - Merge tamarin-redux into tamarin-central

Status: VERIFIED FIXED

(Tamarin Graveyard :: Virtual Machine, defect)

Description

[Edwin Smith]

The tamarin-redux dev branch includes:

Interpreter:
- lots of interpreter optimizations, inlining fast path, etc
- abc-to-wordcode translator, and wordcode interpreter
- table driven state-machine based peephole optimizer
- optional direct-threaded dispatch
- instrumentation & utilities for finding superoperator opcodes

JIT:
- integrates nanojit from tamarin-tracing branch
- adds branch & stack allocation opcodes to nanojit
- better register allocation
- adds VTune support for nanojit
- smaller prolog/epilog sequences
- supports other x86 calling conventions (cdecl, thiscall, fastcall)

General:
- integrates vprof utility from tamarin-tracing

Initial cleanup patch has landed, this is a tracking bug for the rest, which will be a big patch.

see:
http://hg.mozilla.org/users/edwsmith_adobe.com/tamarin-redux/

Comment 1

[Edwin Smith]

Attachment: first draft for review

This patch is the code changes.  Next patch is the extras -- project files, utils, nanojit & vprof additions.

Comment 2

[Edwin Smith]

Attachment: additional new files and build changes

Comment 3

[Steven Johnson]

Comment on attachment 340399 [details] [diff] [review]
first draft for review

Looks good with minor reservations that can be corrected in subsequent patches:

CodegenLIR has code to support Mac Carbon builds — has it been tested? Will it ever be used? If not, perhaps we should remove it.

CodegenLIR.cpp:2072 has a #ifdef AVMPLUS_AMD64 that I suspect should be #ifdef AVMPLUS_64BIT. 

where/how is vm_fops.h generated? Or is it hand-created? If the former, source for generator would be good; if the latter, the standard license boilerplate at the top is appropriate

looks like some #ifdef AVMPLUS_PROFILE code crept back into CodegenMIR.h (it was all expunged recently).

minor naming question: do we want to use FEATURE_NANOJIT as the flag for nanojit? Virtually every other preprocessor flag we use it prefaced with AVMPLUS_ rather than FEATURE_ (and IMHO that’s a good convention to use)

semi-nit: class JITCodeInfo isn’t sorted for minimal padding in 64-bit builds (starts with a uint32_t followed by a pointer, meaning a 32-bit hole results). (not sure if we allocate enough for it to matter much, but grouping & sorting the data fields of any non-singleton structure is a good idea to avoid this sort of thing)

nit: some new/revised code is using uint32, sintptr, etc. instead of C99 types.

nit: in avmshell, the comment for –forcejit says “deprecated, use forcejit”, but the new flag seems to be named “Ojit”...

super duper spelling nit: “interruptable” is misspelled (should be “interruptible”)

The comment explaining the peephole optimizer is a thing of beauty: Lars, you get a gold star :-)

nullterm?
interp_fop

Comment 4

[Steven Johnson]

Oops, left some cruft in my previous comment: is StringNullTerminatedUTF8 actually used anywhere? It doesn't appear to be used in this patch, is it just a cut-n-paste error from TT?

Comment 5

[Steven Johnson]

Comment on attachment 340400 [details] [diff] [review]
additional new files and build changes

Only one serious issue: the Mac XCode projects were changed to build only the current architecture (rather than always building i386 and ppc). At least one client of the projects (FlashPlayer) needs these projects to build both architectures; unless there's a compelling reason to not build both all the time, we should back this change out.

Minor quibbles:

-- nanojit files appear to be a weird mishmash of spaces and tabs; would be good to normalize these.

-- nanojit uses a variety of prefixes for macros: NANOJIT_ NJ_ Nano etc... would be good to move towards standardizing IMHO

-- Now might be a good time to remove all Visual Studio projects earlier than VC2008: AFAIK no one is using them nor are any major contributors committed to supporting them.

Comment 6

[Edwin Smith]

- StringNullTerminatedUTF is used within nanojit/LIR.cpp

- this patch won't touch (edit/add/remove) VS2005 files

- the nanojit module is standalone (like mmgc) so it relies on FEATURE_NANOJIT.  we could add another AVMPLUS_NANOJIT switch, but it just turn on FEATURE_NANOJIT internally.  added as integration task.  we also need an AVMPLUS_JIT switch that doesn't imply MIR or NANOJIT, and embedders should use that instead.

- I will leave cosmetic issues alone in nanojit until nanojit merging activity settles down (space/tabs, preprocessor identifier names)

- sintptr and uint32 fixed in new code

- JITCodeInfo rearranged

pushed changeset:   945:8290c358487d

Comment 7

[Steven Johnson]

Got it. perhaps my cosmetic comments on NJ should be redirected to a changelist specific to NJ?