Bug 183071 - [JSC] Implement $vm.ftlTrue function for FTL testing
Summary: [JSC] Implement $vm.ftlTrue function for FTL testing
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-02-22 20:15 PST by Yusuke Suzuki
Modified: 2018-02-27 01:36 PST (History)
6 users (show)

See Also:


Attachments
Patch (25.28 KB, patch)
2018-02-23 09:40 PST, Yusuke Suzuki
mark.lam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2018-02-22 20:15:16 PST
...
Comment 1 Mark Lam 2018-02-22 21:21:50 PST
Let's name it $vm.ftlTrue instead.
Comment 2 Yusuke Suzuki 2018-02-22 21:50:32 PST
(In reply to Mark Lam from comment #1)
> Let's name it $vm.ftlTrue instead.

Sounds good.
Comment 3 Yusuke Suzuki 2018-02-23 09:40:34 PST
Created attachment 334536 [details]
Patch
Comment 4 Mark Lam 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".
Comment 5 Yusuke Suzuki 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.
Comment 6 Yusuke Suzuki 2018-02-23 09:54:13 PST
Committed r228950: <https://trac.webkit.org/changeset/228950>
Comment 7 Radar WebKit Bug Importer 2018-02-23 09:55:20 PST
<rdar://problem/37829272>
Comment 8 Alexey Proskuryakov 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
Comment 9 Mark Lam 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.
Comment 10 Alexey Proskuryakov 2018-02-25 23:14:36 PST
Please do skip them then!
Comment 11 Yusuke Suzuki 2018-02-26 00:57:53 PST
Committed r228986: <https://trac.webkit.org/changeset/228986>
Comment 12 Yusuke Suzuki 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!
Comment 13 Yusuke Suzuki 2018-02-27 01:36:34 PST
Committed r229057: <https://trac.webkit.org/changeset/229057>