Bug 90198

Created attachment 149988 [details] first draft patch There is some progress on this porting work. Most of the JSCore tests are running now, but some runtime assertions are hit during the test.

Attachment 149988 [details] did not pass style-queue: Source/JavaScriptCore/dfg/DFGOperations.cpp:146: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/dfg/DFGOperations.cpp:157: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/dfg/DFGOperations.cpp:178: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/dfg/DFGOperations.cpp:189: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/dfg/DFGOperations.cpp:1384: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/dfg/DFGOperations.cpp:1393: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/assembler/ARMAssembler.cpp:295: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Source/JavaScriptCore/assembler/ARMAssembler.cpp:331: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Source/JavaScriptCore/assembler/ARMAssembler.h:430: vmov_f64_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:465: vabs_f64_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:470: vneg_f64_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:485: dtr_u is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:490: dtr_ur is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:495: dtr_d is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:500: dtr_dr is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:505: dtrh_u is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:510: dtrh_ur is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:515: dtrh_d is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:520: dtrh_dr is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:525: fdtr_u is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:532: fdtr_d is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:561: vmov_vfp64_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:567: vmov_arm64_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:573: vmov_vfp32_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:579: vmov_arm32_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:597: vcvt_u32_f64_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:603: vcvt_f64_f32_r is incorrectly named. Don't use underscores in your identifieFailed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/assembler/ARMAssembl..." exit_code: 1 r names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:609: vcvt_f32_f64_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:893: Extra space before last semicolon. If this should be an empty statement, use { } instead. [whitespace/semicolon] [5] Source/JavaScriptCore/assembler/ARMAssembler.h:910: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/assembler/ARMAssembler.h:911: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/assembler/ARMAssembler.h:912: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/assembler/ARMAssembler.h:913: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/assembler/ARMAssembler.h:914: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/assembler/ARMAssembler.h:915: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 35 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.

Created attachment 150392 [details] second draft patch 3 regressions in jscore tests.

Attachment 150392 [details] did not pass style-queue: Source/JavaScriptCore/assembler/MacroAssemblerARM.h:1205: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Source/JavaScriptCore/dfg/DFGOperations.cpp:146: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/dfg/DFGOperations.cpp:157: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/dfg/DFGOperations.cpp:178: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/dfg/DFGOperations.cpp:189: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/dfg/DFGOperations.cpp:1382: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/dfg/DFGOperations.cpp:1391: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/assembler/ARMAssembler.cpp:295: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Source/JavaScriptCore/assembler/ARMAssembler.cpp:331: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Source/JavaScriptCore/assembler/ARMAssembler.h:433: vmov_f64_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:468: vabs_f64_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:473: vneg_f64_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:488: dtr_u is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:493: dtr_ur is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:498: dtr_d is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:503: dtr_dr is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:508: dtrh_u is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:513: dtrh_ur is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:518: dtrh_d is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:523: dtrh_dr is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:528: fdtr_u is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:535: fdtr_d is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:564: vmov_vfp64_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:570: vmov_arm64_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:576: vmov_vfp32_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:582: vmov_arm32_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:600: vcvt_u32_f64_r is incorrectly named. DoFailed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/assembler/ARMAssembl..." exit_code: 1 n't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:606: vcvt_f64_f32_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:612: vcvt_f32_f64_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:932: Extra space before last semicolon. If this should be an empty statement, use { } instead. [whitespace/semicolon] [5] Source/JavaScriptCore/assembler/ARMAssembler.h:949: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/assembler/ARMAssembler.h:950: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/assembler/ARMAssembler.h:951: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/assembler/ARMAssembler.h:952: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/assembler/ARMAssembler.h:953: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/assembler/ARMAssembler.h:954: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 36 in 17 files If any of these errors are false positives, please file a bug against check-webkit-style.

Created attachment 150423 [details] working patch The patch is working now. SunSpider: With DFG: 1910.2ms Without DFG 2062.6ms ChangeLog is missing but I think the patch is ready for a first round of review.

Attachment 150423 [details] did not pass style-queue: Source/JavaScriptCore/assembler/MacroAssemblerARM.h:1205: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Source/JavaScriptCore/dfg/DFGOperations.cpp:146: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/dfg/DFGOperations.cpp:157: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/dfg/DFGOperations.cpp:178: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/dfg/DFGOperations.cpp:189: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/dfg/DFGOperations.cpp:1382: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/dfg/DFGOperations.cpp:1391: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/assembler/ARMAssembler.cpp:295: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Source/JavaScriptCore/assembler/ARMAssembler.cpp:331: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Source/JavaScriptCore/assembler/ARMAssembler.h:433: vmov_f64_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:468: vabs_f64_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:473: vneg_f64_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:488: dtr_u is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:493: dtr_ur is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:498: dtr_d is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:503: dtr_dr is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:508: dtrh_u is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:513: dtrh_ur is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:518: dtrh_d is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:523: dtrh_dr is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:528: fdtr_u is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:535: fdtr_d is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:564: vmov_vfp64_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:570: vmov_arm64_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:576: vmov_vfp32_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:582: vmov_arm32_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:600: vcvt_u32_f64_r is incorrectly named. DoFailed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/assembler/ARMAssembl..." exit_code: 1 n't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:606: vcvt_f64_f32_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:612: vcvt_f32_f64_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:932: Extra space before last semicolon. If this should be an empty statement, use { } instead. [whitespace/semicolon] [5] Source/JavaScriptCore/assembler/ARMAssembler.h:949: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/assembler/ARMAssembler.h:950: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/assembler/ARMAssembler.h:951: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/assembler/ARMAssembler.h:952: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/assembler/ARMAssembler.h:953: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/assembler/ARMAssembler.h:954: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 36 in 17 files If any of these errors are false positives, please file a bug against check-webkit-style.

Created attachment 150590 [details] patch Some minor things were refactored and a changelog is added.

Attachment 150590 [details] did not pass style-queue: Source/JavaScriptCore/assembler/MacroAssemblerARM.h:1195: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Source/JavaScriptCore/dfg/DFGOperations.cpp:146: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/dfg/DFGOperations.cpp:157: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/dfg/DFGOperations.cpp:178: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/dfg/DFGOperations.cpp:189: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/dfg/DFGOperations.cpp:1382: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/dfg/DFGOperations.cpp:1391: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/assembler/ARMAssembler.cpp:295: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Source/JavaScriptCore/assembler/ARMAssembler.cpp:331: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Source/JavaScriptCore/assembler/ARMAssembler.h:433: vmov_f64_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:468: vabs_f64_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:473: vneg_f64_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:488: dtr_u is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:493: dtr_ur is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:498: dtr_d is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:503: dtr_dr is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:508: dtrh_u is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:513: dtrh_ur is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:518: dtrh_d is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:523: dtrh_dr is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:528: fdtr_u is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:535: fdtr_d is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:564: vmov_vfp64_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:570: vmov_arm64_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:576: vmov_vfp32_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:582: vmov_arm32_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:600: vcvt_u32_f64_r is incorrectly named. DoFailed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1 n't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:606: vcvt_f64_f32_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:612: vcvt_f32_f64_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:933: Extra space before last semicolon. If this should be an empty statement, use { } instead. [whitespace/semicolon] [5] Total errors found: 30 in 19 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 150590 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=150590&action=review R=me. Please measure V8 and Kraken though. You should see big speed-ups there; if you don't then there's something wrong! > Source/JavaScriptCore/ChangeLog:13 > + Sunspider is improved by 8%. Affect on V8 performance? Kraken?

Comment on attachment 150590 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=150590&action=review >> Source/JavaScriptCore/assembler/ARMAssembler.cpp:295 >> + if (offset == 0) { > > Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] You should refine this and other style errors below. ;) > Source/JavaScriptCore/assembler/ARMAssembler.h:-158 > -#if WTF_ARM_ARCH_AT_LEAST(5) || defined(__ARM_ARCH_4T__) Please mention in the ChangeLog that the support of ARMv4 and below is removed. Additionally you should check WTF_ARM_ARCH_AT_LEAST(4) and similar constructs and modify them in Platform.h and other files. > Source/JavaScriptCore/dfg/DFGAssemblyHelpers.h:269 > +#if CPU(ARM_THUMB2) > m_assembler.vmov(payloadGPR, tagGPR, fpr); > +#else > + m_assembler.vmov_arm64_r(payloadGPR, tagGPR, fpr); > +#endif I suggest to define a common name or a wrapper function reducing #ifdef burden for the same logic. Apart from these, I really like your patch. ;)

(In reply to comment #10) > (From update of attachment 150590 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=150590&action=review > > >> Source/JavaScriptCore/assembler/ARMAssembler.cpp:295 > >> + if (offset == 0) { > > > > Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] > > You should refine this and other style errors below. ;) > > > Source/JavaScriptCore/assembler/ARMAssembler.h:-158 > > -#if WTF_ARM_ARCH_AT_LEAST(5) || defined(__ARM_ARCH_4T__) > > Please mention in the ChangeLog that the support of ARMv4 and below is removed. > > Additionally you should check WTF_ARM_ARCH_AT_LEAST(4) and similar constructs and modify them in Platform.h and other files. > > > Source/JavaScriptCore/dfg/DFGAssemblyHelpers.h:269 > > +#if CPU(ARM_THUMB2) > > m_assembler.vmov(payloadGPR, tagGPR, fpr); > > +#else > > + m_assembler.vmov_arm64_r(payloadGPR, tagGPR, fpr); > > +#endif > > I suggest to define a common name or a wrapper function reducing #ifdef burden for the same logic. > > Apart from these, I really like your patch. ;) +1 I like the idea of wrapping the "m_assembler.vmov_blah" in something nicer. Perhaps you can add a method in DFGAssemblyHelpers.h like: void transferReturnValueToReturnValueFPR() { #if thumb2 the thumb2 thingy #else if arm the arm thingy #else do thing #endif } I think that would be useful in other places in the future, particularly since there are code paths in the DFG that make calls in a manner that bypasses the helper methods for a variety of reasons. Currently none of those deal with floating point returns, but that might change.

I finished the V8 measurements. Without DFG: 15372.2ms With DFG: 8010.5ms So there is a nice progression here.

(In reply to comment #12) > I finished the V8 measurements. > > Without DFG: 15372.2ms > With DFG: 8010.5ms > > So there is a nice progression here. Fantastic!

> Please mention in the ChangeLog that the support of ARMv4 and below is removed. It is already there :) Ok I will do the necessary style changes. I think further refactoring could be done in followup patches, but this patch is already big enough, so I focus on the DFG related changes.

Created attachment 150746 [details] latest patch

Attachment 150746 [details] did not pass style-queue: Source/JavaScriptCore/assembler/MacroAssemblerARM.h:1195: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Source/JavaScriptCore/dfg/DFGOperations.cpp:146: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/dfg/DFGOperations.cpp:157: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/dfg/DFGOperations.cpp:178: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/dfg/DFGOperations.cpp:189: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/dfg/DFGOperations.cpp:1382: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/dfg/DFGOperations.cpp:1391: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/assembler/ARMAssembler.h:433: vmov_f64_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:468: vabs_f64_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:473: vneg_f64_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:488: dtr_u is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:493: dtr_ur is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:498: dtr_d is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:503: dtr_dr is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:508: dtrh_u is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:513: dtrh_ur is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:518: dtrh_d is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:523: dtrh_dr is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:528: fdtr_u is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:535: fdtr_d is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:564: vmov_vfp64_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:570: vmov_arm64_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:576: vmov_vfp32_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:582: vmov_arm32_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:600: vcvt_u32_f64_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:606: vcvt_f64_f32_r is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/assembler/ARMAssembler.h:612: vcvt_f32_f64_r is incorrectly named. Don't use underscores in your identifier names. [readability/namingFailed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1 ] [4] Total errors found: 27 in 19 files If any of these errors are false positives, please file a bug against check-webkit-style.

> void transferReturnValueToReturnValueFPR() > { > #if thumb2 > the thumb2 thingy > #else if arm > the arm thingy > #else > do thing > #endif > } I am not sure I understand this, but I added two vmov-s to the traditional assemblers, which wraps the "arm thingy" and this way I could remove all ifdefs from the code.

(In reply to comment #17) > > void transferReturnValueToReturnValueFPR() > > { > > #if thumb2 > > the thumb2 thingy > > #else if arm > > the arm thingy > > #else > > do thing > > #endif > > } > > I am not sure I understand this, but I added two vmov-s to the traditional assemblers, which wraps the "arm thingy" and this way I could remove all ifdefs from the code. Argh, sorry, I got confused by context. Never mind.

Landed as http://trac.webkit.org/changeset/121885

FYI, this appears to have broken the ARMv7 port. I'll fix it.

(In reply to comment #20) > FYI, this appears to have broken the ARMv7 port. I'll fix it. Fixed in http://trac.webkit.org/changeset/121927

> Fixed in http://trac.webkit.org/changeset/121927 This macro should only used by traditional arm, and as far as I see all of its uses are guarded by CPU(ARM_TRADITIONAL). How does it slip out?

(In reply to comment #22) > > Fixed in http://trac.webkit.org/changeset/121927 > > This macro should only used by traditional arm, and as far as I see all of its uses are guarded by CPU(ARM_TRADITIONAL). How does it slip out? Ah, interesting. Around line 1089 in JITStubs.cpp: #if CPU(ARM_THUMB2) && COMPILER(GCC) #define DEFINE_STUB_FUNCTION(rtype, op) \ extern "C" { \ rtype JITStubThunked_##op(STUB_ARGS_DECLARATION); \ }; \ asm ( \ ".text" "\n" \ ".align 2" "\n" \ ".globl " SYMBOL_STRING(cti_##op) "\n" \ HIDE_SYMBOL(cti_##op) "\n" \ INLINE_ARM_FUNCTION(cti_##op) \ SYMBOL_STRING(cti_##op) ":" "\n" \ "str lr, [sp, #" STRINGIZE_VALUE_OF(THUNK_RETURN_ADDRESS_OFFSET) "]" "\n" \ "bl " SYMBOL_STRING(JITStubThunked_##op) "\n" \ "ldr lr, [sp, #" STRINGIZE_VALUE_OF(THUNK_RETURN_ADDRESS_OFFSET) "]" "\n" \ "bx lr" "\n" \ ); \ rtype JITStubThunked_##op(STUB_ARGS_DECLARATION) \ Maybe you didn't mean to use INLINE_ARM_FUNCTION there?

That is my mistake. I accidentaly replaced that macro as well. I will revert it. - ".thumb" "\n" \ - ".thumb_func " THUMB_FUNC_PARAM(cti_##op) "\n" \ + INLINE_ARM_FUNCTION(cti_##op) \