Quick assembler fix
This CL re-instates the select pattern optimization disabled by
CL 374310, and fixes the underlying problem: improper handling of
the kPseudoBarrier LIR opcode. The bug was introduced in the
recent assembler restructuring. In short, LIR pseudo opcodes (which
have values < 0), should always have size 0 - and thus cause no
bits to be emitted during assembly. In this case, bad logic caused
us to set the size of a kPseudoBarrier opcode via lookup through the
EncodingMap.
Because all pseudo ops are < 0, this meant we did an array underflow
load, picking up whatever garbage was located before the EncodingMap.
This explains why this error showed up recently - we'd previuosly just
gotten a lucky layout.
This CL corrects the faulty logic, and adds DCHECKs to uses of
the EncodingMap to ensure that we don't try to access w/ a
pseudo op. Additionally, the existing is_pseudo_op() macro is
replaced with IsPseudoLirOp(), named similar to the existing
IsPseudoMirOp().
Change-Id: I46761a0275a923d85b545664cadf052e1ab120dc
diff --git a/compiler/dex/quick/arm/assemble_arm.cc b/compiler/dex/quick/arm/assemble_arm.cc
index dac3a21..3c646c4 100644
--- a/compiler/dex/quick/arm/assemble_arm.cc
+++ b/compiler/dex/quick/arm/assemble_arm.cc
@@ -1021,7 +1021,7 @@
void ArmMir2Lir::EncodeLIR(LIR* lir) {
int opcode = lir->opcode;
- if (opcode < 0) {
+ if (IsPseudoLirOp(opcode)) {
if (UNLIKELY(opcode == kPseudoPseudoAlign4)) {
// Note: size for this opcode will be either 0 or 2 depending on final alignment.
lir->u.a.bytes[0] = (PADDING_MOV_R5_R5 & 0xff);
@@ -1594,6 +1594,7 @@
}
int ArmMir2Lir::GetInsnSize(LIR* lir) {
+ DCHECK(!IsPseudoLirOp(lir->opcode));
return EncodingMap[lir->opcode].size;
}
@@ -1613,7 +1614,7 @@
lir->offset = offset;
if (!lir->flags.is_nop) {
if (lir->flags.fixup != kFixupNone) {
- if (lir->opcode >= 0) {
+ if (!IsPseudoLirOp(lir->opcode)) {
lir->flags.size = EncodingMap[lir->opcode].size;
lir->flags.fixup = EncodingMap[lir->opcode].fixup;
} else if (UNLIKELY(lir->opcode == kPseudoPseudoAlign4)) {
diff --git a/compiler/dex/quick/arm/target_arm.cc b/compiler/dex/quick/arm/target_arm.cc
index a4ea10b..933c1a3 100644
--- a/compiler/dex/quick/arm/target_arm.cc
+++ b/compiler/dex/quick/arm/target_arm.cc
@@ -718,14 +718,17 @@
}
uint64_t ArmMir2Lir::GetTargetInstFlags(int opcode) {
+ DCHECK(!IsPseudoLirOp(opcode));
return ArmMir2Lir::EncodingMap[opcode].flags;
}
const char* ArmMir2Lir::GetTargetInstName(int opcode) {
+ DCHECK(!IsPseudoLirOp(opcode));
return ArmMir2Lir::EncodingMap[opcode].name;
}
const char* ArmMir2Lir::GetTargetInstFmt(int opcode) {
+ DCHECK(!IsPseudoLirOp(opcode));
return ArmMir2Lir::EncodingMap[opcode].fmt;
}
diff --git a/compiler/dex/quick/arm/utility_arm.cc b/compiler/dex/quick/arm/utility_arm.cc
index a7b8dfe..00de8de 100644
--- a/compiler/dex/quick/arm/utility_arm.cc
+++ b/compiler/dex/quick/arm/utility_arm.cc
@@ -327,7 +327,7 @@
LOG(FATAL) << "Bad opcode: " << op;
break;
}
- DCHECK_GE(static_cast<int>(opcode), 0);
+ DCHECK(!IsPseudoLirOp(opcode));
if (EncodingMap[opcode].flags & IS_BINARY_OP) {
return NewLIR2(opcode, r_dest_src1, r_src2);
} else if (EncodingMap[opcode].flags & IS_TERTIARY_OP) {
@@ -405,7 +405,7 @@
LOG(FATAL) << "Bad opcode: " << op;
break;
}
- DCHECK_GE(static_cast<int>(opcode), 0);
+ DCHECK(!IsPseudoLirOp(opcode));
if (EncodingMap[opcode].flags & IS_QUAD_OP) {
return NewLIR4(opcode, r_dest, r_src1, r_src2, shift);
} else {