Bug 121064 – DFG should support Int52 for local variables

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Filip Pizlo]

...

[Filip Pizlo]

Created attachment 211385 [details]
it begins

[Filip Pizlo]

Created attachment 211463 [details]
making some small progress

[Filip Pizlo]

Created attachment 211477 [details]
more

[Filip Pizlo]

Created attachment 211507 [details]
it's getting real

[Filip Pizlo]

Created attachment 211614 [details]
more

[Filip Pizlo]

Created attachment 211619 [details]
starting to get there

[Filip Pizlo]

Created attachment 211655 [details]
filling in the backend

[Filip Pizlo]

Created attachment 211666 [details]
backend almost done

[Filip Pizlo]

Created attachment 211679 [details]
more

[Filip Pizlo]

Created attachment 211723 [details]
almost done!

[Filip Pizlo]

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]

Created attachment 211732 [details]
it kind of passes some tests

[Filip Pizlo]

Created attachment 211735 [details]
passes all DFG 64-bit tests

[Filip Pizlo]

Created attachment 211736 [details]
starting on FTL

[Filip Pizlo]

Created attachment 211737 [details]
more FTL

[Filip Pizlo]

Created attachment 211842 [details]
the patch

[WebKit Commit Bot]

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 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 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]

Created attachment 211853 [details]
the patch

Fix build and actually include WTF changes this time.

[WebKit Commit Bot]

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 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]

Created attachment 211854 [details]
the patch

Found and fixed some bugs.

[WebKit Commit Bot]

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 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 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 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]

Created attachment 211866 [details]
the patch

Bunch of fixes, to placate GCC and the style bot.

[Oliver Hunt]

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]

(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]

Why Int48 when you can do Int52?  No reason.  So I changed it.

[Filip Pizlo]

Created attachment 211929 [details]
the patch

Already reviewed by Oliver.

[Filip Pizlo]

Landed in http://trac.webkit.org/changeset/156019

[WebKit Commit Bot]

Re-opened since this is blocked by bug 121530

[Alexey Proskuryakov]

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]

(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]

(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]

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]

This patch caused the bug 121648.

[Filip Pizlo]

This was relanded.