Commit 244ac6de authored by balazs.kilvady's avatar balazs.kilvady Committed by Commit bot

MIPS: Fix 'Assembler support for internal references.'

Added new INTERNAL_REFERENCE_ENCODED RelocInfo type to differentiate MIPS existing use of internal references in instructions from the new raw pointer reference needed for dd(Label*).

BUG=
TEST=cctest/test-assembler-mips/jump_tables1, cctest/test-assembler-mips/jump_tables2, cctest/test-assembler-mips/jump_tables3, cctest/test-run-machops/RunSwitch1

Review URL: https://codereview.chromium.org/922043005

Cr-Commit-Position: refs/heads/master@{#26651}
parent 38d0749c
...@@ -393,6 +393,11 @@ class RelocInfo { ...@@ -393,6 +393,11 @@ class RelocInfo {
NONE64, // never recorded 64-bit value NONE64, // never recorded 64-bit value
CODE_AGE_SEQUENCE, // Not stored in RelocInfo array, used explictly by CODE_AGE_SEQUENCE, // Not stored in RelocInfo array, used explictly by
// code aging. // code aging.
// Encoded internal reference, used only on MIPS and MIPS64.
// Re-uses previous ARM-only encoding, to fit in RealRelocMode space.
INTERNAL_REFERENCE_ENCODED = CONST_POOL,
FIRST_REAL_RELOC_MODE = CODE_TARGET, FIRST_REAL_RELOC_MODE = CODE_TARGET,
LAST_REAL_RELOC_MODE = VENEER_POOL, LAST_REAL_RELOC_MODE = VENEER_POOL,
FIRST_PSEUDO_RELOC_MODE = CODE_AGE_SEQUENCE, FIRST_PSEUDO_RELOC_MODE = CODE_AGE_SEQUENCE,
...@@ -465,6 +470,9 @@ class RelocInfo { ...@@ -465,6 +470,9 @@ class RelocInfo {
static inline bool IsInternalReference(Mode mode) { static inline bool IsInternalReference(Mode mode) {
return mode == INTERNAL_REFERENCE; return mode == INTERNAL_REFERENCE;
} }
static inline bool IsInternalReferenceEncoded(Mode mode) {
return mode == INTERNAL_REFERENCE_ENCODED;
}
static inline bool IsDebugBreakSlot(Mode mode) { static inline bool IsDebugBreakSlot(Mode mode) {
return mode == DEBUG_BREAK_SLOT; return mode == DEBUG_BREAK_SLOT;
} }
......
...@@ -126,10 +126,10 @@ void RelocInfo::apply(intptr_t delta, ICacheFlushMode icache_flush_mode) { ...@@ -126,10 +126,10 @@ void RelocInfo::apply(intptr_t delta, ICacheFlushMode icache_flush_mode) {
Assembler::JumpLabelToJumpRegister(pc_); Assembler::JumpLabelToJumpRegister(pc_);
} }
} }
if (IsInternalReference(rmode_)) { if (IsInternalReference(rmode_) || IsInternalReferenceEncoded(rmode_)) {
// Absolute code pointer inside code object moves with the code object. // Absolute code pointer inside code object moves with the code object.
byte* p = reinterpret_cast<byte*>(pc_); byte* p = reinterpret_cast<byte*>(pc_);
int count = Assembler::RelocateInternalReference(p, delta); int count = Assembler::RelocateInternalReference(rmode_, p, delta);
CpuFeatures::FlushICache(p, count * sizeof(uint32_t)); CpuFeatures::FlushICache(p, count * sizeof(uint32_t));
} }
} }
......
...@@ -197,7 +197,8 @@ Register ToRegister(int num) { ...@@ -197,7 +197,8 @@ Register ToRegister(int num) {
// Implementation of RelocInfo. // Implementation of RelocInfo.
const int RelocInfo::kApplyMask = RelocInfo::kCodeTargetMask | const int RelocInfo::kApplyMask = RelocInfo::kCodeTargetMask |
1 << RelocInfo::INTERNAL_REFERENCE; 1 << RelocInfo::INTERNAL_REFERENCE |
1 << RelocInfo::INTERNAL_REFERENCE_ENCODED;
bool RelocInfo::IsCodedSpecially() { bool RelocInfo::IsCodedSpecially() {
...@@ -662,8 +663,18 @@ bool Assembler::IsAndImmediate(Instr instr) { ...@@ -662,8 +663,18 @@ bool Assembler::IsAndImmediate(Instr instr) {
} }
int Assembler::target_at(int32_t pos) { int Assembler::target_at(int32_t pos, bool is_internal) {
Instr instr = instr_at(pos); 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;
DCHECK(pos > delta);
return pos - delta;
}
}
if ((instr & ~kImm16Mask) == 0) { if ((instr & ~kImm16Mask) == 0) {
// Emitted label constant, not part of a branch. // Emitted label constant, not part of a branch.
if (instr == 0) { if (instr == 0) {
...@@ -712,20 +723,22 @@ int Assembler::target_at(int32_t pos) { ...@@ -712,20 +723,22 @@ int Assembler::target_at(int32_t pos) {
DCHECK(pos > delta); DCHECK(pos > delta);
return pos - delta; return pos - delta;
} }
} else { // IsLabel(instr) } else {
int32_t imm28 = (instr & static_cast<int32_t>(kImm26Mask)) << 2; UNREACHABLE();
if (imm28 == kEndOfJumpChain) { return 0;
// EndOfChain sentinel is returned directly, not relative to pc or pos.
return kEndOfChain;
} else {
return pos + imm28;
}
} }
} }
void Assembler::target_at_put(int32_t pos, int32_t target_pos) { void Assembler::target_at_put(int32_t pos, int32_t target_pos,
bool is_internal) {
Instr instr = instr_at(pos); Instr instr = instr_at(pos);
if (is_internal) {
uint32_t imm = reinterpret_cast<uint32_t>(buffer_) + target_pos;
instr_at_put(pos, imm);
return;
}
if ((instr & ~kImm16Mask) == 0) { if ((instr & ~kImm16Mask) == 0) {
DCHECK(target_pos == kEndOfChain || target_pos >= 0); DCHECK(target_pos == kEndOfChain || target_pos >= 0);
// Emitted label constant, not part of a branch. // Emitted label constant, not part of a branch.
...@@ -768,8 +781,7 @@ void Assembler::target_at_put(int32_t pos, int32_t target_pos) { ...@@ -768,8 +781,7 @@ void Assembler::target_at_put(int32_t pos, int32_t target_pos) {
instr_at_put(pos, instr | (imm26 & kImm26Mask)); instr_at_put(pos, instr | (imm26 & kImm26Mask));
} else { } else {
uint32_t imm = reinterpret_cast<uint32_t>(buffer_) + target_pos; UNREACHABLE();
instr_at_put(pos, imm);
} }
} }
...@@ -790,7 +802,8 @@ void Assembler::print(Label* L) { ...@@ -790,7 +802,8 @@ void Assembler::print(Label* L) {
} else { } else {
PrintF("%d\n", instr); PrintF("%d\n", instr);
} }
next(&l); next(&l, internal_reference_positions_.find(l.pos()) !=
internal_reference_positions_.end());
} }
} else { } else {
PrintF("label in inconsistent state (pos = %d)\n", L->pos_); PrintF("label in inconsistent state (pos = %d)\n", L->pos_);
...@@ -801,6 +814,7 @@ void Assembler::print(Label* L) { ...@@ -801,6 +814,7 @@ void Assembler::print(Label* L) {
void Assembler::bind_to(Label* L, int pos) { void Assembler::bind_to(Label* L, int pos) {
DCHECK(0 <= pos && pos <= pc_offset()); // Must have valid binding position. DCHECK(0 <= pos && pos <= pc_offset()); // Must have valid binding position.
int32_t trampoline_pos = kInvalidSlotPos; int32_t trampoline_pos = kInvalidSlotPos;
bool is_internal = false;
if (L->is_linked() && !trampoline_emitted_) { if (L->is_linked() && !trampoline_emitted_) {
unbound_labels_count_--; unbound_labels_count_--;
next_buffer_check_ += kTrampolineSlotsSize; next_buffer_check_ += kTrampolineSlotsSize;
...@@ -809,22 +823,27 @@ void Assembler::bind_to(Label* L, int pos) { ...@@ -809,22 +823,27 @@ void Assembler::bind_to(Label* L, int pos) {
while (L->is_linked()) { while (L->is_linked()) {
int32_t fixup_pos = L->pos(); int32_t fixup_pos = L->pos();
int32_t dist = pos - fixup_pos; int32_t dist = pos - fixup_pos;
next(L); // Call next before overwriting link with target at 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
// fixup_pos.
Instr instr = instr_at(fixup_pos); Instr instr = instr_at(fixup_pos);
if (IsBranch(instr)) { if (is_internal) {
target_at_put(fixup_pos, pos, is_internal);
} else if (!is_internal && IsBranch(instr)) {
if (dist > kMaxBranchOffset) { if (dist > kMaxBranchOffset) {
if (trampoline_pos == kInvalidSlotPos) { if (trampoline_pos == kInvalidSlotPos) {
trampoline_pos = get_trampoline_entry(fixup_pos); trampoline_pos = get_trampoline_entry(fixup_pos);
CHECK(trampoline_pos != kInvalidSlotPos); CHECK(trampoline_pos != kInvalidSlotPos);
} }
DCHECK((trampoline_pos - fixup_pos) <= kMaxBranchOffset); DCHECK((trampoline_pos - fixup_pos) <= kMaxBranchOffset);
target_at_put(fixup_pos, trampoline_pos); target_at_put(fixup_pos, trampoline_pos, false);
fixup_pos = trampoline_pos; fixup_pos = trampoline_pos;
dist = pos - fixup_pos; dist = pos - fixup_pos;
} }
target_at_put(fixup_pos, pos); target_at_put(fixup_pos, pos, false);
} else { } else {
target_at_put(fixup_pos, pos); target_at_put(fixup_pos, pos, false);
} }
} }
L->bind_to(pos); L->bind_to(pos);
...@@ -842,9 +861,9 @@ void Assembler::bind(Label* L) { ...@@ -842,9 +861,9 @@ void Assembler::bind(Label* L) {
} }
void Assembler::next(Label* L) { void Assembler::next(Label* L, bool is_internal) {
DCHECK(L->is_linked()); DCHECK(L->is_linked());
int link = target_at(L->pos()); int link = target_at(L->pos(), is_internal);
if (link == kEndOfChain) { if (link == kEndOfChain) {
L->Unuse(); L->Unuse();
} else { } else {
...@@ -2332,51 +2351,58 @@ void Assembler::bc1t(int16_t offset, uint16_t cc) { ...@@ -2332,51 +2351,58 @@ void Assembler::bc1t(int16_t offset, uint16_t cc) {
// Debugging. // Debugging.
int Assembler::RelocateInternalReference(byte* pc, intptr_t pc_delta) { int Assembler::RelocateInternalReference(RelocInfo::Mode rmode, byte* pc,
intptr_t pc_delta) {
Instr instr = instr_at(pc); Instr instr = instr_at(pc);
if (IsLui(instr)) {
Instr instr_lui = instr_at(pc + 0 * Assembler::kInstrSize);
Instr instr_ori = instr_at(pc + 1 * Assembler::kInstrSize);
DCHECK(IsOri(instr_ori));
int32_t imm = (instr_lui & static_cast<int32_t>(kImm16Mask)) << kLuiShift;
imm |= (instr_ori & static_cast<int32_t>(kImm16Mask));
if (imm == kEndOfJumpChain) {
return 0; // Number of instructions patched.
}
imm += pc_delta;
DCHECK((imm & 3) == 0);
instr_lui &= ~kImm16Mask;
instr_ori &= ~kImm16Mask;
instr_at_put(pc + 0 * Assembler::kInstrSize,
instr_lui | ((imm >> kLuiShift) & kImm16Mask));
instr_at_put(pc + 1 * Assembler::kInstrSize,
instr_ori | (imm & kImm16Mask));
return 2; // Number of instructions patched.
} else if (IsJ(instr)) {
uint32_t imm28 = (instr & static_cast<int32_t>(kImm26Mask)) << 2;
if (static_cast<int32_t>(imm28) == kEndOfJumpChain) {
return 0; // Number of instructions patched.
}
imm28 += pc_delta;
imm28 &= kImm28Mask;
DCHECK((imm28 & 3) == 0);
instr &= ~kImm26Mask;
uint32_t imm26 = imm28 >> 2;
DCHECK(is_uint26(imm26));
instr_at_put(pc, instr | (imm26 & kImm26Mask)); if (RelocInfo::IsInternalReference(rmode)) {
return 1; // Number of instructions patched.
} else { // IsLabel(instr)
int32_t* p = reinterpret_cast<int32_t*>(pc); int32_t* p = reinterpret_cast<int32_t*>(pc);
uint32_t imm28 = (instr & static_cast<int32_t>(kImm26Mask)) << 2; if (*p == 0) {
if (static_cast<int32_t>(imm28) == kEndOfJumpChain) {
return 0; // Number of instructions patched. return 0; // Number of instructions patched.
} }
*p += pc_delta; *p += pc_delta;
return 1; // Number of instructions patched. return 1; // Number of instructions patched.
} else {
DCHECK(RelocInfo::IsInternalReferenceEncoded(rmode));
if (IsLui(instr)) {
Instr instr_lui = instr_at(pc + 0 * Assembler::kInstrSize);
Instr instr_ori = instr_at(pc + 1 * Assembler::kInstrSize);
DCHECK(IsOri(instr_ori));
int32_t imm = (instr_lui & static_cast<int32_t>(kImm16Mask)) << kLuiShift;
imm |= (instr_ori & static_cast<int32_t>(kImm16Mask));
if (imm == kEndOfJumpChain) {
return 0; // Number of instructions patched.
}
imm += pc_delta;
DCHECK((imm & 3) == 0);
instr_lui &= ~kImm16Mask;
instr_ori &= ~kImm16Mask;
instr_at_put(pc + 0 * Assembler::kInstrSize,
instr_lui | ((imm >> kLuiShift) & kImm16Mask));
instr_at_put(pc + 1 * Assembler::kInstrSize,
instr_ori | (imm & kImm16Mask));
return 2; // Number of instructions patched.
} else if (IsJ(instr)) {
uint32_t imm28 = (instr & static_cast<int32_t>(kImm26Mask)) << 2;
if (static_cast<int32_t>(imm28) == kEndOfJumpChain) {
return 0; // Number of instructions patched.
}
imm28 += pc_delta;
imm28 &= kImm28Mask;
DCHECK((imm28 & 3) == 0);
instr &= ~kImm26Mask;
uint32_t imm26 = imm28 >> 2;
DCHECK(is_uint26(imm26));
instr_at_put(pc, instr | (imm26 & kImm26Mask));
return 1; // Number of instructions patched.
} else {
UNREACHABLE();
return 0;
}
} }
} }
...@@ -2417,12 +2443,12 @@ void Assembler::GrowBuffer() { ...@@ -2417,12 +2443,12 @@ void Assembler::GrowBuffer() {
// Relocate runtime entries. // Relocate runtime entries.
for (RelocIterator it(desc); !it.done(); it.next()) { for (RelocIterator it(desc); !it.done(); it.next()) {
RelocInfo::Mode rmode = it.rinfo()->rmode(); RelocInfo::Mode rmode = it.rinfo()->rmode();
if (rmode == RelocInfo::INTERNAL_REFERENCE) { if (rmode == RelocInfo::INTERNAL_REFERENCE_ENCODED ||
rmode == RelocInfo::INTERNAL_REFERENCE) {
byte* p = reinterpret_cast<byte*>(it.rinfo()->pc()); byte* p = reinterpret_cast<byte*>(it.rinfo()->pc());
RelocateInternalReference(p, pc_delta); RelocateInternalReference(rmode, p, pc_delta);
} }
} }
DCHECK(!overflow()); DCHECK(!overflow());
} }
...@@ -2449,23 +2475,9 @@ void Assembler::dd(Label* label) { ...@@ -2449,23 +2475,9 @@ void Assembler::dd(Label* label) {
*reinterpret_cast<uint32_t*>(pc_) = data; *reinterpret_cast<uint32_t*>(pc_) = data;
pc_ += sizeof(uint32_t); pc_ += sizeof(uint32_t);
} else { } else {
int target_pos; uint32_t target_pos = jump_address(label);
if (label->is_linked()) { emit(target_pos);
// Point to previous instruction that uses the link. internal_reference_positions_.insert(label->pos());
target_pos = label->pos();
} else {
// First entry of the link chain points to itself.
target_pos = pc_offset();
}
label->link_to(pc_offset());
// Encode internal reference to unbound label. We set the least significant
// bit to distinguish unbound internal references in GrowBuffer() below.
int diff = target_pos - pc_offset();
DCHECK_EQ(0, diff & 3);
int imm26 = diff >> 2;
DCHECK(is_int26(imm26));
// Emit special LABEL instruction.
emit(LABEL | (imm26 & kImm26Mask));
} }
} }
...@@ -2550,7 +2562,7 @@ void Assembler::CheckTrampolinePool() { ...@@ -2550,7 +2562,7 @@ void Assembler::CheckTrampolinePool() {
// Buffer growth (and relocation) must be blocked for internal // Buffer growth (and relocation) must be blocked for internal
// references until associated instructions are emitted and available // references until associated instructions are emitted and available
// to be patched. // to be patched.
RecordRelocInfo(RelocInfo::INTERNAL_REFERENCE); RecordRelocInfo(RelocInfo::INTERNAL_REFERENCE_ENCODED);
lui(at, (imm32 & kHiMask) >> kLuiShift); lui(at, (imm32 & kHiMask) >> kLuiShift);
ori(at, at, (imm32 & kImm16Mask)); ori(at, at, (imm32 & kImm16Mask));
} }
......
...@@ -38,6 +38,8 @@ ...@@ -38,6 +38,8 @@
#include <stdio.h> #include <stdio.h>
#include <tr1/unordered_set>
#include "src/assembler.h" #include "src/assembler.h"
#include "src/mips/constants-mips.h" #include "src/mips/constants-mips.h"
#include "src/serialize.h" #include "src/serialize.h"
...@@ -1021,7 +1023,8 @@ class Assembler : public AssemblerBase { ...@@ -1021,7 +1023,8 @@ class Assembler : public AssemblerBase {
void RecordDeoptReason(const int reason, const int raw_position); void RecordDeoptReason(const int reason, const int raw_position);
static int RelocateInternalReference(byte* pc, intptr_t pc_delta); static int RelocateInternalReference(RelocInfo::Mode rmode, byte* pc,
intptr_t pc_delta);
// Writes a single byte or word of data in the code stream. Used for // Writes a single byte or word of data in the code stream. Used for
// inline tables, e.g., jump-tables. // inline tables, e.g., jump-tables.
...@@ -1126,10 +1129,10 @@ class Assembler : public AssemblerBase { ...@@ -1126,10 +1129,10 @@ class Assembler : public AssemblerBase {
int32_t buffer_space() const { return reloc_info_writer.pos() - pc_; } int32_t buffer_space() const { return reloc_info_writer.pos() - pc_; }
// Decode branch instruction at pos and return branch target pos. // Decode branch instruction at pos and return branch target pos.
int target_at(int32_t pos); int target_at(int32_t pos, bool is_internal);
// Patch branch instruction at pos to branch to given branch target pos. // Patch branch instruction at pos to branch to given branch target pos.
void target_at_put(int32_t pos, int32_t target_pos); void target_at_put(int32_t pos, int32_t target_pos, bool is_internal);
// Say if we need to relocate with this mode. // Say if we need to relocate with this mode.
bool MustUseReg(RelocInfo::Mode rmode); bool MustUseReg(RelocInfo::Mode rmode);
...@@ -1298,7 +1301,7 @@ class Assembler : public AssemblerBase { ...@@ -1298,7 +1301,7 @@ class Assembler : public AssemblerBase {
// Labels. // Labels.
void print(Label* L); void print(Label* L);
void bind_to(Label* L, int pos); void bind_to(Label* L, int pos);
void next(Label* L); void next(Label* L, bool is_internal);
// One trampoline consists of: // One trampoline consists of:
// - space for trampoline slots, // - space for trampoline slots,
...@@ -1363,6 +1366,10 @@ class Assembler : public AssemblerBase { ...@@ -1363,6 +1366,10 @@ class Assembler : public AssemblerBase {
static const int kMaxBranchOffset = (1 << (18 - 1)) - 1; static const int kMaxBranchOffset = (1 << (18 - 1)) - 1;
static const int kInvalidSlotPos = -1; static const int kInvalidSlotPos = -1;
// Internal reference positions, required for unbounded internal reference
// labels.
std::tr1::unordered_set<int> internal_reference_positions_;
Trampoline trampoline_; Trampoline trampoline_;
bool internal_trampoline_exception_; bool internal_trampoline_exception_;
......
...@@ -339,7 +339,6 @@ enum Opcode { ...@@ -339,7 +339,6 @@ enum Opcode {
DADDI = ((3 << 3) + 0) << kOpcodeShift, // This is also BNEC. DADDI = ((3 << 3) + 0) << kOpcodeShift, // This is also BNEC.
SPECIAL2 = ((3 << 3) + 4) << kOpcodeShift, SPECIAL2 = ((3 << 3) + 4) << kOpcodeShift,
LABEL = ((3 << 3) + 5) << kOpcodeShift,
SPECIAL3 = ((3 << 3) + 7) << kOpcodeShift, SPECIAL3 = ((3 << 3) + 7) << kOpcodeShift,
LB = ((4 << 3) + 0) << kOpcodeShift, LB = ((4 << 3) + 0) << kOpcodeShift,
......
...@@ -3087,7 +3087,7 @@ void MacroAssembler::J(Label* L, BranchDelaySlot bdslot) { ...@@ -3087,7 +3087,7 @@ void MacroAssembler::J(Label* L, BranchDelaySlot bdslot) {
{ BlockGrowBufferScope block_buf_growth(this); { BlockGrowBufferScope block_buf_growth(this);
// Buffer growth (and relocation) must be blocked for internal references // Buffer growth (and relocation) must be blocked for internal references
// until associated instructions are emitted and available to be patched. // until associated instructions are emitted and available to be patched.
RecordRelocInfo(RelocInfo::INTERNAL_REFERENCE); RecordRelocInfo(RelocInfo::INTERNAL_REFERENCE_ENCODED);
j(imm28); j(imm28);
} }
// Emit a nop in the branch delay slot if required. // Emit a nop in the branch delay slot if required.
...@@ -3104,7 +3104,7 @@ void MacroAssembler::Jr(Label* L, BranchDelaySlot bdslot) { ...@@ -3104,7 +3104,7 @@ void MacroAssembler::Jr(Label* L, BranchDelaySlot bdslot) {
{ BlockGrowBufferScope block_buf_growth(this); { BlockGrowBufferScope block_buf_growth(this);
// Buffer growth (and relocation) must be blocked for internal references // Buffer growth (and relocation) must be blocked for internal references
// until associated instructions are emitted and available to be patched. // until associated instructions are emitted and available to be patched.
RecordRelocInfo(RelocInfo::INTERNAL_REFERENCE); RecordRelocInfo(RelocInfo::INTERNAL_REFERENCE_ENCODED);
lui(at, (imm32 & kHiMask) >> kLuiShift); lui(at, (imm32 & kHiMask) >> kLuiShift);
ori(at, at, (imm32 & kImm16Mask)); ori(at, at, (imm32 & kImm16Mask));
} }
...@@ -3124,7 +3124,7 @@ void MacroAssembler::Jalr(Label* L, BranchDelaySlot bdslot) { ...@@ -3124,7 +3124,7 @@ void MacroAssembler::Jalr(Label* L, BranchDelaySlot bdslot) {
{ BlockGrowBufferScope block_buf_growth(this); { BlockGrowBufferScope block_buf_growth(this);
// Buffer growth (and relocation) must be blocked for internal references // Buffer growth (and relocation) must be blocked for internal references
// until associated instructions are emitted and available to be patched. // until associated instructions are emitted and available to be patched.
RecordRelocInfo(RelocInfo::INTERNAL_REFERENCE); RecordRelocInfo(RelocInfo::INTERNAL_REFERENCE_ENCODED);
lui(at, (imm32 & kHiMask) >> kLuiShift); lui(at, (imm32 & kHiMask) >> kLuiShift);
ori(at, at, (imm32 & kImm16Mask)); ori(at, at, (imm32 & kImm16Mask));
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment