The "conway" test on the new Celtic Kane benchmark spends half its time in op_less. This is because it does conditionals like this: if (a < b && c > d)... and the logical and operator prevents any branch/condition fusion from taking place.
Created attachment 44354 [details] Patch
Attachment 44354 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 JavaScriptCore/jit/JITArithmetic.cpp:151: JIT::emit_op_jless is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] JavaScriptCore/jit/JITArithmetic.cpp:188: JIT::emitSlow_op_jless is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] JavaScriptCore/jit/JITArithmetic.cpp:1529: JIT::emit_op_jless is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] JavaScriptCore/jit/JITArithmetic.cpp:1567: JIT::emitSlow_op_jless is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] JavaScriptCore/jit/JITArithmetic.cpp:1595: More than one command on the same line [whitespace/newline] [4] JavaScriptCore/jit/JITArithmetic.cpp:1636: More than one command on the same line [whitespace/newline] [4] JavaScriptCore/jit/JIT.h:739: emit_op_jless is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] JavaScriptCore/jit/JIT.h:748: emit_op_loop_if_false is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] JavaScriptCore/jit/JIT.h:823: emitSlow_op_jless is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] JavaScriptCore/jit/JIT.h:829: emitSlow_op_loop_if_false is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] JavaScriptCore/jit/JITOpcodes.cpp:746: JIT::emit_op_loop_if_false is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] JavaScriptCore/jit/JITOpcodes.cpp:777: JIT::emitSlow_op_loop_if_false is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] JavaScriptCore/interpreter/Interpreter.cpp:2650: Place brace on its own line for function definitions. [whitespace/braces] [4] JavaScriptCore/interpreter/Interpreter.cpp:2833: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 14
Created attachment 44357 [details] Patch
Attachment 44357 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 JavaScriptCore/jit/JITArithmetic.cpp:151: JIT::emit_op_jless is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] JavaScriptCore/jit/JITArithmetic.cpp:188: JIT::emitSlow_op_jless is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] JavaScriptCore/jit/JITArithmetic.cpp:1529: JIT::emit_op_jless is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] JavaScriptCore/jit/JITArithmetic.cpp:1567: JIT::emitSlow_op_jless is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] JavaScriptCore/jit/JITArithmetic.cpp:1595: More than one command on the same line [whitespace/newline] [4] JavaScriptCore/jit/JITArithmetic.cpp:1636: More than one command on the same line [whitespace/newline] [4] JavaScriptCore/jit/JIT.h:739: emit_op_jless is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] JavaScriptCore/jit/JIT.h:748: emit_op_loop_if_false is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] JavaScriptCore/jit/JIT.h:823: emitSlow_op_jless is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] JavaScriptCore/jit/JIT.h:829: emitSlow_op_loop_if_false is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] JavaScriptCore/jit/JITOpcodes.cpp:746: JIT::emit_op_loop_if_false is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] JavaScriptCore/jit/JITOpcodes.cpp:777: JIT::emitSlow_op_loop_if_false is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] JavaScriptCore/interpreter/Interpreter.cpp:2650: Place brace on its own line for function definitions. [whitespace/braces] [4] JavaScriptCore/interpreter/Interpreter.cpp:2833: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 14
Are these naming warnings useful? Do we plan to change these names to use camel case?
Comment on attachment 44357 [details] Patch r=me on the code, though i'd like gbarra or ggaren to also look at the jit code gen. The changes to the sunspider script seem accidental I'd prefer it if the test case also covered float values in addition to integers. --Oliver
(In reply to comment #5) > Are these naming warnings useful? Do we plan to change these names to use > camel case? The opcode-related names are an exception to the usual naming rules. They should probably be whitelisted somehow for style checking purposes. Allowing names with _op_ in the JavaScriptCore directory would probably do it.
(In reply to comment #4) > > JavaScriptCore/interpreter/Interpreter.cpp:2650: Place brace on its own line > for function definitions. [whitespace/braces] [4] > JavaScriptCore/interpreter/Interpreter.cpp:2833: Place brace on its own line > for function definitions. [whitespace/braces] [4] These are also false positives (the lines in question are funky funky macros followed by a brace, not function definitions).
Created attachment 44358 [details] Fix style issues; remove bogus sunspider change; add tests for float case codegen
Attachment 44358 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 JavaScriptCore/jit/JITArithmetic.cpp:151: JIT::emit_op_jless is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] JavaScriptCore/jit/JITArithmetic.cpp:188: JIT::emitSlow_op_jless is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] JavaScriptCore/jit/JITArithmetic.cpp:1529: JIT::emit_op_jless is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] JavaScriptCore/jit/JITArithmetic.cpp:1567: JIT::emitSlow_op_jless is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] JavaScriptCore/jit/JIT.h:739: emit_op_jless is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] JavaScriptCore/jit/JIT.h:748: emit_op_loop_if_false is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] JavaScriptCore/jit/JIT.h:823: emitSlow_op_jless is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] JavaScriptCore/jit/JIT.h:829: emitSlow_op_loop_if_false is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] JavaScriptCore/jit/JITOpcodes.cpp:746: JIT::emit_op_loop_if_false is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] JavaScriptCore/jit/JITOpcodes.cpp:777: JIT::emitSlow_op_loop_if_false is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] JavaScriptCore/interpreter/Interpreter.cpp:2650: Place brace on its own line for function definitions. [whitespace/braces] [4] JavaScriptCore/interpreter/Interpreter.cpp:2833: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 12
Comment on attachment 44358 [details] Fix style issues; remove bogus sunspider change; add tests for float case codegen r=me
Committed r51735: <http://trac.webkit.org/changeset/51735>
> The opcode-related names are an exception to the usual naming rules. They > should probably be whitelisted somehow for style checking purposes. Allowing > names with _op_ in the JavaScriptCore directory would probably do it. Filed https://bugs.webkit.org/show_bug.cgi?id=32192
> These are also false positives (the lines in question are funky funky macros > followed by a brace, not function definitions). Filed: https://bugs.webkit.org/show_bug.cgi?id=32193