WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
121064
DFG should support Int52 for local variables
https://bugs.webkit.org/show_bug.cgi?id=121064
Summary
DFG should support Int52 for local variables
Filip Pizlo
Reported
2013-09-09 18:54:23 PDT
...
Attachments
it begins
(10.58 KB, patch)
2013-09-11 19:24 PDT
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
making some small progress
(15.59 KB, patch)
2013-09-12 13:17 PDT
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
more
(27.10 KB, patch)
2013-09-12 14:44 PDT
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
it's getting real
(43.50 KB, patch)
2013-09-12 22:16 PDT
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
more
(48.08 KB, patch)
2013-09-13 18:54 PDT
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
starting to get there
(60.95 KB, patch)
2013-09-13 21:58 PDT
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
filling in the backend
(67.60 KB, patch)
2013-09-14 10:33 PDT
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
backend almost done
(73.31 KB, patch)
2013-09-14 13:13 PDT
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
more
(78.77 KB, patch)
2013-09-14 18:24 PDT
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
almost done!
(90.57 KB, patch)
2013-09-15 12:12 PDT
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
it ran some easy program
(133.97 KB, patch)
2013-09-15 15:16 PDT
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
it kind of passes some tests
(135.72 KB, patch)
2013-09-15 15:52 PDT
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
passes all DFG 64-bit tests
(140.88 KB, patch)
2013-09-15 20:24 PDT
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
starting on FTL
(155.12 KB, patch)
2013-09-15 21:49 PDT
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
more FTL
(162.23 KB, patch)
2013-09-15 22:13 PDT
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
the patch
(200.22 KB, patch)
2013-09-16 16:32 PDT
,
Filip Pizlo
webkit-ews
: commit-queue-
Details
Formatted Diff
Diff
the patch
(202.00 KB, patch)
2013-09-16 18:45 PDT
,
Filip Pizlo
webkit-ews
: commit-queue-
Details
Formatted Diff
Diff
the patch
(202.78 KB, patch)
2013-09-16 19:05 PDT
,
Filip Pizlo
webkit-ews
: commit-queue-
Details
Formatted Diff
Diff
the patch
(202.78 KB, patch)
2013-09-16 22:53 PDT
,
Filip Pizlo
oliver
: review+
Details
Formatted Diff
Diff
the patch
(208.92 KB, patch)
2013-09-17 12:05 PDT
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
Show Obsolete
(19)
View All
Add attachment
proposed patch, testcase, etc.
Filip Pizlo
Comment 1
2013-09-11 19:24:42 PDT
Created
attachment 211385
[details]
it begins
Filip Pizlo
Comment 2
2013-09-12 13:17:24 PDT
Created
attachment 211463
[details]
making some small progress
Filip Pizlo
Comment 3
2013-09-12 14:44:01 PDT
Created
attachment 211477
[details]
more
Filip Pizlo
Comment 4
2013-09-12 22:16:17 PDT
Created
attachment 211507
[details]
it's getting real
Filip Pizlo
Comment 5
2013-09-13 18:54:52 PDT
Created
attachment 211614
[details]
more
Filip Pizlo
Comment 6
2013-09-13 21:58:12 PDT
Created
attachment 211619
[details]
starting to get there
Filip Pizlo
Comment 7
2013-09-14 10:33:28 PDT
Created
attachment 211655
[details]
filling in the backend
Filip Pizlo
Comment 8
2013-09-14 13:13:05 PDT
Created
attachment 211666
[details]
backend almost done
Filip Pizlo
Comment 9
2013-09-14 18:24:05 PDT
Created
attachment 211679
[details]
more
Filip Pizlo
Comment 10
2013-09-15 12:12:37 PDT
Created
attachment 211723
[details]
almost done!
Filip Pizlo
Comment 11
2013-09-15 15:16:07 PDT
Created
attachment 211730
[details]
it ran some easy program Here's some groovy IR, including StrictInt48 addition over Int32 inputs. Note the lack of an overflow check: adding two int32's to produce an int48 doesn't need it. :-) Note that this is in debug mode so it probably has some JIT assertions. Generated DFG JIT code for foo#CZzf6Y:[0x10150aca0->0x10244fe70, DFGFunctionCall], instruction count = 12: Optimized with execution counter = 1005.000000/1047.000000, 5 Code at [0x3b25ef202aa0, 0x3b25ef202bdb): 0x3b25ef202aa0: pop %rcx 0x3b25ef202aa1: mov %rcx, 0x10(%r13) 0x3b25ef202aa5: mov $0x10150aca0, %r11 0x3b25ef202aaf: mov %r11, 0x8(%r13) 0x3b25ef202ab3: lea -0x10(%r13), %rdx 0x3b25ef202ab7: mov $0x101501918, %r11 0x3b25ef202ac1: cmp %rdx, (%r11) 0x3b25ef202ac4: ja 0x3b25ef202b18 Block #0 (bc#0): (OSR target) Predecessors: Dominated by: #0 Dominates: #0 0x3b25ef202aca: test $0x7, %r13b 0x3b25ef202ace: jz 0x3b25ef202ad5 0x3b25ef202ad4: int3 1: < 2:-> SetArgument(, arg1(B~<Int32>/FlushedJSValue), W:SideState, bc#0) predicting Int32 2: < 2:-> SetArgument(, arg2(C~<Int32>/FlushedJSValue), W:SideState, bc#0) predicting Int32 3: < 1:0> GetLocal(@1, JS|UseAsOther, Int32, arg1(B~<Int32>/FlushedJSValue), R:Variables(8), bc#1) predicting Int32 0x3b25ef202ad5: mov 0x40(%r13), %rax 4: < 1:-1> GetLocal(@2, JS|UseAsOther, Int32, arg2(C~<Int32>/FlushedJSValue), R:Variables(9), bc#1) predicting Int32 0x3b25ef202ad9: mov 0x48(%r13), %rdx 5: <!1:-1> ValueAdd(Check:MachineInt:@3<Int32>, Check:MachineInt:@4<Int32>, JS|MustGen|MightClobber|UseAsOther|MayOverflow, Int48, bc#1) 0x3b25ef202add: cmp %r14, %rax 0x3b25ef202ae0: jb 0x3b25ef202b99 0x3b25ef202ae6: movsxd %eax, %rax 0x3b25ef202ae9: cmp %r14, %rdx 0x3b25ef202aec: jb 0x3b25ef202bba 0x3b25ef202af2: movsxd %edx, %rdx 0x3b25ef202af5: add %rdx, %rax 6: skipped < 0:-> ZombieHint(r0(D~<Int48>/FlushedJSValue), W:SideState, bc#1) 7: < 1:0> JSConstant(JS|UseAsOther, Int48, $0 = Double: 4748581863621132288, 3000000000.000000, bc#6) 8: <!1:0> CompareEq(MachineInt:@5<Int48>, MachineInt:@7<Int48>, Boolean|MustGen|MightClobber|UseAsOther|CanExit, Bool, bc#6) 0x3b25ef202af8: mov $0xb2d05e00, %rdx 0x3b25ef202b02: cmp %rdx, %rax 0x3b25ef202b05: setz %al 0x3b25ef202b08: movzx %al, %eax 0x3b25ef202b0b: or $0x6, %eax 9: skipped < 0:-> MovHint(@8<Boolean>, r0(E~<Boolean>/FlushedJSValue), W:SideState, bc#6) 10: <!0:-> Flush(@2, MustGen, arg2(C~<Int32>/FlushedJSValue), W:SideState, bc#10) predicting Int32 11: <!0:-> Flush(@1, MustGen, arg1(B~<Int32>/FlushedJSValue), W:SideState, bc#10) predicting Int32 12: <!0:-> Return(@8<Boolean>, MustGen, W:SideState, bc#10) 0x3b25ef202b0e: mov 0x10(%r13), %rdx 0x3b25ef202b12: mov 0x28(%r13), %r13 0x3b25ef202b16: push %rdx 0x3b25ef202b17: ret (End Of Main Path) 0x3b25ef202b18: mov %rsp, %rdi 0x3b25ef202b1b: mov %r13, 0x58(%rsp) 0x3b25ef202b20: mov $0x40000000, 0x34(%r13) 0x3b25ef202b28: mov $0x10036ea00, %r11 0x3b25ef202b32: call %r11 0x3b25ef202b35: jmp 0x3b25ef202aca 0x3b25ef202b3a: pop %rcx 0x3b25ef202b3b: mov %rcx, 0x10(%r13) 0x3b25ef202b3f: mov $0x10150aca0, %r11 0x3b25ef202b49: mov %r11, 0x8(%r13) 0x3b25ef202b4d: mov 0x30(%r13), %edx 0x3b25ef202b51: cmp $0x3, %edx 0x3b25ef202b54: jae 0x3b25ef202ab3 0x3b25ef202b5a: mov %rsp, %rdi 0x3b25ef202b5d: mov %r13, 0x58(%rsp) 0x3b25ef202b62: mov $0x40000001, 0x34(%r13) 0x3b25ef202b6a: mov $0x100374060, %r11 0x3b25ef202b74: call %r11 0x3b25ef202b77: test %eax, %eax 0x3b25ef202b79: jz 0x3b25ef202ab3 0x3b25ef202b7f: mov $0x40000002, 0x34(%r13) 0x3b25ef202b87: mov $0x3b25ef201940, %r11 0x3b25ef202b91: call %r11 0x3b25ef202b94: jmp 0x3b25ef202ab3 0x3b25ef202b99: test $0x7, %r13b 0x3b25ef202b9d: jz 0x3b25ef202ba4 0x3b25ef202ba3: int3 0x3b25ef202ba4: mov $0x101813120, %r11 0x3b25ef202bae: mov $0x0, (%r11) 0x3b25ef202bb5: jmp 0x3b25ef201c60 0x3b25ef202bba: test $0x7, %r13b 0x3b25ef202bbe: jz 0x3b25ef202bc5 0x3b25ef202bc4: int3 0x3b25ef202bc5: mov $0x101813120, %r11 0x3b25ef202bcf: mov $0x1, (%r11) 0x3b25ef202bd6: jmp 0x3b25ef201c60 Structures:
Filip Pizlo
Comment 12
2013-09-15 15:52:22 PDT
Created
attachment 211732
[details]
it kind of passes some tests
Filip Pizlo
Comment 13
2013-09-15 20:24:30 PDT
Created
attachment 211735
[details]
passes all DFG 64-bit tests
Filip Pizlo
Comment 14
2013-09-15 21:49:32 PDT
Created
attachment 211736
[details]
starting on FTL
Filip Pizlo
Comment 15
2013-09-15 22:13:22 PDT
Created
attachment 211737
[details]
more FTL
Filip Pizlo
Comment 16
2013-09-16 16:32:01 PDT
Created
attachment 211842
[details]
the patch
WebKit Commit Bot
Comment 17
2013-09-16 16:34:10 PDT
Attachment 211842
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h', u'Source/JavaScriptCore/assembler/X86Assembler.h', u'Source/JavaScriptCore/bytecode/DataFormat.h', u'Source/JavaScriptCore/bytecode/OperandsInlines.h', u'Source/JavaScriptCore/bytecode/SpeculatedType.cpp', u'Source/JavaScriptCore/bytecode/SpeculatedType.h', u'Source/JavaScriptCore/bytecode/ValueRecovery.h', u'Source/JavaScriptCore/dfg/DFGAbstractInterpreter.h', u'Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h', u'Source/JavaScriptCore/dfg/DFGAbstractValue.cpp', u'Source/JavaScriptCore/dfg/DFGAbstractValue.h', u'Source/JavaScriptCore/dfg/DFGArrayMode.cpp', u'Source/JavaScriptCore/dfg/DFGAssemblyHelpers.h', u'Source/JavaScriptCore/dfg/DFGCSEPhase.cpp', u'Source/JavaScriptCore/dfg/DFGClobberize.h', u'Source/JavaScriptCore/dfg/DFGCommon.h', u'Source/JavaScriptCore/dfg/DFGFixupPhase.cpp', u'Source/JavaScriptCore/dfg/DFGFlushFormat.cpp', u'Source/JavaScriptCore/dfg/DFGFlushFormat.h', u'Source/JavaScriptCore/dfg/DFGGenerationInfo.h', u'Source/JavaScriptCore/dfg/DFGGraph.cpp', u'Source/JavaScriptCore/dfg/DFGGraph.h', u'Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.cpp', u'Source/JavaScriptCore/dfg/DFGJITCode.cpp', u'Source/JavaScriptCore/dfg/DFGMinifiedNode.h', u'Source/JavaScriptCore/dfg/DFGNode.h', u'Source/JavaScriptCore/dfg/DFGNodeFlags.h', u'Source/JavaScriptCore/dfg/DFGNodeType.h', u'Source/JavaScriptCore/dfg/DFGOSRExitCompiler.cpp', u'Source/JavaScriptCore/dfg/DFGOSRExitCompiler64.cpp', u'Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp', u'Source/JavaScriptCore/dfg/DFGSafeToExecute.h', u'Source/JavaScriptCore/dfg/DFGSilentRegisterSavePlan.h', u'Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp', u'Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h', u'Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp', u'Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp', u'Source/JavaScriptCore/dfg/DFGUseKind.cpp', u'Source/JavaScriptCore/dfg/DFGUseKind.h', u'Source/JavaScriptCore/dfg/DFGValueSource.cpp', u'Source/JavaScriptCore/dfg/DFGValueSource.h', u'Source/JavaScriptCore/dfg/DFGVariableAccessData.h', u'Source/JavaScriptCore/ftl/FTLCArgumentGetter.cpp', u'Source/JavaScriptCore/ftl/FTLCArgumentGetter.h', u'Source/JavaScriptCore/ftl/FTLCapabilities.cpp', u'Source/JavaScriptCore/ftl/FTLExitValue.cpp', u'Source/JavaScriptCore/ftl/FTLExitValue.h', u'Source/JavaScriptCore/ftl/FTLIntrinsicRepository.h', u'Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp', u'Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp', u'Source/JavaScriptCore/ftl/FTLOutput.h', u'Source/JavaScriptCore/ftl/FTLValueFormat.cpp', u'Source/JavaScriptCore/ftl/FTLValueFormat.h', u'Source/JavaScriptCore/ftl/FTLValueSource.cpp', u'Source/JavaScriptCore/ftl/FTLValueSource.h', u'Source/JavaScriptCore/interpreter/Register.h', u'Source/JavaScriptCore/runtime/Arguments.cpp', u'Source/JavaScriptCore/runtime/IndexingType.cpp', u'Source/JavaScriptCore/runtime/JSCJSValue.h', u'Source/JavaScriptCore/runtime/JSCJSValueInlines.h']" exit_code: 1 Source/JavaScriptCore/ftl/FTLCArgumentGetter.h:118: The parameter name "format" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:340: One line control clauses should not use braces. [whitespace/braces] [4] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:3372: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4008: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h:466: The parameter name "format" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:858: One line control clauses should not use braces. [whitespace/braces] [4] Total errors found: 6 in 61 files If any of these errors are false positives, please file a bug against check-webkit-style.
Early Warning System Bot
Comment 18
2013-09-16 16:42:14 PDT
Comment on
attachment 211842
[details]
the patch
Attachment 211842
[details]
did not pass qt-ews (qt): Output:
http://webkit-queues.appspot.com/results/1907087
EFL EWS Bot
Comment 19
2013-09-16 17:44:33 PDT
Comment on
attachment 211842
[details]
the patch
Attachment 211842
[details]
did not pass efl-ews (efl): Output:
http://webkit-queues.appspot.com/results/1859127
Filip Pizlo
Comment 20
2013-09-16 18:45:43 PDT
Created
attachment 211853
[details]
the patch Fix build and actually include WTF changes this time.
WebKit Commit Bot
Comment 21
2013-09-16 18:47:51 PDT
Attachment 211853
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h', u'Source/JavaScriptCore/assembler/X86Assembler.h', u'Source/JavaScriptCore/bytecode/DataFormat.h', u'Source/JavaScriptCore/bytecode/OperandsInlines.h', u'Source/JavaScriptCore/bytecode/SpeculatedType.cpp', u'Source/JavaScriptCore/bytecode/SpeculatedType.h', u'Source/JavaScriptCore/bytecode/ValueRecovery.h', u'Source/JavaScriptCore/dfg/DFGAbstractInterpreter.h', u'Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h', u'Source/JavaScriptCore/dfg/DFGAbstractValue.cpp', u'Source/JavaScriptCore/dfg/DFGAbstractValue.h', u'Source/JavaScriptCore/dfg/DFGArrayMode.cpp', u'Source/JavaScriptCore/dfg/DFGAssemblyHelpers.h', u'Source/JavaScriptCore/dfg/DFGCSEPhase.cpp', u'Source/JavaScriptCore/dfg/DFGClobberize.h', u'Source/JavaScriptCore/dfg/DFGCommon.h', u'Source/JavaScriptCore/dfg/DFGFixupPhase.cpp', u'Source/JavaScriptCore/dfg/DFGFlushFormat.cpp', u'Source/JavaScriptCore/dfg/DFGFlushFormat.h', u'Source/JavaScriptCore/dfg/DFGGenerationInfo.h', u'Source/JavaScriptCore/dfg/DFGGraph.cpp', u'Source/JavaScriptCore/dfg/DFGGraph.h', u'Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.cpp', u'Source/JavaScriptCore/dfg/DFGJITCode.cpp', u'Source/JavaScriptCore/dfg/DFGMinifiedNode.h', u'Source/JavaScriptCore/dfg/DFGNode.h', u'Source/JavaScriptCore/dfg/DFGNodeFlags.h', u'Source/JavaScriptCore/dfg/DFGNodeType.h', u'Source/JavaScriptCore/dfg/DFGOSRExitCompiler.cpp', u'Source/JavaScriptCore/dfg/DFGOSRExitCompiler64.cpp', u'Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp', u'Source/JavaScriptCore/dfg/DFGSafeToExecute.h', u'Source/JavaScriptCore/dfg/DFGSilentRegisterSavePlan.h', u'Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp', u'Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h', u'Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp', u'Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp', u'Source/JavaScriptCore/dfg/DFGUseKind.cpp', u'Source/JavaScriptCore/dfg/DFGUseKind.h', u'Source/JavaScriptCore/dfg/DFGValueSource.cpp', u'Source/JavaScriptCore/dfg/DFGValueSource.h', u'Source/JavaScriptCore/dfg/DFGVariableAccessData.h', u'Source/JavaScriptCore/ftl/FTLCArgumentGetter.cpp', u'Source/JavaScriptCore/ftl/FTLCArgumentGetter.h', u'Source/JavaScriptCore/ftl/FTLCapabilities.cpp', u'Source/JavaScriptCore/ftl/FTLExitValue.cpp', u'Source/JavaScriptCore/ftl/FTLExitValue.h', u'Source/JavaScriptCore/ftl/FTLIntrinsicRepository.h', u'Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp', u'Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp', u'Source/JavaScriptCore/ftl/FTLOutput.h', u'Source/JavaScriptCore/ftl/FTLValueFormat.cpp', u'Source/JavaScriptCore/ftl/FTLValueFormat.h', u'Source/JavaScriptCore/ftl/FTLValueSource.cpp', u'Source/JavaScriptCore/ftl/FTLValueSource.h', u'Source/JavaScriptCore/interpreter/Register.h', u'Source/JavaScriptCore/runtime/Arguments.cpp', u'Source/JavaScriptCore/runtime/IndexingType.cpp', u'Source/JavaScriptCore/runtime/JSCJSValue.h', u'Source/JavaScriptCore/runtime/JSCJSValueInlines.h', u'Source/WTF/ChangeLog', u'Source/WTF/wtf/PrintStream.h']" exit_code: 1 Source/JavaScriptCore/ftl/FTLCArgumentGetter.h:118: The parameter name "format" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:340: One line control clauses should not use braces. [whitespace/braces] [4] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:3373: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4009: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h:466: The parameter name "format" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:858: One line control clauses should not use braces. [whitespace/braces] [4] Total errors found: 6 in 63 files If any of these errors are false positives, please file a bug against check-webkit-style.
Early Warning System Bot
Comment 22
2013-09-16 18:55:01 PDT
Comment on
attachment 211853
[details]
the patch
Attachment 211853
[details]
did not pass qt-ews (qt): Output:
http://webkit-queues.appspot.com/results/1844196
Filip Pizlo
Comment 23
2013-09-16 19:05:49 PDT
Created
attachment 211854
[details]
the patch Found and fixed some bugs.
WebKit Commit Bot
Comment 24
2013-09-16 19:07:10 PDT
Attachment 211854
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h', u'Source/JavaScriptCore/assembler/X86Assembler.h', u'Source/JavaScriptCore/bytecode/DataFormat.h', u'Source/JavaScriptCore/bytecode/OperandsInlines.h', u'Source/JavaScriptCore/bytecode/SpeculatedType.cpp', u'Source/JavaScriptCore/bytecode/SpeculatedType.h', u'Source/JavaScriptCore/bytecode/ValueRecovery.h', u'Source/JavaScriptCore/dfg/DFGAbstractInterpreter.h', u'Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h', u'Source/JavaScriptCore/dfg/DFGAbstractValue.cpp', u'Source/JavaScriptCore/dfg/DFGAbstractValue.h', u'Source/JavaScriptCore/dfg/DFGArrayMode.cpp', u'Source/JavaScriptCore/dfg/DFGAssemblyHelpers.h', u'Source/JavaScriptCore/dfg/DFGCSEPhase.cpp', u'Source/JavaScriptCore/dfg/DFGClobberize.h', u'Source/JavaScriptCore/dfg/DFGCommon.h', u'Source/JavaScriptCore/dfg/DFGFixupPhase.cpp', u'Source/JavaScriptCore/dfg/DFGFlushFormat.cpp', u'Source/JavaScriptCore/dfg/DFGFlushFormat.h', u'Source/JavaScriptCore/dfg/DFGGenerationInfo.h', u'Source/JavaScriptCore/dfg/DFGGraph.cpp', u'Source/JavaScriptCore/dfg/DFGGraph.h', u'Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.cpp', u'Source/JavaScriptCore/dfg/DFGJITCode.cpp', u'Source/JavaScriptCore/dfg/DFGMinifiedNode.h', u'Source/JavaScriptCore/dfg/DFGNode.h', u'Source/JavaScriptCore/dfg/DFGNodeFlags.h', u'Source/JavaScriptCore/dfg/DFGNodeType.h', u'Source/JavaScriptCore/dfg/DFGOSRExitCompiler.cpp', u'Source/JavaScriptCore/dfg/DFGOSRExitCompiler64.cpp', u'Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp', u'Source/JavaScriptCore/dfg/DFGSafeToExecute.h', u'Source/JavaScriptCore/dfg/DFGSilentRegisterSavePlan.h', u'Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp', u'Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h', u'Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp', u'Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp', u'Source/JavaScriptCore/dfg/DFGUseKind.cpp', u'Source/JavaScriptCore/dfg/DFGUseKind.h', u'Source/JavaScriptCore/dfg/DFGValueSource.cpp', u'Source/JavaScriptCore/dfg/DFGValueSource.h', u'Source/JavaScriptCore/dfg/DFGVariableAccessData.h', u'Source/JavaScriptCore/ftl/FTLCArgumentGetter.cpp', u'Source/JavaScriptCore/ftl/FTLCArgumentGetter.h', u'Source/JavaScriptCore/ftl/FTLCapabilities.cpp', u'Source/JavaScriptCore/ftl/FTLExitValue.cpp', u'Source/JavaScriptCore/ftl/FTLExitValue.h', u'Source/JavaScriptCore/ftl/FTLIntrinsicRepository.h', u'Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp', u'Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp', u'Source/JavaScriptCore/ftl/FTLOutput.h', u'Source/JavaScriptCore/ftl/FTLValueFormat.cpp', u'Source/JavaScriptCore/ftl/FTLValueFormat.h', u'Source/JavaScriptCore/ftl/FTLValueSource.cpp', u'Source/JavaScriptCore/ftl/FTLValueSource.h', u'Source/JavaScriptCore/interpreter/Register.h', u'Source/JavaScriptCore/runtime/Arguments.cpp', u'Source/JavaScriptCore/runtime/IndexingType.cpp', u'Source/JavaScriptCore/runtime/JSCJSValue.h', u'Source/JavaScriptCore/runtime/JSCJSValueInlines.h', u'Source/WTF/ChangeLog', u'Source/WTF/wtf/PrintStream.h']" exit_code: 1 Source/JavaScriptCore/ftl/FTLCArgumentGetter.h:118: The parameter name "format" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:340: One line control clauses should not use braces. [whitespace/braces] [4] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:3373: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4009: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h:466: The parameter name "format" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:858: One line control clauses should not use braces. [whitespace/braces] [4] Total errors found: 6 in 63 files If any of these errors are false positives, please file a bug against check-webkit-style.
Early Warning System Bot
Comment 25
2013-09-16 19:16:08 PDT
Comment on
attachment 211854
[details]
the patch
Attachment 211854
[details]
did not pass qt-ews (qt): Output:
http://webkit-queues.appspot.com/results/1855221
EFL EWS Bot
Comment 26
2013-09-16 20:45:54 PDT
Comment on
attachment 211854
[details]
the patch
Attachment 211854
[details]
did not pass efl-ews (efl): Output:
http://webkit-queues.appspot.com/results/1893150
EFL EWS Bot
Comment 27
2013-09-16 22:45:40 PDT
Comment on
attachment 211854
[details]
the patch
Attachment 211854
[details]
did not pass efl-wk2-ews (efl-wk2): Output:
http://webkit-queues.appspot.com/results/1836342
Filip Pizlo
Comment 28
2013-09-16 22:53:43 PDT
Created
attachment 211866
[details]
the patch Bunch of fixes, to placate GCC and the style bot.
Oliver Hunt
Comment 29
2013-09-17 09:46:40 PDT
Comment on
attachment 211866
[details]
the patch Looks good -- the only thing i'm really uncomfortable with is the Int48s type - why do we need this? I can't see anywhere that you're not explicitly using the values 1 or 2 and then directly loading indices 0 or 1 - I'm assuming that there's a templatised function somewhere that you pass these to where it's nice to just iterate the list but i can't find it :( Also Int48s seems like an odd name, i'd prefer Int48Tuple or something. Then if you wanted you could typedef Int48Pair, etc into existence.
Filip Pizlo
Comment 30
2013-09-17 12:03:58 PDT
(In reply to
comment #29
)
> (From update of
attachment 211866
[details]
) > Looks good -- the only thing i'm really uncomfortable with is the Int48s type - why do we need this? I can't see anywhere that you're not explicitly using the values 1 or 2 and then directly loading indices 0 or 1 - I'm assuming that there's a templatised function somewhere that you pass these to where it's nice to just iterate the list but i can't find it :( > > Also Int48s seems like an odd name, i'd prefer Int48Tuple or something. Then if you wanted you could typedef Int48Pair, etc into existence.
Agreed, I've fixed this.
Filip Pizlo
Comment 31
2013-09-17 12:04:26 PDT
Why Int48 when you can do Int52? No reason. So I changed it.
Filip Pizlo
Comment 32
2013-09-17 12:05:00 PDT
Created
attachment 211929
[details]
the patch Already reviewed by Oliver.
Filip Pizlo
Comment 33
2013-09-17 18:30:41 PDT
Landed in
http://trac.webkit.org/changeset/156019
WebKit Commit Bot
Comment 34
2013-09-17 18:46:11 PDT
Re-opened since this is blocked by
bug 121530
Alexey Proskuryakov
Comment 35
2013-09-18 00:26:32 PDT
Rolled out in <
https://trac.webkit.org/r156029
>: ** The following js test failures have been introduced: js/dfg-int-overflow-large-constants-in-a-line ** The following JSC stress test failures have been introduced: regress/script-tests/large-int-neg.js.always-trigger-copy-phase The first failure occurs every time, the second one seems flaky.
Filip Pizlo
Comment 36
2013-09-18 08:24:17 PDT
(In reply to
comment #35
)
> Rolled out in <
https://trac.webkit.org/r156029
>: > > ** The following js test failures have been introduced: > js/dfg-int-overflow-large-constants-in-a-line
Yeah, that's a bug. I've got a fix for it.
> > ** The following JSC stress test failures have been introduced: > regress/script-tests/large-int-neg.js.always-trigger-copy-phase
Not seeing this one.
> > The first failure occurs every time, the second one seems flaky.
I think that the second one is caused by the same thing as the first. I'll test on a couple of machines. If I don't see the failure in the second one, I'll reland.
Filip Pizlo
Comment 37
2013-09-18 08:58:19 PDT
(In reply to
comment #36
)
> (In reply to
comment #35
) > > Rolled out in <
https://trac.webkit.org/r156029
>: > > > > ** The following js test failures have been introduced: > > js/dfg-int-overflow-large-constants-in-a-line > > Yeah, that's a bug. I've got a fix for it. > > > > > ** The following JSC stress test failures have been introduced: > > regress/script-tests/large-int-neg.js.always-trigger-copy-phase > > Not seeing this one. > > > > > The first failure occurs every time, the second one seems flaky. > > I think that the second one is caused by the same thing as the first. I'll test on a couple of machines. If I don't see the failure in the second one, I'll reland.
I got a different, but related, test to fail. It was also flaky. Fix that also.
Filip Pizlo
Comment 38
2013-09-18 10:15:31 PDT
Relanded in
http://trac.webkit.org/changeset/156047
with some test fixes: - OSR entry wasn't handling Int52. I had assumed that OSR entry seeing Int52 would be impossible; that's sort of true - the baseline JIT won't produce an Int52 and if it produces a double then OSR entry will be disallowed even if that double is representable in Int52. But we could have the DFG deducing that a local should be an Int52 while the baseline JIT has an Int32 in that variable. Then we still need the logic to up-convert the Int32 to an Int52. - Int52 Add and Sub were reusing registers on overflow checks in a way that led to incorrect values. Disable register reuse for those operations if they do overflow checks.
Ryosuke Niwa
Comment 39
2013-09-19 17:16:39 PDT
This patch caused the
bug 121648
.
Filip Pizlo
Comment 40
2013-12-08 19:54:12 PST
This was relanded.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug