Runtime support for Ahead-of-Time (AOT) Compilation



Virtual Machine
9 years ago
6 years ago


(Reporter: Stan Switzer, Unassigned)




(1 attachment)



9 years ago
User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv: Gecko/20091102 Firefox/3.5.5
Build Identifier: Tamarin Redux, 11 Nov 09

This patch updates the AVM runtime to support Ahead-of-Time compilation.  This involves:

 - Locating precompiled method code
 - Capturing traits so that they can be referred to by precompiled code.
 - Activation Traits for precompiled methods
 - Traits initialization that would normally be done by the verifier
 - Debugger support

These changes are separate from the fixed-slot-offset changes (which is a separate patch).

Reproducible: Always

Steps to Reproduce:
New feature

Comment 1

9 years ago
Created attachment 411870 [details] [diff] [review]
Proposed AOT support patch

Corresponds largely to Perforce changelist 601503 minus the slots changes.


9 years ago
Attachment #411870 - Flags: superreview?(edwsmith)
Attachment #411870 - Flags: review?(stejohns)

Comment 2

9 years ago
Comment on attachment 411870 [details] [diff] [review]
Proposed AOT support patch

No red flags, mostly just code style comments:

-- all new code should use C99 integer types where possible, avoiding  
"int"/"long"/"unsigned" unless absolutely required (typically, only for printf/scanf).

-- code style nit: preferred style in Tamarin is to avoid Hungarian  
warts ("nStateTraits" -> "stateTraitsCount" or similar)

-- AbcParser references some undefined vars, presumably constants declared by the AOT compiler… eg "nAOTInfos". New constants should ideally be declared with a "k" prefix or in all caps, eg "kAOTInfoCount" or "AOT_INFO_COUNT".

-- AbcParser::canParse: do we really want to use 0 as the "magic"  
value for AOT? 

-- if you want a function inlined, use REALLY_INLINE rather than plain  
"inline". (sad but necessary) (and yes, there are plenty of existing  
places this is violated in existing code, we're slowly migrating them  

-- code style nit: preferred style in Tamarin is "if (cond)", not "if 

-- code style nit: preferred style in Tamarin is to use NULL for null  
pointers (vs plain "0")

-- code style nit: if an argument is being passed by a pointer-that-can't-ever-be-null, use a reference rather than a pointer (eg NativeInitializer::getCompiledInfo)… yes, even for "out" parameters.
Attachment #411870 - Flags: review?(stejohns) → review+

Comment 3

9 years ago
Comment on attachment 411870 [details] [diff] [review]
Proposed AOT support patch

MathUtils.cpp: intentional deletion of code?

MethodInfo.h: when code is rebased, inline method bodies should all be in MethodInfo-inlines.h

Remainder looks safe (everything looks carefully ifdef'd) but I haven't reviewed the aot-code carefully.  It really needs a bit of documentation to explain the larger context.
Attachment #411870 - Flags: superreview?(edwsmith) → superreview+

Comment 4

9 years ago
pushed (with minor tweaks) as changeset:   3136:7b6e3a3be69d
Last Resolved: 9 years ago
Resolution: --- → FIXED

Comment 5

9 years ago
Responding to comments...

nAOTInfos -> kAOTInfoCount, OK.  AHould be defined in AOTCompiled.h which I accidentally dropped from the patch (dropped all additions... driver error).

zero as a magic number?  Well, since the ABC bodies are built into the binary, there can never be version skew. It corresponds to no loadable version of the ABC format.  (Consists only of the runtime metadata and no code bodies.)  Zero makes a certain sense to me, but YMMD.

I think in some cases we don't care if it really inlines and are happy to give the compiler the choice.  But if a function (as opposed to a method) is defined in a header, it needs to say inline or else the linker will see multiple definitions.

"if (cond)", not "if (cond)"  ? Sorry, confused.

"yes, even for "out" parameters."  Surely you jest!  Well, when in Rome, I guess.

Shall we put these on a list to revise next go-round?

Also, arguably the private version of ScopeChain::create I introduced should be public and the other two versions that call it should be removed.  I did it this way to minimize the number of changes that were not _directly_ AOT-related.  Making that change touches half a dozen files and so I decided better of it, but it would address the FIXME referring to the use of FrameState in this function.

MathUtils.cpp:  Pops, sorry, that was unrelated.  Some freelance cleanup.  I gasped in surprise at "#define NAN acosh(0)" and then discovered it was never used.  So I deleted it.

Comment 6

8 years ago
Engineering work item.  Marking as verified.
(Felix: adding reference to pushed changeset long after it was pushed.)

changeset:   3145:7b6e3a3be69d
parent:      3135:f454ce7466f4
user:        Steven Johnson <>
date:        Mon Nov 16 13:56:02 2009 -0800
summary:     add runtime support for Ahead-of-Time compilation. (r=stejohns, sr=edwsmith, bug=528111)
You need to log in before you can comment on or make changes to this bug.