Reviewers: paul.l..., dusmil.imgtec, akos.palfi.imgtec, gergely.kis.imgtec,
Description:
MIPS: Fix label position types in binding code.
Also some target_at and target_at_put uniformed on mips and mips64.
BUG=
Please review this at https://codereview.chromium.org/942123002/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+22, -24 lines):
M src/mips/assembler-mips.h
M src/mips/assembler-mips.cc
M src/mips64/assembler-mips64.h
M src/mips64/assembler-mips64.cc
Index: src/mips/assembler-mips.cc
diff --git a/src/mips/assembler-mips.cc b/src/mips/assembler-mips.cc
index
e7cfd57006ae0c10297d10484efa7d87db24231a..13dd0cd2f9ec8a07ecb1bbbdb01d070a942566bf
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<int32_t>(buffer_ + pos);
- int32_t delta = instr_address - instr;
+ int delta = static_cast<int>(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<int32_t>(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<int32_t>(buffer_ + pos);
instr_address &= kImm28Mask;
- int32_t delta = instr_address - imm28;
+ int delta = static_cast<int>(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<uint32_t>(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();
}
}
Index: src/mips/assembler-mips.h
diff --git a/src/mips/assembler-mips.h b/src/mips/assembler-mips.h
index
89af82ad1ac1f87c699d5673ce001f03e633ad17..585cf5e50df44fb1950c4ff6b3cf6b10b7598f54
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);
Index: src/mips64/assembler-mips64.cc
diff --git a/src/mips64/assembler-mips64.cc b/src/mips64/assembler-mips64.cc
index
4ce970da332c9e3a2a6ff0fbed5fedef7209a1c3..9fdcf759449fe1158db28097bbb495a8bd696713
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<int64_t*>(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<int64_t>(p);
- int64_t delta = instr_address - address;
+ DCHECK(instr_address - address < INT_MAX);
+ int delta = static_cast<int>(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<int64_t>(buffer_ + pos);
- int64_t delta = instr_address - imm;
+ DCHECK(instr_address - imm < INT_MAX);
+ int delta = static_cast<int>(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<int64_t>(buffer_ + pos);
instr_address &= kImm28Mask;
- int64_t delta = instr_address - imm28;
+ int delta = static_cast<int>(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<uint64_t>(buffer_) + target_pos;
*reinterpret_cast<uint64_t*>(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
Index: src/mips64/assembler-mips64.h
diff --git a/src/mips64/assembler-mips64.h b/src/mips64/assembler-mips64.h
index
5ad98f6cd88135a43c282b42651f79fd184b26ef..a550ef2c8b575b769cf7b789b830455dfefa5b60
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);
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
