182563 – Experiment with alternative implementation of memcpy/memset

REOPENED182563

Experiment with alternative implementation of memcpy/memset

https://bugs.webkit.org/show_bug.cgi?id=182563

Summary Experiment with alternative implementation of memcpy/memset

Filip Pizlo

Reported 2018-02-06 20:04:17 PST

...

Attachments
work in progress (54.06 KB, patch) 2018-02-06 20:05 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (66.31 KB, patch) 2018-02-08 10:08 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (68.98 KB, patch) 2018-02-08 10:20 PST, Filip Pizlo	msaboff: review+	Details Formatted Diff Diff
patch for landing (68.72 KB, patch) 2018-02-08 12:57 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Filip Pizlo

Comment 1 2018-02-06 20:05:22 PST

Created attachment 333260 [details] work in progress

Filip Pizlo

Comment 2 2018-02-07 15:50:28 PST

Looks like a 1.00882x speed-up on PLT with p = 0.0973514. Gonna test some more benchmarks.

Filip Pizlo

Comment 3 2018-02-08 10:08:42 PST

Created attachment 333383 [details] the patch

EWS Watchlist

Comment 4 2018-02-08 10:11:46 PST

Attachment 333383 [details] did not pass style-queue: ERROR: Source/bmalloc/bmalloc/Algorithm.h:190: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:203: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:210: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:231: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:238: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:259: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:285: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:290: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:295: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:285: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:290: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:295: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:285: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:290: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:295: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:265: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:285: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:290: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:295: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:290: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:295: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:290: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:295: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:301: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:313: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:314: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:314: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:320: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:339: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:340: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:340: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:346: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:365: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:366: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:389: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:390: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:394: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:366: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:389: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:390: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:394: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:389: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:390: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:394: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:371: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:389: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:390: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:394: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:390: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:394: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:394: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:39: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:51: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:52: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:52: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:58: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:77: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:78: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:78: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:84: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:103: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:104: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:127: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:128: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:132: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:136: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:104: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:127: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:128: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:132: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:136: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:127: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:128: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:132: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:136: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:109: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:127: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:128: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:132: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:136: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:128: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:132: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:136: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:132: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:136: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:39: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastCopy.h:52: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:59: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastCopy.h:80: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:87: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastCopy.h:108: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:134: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:139: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:143: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:134: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:139: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:143: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:134: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:139: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:143: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:114: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastCopy.h:134: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:139: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:143: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:139: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:143: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:139: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:143: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] Total errors found: 108 in 41 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 5 2018-02-08 10:20:15 PST

Created attachment 333386 [details] the patch

EWS Watchlist

Comment 6 2018-02-08 10:23:07 PST

Attachment 333386 [details] did not pass style-queue: ERROR: Source/bmalloc/bmalloc/Algorithm.h:190: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:203: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:210: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:231: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:238: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:259: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:285: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:290: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:295: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:285: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:290: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:295: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:285: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:290: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:295: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:265: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:285: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:290: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:295: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:290: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:295: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:290: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:295: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:301: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:313: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:314: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:314: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:320: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:339: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:340: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:340: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:346: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:365: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:366: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:389: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:390: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:394: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:366: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:389: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:390: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:394: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:389: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:390: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:394: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:371: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:389: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:390: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:394: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:390: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:394: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:394: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:39: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:51: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:52: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:52: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:58: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:77: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:78: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:78: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:84: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:103: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:104: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:127: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:128: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:132: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:136: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:104: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:127: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:128: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:132: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:136: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:127: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:128: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:132: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:136: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:109: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:127: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:128: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:132: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:136: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:128: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:132: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:136: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:132: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:136: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:39: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastCopy.h:52: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:59: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastCopy.h:80: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:87: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastCopy.h:108: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:134: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:139: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:143: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:134: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:139: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:143: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:134: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:139: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:143: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:114: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastCopy.h:134: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:139: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:143: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:139: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:143: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:139: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:143: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] Total errors found: 108 in 43 files If any of these errors are false positives, please file a bug against check-webkit-style.

Michael Saboff

Comment 7 2018-02-08 11:21:32 PST

Comment on attachment 333386 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=333386&action=review r=me. Did you consider implementing only in bmalloc and then a forward reference in WTF? > Source/WTF/ChangeLog:36 > + reasoning. But that's not quite right, since then we would lose inlining in the unknonw-size Spelling *unknonw-size*

Mark Lam

Comment 8 2018-02-08 12:20:30 PST

Comment on attachment 333386 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=333386&action=review FYI, I did my review from the bottom up (since the fastCopy and fastZeroFill implementations are located towards the bottom). It might make more sense to read my comments backwards. > Source/WTF/wtf/FastCopy.h:33 > +template<typename T> > +void fastCopy(T* dst, const T* src, size_t length) I assume this is identical to the bmalloc version below and didn't review this thoroughly. > Source/WTF/wtf/FastCopy.h:143 > +inline void fastCopyBytes(void* dst, const void* src, size_t bytes) At first, I was pondering whether we can rename this to just be fastCopy. After doing the entire review, I am strongly of the opinion that we should NOT rename this to fastCopy. There's a strong need to distinguish fastCopy and fastCopyBytes apart. One of the class of errors that can arise from fastCopy() taking a count of T to copy instead of size_t bytes is that if someone edits the code later and innocently changes the type of the pointer, an error may silently creep in. Consider this scenario: Initially, let's say we have this contrived code in a class: void foo(Stuff* myStuff, Stuff* other, int numberOfStuff) { m_myStuff = myStuff; m_other = other; m_length = numberOfStuff; bar(); // where bar contains ... fastCopy(m_myStuff, m_other, m_length); // <======== this is correct. } Stuff m_myStuff; Stuff m_other; size_t m_length; Some time later, for reasons, someone decides to change type of the pointers like so: void* m_myStuff; void* m_other; size_t m_length; The person making the change may not notice that bar() relies on typeof m_myStuff and m_other not being void*. If you can be sure that calling fastCopy() with void* pointers will result in a build error, then this error will not go unnoticed. I think we're fine, but can you please do a quick test to make sure that this is indeed the case i.e. we should get build errors if fastCopy() is called with void* pointers. This is also why it is good to name fastCopyBytes() differently than fastCopy(). > Source/WTF/wtf/FastMalloc.cpp:83 > + fastZeroFill(static_cast<char*>(result), n); Let's just call fastZeroFillBytes() and do away with the cast instead. > Source/WTF/wtf/FastMalloc.cpp:100 > + fastZeroFill(static_cast<char*>(result), n); Let's just call fastZeroFillBytes() and do away with the cast instead. > Source/WTF/wtf/FastZeroFill.h:33 > +void fastZeroFill(T* dst, size_t length) I assume this is identical to the bmalloc version below and didn't review this thoroughly. > Source/WTF/wtf/FastZeroFill.h:136 > +inline void fastZeroFillBytes(void* dst, size_t bytes) Ditto: same issue as with fastCopyBytes() above. > Source/WTF/wtf/MD5.cpp:5 > - * Copyright (C) 2015 Apple Inc. All rights reserved. > + * Copyright (C) 2015-2018 Apple Inc. All rights reserved. Why is this needed? > Source/WTF/wtf/text/StringConcatenate.h:161 > + fastCopy(destination, m_characters, m_length * sizeof(UChar)); This is wrong. You should remove the "* sizeof(UChar)". > Source/bmalloc/bmalloc/Algorithm.h:218 > + "movq (%%rsi, %%rcx), %%rax\n\t" > + "movq %%rax, (%%rdi, %%rcx)\n\t" Ditto with assumption about alignment being a non-issue (see below). > Source/bmalloc/bmalloc/Algorithm.h:229 > + "addq $4, %%rcx\n\t" > + "4:\n\t" > + "movl (%%rsi, %%rcx), %%eax\n\t" > + "movl %%eax, (%%rdi, %%rcx)\n\t" > + "subq $4, %%rcx\n\t" > + "jae 4b\n\t" Same as below: no need for addq, subq, and jae. This reduces to: "movl (%%rsi), %%eax\n\t" "movl %%eax, (%%rdi)\n\t" > Source/bmalloc/bmalloc/Algorithm.h:246 > + "movq (%%rsi, %%rcx), %%rax\n\t" > + "movq %%rax, (%%rdi, %%rcx)\n\t" Ditto with assumption about alignment being a non-issue (see below). > Source/bmalloc/bmalloc/Algorithm.h:272 > + "movq (%%rsi, %%rcx), %%rax\n\t" > + "movq %%rax, (%%rdi, %%rcx)\n\t" Ditto with assumption about alignment being a non-issue (see below). > Source/bmalloc/bmalloc/Algorithm.h:324 > + "rep stosl\n\t" I presume the CPU's implementation of stosl will do the equivalent of stosq for the parts that are 8 byte aligned? Can you confirm? > Source/bmalloc/bmalloc/Algorithm.h:330 > + "1:\n\t" > + "subq $8, %%rcx\n\t" > + "jae 3b\n\t" It looks like you're assuming that fastZeroFill() will only every be called on a dst that is aligned on a 8 byte boundary because you're not checking for the potential of a leading 4 bytes here. Or are you relying on x86 support for unaligned access and just taking the hit if it turns out to be unaligned? > Source/bmalloc/bmalloc/Algorithm.h:337 > + "addq $4, %%rcx\n\t" > + "4:\n\t" > + "movl %%eax, (%%rdi, %%rcx)\n\t" > + "subq $4, %%rcx\n\t" > + "jae 4b\n\t" We already know that sizeof(T) % sizeof(uint32_t) == 0. Hence, the only way that we can get here (%rcx != -8) is if there's one 4 byte word to write. I think you can remove this addq, subq, and jae. This reduces to: "movl %%eax, (%%rdi)\n\t" > Source/bmalloc/bmalloc/Algorithm.h:353 > + "movq %%rax, (%%rdi, %%rcx)\n\t" Ditto with assumption about alignment being a non-issue. > Source/bmalloc/bmalloc/Algorithm.h:377 > + "movq %%rax, (%%rdi, %%rcx)\n\t" Ditto with assumption about alignment being a non-issue.

Filip Pizlo

Comment 9 2018-02-08 12:49:50 PST

(In reply to Mark Lam from comment #8) > Comment on attachment 333386 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=333386&action=review > > FYI, I did my review from the bottom up (since the fastCopy and fastZeroFill > implementations are located towards the bottom). It might make more sense > to read my comments backwards. > > > Source/WTF/wtf/FastCopy.h:33 > > +template<typename T> > > +void fastCopy(T* dst, const T* src, size_t length) > > I assume this is identical to the bmalloc version below and didn't review > this thoroughly. > > > Source/WTF/wtf/FastCopy.h:143 > > +inline void fastCopyBytes(void* dst, const void* src, size_t bytes) > > At first, I was pondering whether we can rename this to just be fastCopy. > After doing the entire review, I am strongly of the opinion that we should > NOT rename this to fastCopy. There's a strong need to distinguish fastCopy > and fastCopyBytes apart. > > One of the class of errors that can arise from fastCopy() taking a count of > T to copy instead of size_t bytes is that if someone edits the code later > and innocently changes the type of the pointer, an error may silently creep > in. Consider this scenario: > > Initially, let's say we have this contrived code in a class: > > void foo(Stuff* myStuff, Stuff* other, int numberOfStuff) { > m_myStuff = myStuff; > m_other = other; > m_length = numberOfStuff; > bar(); // where bar contains ... fastCopy(m_myStuff, m_other, > m_length); // <======== this is correct. > } > > Stuff m_myStuff; > Stuff m_other; > size_t m_length; > > Some time later, for reasons, someone decides to change type of the pointers > like so: > > void* m_myStuff; > void* m_other; > size_t m_length; > > The person making the change may not notice that bar() relies on typeof > m_myStuff and m_other not being void*. If you can be sure that calling > fastCopy() with void* pointers will result in a build error, then this error > will not go unnoticed. I think we're fine, but can you please do a quick > test to make sure that this is indeed the case i.e. we should get build > errors if fastCopy() is called with void* pointers. We do not get build errors if fastCopy() is called with void*. > > This is also why it is good to name fastCopyBytes() differently than > fastCopy(). > > > Source/WTF/wtf/FastMalloc.cpp:83 > > + fastZeroFill(static_cast<char*>(result), n); > > Let's just call fastZeroFillBytes() and do away with the cast instead. Fixed. > > > Source/WTF/wtf/FastMalloc.cpp:100 > > + fastZeroFill(static_cast<char*>(result), n); > > Let's just call fastZeroFillBytes() and do away with the cast instead. Fixed. > > > Source/WTF/wtf/FastZeroFill.h:33 > > +void fastZeroFill(T* dst, size_t length) > > I assume this is identical to the bmalloc version below and didn't review > this thoroughly. Yeah. > > > Source/WTF/wtf/FastZeroFill.h:136 > > +inline void fastZeroFillBytes(void* dst, size_t bytes) > > Ditto: same issue as with fastCopyBytes() above. Yeah. > > > Source/WTF/wtf/MD5.cpp:5 > > - * Copyright (C) 2015 Apple Inc. All rights reserved. > > + * Copyright (C) 2015-2018 Apple Inc. All rights reserved. > > Why is this needed? Reverted. An earlier version replaced memcpy/memset in this file, but later I noticed it was being used to copy constant-size things, so I reverted. > > > Source/WTF/wtf/text/StringConcatenate.h:161 > > + fastCopy(destination, m_characters, m_length * sizeof(UChar)); > > This is wrong. You should remove the "* sizeof(UChar)". Fixed! > > > Source/bmalloc/bmalloc/Algorithm.h:218 > > + "movq (%%rsi, %%rcx), %%rax\n\t" > > + "movq %%rax, (%%rdi, %%rcx)\n\t" > > Ditto with assumption about alignment being a non-issue (see below). I confirmed that alignment is a non-issue! It appears that doing a misaligned 64-bit word load/store is always faster than doing 8 8-bit loads/stores. Also, other memcpy's that I've seen for x86 do the same thing. > > > Source/bmalloc/bmalloc/Algorithm.h:229 > > + "addq $4, %%rcx\n\t" > > + "4:\n\t" > > + "movl (%%rsi, %%rcx), %%eax\n\t" > > + "movl %%eax, (%%rdi, %%rcx)\n\t" > > + "subq $4, %%rcx\n\t" > > + "jae 4b\n\t" > > Same as below: no need for addq, subq, and jae. This reduces to: > "movl (%%rsi), %%eax\n\t" > "movl %%eax, (%%rdi)\n\t" Clever. I'll leave a comment and fix in another patch: https://bugs.webkit.org/show_bug.cgi?id=182617 > > > Source/bmalloc/bmalloc/Algorithm.h:246 > > + "movq (%%rsi, %%rcx), %%rax\n\t" > > + "movq %%rax, (%%rdi, %%rcx)\n\t" > > Ditto with assumption about alignment being a non-issue (see below). Will leave fixme. > > > Source/bmalloc/bmalloc/Algorithm.h:272 > > + "movq (%%rsi, %%rcx), %%rax\n\t" > > + "movq %%rax, (%%rdi, %%rcx)\n\t" > > Ditto with assumption about alignment being a non-issue (see below). Will leave fixme. > > > Source/bmalloc/bmalloc/Algorithm.h:324 > > + "rep stosl\n\t" > > I presume the CPU's implementation of stosl will do the equivalent of stosq > for the parts that are 8 byte aligned? Can you confirm? > > > Source/bmalloc/bmalloc/Algorithm.h:330 > > + "1:\n\t" > > + "subq $8, %%rcx\n\t" > > + "jae 3b\n\t" > > It looks like you're assuming that fastZeroFill() will only every be called > on a dst that is aligned on a 8 byte boundary because you're not checking > for the potential of a leading 4 bytes here. Or are you relying on x86 > support for unaligned access and just taking the hit if it turns out to be > unaligned? Relying on x86 to be awesome. ;-) > > > Source/bmalloc/bmalloc/Algorithm.h:337 > > + "addq $4, %%rcx\n\t" > > + "4:\n\t" > > + "movl %%eax, (%%rdi, %%rcx)\n\t" > > + "subq $4, %%rcx\n\t" > > + "jae 4b\n\t" > > We already know that sizeof(T) % sizeof(uint32_t) == 0. Hence, the only way > that we can get here (%rcx != -8) is if there's one 4 byte word to write. I > think you can remove this addq, subq, and jae. This reduces to: > "movl %%eax, (%%rdi)\n\t" That's a really good observation. https://bugs.webkit.org/show_bug.cgi?id=182617 > > > Source/bmalloc/bmalloc/Algorithm.h:353 > > + "movq %%rax, (%%rdi, %%rcx)\n\t" > > Ditto with assumption about alignment being a non-issue. Not an issue. > > > Source/bmalloc/bmalloc/Algorithm.h:377 > > + "movq %%rax, (%%rdi, %%rcx)\n\t" > > Ditto with assumption about alignment being a non-issue. Not an issue.

Filip Pizlo

Comment 10 2018-02-08 12:52:00 PST

(In reply to Filip Pizlo from comment #9) > (In reply to Mark Lam from comment #8) > > Comment on attachment 333386 [details] > > the patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=333386&action=review > > > > FYI, I did my review from the bottom up (since the fastCopy and fastZeroFill > > implementations are located towards the bottom). It might make more sense > > to read my comments backwards. > > > > > Source/WTF/wtf/FastCopy.h:33 > > > +template<typename T> > > > +void fastCopy(T* dst, const T* src, size_t length) > > > > I assume this is identical to the bmalloc version below and didn't review > > this thoroughly. > > > > > Source/WTF/wtf/FastCopy.h:143 > > > +inline void fastCopyBytes(void* dst, const void* src, size_t bytes) > > > > At first, I was pondering whether we can rename this to just be fastCopy. > > After doing the entire review, I am strongly of the opinion that we should > > NOT rename this to fastCopy. There's a strong need to distinguish fastCopy > > and fastCopyBytes apart. > > > > One of the class of errors that can arise from fastCopy() taking a count of > > T to copy instead of size_t bytes is that if someone edits the code later > > and innocently changes the type of the pointer, an error may silently creep > > in. Consider this scenario: > > > > Initially, let's say we have this contrived code in a class: > > > > void foo(Stuff* myStuff, Stuff* other, int numberOfStuff) { > > m_myStuff = myStuff; > > m_other = other; > > m_length = numberOfStuff; > > bar(); // where bar contains ... fastCopy(m_myStuff, m_other, > > m_length); // <======== this is correct. > > } > > > > Stuff m_myStuff; > > Stuff m_other; > > size_t m_length; > > > > Some time later, for reasons, someone decides to change type of the pointers > > like so: > > > > void* m_myStuff; > > void* m_other; > > size_t m_length; > > > > The person making the change may not notice that bar() relies on typeof > > m_myStuff and m_other not being void*. If you can be sure that calling > > fastCopy() with void* pointers will result in a build error, then this error > > will not go unnoticed. I think we're fine, but can you please do a quick > > test to make sure that this is indeed the case i.e. we should get build > > errors if fastCopy() is called with void* pointers. > > We do not get build errors if fastCopy() is called with void*. I was wrong. We totally do, because of sizeof(void).

Filip Pizlo

Comment 11 2018-02-08 12:57:04 PST

Created attachment 333413 [details] patch for landing

EWS Watchlist

Comment 12 2018-02-08 12:59:56 PST

Attachment 333413 [details] did not pass style-queue: ERROR: Source/bmalloc/bmalloc/Algorithm.h:190: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:203: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:210: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:231: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:238: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:259: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:285: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:290: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:295: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:285: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:290: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:295: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:285: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:290: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:295: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:265: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:285: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:290: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:295: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:290: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:295: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:290: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:295: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:301: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:313: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:314: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:314: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:320: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:339: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:340: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:340: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:346: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:365: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:366: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:389: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:390: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:394: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:366: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:389: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:390: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:394: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:389: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:390: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:394: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:371: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:389: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:390: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:394: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:390: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:394: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:394: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:39: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:51: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:52: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:52: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:58: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:77: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:78: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:78: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:84: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:103: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:104: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:127: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:128: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:132: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:136: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:104: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:127: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:128: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:132: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:136: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:127: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:128: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:132: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:136: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:109: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:127: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:128: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:132: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:136: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:128: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:132: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:136: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:132: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:136: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:39: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastCopy.h:52: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:59: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastCopy.h:80: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:87: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastCopy.h:108: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:134: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:139: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:143: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:134: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:139: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:143: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:134: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:139: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:143: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:114: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastCopy.h:134: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:139: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:143: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:139: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:143: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:139: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:143: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] Total errors found: 108 in 42 files If any of these errors are false positives, please file a bug against check-webkit-style.

Mark Lam

Comment 13 2018-02-08 13:01:44 PST

(In reply to Filip Pizlo from comment #10) > (In reply to Filip Pizlo from comment #9) > > We do not get build errors if fastCopy() is called with void*. > > I was wrong. We totally do, because of sizeof(void). Excellent.

Mark Lam

Comment 14 2018-02-08 13:04:52 PST

Comment on attachment 333413 [details] patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=333413&action=review > Source/WTF/wtf/FastCopy.h:101 > + "addq $6, %%rcx\n\t" // FIXME: This isn't really a loop. https://bugs.webkit.org/show_bug.cgi?id=182617 Wrong place. This comment belongs on the uint32_t case above. > Source/WTF/wtf/FastZeroFill.h:97 > + "addq $6, %%rcx\n\t" // FIXME: This isn't really a loop. https://bugs.webkit.org/show_bug.cgi?id=182617 Wrong place. This comment belongs on the uint32_t case above. > Source/bmalloc/bmalloc/Algorithm.h:252 > + "addq $6, %%rcx\n\t" // FIXME: This isn't really a loop. https://bugs.webkit.org/show_bug.cgi?id=182617 Wrong place. This comment belongs on the uint32_t case above. > Source/bmalloc/bmalloc/Algorithm.h:359 > + "addq $6, %%rcx\n\t" // FIXME: This isn't really a loop. https://bugs.webkit.org/show_bug.cgi?id=182617 Wrong place. This comment belongs on the uint32_t case above.

Filip Pizlo

Comment 15 2018-02-08 18:11:45 PST

(In reply to Mark Lam from comment #14) > Comment on attachment 333413 [details] > patch for landing > > View in context: > https://bugs.webkit.org/attachment.cgi?id=333413&action=review > > > Source/WTF/wtf/FastCopy.h:101 > > + "addq $6, %%rcx\n\t" // FIXME: This isn't really a loop. https://bugs.webkit.org/show_bug.cgi?id=182617 > > Wrong place. This comment belongs on the uint32_t case above. > > > Source/WTF/wtf/FastZeroFill.h:97 > > + "addq $6, %%rcx\n\t" // FIXME: This isn't really a loop. https://bugs.webkit.org/show_bug.cgi?id=182617 > > Wrong place. This comment belongs on the uint32_t case above. > > > Source/bmalloc/bmalloc/Algorithm.h:252 > > + "addq $6, %%rcx\n\t" // FIXME: This isn't really a loop. https://bugs.webkit.org/show_bug.cgi?id=182617 > > Wrong place. This comment belongs on the uint32_t case above. > > > Source/bmalloc/bmalloc/Algorithm.h:359 > > + "addq $6, %%rcx\n\t" // FIXME: This isn't really a loop. https://bugs.webkit.org/show_bug.cgi?id=182617 > > Wrong place. This comment belongs on the uint32_t case above. Fixed!

Filip Pizlo

Comment 16 2018-02-08 18:13:45 PST

Landed in http://trac.webkit.org/changeset/228306/webkit Saam and I will watch the bots to see if this is really a speed-up. It's hard to tell. When I retested PLT, I didn't see anything. But a Speedometer run did see a maybe-speed-up. So who knows!

Filip Pizlo

Comment 17 2018-02-08 18:14:01 PST

Landed in http://trac.webkit.org/changeset/228306/webkit

Radar WebKit Bug Importer

Comment 18 2018-02-08 18:14:27 PST

<rdar://problem/37374970>

Carlos Alberto Lopez Perez

Comment 19 2018-02-09 05:21:30 PST

This seems to have broken Linux 32-bit (ARMv7) builds: - Log from Linux GTK ARMv7 (thumb2) bot: https://build.webkit.org/builders/GTK%20Linux%20ARM%20Release/builds/2258/steps/compile-webkit/logs/stdio/text - Log from Linux JSCOnly ARMv7 (thumb2) bot: https://build.webkit.org/builders/JSCOnly%20Linux%20ARMv7%20Thumb2%20Release/builds/3206/steps/compile-webkit/logs/stdio/text - Log from Linux JSCOnly ARMv7 (traditional) bot: https://build.webkit.org/builders/JSCOnly%20Linux%20ARMv7%20Traditional%20Release/builds/2487/steps/compile-webkit/logs/stdio/text

Carlos Alberto Lopez Perez

Comment 20 2018-02-09 05:37:49 PST

Committed r228317: <https://trac.webkit.org/changeset/228317>

Carlos Alberto Lopez Perez

Comment 21 2018-02-09 05:50:47 PST

Committed r228318: <https://trac.webkit.org/changeset/228318>

Michael Catanzaro

Comment 22 2018-02-09 08:07:32 PST

(In reply to Carlos Alberto Lopez Perez from comment #20) > Committed r228317: <https://trac.webkit.org/changeset/228317> We should avoid architecture-specific #includes whenever possible. Adding one #include <string.h> is not going to slow down the build very much at all; it's not worth the cost of the include guards, IMO.

Carlos Alberto Lopez Perez

Comment 23 2018-02-09 09:04:31 PST

(In reply to Michael Catanzaro from comment #22) > (In reply to Carlos Alberto Lopez Perez from comment #20) > > Committed r228317: <https://trac.webkit.org/changeset/228317> > > We should avoid architecture-specific #includes whenever possible. Adding > one #include <string.h> is not going to slow down the build very much at > all; it's not worth the cost of the include guards, IMO. Thanks for the suggestion. I have uploaded a patch for this to bug 182642

Matt Lewis

Comment 24 2018-02-16 14:06:37 PST

Reverted r228318 for reason: The patch that this attempted to fix was rolled out already. Committed r228582: <https://trac.webkit.org/changeset/228582>

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

Status REOPENED

Resolution

Priority P2

Severity Normal

Classification Unclassified

Version WebKit Nightly Build

Hardware All

OS All

Product WebKit

Component Web Template Framework

Assignee

Filip Pizlo

Reported

2018-02-06 20:04 PST

Modified

2018-02-16 14:06 PST History

CC List

14 users Show

URL

Keywords InRadar

Depends on

Blocks