Summary: | [B3] Add more tests for Check and fix bugs this found | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Filip Pizlo <fpizlo> | ||||||
Component: | JavaScriptCore | Assignee: | Filip Pizlo <fpizlo> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue | ||||||
Priority: | P2 | ||||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
Filip Pizlo
2015-11-09 17:53:59 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]
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.
Created attachment 265197 [details]
the patch
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.
Comment on attachment 265197 [details]
the patch
r=me
Landed in http://trac.webkit.org/changeset/192257 |