RESOLVED FIXED 183071
[JSC] Implement $vm.ftlTrue function for FTL testing
https://bugs.webkit.org/show_bug.cgi?id=183071
Summary [JSC] Implement $vm.ftlTrue function for FTL testing
Yusuke Suzuki
Reported 2018-02-22 20:15:16 PST
...
Attachments
Patch (25.28 KB, patch)
2018-02-23 09:40 PST, Yusuke Suzuki
mark.lam: review+
Mark Lam
Comment 1 2018-02-22 21:21:50 PST
Let's name it $vm.ftlTrue instead.
Yusuke Suzuki
Comment 2 2018-02-22 21:50:32 PST
(In reply to Mark Lam from comment #1) > Let's name it $vm.ftlTrue instead. Sounds good.
Yusuke Suzuki
Comment 3 2018-02-23 09:40:34 PST
Mark Lam
Comment 4 2018-02-23 09:46:58 PST
Comment on attachment 334536 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=334536&action=review r=me with suggestions. > Source/JavaScriptCore/jsc.cpp:-292 > static EncodedJSValue JSC_HOST_CALL functionFalse1(ExecState*); > -static EncodedJSValue JSC_HOST_CALL functionFalse2(ExecState*); You can rename functionFalse1 to functionFalse now that there's only one. > Source/JavaScriptCore/jsc.cpp:518 > - putDirectNativeFunction(vm, this, Identifier::fromString(&vm, "isFinalTier"), 0, functionFalse2, IsFinalTierIntrinsic, static_cast<unsigned>(PropertyAttribute::DontEnum)); > + putDirectNativeFunction(vm, this, Identifier::fromString(&vm, "isFinalTier"), 0, functionFalse1, IsFinalTierIntrinsic, static_cast<unsigned>(PropertyAttribute::DontEnum)); Ditto. Use functionFalse. > Source/JavaScriptCore/jsc.cpp:-1831 > EncodedJSValue JSC_HOST_CALL functionFalse1(ExecState*) { return JSValue::encode(jsBoolean(false)); } > -EncodedJSValue JSC_HOST_CALL functionFalse2(ExecState*) { return JSValue::encode(jsBoolean(false)); } Ditto. Use functionFalse. > JSTests/stress/has-indexed-property-array-storage-ftl.js:8 > +var flag = false; I suggest renaming "flag" to "didFTLCompile". > JSTests/stress/has-indexed-property-slow-put-array-storage-ftl.js:8 > +var flag = false; I suggest renaming "flag" to "didFTLCompile".
Yusuke Suzuki
Comment 5 2018-02-23 09:50:40 PST
Comment on attachment 334536 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=334536&action=review Thanks! >> Source/JavaScriptCore/jsc.cpp:-292 >> -static EncodedJSValue JSC_HOST_CALL functionFalse2(ExecState*); > > You can rename functionFalse1 to functionFalse now that there's only one. Nice, fixed. >> Source/JavaScriptCore/jsc.cpp:518 >> + putDirectNativeFunction(vm, this, Identifier::fromString(&vm, "isFinalTier"), 0, functionFalse1, IsFinalTierIntrinsic, static_cast<unsigned>(PropertyAttribute::DontEnum)); > > Ditto. Use functionFalse. Fixed. >> Source/JavaScriptCore/jsc.cpp:-1831 >> -EncodedJSValue JSC_HOST_CALL functionFalse2(ExecState*) { return JSValue::encode(jsBoolean(false)); } > > Ditto. Use functionFalse. Fixed. >> JSTests/stress/has-indexed-property-array-storage-ftl.js:8 >> +var flag = false; > > I suggest renaming "flag" to "didFTLCompile". Nice. Changed. >> JSTests/stress/has-indexed-property-slow-put-array-storage-ftl.js:8 >> +var flag = false; > > I suggest renaming "flag" to "didFTLCompile". Changed.
Yusuke Suzuki
Comment 6 2018-02-23 09:54:13 PST
Radar WebKit Bug Importer
Comment 7 2018-02-23 09:55:20 PST
Alexey Proskuryakov
Comment 8 2018-02-23 14:51:46 PST
32-bit test failures on new tests: https://build.webkit.org/builders/Apple%20High%20Sierra%2032-bit%20JSC%20%28BuildAndTest%29/builds/1263/steps/webkit-32bit-jsc-test/logs/stdio stress/has-indexed-property-array-storage-ftl.js.misc-ftl-no-cjit: Exception: Error: bad value: false stress/has-indexed-property-array-storage-ftl.js.misc-ftl-no-cjit: shouldBe@has-indexed-property-array-storage-ftl.js:5:24 stress/has-indexed-property-array-storage-ftl.js.misc-ftl-no-cjit: global code@has-indexed-property-array-storage-ftl.js:22:9 stress/has-indexed-property-array-storage-ftl.js.misc-ftl-no-cjit: ERROR: Unexpected exit code: 3
Mark Lam
Comment 9 2018-02-23 14:53:10 PST
(In reply to Alexey Proskuryakov from comment #8) > 32-bit test failures on new tests: the FTL is not supported on 32-bit. Hence, these are guaranteed to fail. We should skip these tests on 32-bit.
Alexey Proskuryakov
Comment 10 2018-02-25 23:14:36 PST
Please do skip them then!
Yusuke Suzuki
Comment 11 2018-02-26 00:57:53 PST
Yusuke Suzuki
Comment 12 2018-02-26 03:22:09 PST
(In reply to Yusuke Suzuki from comment #11) > Committed r228986: <https://trac.webkit.org/changeset/228986> OK, skipped!
Yusuke Suzuki
Comment 13 2018-02-27 01:36:34 PST
Note You need to log in before you can comment on or make changes to this bug.