Bug 151073

Summary: [B3] Add more tests for Check and fix bugs this found
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue
Priority: P2    
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
the patch
none
the patch saam: review+

Filip Pizlo
Reported 2015-11-09 17:53:59 PST
Patch forthcoming.
Attachments
the patch (7.28 KB, patch)
2015-11-09 17:57 PST, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
the patch (7.24 KB, patch)
2015-11-10 10:33 PST, Filip Pizlo 		saam: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Filip Pizlo
Comment 1 2015-11-09 17:57:46 PST
Created attachment 265131 [details] the patch It's cool to see the MegaCombo test in action: [pizlo@shakezilla OpenSource] JSC_dumpDisassembly=true JSC_dumpGraphAtEachPhase=true JSC_validateGraphAtEachPhase=true DYLD_FRAMEWORK_PATH=WebKitBuild/Debug/ WebKitBuild/Debug/testb3 CheckMegaCombo testCheckMegaCombo()... B3 after initial, before reduceStrength: BB#0: ; frequency = nan Int64 @0 = ArgumentReg(%rdi) Int64 @1 = ArgumentReg(%rsi) Int32 @2 = Trunc(@1) Int64 @3 = ZExt32(@2) Int32 @4 = Const32(1) Int64 @5 = Shl(@3, @4) Int64 @6 = Add(@0, @5) Int32 @7 = Load8S(@6, ControlDependent|Reads:Top) Int32 @8 = Const32(42) Int32 @9 = LessThan(@7, @8) Void @10 = Check(@9:Any, generator = 0x10a3eb010, clobbered = [], usedRegisters = [], ExitsSideways) Int32 @11 = Const32(0) Void @12 = Return(@11, Terminal) B3 after reduceStrength, before moveConstants: BB#0: ; frequency = nan Int64 @0 = ArgumentReg(%rdi) Int64 @1 = ArgumentReg(%rsi) Int32 @2 = Trunc(@1) Int64 @3 = ZExt32(@2) Int32 @4 = Const32(1) Int64 @5 = Shl(@3, @4) Int64 @6 = Add(@0, @5) Int32 @7 = Load8S(@6, ControlDependent|Reads:Top) Int32 @8 = Const32(42) Int32 @9 = LessThan(@7, @8) Void @10 = Check(@9:Any, generator = 0x10a3eb010, clobbered = [], usedRegisters = [], ExitsSideways) Int32 @11 = Const32(0) Void @12 = Return(@11, Terminal) B3 after moveConstants, before lowerToAir: BB#0: ; frequency = nan Int64 @0 = ArgumentReg(%rdi) Int64 @1 = ArgumentReg(%rsi) Int32 @2 = Trunc(@1) Int64 @3 = ZExt32(@2) Int32 @4 = Const32(1) Int64 @5 = Shl(@3, @4) Int64 @6 = Add(@0, @5) Int32 @7 = Load8S(@6, ControlDependent|Reads:Top) Int32 @8 = Const32(42) Int32 @9 = LessThan(@7, @8) Void @10 = Check(@9:Any, generator = 0x10a3eb010, clobbered = [], usedRegisters = [], ExitsSideways) Int32 @11 = Const32(0) Void @12 = Return(@11, Terminal) Air after initial, before eliminateDeadCode: BB#0: ; frequency = nan Move %rsi, %tmp7, @1 Move %rdi, %tmp1, @0 Move32 %tmp7, %tmp2, @3 Move $1, %tmp6, @4 Move %tmp2, %tmp5, @5 Lshift64 $1, %tmp5, @5 Move %tmp1, %tmp4, @6 Add64 %tmp5, %tmp4, @6 Move $42, %tmp3, @8 Patch &Branch8(3), LessThan, (%tmp1,%tmp2,2), $42, @10 Move $0, %tmp0, @11 Move $0, %rax, @12 Ret @12 Specials: &Branch8(3): B3::CheckValue lowered to Branch8 with 3 args. Air after eliminateDeadCode, before spillEverything: BB#0: ; frequency = nan Move %rsi, %tmp7, @1 Move %rdi, %tmp1, @0 Move32 %tmp7, %tmp2, @3 Patch &Branch8(3), LessThan, (%tmp1,%tmp2,2), $42, @10 Move $0, %rax, @12 Ret @12 Specials: &Branch8(3): B3::CheckValue lowered to Branch8 with 3 args. Air after spillEverything, before handleCalleeSaves: BB#0: ; frequency = nan Move %rsi, (stack0), @1 Move %rdi, (stack6), @0 Move32 (stack0), %rax, @3 Move %rax, (stack5), @3 Move (stack6), %rcx, @10 Move (stack5), %rdx, @10 Patch &Branch8(3), LessThan, (%rcx,%rdx,2), $42, @10 Move $0, %rax, @12 Ret @12 Stack slots: stack0: byteSize = 8, offsetFromFP = 0, kind = Anonymous stack1: byteSize = 8, offsetFromFP = 0, kind = Anonymous stack2: byteSize = 8, offsetFromFP = 0, kind = Anonymous stack3: byteSize = 8, offsetFromFP = 0, kind = Anonymous stack4: byteSize = 8, offsetFromFP = 0, kind = Anonymous stack5: byteSize = 8, offsetFromFP = 0, kind = Anonymous stack6: byteSize = 8, offsetFromFP = 0, kind = Anonymous stack7: byteSize = 8, offsetFromFP = 0, kind = Anonymous Specials: &Branch8(3): B3::CheckValue lowered to Branch8 with 3 args. Air after handleCalleeSaves, before allocateStack: BB#0: ; frequency = nan Move %rsi, (stack0), @1 Move %rdi, (stack6), @0 Move32 (stack0), %rax, @3 Move %rax, (stack5), @3 Move (stack6), %rcx, @10 Move (stack5), %rdx, @10 Patch &Branch8(3), LessThan, (%rcx,%rdx,2), $42, @10 Move $0, %rax, @12 Ret @12 Stack slots: stack0: byteSize = 8, offsetFromFP = 0, kind = Anonymous stack1: byteSize = 8, offsetFromFP = 0, kind = Anonymous stack2: byteSize = 8, offsetFromFP = 0, kind = Anonymous stack3: byteSize = 8, offsetFromFP = 0, kind = Anonymous stack4: byteSize = 8, offsetFromFP = 0, kind = Anonymous stack5: byteSize = 8, offsetFromFP = 0, kind = Anonymous stack6: byteSize = 8, offsetFromFP = 0, kind = Anonymous stack7: byteSize = 8, offsetFromFP = 0, kind = Anonymous Specials: &Branch8(3): B3::CheckValue lowered to Branch8 with 3 args. Air after allocateStack, before simplifyCFG: BB#0: ; frequency = nan Move %rsi, -8(%rbp), @1 Move %rdi, -16(%rbp), @0 Move32 -8(%rbp), %rax, @3 Move %rax, -8(%rbp), @3 Move -16(%rbp), %rcx, @10 Move -8(%rbp), %rdx, @10 Patch &Branch8(3), LessThan, (%rcx,%rdx,2), $42, @10 Move $0, %rax, @12 Ret @12 Stack slots: stack0: byteSize = 8, offsetFromFP = -8, kind = Anonymous stack1: byteSize = 8, offsetFromFP = -8, kind = Anonymous stack2: byteSize = 8, offsetFromFP = -8, kind = Anonymous stack3: byteSize = 8, offsetFromFP = -8, kind = Anonymous stack4: byteSize = 8, offsetFromFP = -8, kind = Anonymous stack5: byteSize = 8, offsetFromFP = -8, kind = Anonymous stack6: byteSize = 8, offsetFromFP = -16, kind = Anonymous stack7: byteSize = 8, offsetFromFP = -8, kind = Anonymous Specials: &Branch8(3): B3::CheckValue lowered to Branch8 with 3 args. Frame size: 16 Air after simplifyCFG, before reportUsedRegisters: BB#0: ; frequency = nan Move %rsi, -8(%rbp), @1 Move %rdi, -16(%rbp), @0 Move32 -8(%rbp), %rax, @3 Move %rax, -8(%rbp), @3 Move -16(%rbp), %rcx, @10 Move -8(%rbp), %rdx, @10 Patch &Branch8(3), LessThan, (%rcx,%rdx,2), $42, @10 Move $0, %rax, @12 Ret @12 Stack slots: stack0: byteSize = 8, offsetFromFP = -8, kind = Anonymous stack1: byteSize = 8, offsetFromFP = -8, kind = Anonymous stack2: byteSize = 8, offsetFromFP = -8, kind = Anonymous stack3: byteSize = 8, offsetFromFP = -8, kind = Anonymous stack4: byteSize = 8, offsetFromFP = -8, kind = Anonymous stack5: byteSize = 8, offsetFromFP = -8, kind = Anonymous stack6: byteSize = 8, offsetFromFP = -16, kind = Anonymous stack7: byteSize = 8, offsetFromFP = -8, kind = Anonymous Specials: &Branch8(3): B3::CheckValue lowered to Branch8 with 3 args. Frame size: 16 Air after reportUsedRegisters, before generation: BB#0: ; frequency = nan Move %rsi, -8(%rbp), @1 Move %rdi, -16(%rbp), @0 Move32 -8(%rbp), %rax, @3 Move %rax, -8(%rbp), @3 Move -16(%rbp), %rcx, @10 Move -8(%rbp), %rdx, @10 Patch &Branch8(3), LessThan, (%rcx,%rdx,2), $42, @10 Move $0, %rax, @12 Ret @12 Stack slots: stack0: byteSize = 8, offsetFromFP = -8, kind = Anonymous stack1: byteSize = 8, offsetFromFP = -8, kind = Anonymous stack2: byteSize = 8, offsetFromFP = -8, kind = Anonymous stack3: byteSize = 8, offsetFromFP = -8, kind = Anonymous stack4: byteSize = 8, offsetFromFP = -8, kind = Anonymous stack5: byteSize = 8, offsetFromFP = -8, kind = Anonymous stack6: byteSize = 8, offsetFromFP = -16, kind = Anonymous stack7: byteSize = 8, offsetFromFP = -8, kind = Anonymous Specials: &Branch8(3): B3::CheckValue lowered to Branch8 with 3 args. Frame size: 16 Generated JIT code for B3::Compilation: Code at [0x431355e01000, 0x431355e01040): 0x431355e01000: push %rbp 0x431355e01001: mov %rsp, %rbp 0x431355e01004: add $0xfffffffffffffff0, %rsp 0x431355e01008: mov %rsi, -0x8(%rbp) 0x431355e0100c: mov %rdi, -0x10(%rbp) 0x431355e01010: mov -0x8(%rbp), %eax 0x431355e01013: mov %rax, -0x8(%rbp) 0x431355e01017: mov -0x10(%rbp), %rcx 0x431355e0101b: mov -0x8(%rbp), %rdx 0x431355e0101f: cmp $0x2a, (%rcx,%rdx,2) 0x431355e01023: jl 0x431355e01035 0x431355e01029: mov $0x0, %rax 0x431355e01030: mov %rbp, %rsp 0x431355e01033: pop %rbp 0x431355e01034: ret 0x431355e01035: mov $0x2a, %eax 0x431355e0103a: mov %rbp, %rsp 0x431355e0103d: pop %rbp 0x431355e0103e: ret testCheckMegaCombo(): OK! [pizlo@shakezilla OpenSource]
WebKit Commit Bot
Comment 2 2015-11-09 18:00:21 PST
Attachment 265131 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/b3/testb3.cpp:2417: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2418: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2420: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2433: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2433: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2434: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2434: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2435: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2436: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2437: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2464: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2465: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2467: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2482: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2482: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2484: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2484: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2486: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2488: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2490: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] Total errors found: 20 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 3 2015-11-10 10:33:34 PST
Created attachment 265197 [details] the patch
WebKit Commit Bot
Comment 4 2015-11-10 10:36:15 PST
Attachment 265197 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/b3/testb3.cpp:2417: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2418: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2420: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2433: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2433: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2434: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2434: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2435: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2436: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2437: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2464: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2465: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2467: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2482: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2482: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2484: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2484: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2486: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2488: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] ERROR: Source/JavaScriptCore/b3/testb3.cpp:2490: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2] Total errors found: 20 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Saam Barati
Comment 5 2015-11-10 10:39:48 PST
Comment on attachment 265197 [details] the patch r=me
Filip Pizlo
Comment 6 2015-11-10 11:15:03 PST
Landed in http://trac.webkit.org/changeset/192257
Note You need to log in before you can comment on or make changes to this bug.