Attachment #226021 for bug #129810

View Details Raw Unified Return to Bug 129810



2014-03-06  Filip Pizlo  <fpizlo@apple.com>

        FTL arity fixup should work on ARM64
        https://bugs.webkit.org/show_bug.cgi?id=129810

        Reviewed by NOBODY (OOPS!).
        
        - Using regT5 to pass the thunk return address to arityFixup is shady since that's a
          callee-save.
        
        - The FTL path was assuming X86 conventions for where SP points at the top of the prologue.
        
        This makes some more tests pass.

        * dfg/DFGJITCompiler.cpp:
        (JSC::DFG::JITCompiler::compileFunction):
        * ftl/FTLLink.cpp:
        (JSC::FTL::link):
        * jit/AssemblyHelpers.h:
        (JSC::AssemblyHelpers::prologueStackPointerDelta):
        * jit/JIT.cpp:
        (JSC::JIT::privateCompile):
        * jit/ThunkGenerators.cpp:
        (JSC::arityFixup):
        * llint/LowLevelInterpreter64.asm:
        * offlineasm/arm64.rb:
        * offlineasm/x86.rb: In addition to the t7 change, make t6 agree with GPRInfo.h.

2014-03-06  Joseph Pecoraro  <pecoraro@apple.com>

        Web Inspector: Expose the console object in JSContexts to interact with Web Inspector

Lines 381-388 void JITCompiler::compileFunction() Source/JavaScriptCore/dfg/DFGJITCompiler.cpp_sec1
- Source/JavaScriptCore/dfg/DFGJITCompiler.cpp -2 / +8 lines
381	addPtr(TrustedImm32(maxFrameExtentForSlowPathCall), stackPointerRegister);	381	addPtr(TrustedImm32(maxFrameExtentForSlowPathCall), stackPointerRegister);
382	branchTest32(Zero, GPRInfo::regT0).linkTo(fromArityCheck, this);	382	branchTest32(Zero, GPRInfo::regT0).linkTo(fromArityCheck, this);
383	emitStoreCodeOrigin(CodeOrigin(0));	383	emitStoreCodeOrigin(CodeOrigin(0));
384	move(TrustedImmPtr(m_vm->arityCheckFailReturnThunks->returnPCsFor(*m_vm, m_codeBlock->numParameters())), GPRInfo::regT5);	384	GPRReg thunkReg;
385	loadPtr(BaseIndex(GPRInfo::regT5, GPRInfo::regT0, timesPtr()), GPRInfo::regT5);	385	#if USE(JSVALUE64)
		386	thunkReg = GPRInfo::regT7;
		387	#else
		388	thunkReg = GPRInfo::regT5;
		389	#endif
		390	move(TrustedImmPtr(m_vm->arityCheckFailReturnThunks->returnPCsFor(*m_vm, m_codeBlock->numParameters())), thunkReg);
		391	loadPtr(BaseIndex(thunkReg, GPRInfo::regT0, timesPtr()), thunkReg);
386	m_callArityFixup = call();	392	m_callArityFixup = call();
387	jump(fromArityCheck);	393	jump(fromArityCheck);
388		394



    CCallHelpers::Label arityCheck;

    CCallHelpers::Address frame = CCallHelpers::Address(
        CCallHelpers::stackPointerRegister, -static_cast<int32_t>(AssemblyHelpers::prologueStackPointerDelta()));
    
    if (Profiler::Compilation* compilation = graph.compilation()) {
        compilation->addDescription(

        jit.emitFunctionEpilogue();
        mainPathJumps.append(jit.branchTest32(CCallHelpers::Zero, GPRInfo::regT0));
        jit.emitFunctionPrologue();
        jit.move(CCallHelpers::TrustedImmPtr(vm.arityCheckFailReturnThunks->returnPCsFor(vm, codeBlock->numParameters())), GPRInfo::regT7);
        jit.loadPtr(CCallHelpers::BaseIndex(GPRInfo::regT7, GPRInfo::regT0, CCallHelpers::timesPtr()), GPRInfo::regT7);
        CCallHelpers::Call callArityFixup = jit.call();
        jit.emitFunctionEpilogue();
        mainPathJumps.append(jit.jump());



    }

#if CPU(X86_64) || CPU(X86)
    static size_t prologueStackPointerDelta()
    {
        // Prologue only saves the framePointerRegister
        return sizeof(void*);

#endif // CPU(X86_64) || CPU(X86)

#if CPU(ARM) || CPU(ARM64)
    static size_t prologueStackPointerDelta()
    {
        // Prologue saves the framePointerRegister and linkRegister
        return 2 * sizeof(void*);

#endif

#if CPU(MIPS)
    static size_t prologueStackPointerDelta()
    {
        // Prologue saves the framePointerRegister and returnAddressRegister
        return 2 * sizeof(void*);

#endif

#if CPU(SH4)
    static size_t prologueStackPointerDelta()
    {
        // Prologue saves the framePointerRegister and link register
        return 2 * sizeof(void*);

Lines 558-565 CompilationResult JIT::privateCompile(JI Source/JavaScriptCore/jit/JIT.cpp_sec1
- Source/JavaScriptCore/jit/JIT.cpp -2 / +8 lines
558	if (returnValueGPR != regT0)	558	if (returnValueGPR != regT0)
559	move(returnValueGPR, regT0);	559	move(returnValueGPR, regT0);
560	branchTest32(Zero, regT0).linkTo(beginLabel, this);	560	branchTest32(Zero, regT0).linkTo(beginLabel, this);
561	move(TrustedImmPtr(m_vm->arityCheckFailReturnThunks->returnPCsFor(*m_vm, m_codeBlock->numParameters())), regT5);	561	GPRReg thunkReg;
562	loadPtr(BaseIndex(regT5, regT0, timesPtr()), regT5);	562	#if USE(JSVALUE64)
		563	thunkReg = GPRInfo::regT7;
		564	#else
		565	thunkReg = GPRInfo::regT5;
		566	#endif
		567	move(TrustedImmPtr(m_vm->arityCheckFailReturnThunks->returnPCsFor(*m_vm, m_codeBlock->numParameters())), thunkReg);
		568	loadPtr(BaseIndex(thunkReg, regT0, timesPtr()), thunkReg);
563	emitNakedCall(m_vm->getCTIStub(arityFixup).code());	569	emitNakedCall(m_vm->getCTIStub(arityFixup).code());
564		570
565	#if !ASSERT_DISABLED	571	#if !ASSERT_DISABLED



    JSInterfaceJIT jit(vm);

    // We enter with fixup count, in aligned stack units, in regT0 and the return thunk in
    // regT5 on 32-bit and regT7 on 64-bit.
#if USE(JSVALUE64)
#  if CPU(X86_64)
    jit.pop(JSInterfaceJIT::regT4);

    jit.storePtr(GPRInfo::regT1, MacroAssembler::BaseIndex(JSInterfaceJIT::regT6, JSInterfaceJIT::regT0, JSInterfaceJIT::TimesEight));
    
    // Install the new return PC.
    jit.storePtr(GPRInfo::regT7, JSInterfaceJIT::Address(JSInterfaceJIT::callFrameRegister, CallFrame::returnPCOffset()));

#  if CPU(X86_64)
    jit.push(JSInterfaceJIT::regT4);

Lines 483-489 macro functionArityCheck(doneLabel, slow Source/JavaScriptCore/llint/LowLevelInterpreter64.asm_sec1
- Source/JavaScriptCore/llint/LowLevelInterpreter64.asm -1 / +1 lines
483	loadp CommonSlowPaths::ArityCheckData::thunkToCall[t1], t2	483	loadp CommonSlowPaths::ArityCheckData::thunkToCall[t1], t2
484	btpz t2, .proceedInline	484	btpz t2, .proceedInline
485		485
486	loadp CommonSlowPaths::ArityCheckData::returnPC[t1], t5	486	loadp CommonSlowPaths::ArityCheckData::returnPC[t1], t7
487	loadp CommonSlowPaths::ArityCheckData::paddedStackSpace[t1], t0	487	loadp CommonSlowPaths::ArityCheckData::paddedStackSpace[t1], t0
488	call t2	488	call t2
489	if ASSERT_ENABLED	489	if ASSERT_ENABLED

Lines 1-4 Source/JavaScriptCore/offlineasm/arm64.rb_sec1
- Source/JavaScriptCore/offlineasm/arm64.rb -1 / +3 lines
1	# Copyright (C) 2011, 2012 Apple Inc. All rights reserved.	1	# Copyright (C) 2011, 2012, 2014 Apple Inc. All rights reserved.
2	#	2	#
3	# Redistribution and use in source and binary forms, with or without	3	# Redistribution and use in source and binary forms, with or without
4	# modification, are permitted provided that the following conditions	4	# modification, are permitted provided that the following conditions
Lines 118-123 class RegisterID Source/JavaScriptCore/offlineasm/arm64.rb_sec2
118	arm64GPRName('x24', kind)	118	arm64GPRName('x24', kind)
119	when 't6'	119	when 't6'
120	arm64GPRName('x6', kind)	120	arm64GPRName('x6', kind)
		121	when 't7'
		122	arm64GPRName('x7', kind)
121	when 'cfr'	123	when 'cfr'
122	arm64GPRName('x29', kind)	124	arm64GPRName('x29', kind)
123	when 'csr1'	125	when 'csr1'



# Copyright (C) 2012, 2014 Apple Inc. All rights reserved.
# Copyright (C) 2013 Digia Plc. and/or its subsidiary(-ies)
#
# Redistribution and use in source and binary forms, with or without

            raise "Cannot use #{name} in 32-bit X86 at #{codeOriginString}" unless isX64
            case kind
            when :half
                "%r8w"
            when :int
                "%r8d"
            when :ptr
                "%r8"
            when :quad
                "%r8"
            end
        when "t7"
            raise "Cannot use #{name} in 32-bit X86 at #{codeOriginString}" unless isX64
            case kind
            when :half
                "%r9w"
            when :int
                "%r9d"
            when :ptr
                "%r9"
            when :quad
                "%r9"
            end
        when "csr1"
            raise "Cannot use #{name} in 32-bit X86 at #{codeOriginString}" unless isX64

Return to Bug 129810

Lines 68-74 public: Source/JavaScriptCore/jit/AssemblyHelpers.h_sec1
- Source/JavaScriptCore/jit/AssemblyHelpers.h -4 / +4 lines
68	}	68	}
69		69
70	#if CPU(X86_64) \|\| CPU(X86)	70	#if CPU(X86_64) \|\| CPU(X86)
71	size_t prologueStackPointerDelta()	71	static size_t prologueStackPointerDelta()
72	{	72	{
73	// Prologue only saves the framePointerRegister	73	// Prologue only saves the framePointerRegister
74	return sizeof(void*);	74	return sizeof(void*);
Lines 103-109 public: Source/JavaScriptCore/jit/AssemblyHelpers.h_sec2
103	#endif // CPU(X86_64) \|\| CPU(X86)	103	#endif // CPU(X86_64) \|\| CPU(X86)
104		104
105	#if CPU(ARM) \|\| CPU(ARM64)	105	#if CPU(ARM) \|\| CPU(ARM64)
106	size_t prologueStackPointerDelta()	106	static size_t prologueStackPointerDelta()
107	{	107	{
108	// Prologue saves the framePointerRegister and linkRegister	108	// Prologue saves the framePointerRegister and linkRegister
109	return 2 * sizeof(void*);	109	return 2 * sizeof(void*);
Lines 138-144 public: Source/JavaScriptCore/jit/AssemblyHelpers.h_sec3
138	#endif	138	#endif
139		139
140	#if CPU(MIPS)	140	#if CPU(MIPS)
141	size_t prologueStackPointerDelta()	141	static size_t prologueStackPointerDelta()
142	{	142	{
143	// Prologue saves the framePointerRegister and returnAddressRegister	143	// Prologue saves the framePointerRegister and returnAddressRegister
144	return 2 * sizeof(void*);	144	return 2 * sizeof(void*);
Lines 161-167 public: Source/JavaScriptCore/jit/AssemblyHelpers.h_sec4
161	#endif	161	#endif
162		162
163	#if CPU(SH4)	163	#if CPU(SH4)
164	size_t prologueStackPointerDelta()	164	static size_t prologueStackPointerDelta()
165	{	165	{
166	// Prologue saves the framePointerRegister and link register	166	// Prologue saves the framePointerRegister and link register
167	return 2 * sizeof(void*);	167	return 2 * sizeof(void*);

Lines 429-435 MacroAssemblerCodeRef arityFixup(VM* vm) Source/JavaScriptCore/jit/ThunkGenerators.cpp_sec1
- Source/JavaScriptCore/jit/ThunkGenerators.cpp -2 / +2 lines
429	JSInterfaceJIT jit(vm);	429	JSInterfaceJIT jit(vm);
430		430
431	// We enter with fixup count, in aligned stack units, in regT0 and the return thunk in	431	// We enter with fixup count, in aligned stack units, in regT0 and the return thunk in
432	// regT5.	432	// regT5 on 32-bit and regT7 on 64-bit.
433	#if USE(JSVALUE64)	433	#if USE(JSVALUE64)
434	# if CPU(X86_64)	434	# if CPU(X86_64)
435	jit.pop(JSInterfaceJIT::regT4);	435	jit.pop(JSInterfaceJIT::regT4);
Lines 467-473 MacroAssemblerCodeRef arityFixup(VM* vm) Source/JavaScriptCore/jit/ThunkGenerators.cpp_sec2
467	jit.storePtr(GPRInfo::regT1, MacroAssembler::BaseIndex(JSInterfaceJIT::regT6, JSInterfaceJIT::regT0, JSInterfaceJIT::TimesEight));	467	jit.storePtr(GPRInfo::regT1, MacroAssembler::BaseIndex(JSInterfaceJIT::regT6, JSInterfaceJIT::regT0, JSInterfaceJIT::TimesEight));
468		468
469	// Install the new return PC.	469	// Install the new return PC.
470	jit.storePtr(GPRInfo::regT5, JSInterfaceJIT::Address(JSInterfaceJIT::callFrameRegister, CallFrame::returnPCOffset()));	470	jit.storePtr(GPRInfo::regT7, JSInterfaceJIT::Address(JSInterfaceJIT::callFrameRegister, CallFrame::returnPCOffset()));
471		471
472	# if CPU(X86_64)	472	# if CPU(X86_64)
473	jit.push(JSInterfaceJIT::regT4);	473	jit.push(JSInterfaceJIT::regT4);

Lines 1-4 Source/JavaScriptCore/offlineasm/x86.rb_sec1
- Source/JavaScriptCore/offlineasm/x86.rb -5 / +17 lines
1	# Copyright (C) 2012 Apple Inc. All rights reserved.	1	# Copyright (C) 2012, 2014 Apple Inc. All rights reserved.
2	# Copyright (C) 2013 Digia Plc. and/or its subsidiary(-ies)	2	# Copyright (C) 2013 Digia Plc. and/or its subsidiary(-ies)
3	#	3	#
4	# Redistribution and use in source and binary forms, with or without	4	# Redistribution and use in source and binary forms, with or without
Lines 284-296 class RegisterID Source/JavaScriptCore/offlineasm/x86.rb_sec2
284	raise "Cannot use #{name} in 32-bit X86 at #{codeOriginString}" unless isX64	284	raise "Cannot use #{name} in 32-bit X86 at #{codeOriginString}" unless isX64
285	case kind	285	case kind
286	when :half	286	when :half
287	"%r10w"	287	"%r8w"
288	when :int	288	when :int
289	"%r10d"	289	"%r8d"
290	when :ptr	290	when :ptr
291	"%r10"	291	"%r8"
292	when :quad	292	when :quad
293	"%r10"	293	"%r8"
		294	end
		295	when "t7"
		296	raise "Cannot use #{name} in 32-bit X86 at #{codeOriginString}" unless isX64
		297	case kind
		298	when :half
		299	"%r9w"
		300	when :int
		301	"%r9d"
		302	when :ptr
		303	"%r9"
		304	when :quad
		305	"%r9"
294	end	306	end
295	when "csr1"	307	when "csr1"
296	raise "Cannot use #{name} in 32-bit X86 at #{codeOriginString}" unless isX64	308	raise "Cannot use #{name} in 32-bit X86 at #{codeOriginString}" unless isX64

Lines 1-3 Source/JavaScriptCore/ChangeLog_sec1
- Source/JavaScriptCore/ChangeLog +28 lines
		1	2014-03-06 Filip Pizlo <fpizlo@apple.com>
		2
		3	FTL arity fixup should work on ARM64
		4	https://bugs.webkit.org/show_bug.cgi?id=129810
		5
		6	Reviewed by NOBODY (OOPS!).
		7
		8	- Using regT5 to pass the thunk return address to arityFixup is shady since that's a
		9	callee-save.
		10
		11	- The FTL path was assuming X86 conventions for where SP points at the top of the prologue.
		12
		13	This makes some more tests pass.
		14
		15	* dfg/DFGJITCompiler.cpp:
		16	(JSC::DFG::JITCompiler::compileFunction):
		17	* ftl/FTLLink.cpp:
		18	(JSC::FTL::link):
		19	* jit/AssemblyHelpers.h:
		20	(JSC::AssemblyHelpers::prologueStackPointerDelta):
		21	* jit/JIT.cpp:
		22	(JSC::JIT::privateCompile):
		23	* jit/ThunkGenerators.cpp:
		24	(JSC::arityFixup):
		25	* llint/LowLevelInterpreter64.asm:
		26	* offlineasm/arm64.rb:
		27	* offlineasm/x86.rb: In addition to the t7 change, make t6 agree with GPRInfo.h.
		28
1	2014-03-06 Joseph Pecoraro <pecoraro@apple.com>	29	2014-03-06 Joseph Pecoraro <pecoraro@apple.com>
2		30
3	Web Inspector: Expose the console object in JSContexts to interact with Web Inspector	31	Web Inspector: Expose the console object in JSContexts to interact with Web Inspector

Lines 71-77 void link(State& state) Source/JavaScriptCore/ftl/FTLLink.cpp_sec1
- Source/JavaScriptCore/ftl/FTLLink.cpp -3 / +3 lines
71	CCallHelpers::Label arityCheck;	71	CCallHelpers::Label arityCheck;
72		72
73	CCallHelpers::Address frame = CCallHelpers::Address(	73	CCallHelpers::Address frame = CCallHelpers::Address(
74	CCallHelpers::stackPointerRegister, -static_cast<int32_t>(sizeof(void*)));	74	CCallHelpers::stackPointerRegister, -static_cast<int32_t>(AssemblyHelpers::prologueStackPointerDelta()));
75		75
76	if (Profiler::Compilation* compilation = graph.compilation()) {	76	if (Profiler::Compilation* compilation = graph.compilation()) {
77	compilation->addDescription(	77	compilation->addDescription(
Lines 170-177 void link(State& state) Source/JavaScriptCore/ftl/FTLLink.cpp_sec2
170	jit.emitFunctionEpilogue();	170	jit.emitFunctionEpilogue();
171	mainPathJumps.append(jit.branchTest32(CCallHelpers::Zero, GPRInfo::regT0));	171	mainPathJumps.append(jit.branchTest32(CCallHelpers::Zero, GPRInfo::regT0));
172	jit.emitFunctionPrologue();	172	jit.emitFunctionPrologue();
173	jit.move(CCallHelpers::TrustedImmPtr(vm.arityCheckFailReturnThunks->returnPCsFor(vm, codeBlock->numParameters())), GPRInfo::regT5);	173	jit.move(CCallHelpers::TrustedImmPtr(vm.arityCheckFailReturnThunks->returnPCsFor(vm, codeBlock->numParameters())), GPRInfo::regT7);
174	jit.loadPtr(CCallHelpers::BaseIndex(GPRInfo::regT5, GPRInfo::regT0, CCallHelpers::timesPtr()), GPRInfo::regT5);	174	jit.loadPtr(CCallHelpers::BaseIndex(GPRInfo::regT7, GPRInfo::regT0, CCallHelpers::timesPtr()), GPRInfo::regT7);
175	CCallHelpers::Call callArityFixup = jit.call();	175	CCallHelpers::Call callArityFixup = jit.call();
176	jit.emitFunctionEpilogue();	176	jit.emitFunctionEpilogue();
177	mainPathJumps.append(jit.jump());	177	mainPathJumps.append(jit.jump());