From a2bd4706c2366987c4a08899720b852a1ab4b9d6 Mon Sep 17 00:00:00 2001 From: "mvstanton@chromium.org" Date: Wed, 6 Mar 2013 16:15:01 +0000 Subject: [PATCH] Make sure that on x86 we don't generate SSE2 code in the snapshot. BUG= Review URL: https://codereview.chromium.org/12391033 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@13844 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/assembler-arm.h | 29 ++++------------------------- src/arm/builtins-arm.cc | 6 ------ src/assembler.cc | 4 +--- src/ia32/assembler-ia32.h | 29 ++++------------------------- src/ia32/builtins-ia32.cc | 8 ++------ src/ia32/code-stubs-ia32.cc | 8 +++----- src/ia32/code-stubs-ia32.h | 5 ++++- src/ia32/lithium-codegen-ia32.cc | 16 +++++++++++----- src/isolate.cc | 4 +++- src/mips/assembler-mips.h | 26 -------------------------- src/mips/builtins-mips.cc | 6 ------ src/x64/assembler-x64.h | 5 +++++ 12 files changed, 37 insertions(+), 109 deletions(-) diff --git a/src/arm/assembler-arm.h b/src/arm/assembler-arm.h index 6b00f32..045638e 100644 --- a/src/arm/assembler-arm.h +++ b/src/arm/assembler-arm.h @@ -74,31 +74,10 @@ class CpuFeatures : public AllStatic { (static_cast(1) << f)) != 0; } - class TryForceFeatureScope BASE_EMBEDDED { - public: - explicit TryForceFeatureScope(CpuFeature f) - : old_supported_(CpuFeatures::supported_) { - if (CanForce()) { - CpuFeatures::supported_ |= (1u << f); - } - } - - ~TryForceFeatureScope() { - if (CanForce()) { - CpuFeatures::supported_ = old_supported_; - } - } - - private: - static bool CanForce() { - // It's only safe to temporarily force support of CPU features - // when there's only a single isolate, which is guaranteed when - // the serializer is enabled. - return Serializer::enabled(); - } - - const unsigned old_supported_; - }; + static bool IsSafeForSnapshot(CpuFeature f) { + return (IsSupported(f) && + (!Serializer::enabled() || !IsFoundByRuntimeProbingOnly(f))); + } private: #ifdef DEBUG diff --git a/src/arm/builtins-arm.cc b/src/arm/builtins-arm.cc index a122800..b2c2193 100644 --- a/src/arm/builtins-arm.cc +++ b/src/arm/builtins-arm.cc @@ -1377,12 +1377,6 @@ void Builtins::Generate_NotifyOSR(MacroAssembler* masm) { void Builtins::Generate_OnStackReplacement(MacroAssembler* masm) { - CpuFeatures::TryForceFeatureScope scope(VFP3); - if (!CPU::SupportsCrankshaft()) { - __ Abort("Unreachable code: Cannot optimize without VFP3 support."); - return; - } - // Lookup the function in the JavaScript frame and push it as an // argument to the on-stack replacement function. __ ldr(r0, MemOperand(fp, JavaScriptFrameConstants::kFunctionOffset)); diff --git a/src/assembler.cc b/src/assembler.cc index fdc000e..8d8208d 100644 --- a/src/assembler.cc +++ b/src/assembler.cc @@ -186,9 +186,7 @@ PredictableCodeSizeScope::~PredictableCodeSizeScope() { #ifdef DEBUG CpuFeatureScope::CpuFeatureScope(AssemblerBase* assembler, CpuFeature f) : assembler_(assembler) { - ASSERT(CpuFeatures::IsSupported(f)); - ASSERT(!Serializer::enabled() || - !CpuFeatures::IsFoundByRuntimeProbingOnly(f)); + ASSERT(CpuFeatures::IsSafeForSnapshot(f)); old_enabled_ = assembler_->enabled_cpu_features(); uint64_t mask = static_cast(1) << f; // TODO(svenpanne) This special case below doesn't belong here! diff --git a/src/ia32/assembler-ia32.h b/src/ia32/assembler-ia32.h index 9fa092a..a3da9af 100644 --- a/src/ia32/assembler-ia32.h +++ b/src/ia32/assembler-ia32.h @@ -536,31 +536,10 @@ class CpuFeatures : public AllStatic { (static_cast(1) << f)) != 0; } - class TryForceFeatureScope BASE_EMBEDDED { - public: - explicit TryForceFeatureScope(CpuFeature f) - : old_supported_(CpuFeatures::supported_) { - if (CanForce()) { - CpuFeatures::supported_ |= (static_cast(1) << f); - } - } - - ~TryForceFeatureScope() { - if (CanForce()) { - CpuFeatures::supported_ = old_supported_; - } - } - - private: - static bool CanForce() { - // It's only safe to temporarily force support of CPU features - // when there's only a single isolate, which is guaranteed when - // the serializer is enabled. - return Serializer::enabled(); - } - - const uint64_t old_supported_; - }; + static bool IsSafeForSnapshot(CpuFeature f) { + return (IsSupported(f) && + (!Serializer::enabled() || !IsFoundByRuntimeProbingOnly(f))); + } private: #ifdef DEBUG diff --git a/src/ia32/builtins-ia32.cc b/src/ia32/builtins-ia32.cc index e3b2b7b..7ce211d 100644 --- a/src/ia32/builtins-ia32.cc +++ b/src/ia32/builtins-ia32.cc @@ -643,6 +643,8 @@ void Builtins::Generate_NotifyLazyDeoptimized(MacroAssembler* masm) { void Builtins::Generate_NotifyOSR(MacroAssembler* masm) { // TODO(kasperl): Do we need to save/restore the XMM registers too? + // TODO(mvstanton): We should save these regs, do this in a future + // checkin. // For now, we are relying on the fact that Runtime::NotifyOSR // doesn't do any garbage collection which allows us to save/restore @@ -1792,12 +1794,6 @@ void Builtins::Generate_ArgumentsAdaptorTrampoline(MacroAssembler* masm) { void Builtins::Generate_OnStackReplacement(MacroAssembler* masm) { - CpuFeatures::TryForceFeatureScope scope(SSE2); - if (!CpuFeatures::IsSupported(SSE2) && FLAG_debug_code) { - __ Abort("Unreachable code: Cannot optimize without SSE2 support."); - return; - } - // Get the loop depth of the stack guard check. This is recorded in // a test(eax, depth) instruction right after the call. Label stack_check; diff --git a/src/ia32/code-stubs-ia32.cc b/src/ia32/code-stubs-ia32.cc index c9c1933..1b27540 100644 --- a/src/ia32/code-stubs-ia32.cc +++ b/src/ia32/code-stubs-ia32.cc @@ -7516,11 +7516,9 @@ bool RecordWriteStub::IsPregenerated() { void StoreBufferOverflowStub::GenerateFixedRegStubsAheadOfTime( Isolate* isolate) { - StoreBufferOverflowStub stub1(kDontSaveFPRegs); - stub1.GetCode(isolate)->set_is_pregenerated(true); - - CpuFeatures::TryForceFeatureScope scope(SSE2); - if (CpuFeatures::IsSupported(SSE2)) { + StoreBufferOverflowStub stub(kDontSaveFPRegs); + stub.GetCode(isolate)->set_is_pregenerated(true); + if (CpuFeatures::IsSafeForSnapshot(SSE2)) { StoreBufferOverflowStub stub2(kSaveFPRegs); stub2.GetCode(isolate)->set_is_pregenerated(true); } diff --git a/src/ia32/code-stubs-ia32.h b/src/ia32/code-stubs-ia32.h index 9d5c0be..c2ae5f0 100644 --- a/src/ia32/code-stubs-ia32.h +++ b/src/ia32/code-stubs-ia32.h @@ -64,7 +64,9 @@ class TranscendentalCacheStub: public PlatformCodeStub { class StoreBufferOverflowStub: public PlatformCodeStub { public: explicit StoreBufferOverflowStub(SaveFPRegsMode save_fp) - : save_doubles_(save_fp) { } + : save_doubles_(save_fp) { + ASSERT(CpuFeatures::IsSafeForSnapshot(SSE2) || save_fp == kDontSaveFPRegs); + } void Generate(MacroAssembler* masm); @@ -397,6 +399,7 @@ class RecordWriteStub: public PlatformCodeStub { regs_(object, // An input reg. address, // An input reg. value) { // One scratch reg. + ASSERT(CpuFeatures::IsSafeForSnapshot(SSE2) || fp_mode == kDontSaveFPRegs); } enum Mode { diff --git a/src/ia32/lithium-codegen-ia32.cc b/src/ia32/lithium-codegen-ia32.cc index 6fa1c42..7ce2f2d 100644 --- a/src/ia32/lithium-codegen-ia32.cc +++ b/src/ia32/lithium-codegen-ia32.cc @@ -40,6 +40,12 @@ namespace v8 { namespace internal { +static SaveFPRegsMode GetSaveFPRegsMode() { + // We don't need to save floating point regs when generating the snapshot + return CpuFeatures::IsSafeForSnapshot(SSE2) ? kSaveFPRegs : kDontSaveFPRegs; +} + + // When invoking builtins, we need to record the safepoint in the middle of // the invoke instruction sequence generated by the macro assembler. class SafepointGenerator : public CallWrapper { @@ -2847,7 +2853,7 @@ void LCodeGen::DoStoreContextSlot(LStoreContextSlot* instr) { offset, value, temp, - kSaveFPRegs, + GetSaveFPRegsMode(), EMIT_REMEMBERED_SET, check_needed); } @@ -4179,7 +4185,7 @@ void LCodeGen::DoStoreNamedField(LStoreNamedField* instr) { HeapObject::kMapOffset, temp_map, temp, - kSaveFPRegs, + GetSaveFPRegsMode(), OMIT_REMEMBERED_SET, OMIT_SMI_CHECK); } @@ -4198,7 +4204,7 @@ void LCodeGen::DoStoreNamedField(LStoreNamedField* instr) { offset, value, temp, - kSaveFPRegs, + GetSaveFPRegsMode(), EMIT_REMEMBERED_SET, check_needed); } @@ -4213,7 +4219,7 @@ void LCodeGen::DoStoreNamedField(LStoreNamedField* instr) { offset, value, object, - kSaveFPRegs, + GetSaveFPRegsMode(), EMIT_REMEMBERED_SET, check_needed); } @@ -4360,7 +4366,7 @@ void LCodeGen::DoStoreKeyedFixedArray(LStoreKeyed* instr) { __ RecordWrite(elements, key, value, - kSaveFPRegs, + GetSaveFPRegsMode(), EMIT_REMEMBERED_SET, check_needed); } diff --git a/src/isolate.cc b/src/isolate.cc index 5129dbd..5e61761 100644 --- a/src/isolate.cc +++ b/src/isolate.cc @@ -2156,8 +2156,10 @@ bool Isolate::Init(Deserializer* des) { } if (!Serializer::enabled()) { - // Ensure that the stub failure trampoline has been generated. + // Ensure that all stubs which need to be generated ahead of time, but + // cannot be serialized into the snapshot have been generated. HandleScope scope(this); + StoreBufferOverflowStub::GenerateFixedRegStubsAheadOfTime(this); CodeStub::GenerateFPStubs(this); StubFailureTrampolineStub::GenerateAheadOfTime(this); } diff --git a/src/mips/assembler-mips.h b/src/mips/assembler-mips.h index c235a4d..1177181 100644 --- a/src/mips/assembler-mips.h +++ b/src/mips/assembler-mips.h @@ -413,32 +413,6 @@ class CpuFeatures : public AllStatic { (static_cast(1) << f)) != 0; } - class TryForceFeatureScope BASE_EMBEDDED { - public: - explicit TryForceFeatureScope(CpuFeature f) - : old_supported_(CpuFeatures::supported_) { - if (CanForce()) { - CpuFeatures::supported_ |= (1u << f); - } - } - - ~TryForceFeatureScope() { - if (CanForce()) { - CpuFeatures::supported_ = old_supported_; - } - } - - private: - static bool CanForce() { - // It's only safe to temporarily force support of CPU features - // when there's only a single isolate, which is guaranteed when - // the serializer is enabled. - return Serializer::enabled(); - } - - const unsigned old_supported_; - }; - private: #ifdef DEBUG static bool initialized_; diff --git a/src/mips/builtins-mips.cc b/src/mips/builtins-mips.cc index 320dcc4..8edfe71 100644 --- a/src/mips/builtins-mips.cc +++ b/src/mips/builtins-mips.cc @@ -1409,12 +1409,6 @@ void Builtins::Generate_NotifyOSR(MacroAssembler* masm) { void Builtins::Generate_OnStackReplacement(MacroAssembler* masm) { - CpuFeatures::TryForceFeatureScope scope(VFP3); - if (!CpuFeatures::IsSupported(FPU)) { - __ Abort("Unreachable code: Cannot optimize without FPU support."); - return; - } - // Lookup the function in the JavaScript frame and push it as an // argument to the on-stack replacement function. __ lw(a0, MemOperand(fp, JavaScriptFrameConstants::kFunctionOffset)); diff --git a/src/x64/assembler-x64.h b/src/x64/assembler-x64.h index 86ae057..03e742d 100644 --- a/src/x64/assembler-x64.h +++ b/src/x64/assembler-x64.h @@ -474,6 +474,11 @@ class CpuFeatures : public AllStatic { (static_cast(1) << f)) != 0; } + static bool IsSafeForSnapshot(CpuFeature f) { + return (IsSupported(f) && + (!Serializer::enabled() || !IsFoundByRuntimeProbingOnly(f))); + } + private: // Safe defaults include SSE2 and CMOV for X64. It is always available, if // anyone checks, but they shouldn't need to check. -- 2.7.4