From 3c0f21390e5e7c5ce554b69daa56fedcb09f4336 Mon Sep 17 00:00:00 2001 From: "Nicolas B. Pierron" Date: Wed, 25 Mar 2015 15:59:02 +0100 Subject: [PATCH] Bug 1130089 - Use constexpr for JitStackValueAlignment. r=bbouvier --- js/src/jit/CodeGenerator.cpp | 9 ++++----- js/src/jit/LIR-Common.h | 5 ++--- js/src/jit/LIR.h | 11 ++++++----- js/src/jit/Lowering.cpp | 5 ++--- js/src/jit/MacroAssembler.cpp | 10 ++++------ js/src/jit/arm/Assembler-arm.h | 14 +++++++++----- js/src/jit/mips/Assembler-mips.h | 18 +++++++++++------- js/src/jit/none/MacroAssembler-none.h | 7 ++++--- js/src/jit/x64/Assembler-x64.h | 14 +++++++++----- js/src/jit/x64/Trampoline-x64.cpp | 7 +++---- js/src/jit/x86/Assembler-x86.h | 16 ++++++++++------ js/src/jit/x86/Trampoline-x86.cpp | 7 +++---- 12 files changed, 67 insertions(+), 56 deletions(-) diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp index f0474d8c8cb6..10685a0d380b 100644 --- a/js/src/jit/CodeGenerator.cpp +++ b/js/src/jit/CodeGenerator.cpp @@ -3140,11 +3140,10 @@ CodeGenerator::emitPushArguments(LApplyArgsGeneric *apply, Register extraStackSp masm.movePtr(argcreg, extraStackSpace); // Align the JitFrameLayout on the JitStackAlignment. - const uint32_t alignment = JitStackAlignment / sizeof(Value); - if (alignment > 1) { + if (JitStackValueAlignment > 1) { MOZ_ASSERT(frameSize() % JitStackAlignment == 0, "Stack padding assumes that the frameSize is correct"); - MOZ_ASSERT(alignment == 2); + MOZ_ASSERT(JitStackValueAlignment == 2); Label noPaddingNeeded; // if the number of arguments is odd, then we do not need any padding. masm.branchTestPtr(Assembler::NonZero, argcreg, Imm32(1), &noPaddingNeeded); @@ -3161,8 +3160,8 @@ CodeGenerator::emitPushArguments(LApplyArgsGeneric *apply, Register extraStackSp // Put a magic value in the space reserved for padding. Note, this code // cannot be merged with the previous test, as not all architectures can // write below their stack pointers. - if (alignment > 1) { - MOZ_ASSERT(alignment == 2); + if (JitStackValueAlignment > 1) { + MOZ_ASSERT(JitStackValueAlignment == 2); Label noPaddingNeeded; // if the number of arguments is odd, then we do not need any padding. masm.branchTestPtr(Assembler::NonZero, argcreg, Imm32(1), &noPaddingNeeded); diff --git a/js/src/jit/LIR-Common.h b/js/src/jit/LIR-Common.h index 1585e20dc0a2..3594c77d4f79 100644 --- a/js/src/jit/LIR-Common.h +++ b/js/src/jit/LIR-Common.h @@ -1537,9 +1537,8 @@ class LJSCallInstructionHelper : public LCallInstructionHelper 1) - return AlignBytes(mir()->numStackArgs(), alignment); + if (JitStackValueAlignment > 1) + return AlignBytes(mir()->numStackArgs(), JitStackValueAlignment); return mir()->numStackArgs(); } MCall *mir() const { diff --git a/js/src/jit/LIR.h b/js/src/jit/LIR.h index 61304ef5dc83..0d25e88d21fa 100644 --- a/js/src/jit/LIR.h +++ b/js/src/jit/LIR.h @@ -1760,11 +1760,12 @@ class LIRGraph // platform stack alignment requirement, and so that it's a multiple of // the number of slots per Value. uint32_t paddedLocalSlotCount() const { - // Round to ABIStackAlignment, but also round to at least sizeof(Value) - // in case that's greater, because StackOffsetOfPassedArg rounds - // argument slots to 8-byte boundaries. - size_t Alignment = Max(size_t(JitStackAlignment), sizeof(Value)); - return AlignBytes(localSlotCount(), Alignment); + // Round to JitStackAlignment, and implicitly to sizeof(Value) as + // JitStackAlignment is a multiple of sizeof(Value). These alignments + // are needed for spilling SIMD registers properly, and for + // StackOffsetOfPassedArg which rounds argument slots to 8-byte + // boundaries. + return AlignBytes(localSlotCount(), JitStackAlignment); } size_t paddedLocalSlotsSize() const { return paddedLocalSlotCount(); diff --git a/js/src/jit/Lowering.cpp b/js/src/jit/Lowering.cpp index a7c9bee9a38c..9e5dee03f784 100644 --- a/js/src/jit/Lowering.cpp +++ b/js/src/jit/Lowering.cpp @@ -393,9 +393,8 @@ LIRGenerator::lowerCallArguments(MCall *call) // Align the arguments of a call such that the callee would keep the same // alignment as the caller. uint32_t baseSlot = 0; - static const uint32_t alignment = JitStackAlignment / sizeof(Value); - if (alignment > 1) - baseSlot = AlignBytes(argc, alignment); + if (JitStackValueAlignment > 1) + baseSlot = AlignBytes(argc, JitStackValueAlignment); else baseSlot = argc; diff --git a/js/src/jit/MacroAssembler.cpp b/js/src/jit/MacroAssembler.cpp index 1a3428d23a92..0eecf25c61f2 100644 --- a/js/src/jit/MacroAssembler.cpp +++ b/js/src/jit/MacroAssembler.cpp @@ -2486,8 +2486,7 @@ MacroAssembler::profilerPreCallImpl(Register reg, Register reg2) void MacroAssembler::alignJitStackBasedOnNArgs(Register nargs) { - const uint32_t alignment = JitStackAlignment / sizeof(Value); - if (alignment == 1) + if (JitStackValueAlignment == 1) return; // A JitFrameLayout is composed of the following: @@ -2500,7 +2499,7 @@ MacroAssembler::alignJitStackBasedOnNArgs(Register nargs) // Which implies that |argN| is aligned if |nargs| is even, and offset by // |sizeof(Value)| if |nargs| is odd. - MOZ_ASSERT(alignment == 2); + MOZ_ASSERT(JitStackValueAlignment == 2); // Thus the |padding| is offset by |sizeof(Value)| if |nargs| is even, and // aligned if |nargs| is odd. @@ -2535,8 +2534,7 @@ MacroAssembler::alignJitStackBasedOnNArgs(Register nargs) void MacroAssembler::alignJitStackBasedOnNArgs(uint32_t nargs) { - const uint32_t alignment = JitStackAlignment / sizeof(Value); - if (alignment == 1) + if (JitStackValueAlignment == 1) return; // A JitFrameLayout is composed of the following: @@ -2549,7 +2547,7 @@ MacroAssembler::alignJitStackBasedOnNArgs(uint32_t nargs) // Which implies that |argN| is aligned if |nargs| is even, and offset by // |sizeof(Value)| if |nargs| is odd. - MOZ_ASSERT(alignment == 2); + MOZ_ASSERT(JitStackValueAlignment == 2); // Thus the |padding| is offset by |sizeof(Value)| if |nargs| is even, and // aligned if |nargs| is odd. diff --git a/js/src/jit/arm/Assembler-arm.h b/js/src/jit/arm/Assembler-arm.h index b9bc9e751562..f59e54d62677 100644 --- a/js/src/jit/arm/Assembler-arm.h +++ b/js/src/jit/arm/Assembler-arm.h @@ -146,16 +146,20 @@ static MOZ_CONSTEXPR_VAR FloatRegister d15 = {FloatRegisters::d15, VFPRegister:: // load/store) operate in a single cycle when the address they are dealing with // is 8 byte aligned. Also, the ARM abi wants the stack to be 8 byte aligned at // function boundaries. I'm trying to make sure this is always true. -static const uint32_t ABIStackAlignment = 8; -static const uint32_t CodeAlignment = 8; -static const uint32_t JitStackAlignment = 8; +static MOZ_CONSTEXPR_VAR uint32_t ABIStackAlignment = 8; +static MOZ_CONSTEXPR_VAR uint32_t CodeAlignment = 8; +static MOZ_CONSTEXPR_VAR uint32_t JitStackAlignment = 8; + +static MOZ_CONSTEXPR_VAR uint32_t JitStackValueAlignment = JitStackAlignment / sizeof(Value); +static_assert(JitStackAlignment % sizeof(Value) == 0 && JitStackValueAlignment >= 1, + "Stack alignment should be a non-zero multiple of sizeof(Value)"); // This boolean indicates whether we support SIMD instructions flavoured for // this architecture or not. Rather than a method in the LIRGenerator, it is // here such that it is accessible from the entire codebase. Once full support // for SIMD is reached on all tier-1 platforms, this constant can be deleted. -static const bool SupportsSimd = false; -static const uint32_t SimdMemoryAlignment = 8; +static MOZ_CONSTEXPR_VAR bool SupportsSimd = false; +static MOZ_CONSTEXPR_VAR uint32_t SimdMemoryAlignment = 8; static_assert(CodeAlignment % SimdMemoryAlignment == 0, "Code alignment should be larger than any of the alignments which are used for " diff --git a/js/src/jit/mips/Assembler-mips.h b/js/src/jit/mips/Assembler-mips.h index cc54d3d6ed7b..6e555ec4438a 100644 --- a/js/src/jit/mips/Assembler-mips.h +++ b/js/src/jit/mips/Assembler-mips.h @@ -161,23 +161,27 @@ static MOZ_CONSTEXPR_VAR FloatRegister f30 = { FloatRegisters::f30, FloatRegiste // MIPS CPUs can only load multibyte data that is "naturally" // four-byte-aligned, sp register should be eight-byte-aligned. -static const uint32_t ABIStackAlignment = 8; -static const uint32_t CodeAlignment = 4; -static const uint32_t JitStackAlignment = 8; +static MOZ_CONSTEXPR_VAR uint32_t ABIStackAlignment = 8; +static MOZ_CONSTEXPR_VAR uint32_t CodeAlignment = 4; +static MOZ_CONSTEXPR_VAR uint32_t JitStackAlignment = 8; + +static MOZ_CONSTEXPR_VAR uint32_t JitStackValueAlignment = JitStackAlignment / sizeof(Value); +static_assert(JitStackAlignment % sizeof(Value) == 0 && JitStackValueAlignment >= 1, + "Stack alignment should be a non-zero multiple of sizeof(Value)"); // This boolean indicates whether we support SIMD instructions flavoured for // this architecture or not. Rather than a method in the LIRGenerator, it is // here such that it is accessible from the entire codebase. Once full support // for SIMD is reached on all tier-1 platforms, this constant can be deleted. -static const bool SupportsSimd = false; +static MOZ_CONSTEXPR_VAR bool SupportsSimd = false; // TODO this is just a filler to prevent a build failure. The MIPS SIMD // alignment requirements still need to be explored. // TODO Copy the static_asserts from x64/x86 assembler files. -static const uint32_t SimdMemoryAlignment = 8; +static MOZ_CONSTEXPR_VAR uint32_t SimdMemoryAlignment = 8; -static const uint32_t AsmJSStackAlignment = SimdMemoryAlignment; +static MOZ_CONSTEXPR_VAR uint32_t AsmJSStackAlignment = SimdMemoryAlignment; -static const Scale ScalePointer = TimesFour; +static MOZ_CONSTEXPR_VAR Scale ScalePointer = TimesFour; // MIPS instruction types // +---------------------------------------------------------------+ diff --git a/js/src/jit/none/MacroAssembler-none.h b/js/src/jit/none/MacroAssembler-none.h index 5acaf82ee8c1..10128b44f7b5 100644 --- a/js/src/jit/none/MacroAssembler-none.h +++ b/js/src/jit/none/MacroAssembler-none.h @@ -71,9 +71,10 @@ static MOZ_CONSTEXPR_VAR ValueOperand JSReturnOperand(InvalidReg); #error "Bad architecture" #endif -static const uint32_t ABIStackAlignment = 4; -static const uint32_t CodeAlignment = 4; -static const uint32_t JitStackAlignment = 4; +static MOZ_CONSTEXPR_VAR uint32_t ABIStackAlignment = 4; +static MOZ_CONSTEXPR_VAR uint32_t CodeAlignment = 4; +static MOZ_CONSTEXPR_VAR uint32_t JitStackAlignment = 8; +static MOZ_CONSTEXPR_VAR uint32_t JitStackValueAlignment = JitStackAlignment / sizeof(Value); static const Scale ScalePointer = TimesOne; diff --git a/js/src/jit/x64/Assembler-x64.h b/js/src/jit/x64/Assembler-x64.h index 154d4a1adb01..5d9758a9518e 100644 --- a/js/src/jit/x64/Assembler-x64.h +++ b/js/src/jit/x64/Assembler-x64.h @@ -178,16 +178,20 @@ static MOZ_CONSTEXPR_VAR Register OsrFrameReg = IntArgReg3; static MOZ_CONSTEXPR_VAR Register PreBarrierReg = rdx; -static const uint32_t ABIStackAlignment = 16; -static const uint32_t CodeAlignment = 16; -static const uint32_t JitStackAlignment = 16; +static MOZ_CONSTEXPR_VAR uint32_t ABIStackAlignment = 16; +static MOZ_CONSTEXPR_VAR uint32_t CodeAlignment = 16; +static MOZ_CONSTEXPR_VAR uint32_t JitStackAlignment = 16; + +static MOZ_CONSTEXPR_VAR uint32_t JitStackValueAlignment = JitStackAlignment / sizeof(Value); +static_assert(JitStackAlignment % sizeof(Value) == 0 && JitStackValueAlignment >= 1, + "Stack alignment should be a non-zero multiple of sizeof(Value)"); // This boolean indicates whether we support SIMD instructions flavoured for // this architecture or not. Rather than a method in the LIRGenerator, it is // here such that it is accessible from the entire codebase. Once full support // for SIMD is reached on all tier-1 platforms, this constant can be deleted. -static const bool SupportsSimd = true; -static const uint32_t SimdMemoryAlignment = 16; +static MOZ_CONSTEXPR_VAR bool SupportsSimd = true; +static MOZ_CONSTEXPR_VAR uint32_t SimdMemoryAlignment = 16; static_assert(CodeAlignment % SimdMemoryAlignment == 0, "Code alignment should be larger than any of the alignments which are used for " diff --git a/js/src/jit/x64/Trampoline-x64.cpp b/js/src/jit/x64/Trampoline-x64.cpp index 0249922eeeef..92147f84755f 100644 --- a/js/src/jit/x64/Trampoline-x64.cpp +++ b/js/src/jit/x64/Trampoline-x64.cpp @@ -405,10 +405,9 @@ JitRuntime::generateArgumentsRectifier(JSContext *cx, void **returnAddrOut) static_assert(JitStackAlignment % sizeof(Value) == 0, "Ensure that we can pad the stack by pushing extra UndefinedValue"); - const uint32_t alignment = JitStackAlignment / sizeof(Value); - MOZ_ASSERT(IsPowerOfTwo(alignment)); - masm.addl(Imm32(alignment - 1 /* for padding */ + 1 /* for |this| */), rcx); - masm.andl(Imm32(~(alignment - 1)), rcx); + MOZ_ASSERT(IsPowerOfTwo(JitStackValueAlignment)); + masm.addl(Imm32(JitStackValueAlignment - 1 /* for padding */ + 1 /* for |this| */), rcx); + masm.andl(Imm32(~(JitStackValueAlignment - 1)), rcx); // Load the number of |undefined|s to push into %rcx. masm.subq(r8, rcx); diff --git a/js/src/jit/x86/Assembler-x86.h b/js/src/jit/x86/Assembler-x86.h index e68f15d4423c..1e42dd8e04fc 100644 --- a/js/src/jit/x86/Assembler-x86.h +++ b/js/src/jit/x86/Assembler-x86.h @@ -105,19 +105,23 @@ static MOZ_CONSTEXPR_VAR Register AsmJSIonExitRegD2 = esi; // GCC stack is aligned on 16 bytes. Ion does not maintain this for internal // calls. asm.js code does. #if defined(__GNUC__) -static const uint32_t ABIStackAlignment = 16; +static MOZ_CONSTEXPR_VAR uint32_t ABIStackAlignment = 16; #else -static const uint32_t ABIStackAlignment = 4; +static MOZ_CONSTEXPR_VAR uint32_t ABIStackAlignment = 4; #endif -static const uint32_t CodeAlignment = 16; -static const uint32_t JitStackAlignment = 16; +static MOZ_CONSTEXPR_VAR uint32_t CodeAlignment = 16; +static MOZ_CONSTEXPR_VAR uint32_t JitStackAlignment = 16; + +static MOZ_CONSTEXPR_VAR uint32_t JitStackValueAlignment = JitStackAlignment / sizeof(Value); +static_assert(JitStackAlignment % sizeof(Value) == 0 && JitStackValueAlignment >= 1, + "Stack alignment should be a non-zero multiple of sizeof(Value)"); // This boolean indicates whether we support SIMD instructions flavoured for // this architecture or not. Rather than a method in the LIRGenerator, it is // here such that it is accessible from the entire codebase. Once full support // for SIMD is reached on all tier-1 platforms, this constant can be deleted. -static const bool SupportsSimd = true; -static const uint32_t SimdMemoryAlignment = 16; +static MOZ_CONSTEXPR_VAR bool SupportsSimd = true; +static MOZ_CONSTEXPR_VAR uint32_t SimdMemoryAlignment = 16; static_assert(CodeAlignment % SimdMemoryAlignment == 0, "Code alignment should be larger than any of the alignments which are used for " diff --git a/js/src/jit/x86/Trampoline-x86.cpp b/js/src/jit/x86/Trampoline-x86.cpp index fc0fe0740a66..892fdbc60c10 100644 --- a/js/src/jit/x86/Trampoline-x86.cpp +++ b/js/src/jit/x86/Trampoline-x86.cpp @@ -395,10 +395,9 @@ JitRuntime::generateArgumentsRectifier(JSContext *cx, void **returnAddrOut) static_assert(JitStackAlignment % sizeof(Value) == 0, "Ensure that we can pad the stack by pushing extra UndefinedValue"); - const uint32_t alignment = JitStackAlignment / sizeof(Value); - MOZ_ASSERT(IsPowerOfTwo(alignment)); - masm.addl(Imm32(alignment - 1 /* for padding */), ecx); - masm.andl(Imm32(~(alignment - 1)), ecx); + MOZ_ASSERT(IsPowerOfTwo(JitStackValueAlignment)); + masm.addl(Imm32(JitStackValueAlignment - 1 /* for padding */), ecx); + masm.andl(Imm32(~(JitStackValueAlignment - 1)), ecx); masm.subl(esi, ecx); // Copy the number of actual arguments.