TT: add early and optimistic binding

VERIFIED FIXED

Status

Tamarin
Virtual Machine
VERIFIED FIXED
11 years ago
9 years ago

People

(Reporter: Edwin Smith, Assigned: Edwin Smith)

Tracking

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

11 years ago
tamarin-central supports early binding based on static analysis.  TT has the same analysis code but ignores the result.  in addition TT could optimistically early bind by inserting guards that assume the type of a reference isn't changing at a given call site.
(Assignee)

Comment 1

11 years ago
Created attachment 295832 [details] [diff] [review]
Add early and optimistic binding 

First draft for review

* optimize array somewhat, redo hot native methods in forth (callnative_x_glue is slow)
* verifier modifies abc in-place
* getglobal0 and getbinding0 renamed to getglobal2/getbinding2
* getbinding0 added (no rt args), guardConstant used on traitsEnv arg
* rewrite tobool in forth
* cktotraitsenv turned into CASE and merged with totraitsenv
(Assignee)

Updated

11 years ago
Attachment #295832 - Flags: review?(stejohns)

Comment 2

11 years ago
Initial thoughts:
-- on the whole, looks good (I didn't try to verify the accuracy of the rewriting in Verifier)
-- modifying ABC in place sounds good; longterm we should figure out how to pre-modify the builtin ABC glue to avoid memory duplication & speed startup
-- AvmHash64 is enabled, what's the speedup? (Before it was insignificant by my measurement)
-- MAXTRACE/HOTLOOP/HOTEXIT are modified, is this intentional?
-- in the ABC rewriting code in Verifier, I wonder if we should collapse or bottleneck it somehow to allow for verification that the rewriting is always <= the original size? (rather than various *wpc++ statements inserted in various spots)
(Assignee)

Comment 3

11 years ago
* MAXTRACE/HOT*, yes intentional, experimenting with settings.  

* the verifier rewrites opcodes in place, we don't actually rewrite the whole method body.  doing the latter would also require recalculating branch offsets, outt of scope for this change.  will create a separate bug to track.

* AvmHash64 speedup is negligible, will disable.

(Assignee)

Comment 4

11 years ago
Created attachment 296150 [details] [diff] [review]
Merged & incorporated review feedback
Attachment #295832 - Attachment is obsolete: true
Attachment #296150 - Flags: review?(stejohns)
Attachment #295832 - Flags: review?(stejohns)

Comment 5

11 years ago
is this redundant code in _fixstr?

+: _fixstr  ( name -- name' )
    isobject IF
        isqname_obj NOT IF
+       tostring sbox
            tostring sbox
        THEN
    THEN ;

otherwise looks good.

Comment 6

11 years ago
Comment on attachment 296150 [details] [diff] [review]
Merged & incorporated review feedback

aside from the redundant-but-probably-harmless code in _fixstr, looks good
Attachment #296150 - Flags: review?(stejohns) → review+
(Assignee)

Comment 7

11 years ago
(In reply to comment #5)
> is this redundant code in _fixstr?

yes, fixed.

Still working on the verify-once flag, then ready to go.
(Assignee)

Comment 8

11 years ago
pushed
http://hg.mozilla.org/tamarin-tracing/?rev/1bf60b9177bd
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED

Updated

9 years ago
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.