Bug 121064

Summary: DFG should support Int52 for local variables
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, benjamin, cmarcelo, commit-queue, eflews.bot, ggaren, gyuyoung.kim, mark.lam, mhahnenberg, msaboff, oliver, rniwa, sam
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 121065, 121116, 121132, 121141, 121142, 121202, 121211, 121250, 121253, 121268, 121371, 121374, 121530, 121540    
Bug Blocks: 121063, 121648    
Attachments:
Description Flags
it begins
none
making some small progress
none
more
none
it's getting real
none
more
none
starting to get there
none
filling in the backend
none
backend almost done
none
more
none
almost done!
none
it ran some easy program
none
it kind of passes some tests
none
passes all DFG 64-bit tests
none
starting on FTL
none
more FTL
none
the patch
webkit-ews: commit-queue-
the patch
webkit-ews: commit-queue-
the patch
webkit-ews: commit-queue-
the patch
oliver: review+
the patch none

Description Filip Pizlo 2013-09-09 18:54:23 PDT 
...
Comment 1 Filip Pizlo 2013-09-11 19:24:42 PDT 
Created attachment 211385 [details]
it begins
Comment 2 Filip Pizlo 2013-09-12 13:17:24 PDT 
Created attachment 211463 [details]
making some small progress
Comment 3 Filip Pizlo 2013-09-12 14:44:01 PDT 
Created attachment 211477 [details]
more
Comment 4 Filip Pizlo 2013-09-12 22:16:17 PDT 
Created attachment 211507 [details]
it's getting real
Comment 5 Filip Pizlo 2013-09-13 18:54:52 PDT 
Created attachment 211614 [details]
more
Comment 6 Filip Pizlo 2013-09-13 21:58:12 PDT 
Created attachment 211619 [details]
starting to get there
Comment 7 Filip Pizlo 2013-09-14 10:33:28 PDT 
Created attachment 211655 [details]
filling in the backend
Comment 8 Filip Pizlo 2013-09-14 13:13:05 PDT 
Created attachment 211666 [details]
backend almost done
Comment 9 Filip Pizlo 2013-09-14 18:24:05 PDT 
Created attachment 211679 [details]
more
Comment 10 Filip Pizlo 2013-09-15 12:12:37 PDT 
Created attachment 211723 [details]
almost done!
Comment 11 Filip Pizlo 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:
Comment 12 Filip Pizlo 2013-09-15 15:52:22 PDT 
Created attachment 211732 [details]
it kind of passes some tests
Comment 13 Filip Pizlo 2013-09-15 20:24:30 PDT 
Created attachment 211735 [details]
passes all DFG 64-bit tests
Comment 14 Filip Pizlo 2013-09-15 21:49:32 PDT 
Created attachment 211736 [details]
starting on FTL
Comment 15 Filip Pizlo 2013-09-15 22:13:22 PDT 
Created attachment 211737 [details]
more FTL
Comment 16 Filip Pizlo 2013-09-16 16:32:01 PDT 
Created attachment 211842 [details]
the patch
Comment 17 WebKit Commit Bot 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.
Comment 18 Early Warning System Bot 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
Comment 19 EFL EWS Bot 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
Comment 20 Filip Pizlo 2013-09-16 18:45:43 PDT 
Created attachment 211853 [details]
the patch

Fix build and actually include WTF changes this time.
Comment 21 WebKit Commit Bot 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.
Comment 22 Early Warning System Bot 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
Comment 23 Filip Pizlo 2013-09-16 19:05:49 PDT 
Created attachment 211854 [details]
the patch

Found and fixed some bugs.
Comment 24 WebKit Commit Bot 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.
Comment 25 Early Warning System Bot 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
Comment 26 EFL EWS Bot 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
Comment 27 EFL EWS Bot 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
Comment 28 Filip Pizlo 2013-09-16 22:53:43 PDT 
Created attachment 211866 [details]
the patch

Bunch of fixes, to placate GCC and the style bot.
Comment 29 Oliver Hunt 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.
Comment 30 Filip Pizlo 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.
Comment 31 Filip Pizlo 2013-09-17 12:04:26 PDT 
Why Int48 when you can do Int52?  No reason.  So I changed it.
Comment 32 Filip Pizlo 2013-09-17 12:05:00 PDT 
Created attachment 211929 [details]
the patch

Already reviewed by Oliver.
Comment 33 Filip Pizlo 2013-09-17 18:30:41 PDT 
Landed in http://trac.webkit.org/changeset/156019
Comment 34 WebKit Commit Bot 2013-09-17 18:46:11 PDT 
Re-opened since this is blocked by bug 121530
Comment 35 Alexey Proskuryakov 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.
Comment 36 Filip Pizlo 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.
Comment 37 Filip Pizlo 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.
Comment 38 Filip Pizlo 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.
Comment 39 Ryosuke Niwa 2013-09-19 17:16:39 PDT 
This patch caused the bug 121648.
Comment 40 Filip Pizlo 2013-12-08 19:54:12 PST 
This was relanded.