From 74d49a1e7e07bddb6582401c1c016e914cbdcbfb Mon Sep 17 00:00:00 2001 From: "Jacob.Bramley@arm.com" Date: Thu, 10 Apr 2014 15:47:45 +0000 Subject: [PATCH] ARM64: Preserve x8 and x9 when necessary. Fix a couple of places were x8 and x9 are excluded from lists of saved registers. These are caller-saved registers, so C code can corrupt them. x8 and x9 were originally reserved for debug code in the ARM64 port, so we didn't bother preserving them, but they are now normal allocatable registers. BUG=v8:3263 LOG=N R=rmcilroy@chromium.org, ulan@chromium.org Review URL: https://codereview.chromium.org/233373002 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@20658 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm64/assembler-arm64.h | 4 ++-- src/arm64/code-stubs-arm64.cc | 16 ++++++++-------- src/arm64/code-stubs-arm64.h | 24 +++++++++++------------- src/arm64/macro-assembler-arm64.cc | 17 ++++++++++++++--- src/arm64/macro-assembler-arm64.h | 7 +++---- 5 files changed, 38 insertions(+), 30 deletions(-) diff --git a/src/arm64/assembler-arm64.h b/src/arm64/assembler-arm64.h index 9a8cd46..a5784a6 100644 --- a/src/arm64/assembler-arm64.h +++ b/src/arm64/assembler-arm64.h @@ -516,8 +516,8 @@ class CPURegList { void Combine(const CPURegList& other); // Remove every register in the other CPURegList from this one. Registers that - // do not exist in this list are ignored. The type and size of the registers - // in the 'other' list must match those in this list. + // do not exist in this list are ignored. The type of the registers in the + // 'other' list must match those in this list. void Remove(const CPURegList& other); // Variants of Combine and Remove which take CPURegisters. diff --git a/src/arm64/code-stubs-arm64.cc b/src/arm64/code-stubs-arm64.cc index d112271..9bd2dca 100644 --- a/src/arm64/code-stubs-arm64.cc +++ b/src/arm64/code-stubs-arm64.cc @@ -1095,20 +1095,20 @@ void ICCompareStub::GenerateGeneric(MacroAssembler* masm) { void StoreBufferOverflowStub::Generate(MacroAssembler* masm) { - // Preserve caller-saved registers x0-x7 and x10-x15. We don't care if x8, x9, - // ip0 and ip1 are corrupted by the call into C. CPURegList saved_regs = kCallerSaved; - saved_regs.Remove(ip0); - saved_regs.Remove(ip1); - saved_regs.Remove(x8); - saved_regs.Remove(x9); + CPURegList saved_fp_regs = kCallerSavedFP; // We don't allow a GC during a store buffer overflow so there is no need to // store the registers in any particular way, but we do have to store and // restore them. + + // We don't care if MacroAssembler scratch registers are corrupted. + saved_regs.Remove(*(masm->TmpList())); + saved_fp_regs.Remove(*(masm->FPTmpList())); + __ PushCPURegList(saved_regs); if (save_doubles_ == kSaveFPRegs) { - __ PushCPURegList(kCallerSavedFP); + __ PushCPURegList(saved_fp_regs); } AllowExternalCallThatCantCauseGC scope(masm); @@ -1118,7 +1118,7 @@ void StoreBufferOverflowStub::Generate(MacroAssembler* masm) { 1, 0); if (save_doubles_ == kSaveFPRegs) { - __ PopCPURegList(kCallerSavedFP); + __ PopCPURegList(saved_fp_regs); } __ PopCPURegList(saved_regs); __ Ret(); diff --git a/src/arm64/code-stubs-arm64.h b/src/arm64/code-stubs-arm64.h index 7e09ffa..f290a08 100644 --- a/src/arm64/code-stubs-arm64.h +++ b/src/arm64/code-stubs-arm64.h @@ -210,9 +210,15 @@ class RecordWriteStub: public PlatformCodeStub { : object_(object), address_(address), scratch0_(scratch), - saved_regs_(kCallerSaved) { + saved_regs_(kCallerSaved), + saved_fp_regs_(kCallerSavedFP) { ASSERT(!AreAliased(scratch, object, address)); + // The SaveCallerSaveRegisters method needs to save caller-saved + // registers, but we don't bother saving MacroAssembler scratch registers. + saved_regs_.Remove(MacroAssembler::DefaultTmpList()); + saved_fp_regs_.Remove(MacroAssembler::DefaultFPTmpList()); + // We would like to require more scratch registers for this stub, // but the number of registers comes down to the ones used in // FullCodeGen::SetVar(), which is architecture independent. @@ -223,12 +229,6 @@ class RecordWriteStub: public PlatformCodeStub { scratch1_ = Register(pool_available.PopLowestIndex()); scratch2_ = Register(pool_available.PopLowestIndex()); - // SaveCallerRegisters method needs to save caller saved register, however - // we don't bother saving ip0 and ip1 because they are used as scratch - // registers by the MacroAssembler. - saved_regs_.Remove(ip0); - saved_regs_.Remove(ip1); - // The scratch registers will be restored by other means so we don't need // to save them with the other caller saved registers. saved_regs_.Remove(scratch0_); @@ -253,7 +253,7 @@ class RecordWriteStub: public PlatformCodeStub { // register will need to be preserved. Can we improve this? masm->PushCPURegList(saved_regs_); if (mode == kSaveFPRegs) { - masm->PushCPURegList(kCallerSavedFP); + masm->PushCPURegList(saved_fp_regs_); } } @@ -261,7 +261,7 @@ class RecordWriteStub: public PlatformCodeStub { // TODO(all): This can be very expensive, and it is likely that not every // register will need to be preserved. Can we improve this? if (mode == kSaveFPRegs) { - masm->PopCPURegList(kCallerSavedFP); + masm->PopCPURegList(saved_fp_regs_); } masm->PopCPURegList(saved_regs_); } @@ -279,6 +279,7 @@ class RecordWriteStub: public PlatformCodeStub { Register scratch1_; Register scratch2_; CPURegList saved_regs_; + CPURegList saved_fp_regs_; // TODO(all): We should consider moving this somewhere else. static CPURegList GetValidRegistersForAllocation() { @@ -296,10 +297,7 @@ class RecordWriteStub: public PlatformCodeStub { CPURegList list(CPURegister::kRegister, kXRegSizeInBits, 0, 25); // We also remove MacroAssembler's scratch registers. - list.Remove(ip0); - list.Remove(ip1); - list.Remove(x8); - list.Remove(x9); + list.Remove(MacroAssembler::DefaultTmpList()); return list; } diff --git a/src/arm64/macro-assembler-arm64.cc b/src/arm64/macro-assembler-arm64.cc index 97361f5..70d601b 100644 --- a/src/arm64/macro-assembler-arm64.cc +++ b/src/arm64/macro-assembler-arm64.cc @@ -53,7 +53,9 @@ MacroAssembler::MacroAssembler(Isolate* arg_isolate, #endif has_frame_(false), use_real_aborts_(true), - sp_(jssp), tmp_list_(ip0, ip1), fptmp_list_(fp_scratch1, fp_scratch2) { + sp_(jssp), + tmp_list_(DefaultTmpList()), + fptmp_list_(DefaultFPTmpList()) { if (isolate() != NULL) { code_object_ = Handle(isolate()->heap()->undefined_value(), isolate()); @@ -61,6 +63,16 @@ MacroAssembler::MacroAssembler(Isolate* arg_isolate, } +CPURegList MacroAssembler::DefaultTmpList() { + return CPURegList(ip0, ip1); +} + + +CPURegList MacroAssembler::DefaultFPTmpList() { + return CPURegList(fp_scratch1, fp_scratch2); +} + + void MacroAssembler::LogicalMacro(const Register& rd, const Register& rn, const Operand& operand, @@ -4729,8 +4741,7 @@ void MacroAssembler::Abort(BailoutReason reason) { // We need some scratch registers for the MacroAssembler, so make sure we have // some. This is safe here because Abort never returns. RegList old_tmp_list = TmpList()->list(); - TmpList()->Combine(ip0); - TmpList()->Combine(ip1); + TmpList()->Combine(MacroAssembler::DefaultTmpList()); if (use_real_aborts()) { // Avoid infinite recursion; Push contains some assertions that use Abort. diff --git a/src/arm64/macro-assembler-arm64.h b/src/arm64/macro-assembler-arm64.h index 5b57a28..904cf50 100644 --- a/src/arm64/macro-assembler-arm64.h +++ b/src/arm64/macro-assembler-arm64.h @@ -1927,6 +1927,9 @@ class MacroAssembler : public Assembler { CPURegList* TmpList() { return &tmp_list_; } CPURegList* FPTmpList() { return &fptmp_list_; } + static CPURegList DefaultTmpList(); + static CPURegList DefaultFPTmpList(); + // Like printf, but print at run-time from generated code. // // The caller must ensure that arguments for floating-point placeholders @@ -1947,10 +1950,6 @@ class MacroAssembler : public Assembler { // PrintfNoPreserve. Callee-saved registers are not used by Printf, and are // implicitly preserved. // - // Unlike many MacroAssembler functions, x8 and x9 are guaranteed to be - // preserved, and can be printed. This allows Printf to be used during debug - // code. - // // This function assumes (and asserts) that the current stack pointer is // callee-saved, not caller-saved. This is most likely the case anyway, as a // caller-saved stack pointer doesn't make a lot of sense. -- 2.7.4