| Summary: | FTL should support GetById(Untyped:) | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Filip Pizlo <fpizlo> | ||||||||||
| Component: | JavaScriptCore | Assignee: | Filip Pizlo <fpizlo> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | barraclough, ggaren, mark.lam, mhahnenberg, mmirman, msaboff, nrotem, oliver, sam | ||||||||||
| Priority: | P2 | ||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||
| Hardware: | All | ||||||||||||
| OS: | All | ||||||||||||
| Bug Depends on: | |||||||||||||
| Bug Blocks: | 127830, 127746 | ||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Filip Pizlo
2014-01-27 18:12:29 PST
Created attachment 222555 [details]
work in progress
Heh, this reveals some bugs: ** The following JSC stress test failures have been introduced: internal-js-tests.yaml/V8v7/deltablue.js.ftl-no-cjit internal-js-tests.yaml/V8v7/deltablue.js.default-ftl sunspider-1.0/access-fannkuch.js.ftl-no-cjit-validate sunspider-1.0/access-fannkuch.js.ftl-no-cjit-osr-validation sunspider-1.0/access-fannkuch.js.ftl-eager-no-cjit sunspider-1.0/access-fannkuch.js.ftl-eager-no-cjit-osr-validation internal-js-tests.yaml/Kraken/stanford-crypto-aes.js.ftl-no-cjit v8-v6/v8-deltablue.js.ftl-eager-no-cjit v8-v6/v8-deltablue.js.ftl-eager-no-cjit-osr-validation I am investigating. Lol. The bug is that LLVM may duplicate code that does patchpoints and stackmaps. Our code gets confused by this, because we incorrectly assume that for a patchpoint/stackmap that we emit in IR, LLVM will only generate one copy in the resulting code. That's clearly bogus. This will require some fun reworking. Since GetById(Untyped:) is relative straight-forward, I think I will just include the fix in the patch. It'll take a while. Created attachment 222624 [details]
more
Working on a fix
Created attachment 222626 [details]
the patch
Created attachment 222629 [details]
the patch
Comment on attachment 222629 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=222629&action=review > Source/JavaScriptCore/ftl/FTLStackMaps.h:91 > + typedef HashMap<uint32_t, Vector<Record>, WTF::IntHash<uint32_t>, WTF::UnsignedWithZeroKeyHashTraits<uint32_t>> RecordMap; Since the common case is one entry, can we use Vector<Record, 1>? That will substantially reduce malloc traffic, with very little space overhead. (In reply to comment #7) > (From update of attachment 222629 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=222629&action=review > > > Source/JavaScriptCore/ftl/FTLStackMaps.h:91 > > + typedef HashMap<uint32_t, Vector<Record>, WTF::IntHash<uint32_t>, WTF::UnsignedWithZeroKeyHashTraits<uint32_t>> RecordMap; > > Since the common case is one entry, can we use Vector<Record, 1>? That will substantially reduce malloc traffic, with very little space overhead. Good point! I will make that change. Landed in http://trac.webkit.org/changeset/163119 |