Status

Tamarin
Virtual Machine
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: Alex Macdonald, Unassigned)

Tracking

Details

Attachments

(1 attachment)

12.80 KB, patch
Steven Johnson
: review+
Chris Peyer
: review+
Details | Diff | Splinter Review
(Reporter)

Description

8 years ago
User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_6_4; en-us) AppleWebKit/533.18.1 (KHTML, like Gecko) Version/5.0.2 Safari/533.18.5
Build Identifier: 

In preparation for open-sourcing the AOT opcode stub implementations this patch cleans up the rest of the AOT related code by:

-- fixing some compiler warnings
-- removing some dead code in Cdecl
-- properly ifdefing Cdecl with VMCFG_CDECL
-- adding a friendly error message if avmshell-aot is compiled without any aot compiled abcs
-- removing unnecessary AOTCompiler.h includes
-- fixing the avmfeatures aot spec to properly prevent AOT being turned on along with the JIT or interpreter


Reproducible: Always
(Reporter)

Comment 1

8 years ago
Created attachment 485976 [details] [diff] [review]
AOT tidyup
Attachment #485976 - Flags: review?(stejohns)
(Reporter)

Updated

8 years ago
Attachment #485976 - Flags: review?(cpeyer)

Comment 2

8 years ago
Comment on attachment 485976 [details] [diff] [review]
AOT tidyup

R+ for build files.
Attachment #485976 - Flags: review?(cpeyer) → review+

Updated

8 years ago
Attachment #485976 - Flags: review?(stejohns) → review+

Comment 3

8 years ago
maybe better late than never, but some documentation on the new AOT specific workaround functions would be very helpful, for example on:

hookUpActivationTraitsInitMethodForTraitsMethod() in Traits.cpp.

if there was a single doc under docs explaining "how it works" then the per-function comments should be brief.  I'd just like to see enough breadcrumbs so a person new to the code can find their way to a description of how the odd cases are handled, like activation traits, object initialization, etc.

(unrelated: the VM no longer generates ABC on the fly for object initialization.  Did this simplify AOT? one would think so).

Comment 4

8 years ago
TR 5402:8c05245736c6
Status: UNCONFIRMED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(Reporter)

Comment 5

8 years ago
(In reply to comment #3)
> maybe better late than never, but some documentation on the new AOT specific
> workaround functions would be very helpful, for example on:
> 
> hookUpActivationTraitsInitMethodForTraitsMethod() in Traits.cpp.
> 
> if there was a single doc under docs explaining "how it works" then the
> per-function comments should be brief.  I'd just like to see enough breadcrumbs
> so a person new to the code can find their way to a description of how the odd
> cases are handled, like activation traits, object initialization, etc.

absolutely, I'm working on a writeup of AOT as we speak.

> (unrelated: the VM no longer generates ABC on the fly for object
> initialization.  Did this simplify AOT? one would think so).

Yeah we had to synthesize init methods in the AOT compiler to handle just this case. If the VM now does that without generating abc then we can definitely take advantage of it.

Comment 6

8 years ago
(In reply to comment #5)
> Yeah we had to synthesize init methods in the AOT compiler to handle just this
> case. If the VM now does that without generating abc then we can definitely
> take advantage of it.

See BaseExecMgr::initObj(), called by initInterpGPR(), initInterpFPR(), initInvokeInterp(), and initInvokeInterpNoCoerce().  When calling an interpreted constructor, these stubs make the object get initialized first.  (same as what the generated abc used to do).

When we invoke a jit-compiled constructor, the field init assignments are still generated.  the AOT compiler could use initObj(), or could generate the initializing assignments like the JIT does, based on the traits. (come to think of it, maybe this doesn't simplify anything for AOT; at least abc generation is gone).
(Reporter)

Comment 7

8 years ago
(In reply to comment #6)
> When we invoke a jit-compiled constructor, the field init assignments are still
> generated.  the AOT compiler could use initObj(), or could generate the
> initializing assignments like the JIT does, based on the traits. (come to think
> of it, maybe this doesn't simplify anything for AOT; at least abc generation is
> gone).

so the jit isn't generating these initializers as abc, but it's still jitting code right? If that's the case we can't do whatever it is doing. We could definitely use initObj, but presumably that will be slower than just sticking with the methods we generate right now.

Comment 8

8 years ago
(In reply to comment #7)
> so the jit isn't generating these initializers as abc, but it's still jitting
> code right?

Right

> If that's the case we can't do whatever it is doing. 

You can't JIT-compile, but you can iterate over the concrete fields of the declaring type, and generate code at the beginning of the AOT-generated constructor, analogous to what the JIT does.

> We could definitely use initObj, but presumably that will be slower than
> just sticking with the methods we generate right now.

Agreed.  After writing comment #6 it just became obvious that the AOT compiler won't be improved after all.
(Reporter)

Comment 9

8 years ago
(In reply to comment #8)
> You can't JIT-compile, but you can iterate over the concrete fields of the
> declaring type, and generate code at the beginning of the AOT-generated
> constructor, analogous to what the JIT does.

yeah, we do exactly this, but of course we have to do this at compile time outside the vm as we can't execute generated code at runtime.

> 
> > We could definitely use initObj, but presumably that will be slower than
> > just sticking with the methods we generate right now.
> 
> Agreed.  After writing comment #6 it just became obvious that the AOT compiler
> won't be improved after all.

Although as Stan just pointed out, this probably wouldn't affect us right now as we have bigger performance bottlenecks in other places. It might however simplify the AOT compiler and make maintaining it easier.

It might also improve AOT compile times so it could be a useful addition to debug mode where we want to generate code as fast as possible.
You need to log in before you can comment on or make changes to this bug.