From 39f58c2451e087dcdece0916d750ae5719483234 Mon Sep 17 00:00:00 2001 From: "balazs.kilvady" Date: Fri, 20 Feb 2015 11:29:28 -0800 Subject: [PATCH] MIPS: Fix label position types in binding code. Also some target_at and target_at_put uniformed on mips and mips64. BUG= Review URL: https://codereview.chromium.org/942123002 Cr-Commit-Position: refs/heads/master@{#26785} --- src/mips/assembler-mips.cc | 18 ++++++++---------- src/mips/assembler-mips.h | 4 ++-- src/mips64/assembler-mips64.cc | 20 ++++++++++---------- src/mips64/assembler-mips64.h | 4 ++-- 4 files changed, 22 insertions(+), 24 deletions(-) diff --git a/src/mips/assembler-mips.cc b/src/mips/assembler-mips.cc index e7cfd57006..13dd0cd2f9 100644 --- a/src/mips/assembler-mips.cc +++ b/src/mips/assembler-mips.cc @@ -663,14 +663,14 @@ bool Assembler::IsAndImmediate(Instr instr) { } -int Assembler::target_at(int32_t pos, bool is_internal) { +int Assembler::target_at(int pos, bool is_internal) { Instr instr = instr_at(pos); if (is_internal) { if (instr == 0) { return kEndOfChain; } else { int32_t instr_address = reinterpret_cast(buffer_ + pos); - int32_t delta = instr_address - instr; + int delta = static_cast(instr_address - instr); DCHECK(pos > delta); return pos - delta; } @@ -684,6 +684,8 @@ int Assembler::target_at(int32_t pos, bool is_internal) { return (imm18 + pos); } } + // Check we have a branch or jump instruction. + DCHECK(IsBranch(instr) || IsJ(instr) || IsLui(instr)); // Do NOT change this to <<2. We rely on arithmetic shifts here, assuming // the compiler uses arithmectic shifts for signed integers. if (IsBranch(instr)) { @@ -711,7 +713,7 @@ int Assembler::target_at(int32_t pos, bool is_internal) { DCHECK(pos > delta); return pos - delta; } - } else if (IsJ(instr)) { + } else { int32_t imm28 = (instr & static_cast(kImm26Mask)) << 2; if (imm28 == kEndOfJumpChain) { // EndOfChain sentinel is returned directly, not relative to pc or pos. @@ -719,13 +721,10 @@ int Assembler::target_at(int32_t pos, bool is_internal) { } else { uint32_t instr_address = reinterpret_cast(buffer_ + pos); instr_address &= kImm28Mask; - int32_t delta = instr_address - imm28; + int delta = static_cast(instr_address - imm28); DCHECK(pos > delta); return pos - delta; } - } else { - UNREACHABLE(); - return 0; } } @@ -747,6 +746,7 @@ void Assembler::target_at_put(int32_t pos, int32_t target_pos, return; } + DCHECK(IsBranch(instr) || IsJ(instr) || IsLui(instr)); if (IsBranch(instr)) { int32_t imm18 = target_pos - (pos + kBranchPCOffset); DCHECK((imm18 & 3) == 0); @@ -770,7 +770,7 @@ void Assembler::target_at_put(int32_t pos, int32_t target_pos, instr_lui | ((imm & kHiMask) >> kLuiShift)); instr_at_put(pos + 1 * Assembler::kInstrSize, instr_ori | (imm & kImm16Mask)); - } else if (IsJ(instr)) { + } else { uint32_t imm28 = reinterpret_cast(buffer_) + target_pos; imm28 &= kImm28Mask; DCHECK((imm28 & 3) == 0); @@ -780,8 +780,6 @@ void Assembler::target_at_put(int32_t pos, int32_t target_pos, DCHECK(is_uint26(imm26)); instr_at_put(pos, instr | (imm26 & kImm26Mask)); - } else { - UNREACHABLE(); } } diff --git a/src/mips/assembler-mips.h b/src/mips/assembler-mips.h index 89af82ad1a..585cf5e50d 100644 --- a/src/mips/assembler-mips.h +++ b/src/mips/assembler-mips.h @@ -1129,10 +1129,10 @@ class Assembler : public AssemblerBase { int32_t buffer_space() const { return reloc_info_writer.pos() - pc_; } // Decode branch instruction at pos and return branch target pos. - int target_at(int32_t pos, bool is_internal); + int target_at(int pos, bool is_internal); // Patch branch instruction at pos to branch to given branch target pos. - void target_at_put(int32_t pos, int32_t target_pos, bool is_internal); + void target_at_put(int pos, int target_pos, bool is_internal); // Say if we need to relocate with this mode. bool MustUseReg(RelocInfo::Mode rmode); diff --git a/src/mips64/assembler-mips64.cc b/src/mips64/assembler-mips64.cc index 4ce970da33..9fdcf75944 100644 --- a/src/mips64/assembler-mips64.cc +++ b/src/mips64/assembler-mips64.cc @@ -32,7 +32,6 @@ // modified significantly by Google Inc. // Copyright 2012 the V8 project authors. All rights reserved. - #include "src/v8.h" #if V8_TARGET_ARCH_MIPS64 @@ -635,7 +634,7 @@ bool Assembler::IsAndImmediate(Instr instr) { } -int64_t Assembler::target_at(int64_t pos, bool is_internal) { +int Assembler::target_at(int pos, bool is_internal) { if (is_internal) { int64_t* p = reinterpret_cast(buffer_ + pos); int64_t address = *p; @@ -643,7 +642,8 @@ int64_t Assembler::target_at(int64_t pos, bool is_internal) { return kEndOfChain; } else { int64_t instr_address = reinterpret_cast(p); - int64_t delta = instr_address - address; + DCHECK(instr_address - address < INT_MAX); + int delta = static_cast(instr_address - address); DCHECK(pos > delta); return pos - delta; } @@ -689,7 +689,8 @@ int64_t Assembler::target_at(int64_t pos, bool is_internal) { return kEndOfChain; } else { uint64_t instr_address = reinterpret_cast(buffer_ + pos); - int64_t delta = instr_address - imm; + DCHECK(instr_address - imm < INT_MAX); + int delta = static_cast(instr_address - imm); DCHECK(pos > delta); return pos - delta; } @@ -701,7 +702,7 @@ int64_t Assembler::target_at(int64_t pos, bool is_internal) { } else { uint64_t instr_address = reinterpret_cast(buffer_ + pos); instr_address &= kImm28Mask; - int64_t delta = instr_address - imm28; + int delta = static_cast(instr_address - imm28); DCHECK(pos > delta); return pos - delta; } @@ -709,8 +710,7 @@ int64_t Assembler::target_at(int64_t pos, bool is_internal) { } -void Assembler::target_at_put(int64_t pos, int64_t target_pos, - bool is_internal) { +void Assembler::target_at_put(int pos, int target_pos, bool is_internal) { if (is_internal) { uint64_t imm = reinterpret_cast(buffer_) + target_pos; *reinterpret_cast(buffer_ + pos) = imm; @@ -796,7 +796,7 @@ void Assembler::print(Label* L) { void Assembler::bind_to(Label* L, int pos) { DCHECK(0 <= pos && pos <= pc_offset()); // Must have valid binding position. - int32_t trampoline_pos = kInvalidSlotPos; + int trampoline_pos = kInvalidSlotPos; bool is_internal = false; if (L->is_linked() && !trampoline_emitted_) { unbound_labels_count_--; @@ -804,8 +804,8 @@ void Assembler::bind_to(Label* L, int pos) { } while (L->is_linked()) { - int32_t fixup_pos = L->pos(); - int32_t dist = pos - fixup_pos; + int fixup_pos = L->pos(); + int dist = pos - fixup_pos; is_internal = internal_reference_positions_.find(fixup_pos) != internal_reference_positions_.end(); next(L, is_internal); // Call next before overwriting link with target at diff --git a/src/mips64/assembler-mips64.h b/src/mips64/assembler-mips64.h index 5ad98f6cd8..a550ef2c8b 100644 --- a/src/mips64/assembler-mips64.h +++ b/src/mips64/assembler-mips64.h @@ -1168,10 +1168,10 @@ class Assembler : public AssemblerBase { int64_t buffer_space() const { return reloc_info_writer.pos() - pc_; } // Decode branch instruction at pos and return branch target pos. - int64_t target_at(int64_t pos, bool is_internal); + int target_at(int pos, bool is_internal); // Patch branch instruction at pos to branch to given branch target pos. - void target_at_put(int64_t pos, int64_t target_pos, bool is_internal); + void target_at_put(int pos, int target_pos, bool is_internal); // Say if we need to relocate with this mode. bool MustUseReg(RelocInfo::Mode rmode);