Bug 183756

Summary:

Improve FunctionPtr and use it in the JIT CallRecord.

Product:

WebKit

Reporter:

Mark Lam <mark.lam>

Component:

JavaScriptCore

Assignee:

Mark Lam <mark.lam>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ews-watchlist, fpizlo, jfbastien, keith_miller, msaboff, rmorisset, saam, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
proposed patch.	ews-watchlist: commit-queue-
proposed patch w/ 32-bit build fix.	jfbastien: review+, ews-watchlist: commit-queue-
patch for landing.	none

Mark Lam

Reported 2018-03-19 16:40:42 PDT

Patch coming.

Attachments
proposed patch. (48.84 KB, patch) 2018-03-19 16:46 PDT, Mark Lam	ews-watchlist: commit-queue-	Details Formatted Diff Diff
proposed patch w/ 32-bit build fix. (50.34 KB, patch) 2018-03-19 18:17 PDT, Mark Lam	jfbastien: review+ ews-watchlist: commit-queue-	Details Formatted Diff Diff
patch for landing. (50.71 KB, patch) 2018-03-19 22:49 PDT, Mark Lam	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2018-03-19 16:41:32 PDT

<rdar://problem/38641335>

Mark Lam

Comment 2 2018-03-19 16:46:05 PDT

Created attachment 336084 [details] proposed patch.

EWS Watchlist

Comment 3 2018-03-19 18:04:55 PDT

Comment on attachment 336084 [details] proposed patch. Attachment 336084 [details] did not pass jsc-ews (mac): Output: http://webkit-queues.webkit.org/results/7031415 New failing tests: stress/arith-atanh-on-various-types.js.no-llint stress/arith-log10-on-various-types.js.no-ftl stress/arith-cosh-on-various-types.js.no-llint stress/arith-round-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-asinh-on-various-types.js.no-ftl stress/arith-atanh-on-various-types.js.no-cjit-validate-phases stress/arith-atanh-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-ceil-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-fround-on-various-types.js.no-llint stress/arith-sqrt-on-various-types.js.default stress/arith-tan-on-various-types.js.no-ftl stress/arith-sqrt-on-various-types.js.no-llint stress/arith-acos-on-various-types.js.no-cjit-collect-continuously stress/arith-atanh-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-log2-on-various-types.js.no-cjit-collect-continuously stress/arith-trunc-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-cos-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-acosh-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-asin-on-various-types.js.no-cjit-collect-continuously stress/arith-log10-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-acos-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-cbrt-on-various-types.js.no-cjit-collect-continuously stress/arith-clz32-on-various-types.js.no-ftl stress/arith-log10-on-various-types.js.no-llint stress/arith-negate-on-various-types.js.no-llint stress/arith-log10-on-various-types.js.default stress/arith-sqrt-on-various-types.js.no-cjit-collect-continuously stress/arith-sin-on-various-types.js.ftl-no-cjit-validate-sampling-profiler jsc-layout-tests.yaml/js/script-tests/dfg-proven-sqrt-backwards-propagation.js.layout-no-ftl stress/arith-asinh-on-various-types.js.no-cjit-collect-continuously stress/arith-negate-on-various-types.js.no-cjit-validate-phases stress/arith-cbrt-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-fround-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-tanh-on-various-types.js.default stress/arith-atan-on-various-types.js.default stress/arith-abs-on-various-types.js.no-cjit-validate-phases stress/arith-log2-on-various-types.js.no-llint stress/arith-sin-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-expm1-on-various-types.js.no-cjit-validate-phases stress/arith-atan-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-asin-on-various-types.js.default stress/arith-floor-on-various-types.js.no-cjit-validate-phases stress/arith-round-on-various-types.js.no-cjit-collect-continuously stress/arith-cosh-on-various-types.js.no-ftl stress/arith-abs-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-acos-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-floor-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-trunc-on-various-types.js.no-cjit-collect-continuously stress/arith-ceil-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-sinh-on-various-types.js.no-ftl jsc-layout-tests.yaml/js/script-tests/dfg-proven-sqrt-backwards-propagation.js.layout-dfg-eager-no-cjit stress/arith-atan-on-various-types.js.no-cjit-collect-continuously stress/arith-tanh-on-various-types.js.no-cjit-validate-phases stress/arith-tan-on-various-types.js.no-llint stress/arith-sin-on-various-types.js.no-cjit-validate-phases stress/arith-expm1-on-various-types.js.no-llint stress/arith-negate-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-trunc-on-various-types.js.no-llint stress/arith-floor-on-various-types.js.no-ftl stress/arith-atan-on-various-types.js.no-ftl stress/arith-negate-on-various-types.js.no-ftl stress/arith-atan-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-acos-on-various-types.js.no-llint stress/arith-atanh-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-acos-on-various-types.js.no-ftl stress/arith-sinh-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-cos-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-cos-on-various-types.js.no-cjit-validate-phases stress/arith-expm1-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-expm1-on-various-types.js.no-ftl stress/arith-cos-on-various-types.js.no-cjit-collect-continuously stress/arith-asin-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-sinh-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-acos-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-ceil-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-sinh-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-log2-on-various-types.js.ftl-no-cjit-b3o1 jsc-layout-tests.yaml/js/script-tests/dfg-proven-sqrt-backwards-propagation.js.layout-ftl-eager-no-cjit stress/arith-log2-on-various-types.js.no-cjit-validate-phases stress/arith-log2-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-sqrt-on-various-types.js.no-ftl stress/arith-expm1-on-various-types.js.default stress/arith-cos-on-various-types.js.no-ftl stress/arith-sqrt-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-acosh-on-various-types.js.no-cjit-validate-phases stress/arith-acosh-on-various-types.js.no-llint stress/arith-clz32-on-various-types.js.no-cjit-validate-phases stress/arith-abs-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-asinh-on-various-types.js.ftl-no-cjit-no-inline-validate jsc-layout-tests.yaml/js/script-tests/dfg-proven-sqrt-backwards-propagation.js.layout-no-llint stress/arith-asin-on-various-types.js.no-llint stress/arith-atanh-on-various-types.js.default stress/arith-fround-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-ceil-on-various-types.js.no-cjit-validate-phases stress/arith-trunc-on-various-types.js.default stress/arith-asinh-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-ceil-on-various-types.js.no-llint stress/arith-round-on-various-types.js.no-llint stress/arith-cosh-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-fround-on-various-types.js.no-cjit-collect-continuously stress/arith-round-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-sqrt-on-various-types.js.no-cjit-validate-phases stress/arith-acos-on-various-types.js.default stress/arith-sin-on-various-types.js.no-ftl stress/arith-asin-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-tanh-on-various-types.js.no-cjit-collect-continuously stress/arith-log10-on-various-types.js.no-cjit-validate-phases stress/arith-tanh-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-floor-on-various-types.js.default stress/arith-atan-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-expm1-on-various-types.js.no-cjit-collect-continuously stress/arith-fround-on-various-types.js.no-cjit-validate-phases stress/arith-ceil-on-various-types.js.default stress/arith-cosh-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-floor-on-various-types.js.no-llint stress/arith-log-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-tan-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-clz32-on-various-types.js.default stress/arith-clz32-on-various-types.js.no-llint stress/arith-cosh-on-various-types.js.no-cjit-validate-phases stress/arith-asin-on-various-types.js.no-cjit-validate-phases stress/arith-cos-on-various-types.js.no-llint stress/arith-tanh-on-various-types.js.no-llint stress/arith-cbrt-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-asinh-on-various-types.js.default stress/arith-acosh-on-various-types.js.default stress/arith-sin-on-various-types.js.no-cjit-collect-continuously stress/arith-floor-on-various-types.js.no-cjit-collect-continuously stress/arith-sinh-on-various-types.js.no-cjit-validate-phases stress/arith-sinh-on-various-types.js.default stress/arith-cbrt-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-acosh-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-cosh-on-various-types.js.no-cjit-collect-continuously stress/arith-round-on-various-types.js.no-ftl stress/arith-trunc-on-various-types.js.no-ftl stress/arith-log10-on-various-types.js.no-cjit-collect-continuously stress/arith-log10-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-expm1-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-tan-on-various-types.js.no-cjit-collect-continuously stress/arith-fround-on-various-types.js.default stress/arith-negate-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-cos-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-acosh-on-various-types.js.no-cjit-collect-continuously stress/arith-cosh-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-clz32-on-various-types.js.no-cjit-collect-continuously stress/arith-acosh-on-various-types.js.no-ftl stress/arith-abs-on-various-types.js.no-ftl stress/arith-sinh-on-various-types.js.no-cjit-collect-continuously stress/arith-floor-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-tan-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-cbrt-on-various-types.js.default stress/arith-cbrt-on-various-types.js.no-llint stress/arith-asinh-on-various-types.js.no-llint stress/arith-round-on-various-types.js.no-cjit-validate-phases stress/arith-tan-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-cosh-on-various-types.js.default stress/arith-acosh-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-ceil-on-various-types.js.no-ftl stress/arith-log2-on-various-types.js.no-ftl stress/arith-negate-on-various-types.js.no-cjit-collect-continuously stress/arith-abs-on-various-types.js.no-cjit-collect-continuously stress/arith-trunc-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-floor-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-fround-on-various-types.js.no-ftl stress/arith-tanh-on-various-types.js.no-ftl stress/arith-sinh-on-various-types.js.no-llint stress/arith-sin-on-various-types.js.default stress/arith-asinh-on-various-types.js.no-cjit-validate-phases stress/arith-abs-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-round-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-cbrt-on-various-types.js.no-cjit-validate-phases stress/arith-asin-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-trunc-on-various-types.js.no-cjit-validate-phases stress/arith-abs-on-various-types.js.no-llint stress/arith-abs-on-various-types.js.default stress/arith-ceil-on-various-types.js.no-cjit-collect-continuously stress/arith-sin-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-atanh-on-various-types.js.no-cjit-collect-continuously stress/arith-negate-on-various-types.js.default stress/arith-acos-on-various-types.js.no-cjit-validate-phases stress/arith-tan-on-various-types.js.default stress/arith-clz32-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-log10-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-trunc-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-tanh-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-log2-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-expm1-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-asinh-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-sqrt-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-log2-on-various-types.js.default stress/arith-log-on-various-types.js.no-cjit-validate-phases stress/arith-clz32-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-atan-on-various-types.js.no-cjit-validate-phases stress/arith-atan-on-various-types.js.no-llint stress/arith-round-on-various-types.js.default stress/arith-clz32-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-asin-on-various-types.js.no-ftl stress/arith-cbrt-on-various-types.js.no-ftl stress/arith-sqrt-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-tan-on-various-types.js.no-cjit-validate-phases stress/arith-negate-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-cos-on-various-types.js.default stress/arith-fround-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-atanh-on-various-types.js.no-ftl stress/arith-sin-on-various-types.js.no-llint stress/arith-tanh-on-various-types.js.ftl-no-cjit-no-inline-validate

Mark Lam

Comment 4 2018-03-19 18:17:30 PDT

Created attachment 336091 [details] proposed patch w/ 32-bit build fix.

Mark Lam

Comment 5 2018-03-19 18:32:19 PDT

Comment on attachment 336091 [details] proposed patch w/ 32-bit build fix. Something is weird. The EWS is reporting failed tests. I'm taking this out of review to investigate first.

EWS Watchlist

Comment 6 2018-03-19 19:30:58 PDT

Comment on attachment 336091 [details] proposed patch w/ 32-bit build fix. Attachment 336091 [details] did not pass jsc-ews (mac): Output: http://webkit-queues.webkit.org/results/7032868 New failing tests: stress/arith-atanh-on-various-types.js.no-llint stress/arith-log10-on-various-types.js.no-ftl stress/arith-cosh-on-various-types.js.no-llint stress/arith-round-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-asinh-on-various-types.js.no-ftl stress/arith-atanh-on-various-types.js.no-cjit-validate-phases stress/arith-atanh-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-ceil-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-fround-on-various-types.js.no-llint stress/arith-sqrt-on-various-types.js.default stress/arith-tan-on-various-types.js.no-ftl stress/arith-sqrt-on-various-types.js.no-llint stress/arith-acos-on-various-types.js.no-cjit-collect-continuously stress/arith-atanh-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-log2-on-various-types.js.no-cjit-collect-continuously stress/arith-trunc-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-cos-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-acosh-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-asin-on-various-types.js.no-cjit-collect-continuously stress/arith-log10-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-acos-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-cbrt-on-various-types.js.no-cjit-collect-continuously stress/arith-clz32-on-various-types.js.no-ftl stress/arith-log10-on-various-types.js.no-llint stress/arith-negate-on-various-types.js.no-llint stress/arith-log10-on-various-types.js.default stress/arith-sqrt-on-various-types.js.no-cjit-collect-continuously stress/arith-sin-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-trunc-on-various-types.js.no-cjit-validate-phases stress/arith-asinh-on-various-types.js.no-cjit-collect-continuously stress/arith-negate-on-various-types.js.no-cjit-validate-phases stress/arith-cbrt-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-fround-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-tanh-on-various-types.js.default stress/arith-atan-on-various-types.js.default stress/arith-abs-on-various-types.js.no-cjit-validate-phases stress/arith-log2-on-various-types.js.no-llint stress/arith-sin-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-expm1-on-various-types.js.no-cjit-validate-phases stress/arith-atan-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-asin-on-various-types.js.default stress/arith-floor-on-various-types.js.no-cjit-validate-phases stress/arith-round-on-various-types.js.no-cjit-collect-continuously stress/arith-cosh-on-various-types.js.no-ftl stress/arith-abs-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-acos-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-floor-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-trunc-on-various-types.js.no-cjit-collect-continuously stress/arith-ceil-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-sinh-on-various-types.js.no-ftl jsc-layout-tests.yaml/js/script-tests/dfg-proven-sqrt-backwards-propagation.js.layout-dfg-eager-no-cjit stress/arith-atan-on-various-types.js.no-cjit-collect-continuously stress/arith-tanh-on-various-types.js.no-cjit-validate-phases stress/arith-tan-on-various-types.js.no-llint stress/arith-sin-on-various-types.js.no-cjit-validate-phases stress/arith-expm1-on-various-types.js.no-llint stress/arith-negate-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-trunc-on-various-types.js.no-llint stress/arith-floor-on-various-types.js.no-ftl stress/arith-atan-on-various-types.js.no-ftl stress/arith-negate-on-various-types.js.no-ftl stress/arith-atan-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-acos-on-various-types.js.no-llint stress/arith-atanh-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-acos-on-various-types.js.no-ftl stress/arith-sinh-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-cos-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-cos-on-various-types.js.no-cjit-validate-phases stress/arith-expm1-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-expm1-on-various-types.js.no-ftl stress/arith-cos-on-various-types.js.no-cjit-collect-continuously stress/arith-asin-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-sinh-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-acos-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-ceil-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-sinh-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-log2-on-various-types.js.ftl-no-cjit-b3o1 jsc-layout-tests.yaml/js/script-tests/dfg-proven-sqrt-backwards-propagation.js.layout-ftl-eager-no-cjit stress/arith-log2-on-various-types.js.no-cjit-validate-phases stress/arith-log2-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-sqrt-on-various-types.js.no-ftl stress/arith-expm1-on-various-types.js.default stress/arith-cos-on-various-types.js.no-ftl stress/arith-sqrt-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-acosh-on-various-types.js.no-cjit-validate-phases stress/arith-acosh-on-various-types.js.no-llint stress/arith-clz32-on-various-types.js.no-cjit-validate-phases stress/arith-abs-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-asinh-on-various-types.js.ftl-no-cjit-no-inline-validate jsc-layout-tests.yaml/js/script-tests/dfg-proven-sqrt-backwards-propagation.js.layout-no-llint stress/arith-asin-on-various-types.js.no-llint stress/arith-atanh-on-various-types.js.default stress/arith-fround-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-ceil-on-various-types.js.no-cjit-validate-phases stress/arith-trunc-on-various-types.js.default stress/arith-asinh-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-ceil-on-various-types.js.no-llint stress/arith-round-on-various-types.js.no-llint stress/arith-cosh-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-fround-on-various-types.js.no-cjit-collect-continuously stress/arith-round-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-sqrt-on-various-types.js.no-cjit-validate-phases stress/arith-acos-on-various-types.js.default stress/arith-sin-on-various-types.js.no-ftl stress/arith-asin-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-tanh-on-various-types.js.no-cjit-collect-continuously stress/arith-log10-on-various-types.js.no-cjit-validate-phases stress/arith-tanh-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-floor-on-various-types.js.default stress/arith-atan-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-expm1-on-various-types.js.no-cjit-collect-continuously stress/arith-fround-on-various-types.js.no-cjit-validate-phases stress/arith-ceil-on-various-types.js.default stress/arith-cosh-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-floor-on-various-types.js.no-llint stress/arith-log-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-tan-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-clz32-on-various-types.js.default stress/arith-clz32-on-various-types.js.no-llint stress/arith-cosh-on-various-types.js.no-cjit-validate-phases stress/arith-asin-on-various-types.js.no-cjit-validate-phases stress/arith-cos-on-various-types.js.no-llint stress/arith-tanh-on-various-types.js.no-llint stress/arith-cbrt-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-asinh-on-various-types.js.default stress/arith-acosh-on-various-types.js.default stress/arith-sin-on-various-types.js.no-cjit-collect-continuously stress/arith-floor-on-various-types.js.no-cjit-collect-continuously stress/arith-sinh-on-various-types.js.no-cjit-validate-phases stress/arith-sinh-on-various-types.js.default stress/arith-cbrt-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-acosh-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-cosh-on-various-types.js.no-cjit-collect-continuously stress/arith-round-on-various-types.js.no-ftl stress/arith-trunc-on-various-types.js.no-ftl stress/arith-log10-on-various-types.js.no-cjit-collect-continuously stress/arith-log10-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-expm1-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-tan-on-various-types.js.no-cjit-collect-continuously stress/arith-fround-on-various-types.js.default stress/arith-negate-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-cos-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-acosh-on-various-types.js.no-cjit-collect-continuously stress/arith-cosh-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-clz32-on-various-types.js.no-cjit-collect-continuously stress/arith-acosh-on-various-types.js.no-ftl stress/arith-abs-on-various-types.js.no-ftl stress/arith-sinh-on-various-types.js.no-cjit-collect-continuously stress/arith-floor-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-tan-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-cbrt-on-various-types.js.default stress/arith-cbrt-on-various-types.js.no-llint stress/arith-asinh-on-various-types.js.no-llint stress/arith-round-on-various-types.js.no-cjit-validate-phases stress/arith-tan-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-cosh-on-various-types.js.default stress/arith-acosh-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-ceil-on-various-types.js.no-ftl stress/arith-log2-on-various-types.js.no-ftl stress/arith-negate-on-various-types.js.no-cjit-collect-continuously stress/arith-abs-on-various-types.js.no-cjit-collect-continuously stress/arith-trunc-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-floor-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-fround-on-various-types.js.no-ftl stress/arith-tanh-on-various-types.js.no-ftl stress/arith-sinh-on-various-types.js.no-llint stress/arith-sin-on-various-types.js.default stress/arith-asinh-on-various-types.js.no-cjit-validate-phases stress/arith-abs-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-round-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-cbrt-on-various-types.js.no-cjit-validate-phases stress/arith-asin-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-abs-on-various-types.js.no-llint stress/arith-abs-on-various-types.js.default stress/arith-ceil-on-various-types.js.no-cjit-collect-continuously stress/arith-sin-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-atanh-on-various-types.js.no-cjit-collect-continuously stress/arith-negate-on-various-types.js.default stress/arith-acos-on-various-types.js.no-cjit-validate-phases stress/arith-tan-on-various-types.js.default stress/arith-clz32-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-log10-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-trunc-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-tanh-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-log2-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-expm1-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-asinh-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-sqrt-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-log2-on-various-types.js.default stress/arith-log-on-various-types.js.no-cjit-validate-phases stress/arith-clz32-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-atan-on-various-types.js.no-cjit-validate-phases stress/arith-atan-on-various-types.js.no-llint stress/arith-round-on-various-types.js.default stress/arith-clz32-on-various-types.js.ftl-no-cjit-validate-sampling-profiler stress/arith-asin-on-various-types.js.no-ftl stress/arith-cbrt-on-various-types.js.no-ftl stress/arith-sqrt-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-tan-on-various-types.js.no-cjit-validate-phases stress/arith-negate-on-various-types.js.ftl-no-cjit-no-inline-validate stress/arith-cos-on-various-types.js.default stress/arith-fround-on-various-types.js.ftl-no-cjit-b3o1 stress/arith-atanh-on-various-types.js.no-ftl stress/arith-sin-on-various-types.js.no-llint stress/arith-tanh-on-various-types.js.ftl-no-cjit-no-inline-validate

JF Bastien

Comment 7 2018-03-19 21:25:18 PDT

Comment on attachment 336091 [details] proposed patch w/ 32-bit build fix. View in context: https://bugs.webkit.org/attachment.cgi?id=336091&action=review r=me modulo EWS. > Source/JavaScriptCore/assembler/MacroAssemblerCodeRef.h:81 > + FunctionPtr(ReturnType(CDECL *value)(Arguments...), PtrTag tag = CFunctionPtrTag) Does it matter that this is CDECL (and below, FASTCALL), as opposed to the default calling convention? It seems like you could have a different PtrTag for those. > Source/JavaScriptCore/assembler/MacroAssemblerCodeRef.h:225 > + return MacroAssemblerCodePtr(retagCodePtr(executableAddress(), oldTag, newTag)); Are there any relationships that we want to enforce between tags? Like if there's a lattice of tags, we could have a debug assert here that the PtrTags are compatible (that function would come from the PtrTag header).

Mark Lam

Comment 8 2018-03-19 22:44:47 PDT

(In reply to JF Bastien from comment #7) > Comment on attachment 336091 [details] > proposed patch w/ 32-bit build fix. > > View in context: > https://bugs.webkit.org/attachment.cgi?id=336091&action=review > > r=me modulo EWS. > > > Source/JavaScriptCore/assembler/MacroAssemblerCodeRef.h:81 > > + FunctionPtr(ReturnType(CDECL *value)(Arguments...), PtrTag tag = CFunctionPtrTag) > > Does it matter that this is CDECL (and below, FASTCALL), as opposed to the > default calling convention? It seems like you could have a different PtrTag > for those. For now, we don't need to differentiate them yet. > > Source/JavaScriptCore/assembler/MacroAssemblerCodeRef.h:225 > > + return MacroAssemblerCodePtr(retagCodePtr(executableAddress(), oldTag, newTag)); > > Are there any relationships that we want to enforce between tags? Like if > there's a lattice of tags, we could have a debug assert here that the > PtrTags are compatible (that function would come from the PtrTag header). No special relationships between tags yet.

Mark Lam

Comment 9 2018-03-19 22:49:59 PDT

Created attachment 336105 [details] patch for landing.

Mark Lam

Comment 10 2018-03-20 11:11:17 PDT

Thanks for the review. Landed in r229767: <http://trac.webkit.org/r229767>.

Note You need to log in before you can comment on or make changes to this bug.