Revert "Revert "Introduce a NearLabel in thumb2.""
This reverts commit 1f277e3cef6c33cd35e91123978491d83338d2ad.
- Fix CompareAndBranch to not use cbz/cbnz with high registers.
- Add a test for CompareAndBranch with the *inc file, as the
other assembler test infrastructure does not handle labels.
Change-Id: If552bf1112b96caa3b9bb6c73c4b40bb90a33db7
diff --git a/compiler/optimizing/code_generator_arm.cc b/compiler/optimizing/code_generator_arm.cc
index 1c76630..09ed9c7 100644
--- a/compiler/optimizing/code_generator_arm.cc
+++ b/compiler/optimizing/code_generator_arm.cc
@@ -2883,7 +2883,7 @@
Location left = locations->InAt(0);
Location right = locations->InAt(1);
- Label less, greater, done;
+ NearLabel less, greater, done;
Primitive::Type type = compare->InputAt(0)->GetType();
switch (type) {
case Primitive::kPrimLong: {
@@ -2979,7 +2979,7 @@
Register temp1,
Register temp2,
HInstruction* instruction) {
- Label fail;
+ NearLabel fail;
if (offset != 0) {
__ LoadImmediate(temp1, offset);
__ add(IP, addr, ShifterOperand(temp1));
@@ -3659,7 +3659,7 @@
Register object,
Register value,
bool can_be_null) {
- Label is_null;
+ NearLabel is_null;
if (can_be_null) {
__ CompareAndBranchIfZero(value, &is_null);
}
@@ -4081,14 +4081,13 @@
Register cls = locations->InAt(1).AsRegister<Register>();
Register out = locations->Out().AsRegister<Register>();
uint32_t class_offset = mirror::Object::ClassOffset().Int32Value();
- Label done, zero;
+ NearLabel done, zero;
SlowPathCodeARM* slow_path = nullptr;
// Return 0 if `obj` is null.
// avoid null check if we know obj is not null.
if (instruction->MustDoNullCheck()) {
- __ cmp(obj, ShifterOperand(0));
- __ b(&zero, EQ);
+ __ CompareAndBranchIfZero(obj, &zero);
}
// Compare the class of `obj` with `cls`.
__ LoadFromOffset(kLoadWord, out, obj, class_offset);
@@ -4139,16 +4138,19 @@
instruction, locations->InAt(1), locations->GetTemp(0), instruction->GetDexPc());
codegen_->AddSlowPath(slow_path);
+ NearLabel done;
// avoid null check if we know obj is not null.
if (instruction->MustDoNullCheck()) {
- __ cmp(obj, ShifterOperand(0));
- __ b(slow_path->GetExitLabel(), EQ);
+ __ CompareAndBranchIfZero(obj, &done);
}
// Compare the class of `obj` with `cls`.
__ LoadFromOffset(kLoadWord, temp, obj, class_offset);
__ cmp(temp, ShifterOperand(cls));
__ b(slow_path->GetEntryLabel(), NE);
__ Bind(slow_path->GetExitLabel());
+ if (instruction->MustDoNullCheck()) {
+ __ Bind(&done);
+ }
}
void LocationsBuilderARM::VisitMonitorOperation(HMonitorOperation* instruction) {
diff --git a/compiler/utils/arm/assembler_arm.h b/compiler/utils/arm/assembler_arm.h
index dee8287..52a69ca 100644
--- a/compiler/utils/arm/assembler_arm.h
+++ b/compiler/utils/arm/assembler_arm.h
@@ -33,6 +33,16 @@
class Arm32Assembler;
class Thumb2Assembler;
+// This class indicates that the label and its uses
+// will fall into a range that is encodable in 16bits on thumb2.
+class NearLabel : public Label {
+ public:
+ NearLabel() {}
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(NearLabel);
+};
+
class ShifterOperand {
public:
ShifterOperand() : type_(kUnknown), rm_(kNoRegister), rs_(kNoRegister),
@@ -519,6 +529,9 @@
// Branch instructions.
virtual void b(Label* label, Condition cond = AL) = 0;
+ virtual void b(NearLabel* label, Condition cond = AL) {
+ b(static_cast<Label*>(label), cond);
+ }
virtual void bl(Label* label, Condition cond = AL) = 0;
virtual void blx(Register rm, Condition cond = AL) = 0;
virtual void bx(Register rm, Condition cond = AL) = 0;
@@ -654,6 +667,9 @@
virtual void Bind(Label* label) = 0;
virtual void CompareAndBranchIfZero(Register r, Label* label) = 0;
+ virtual void CompareAndBranchIfZero(Register r, NearLabel* label) {
+ CompareAndBranchIfZero(r, static_cast<Label*>(label));
+ }
virtual void CompareAndBranchIfNonZero(Register r, Label* label) = 0;
//
diff --git a/compiler/utils/arm/assembler_arm32.h b/compiler/utils/arm/assembler_arm32.h
index 55ec7b4..4564767 100644
--- a/compiler/utils/arm/assembler_arm32.h
+++ b/compiler/utils/arm/assembler_arm32.h
@@ -201,8 +201,8 @@
void vpopd(DRegister reg, int nregs, Condition cond = AL) OVERRIDE;
// Branch instructions.
- void b(Label* label, Condition cond = AL);
- void bl(Label* label, Condition cond = AL);
+ void b(Label* label, Condition cond = AL) OVERRIDE;
+ void bl(Label* label, Condition cond = AL) OVERRIDE;
void blx(Register rm, Condition cond = AL) OVERRIDE;
void bx(Register rm, Condition cond = AL) OVERRIDE;
void Lsl(Register rd, Register rm, uint32_t shift_imm, bool setcc = false,
diff --git a/compiler/utils/arm/assembler_thumb2.cc b/compiler/utils/arm/assembler_thumb2.cc
index 75f2b77..4ff3aeb 100644
--- a/compiler/utils/arm/assembler_thumb2.cc
+++ b/compiler/utils/arm/assembler_thumb2.cc
@@ -671,11 +671,17 @@
EmitVFPddd(cond, B23 | B21 | B20 | B18 | B16 | B6, dd, D0, D0);
}
+
void Thumb2Assembler::b(Label* label, Condition cond) {
EmitBranch(cond, label, false, false);
}
+void Thumb2Assembler::b(NearLabel* label, Condition cond) {
+ EmitBranch(cond, label, false, false, /* is_near */ true);
+}
+
+
void Thumb2Assembler::bl(Label* label, Condition cond) {
CheckCondition(cond);
EmitBranch(cond, label, true, false);
@@ -1369,6 +1375,7 @@
uint16_t Thumb2Assembler::EmitCompareAndBranch(Register rn, uint16_t prev, bool n) {
+ CHECK(IsLowRegister(rn));
uint32_t location = buffer_.Size();
// This is always unresolved as it must be a forward branch.
@@ -1613,7 +1620,7 @@
}
-void Thumb2Assembler::EmitBranch(Condition cond, Label* label, bool link, bool x) {
+void Thumb2Assembler::EmitBranch(Condition cond, Label* label, bool link, bool x, bool is_near) {
uint32_t pc = buffer_.Size();
Branch::Type branch_type;
if (cond == AL) {
@@ -1644,8 +1651,8 @@
}
} else {
// Branch is to an unbound label. Emit space for it.
- uint16_t branch_id = AddBranch(branch_type, pc, cond); // Unresolved branch.
- if (!CanRelocateBranches() || force_32bit_) {
+ uint16_t branch_id = AddBranch(branch_type, pc, cond, is_near); // Unresolved branch.
+ if (force_32bit_ || (!CanRelocateBranches() && !is_near)) {
Emit16(static_cast<uint16_t>(label->position_)); // Emit current label link.
Emit16(0); // another 16 bits.
} else {
@@ -2199,6 +2206,9 @@
if (label->IsBound()) {
LOG(FATAL) << "cbz can only be used to branch forwards";
UNREACHABLE();
+ } else if (IsHighRegister(rn)) {
+ LOG(FATAL) << "cbz can only be used with low registers";
+ UNREACHABLE();
} else {
uint16_t branchid = EmitCompareAndBranch(rn, static_cast<uint16_t>(label->position_), false);
label->LinkTo(branchid);
@@ -2211,6 +2221,9 @@
if (label->IsBound()) {
LOG(FATAL) << "cbnz can only be used to branch forwards";
UNREACHABLE();
+ } else if (IsHighRegister(rn)) {
+ LOG(FATAL) << "cbnz can only be used with low registers";
+ UNREACHABLE();
} else {
uint16_t branchid = EmitCompareAndBranch(rn, static_cast<uint16_t>(label->position_), true);
label->LinkTo(branchid);
@@ -2741,7 +2754,17 @@
void Thumb2Assembler::CompareAndBranchIfZero(Register r, Label* label) {
- if (CanRelocateBranches()) {
+ if (CanRelocateBranches() && IsLowRegister(r)) {
+ cbz(r, label);
+ } else {
+ cmp(r, ShifterOperand(0));
+ b(label, EQ);
+ }
+}
+
+
+void Thumb2Assembler::CompareAndBranchIfZero(Register r, NearLabel* label) {
+ if (IsLowRegister(r)) {
cbz(r, label);
} else {
cmp(r, ShifterOperand(0));
@@ -2751,7 +2774,7 @@
void Thumb2Assembler::CompareAndBranchIfNonZero(Register r, Label* label) {
- if (CanRelocateBranches()) {
+ if (CanRelocateBranches() && IsLowRegister(r)) {
cbnz(r, label);
} else {
cmp(r, ShifterOperand(0));
diff --git a/compiler/utils/arm/assembler_thumb2.h b/compiler/utils/arm/assembler_thumb2.h
index 90d489f..9f02e56 100644
--- a/compiler/utils/arm/assembler_thumb2.h
+++ b/compiler/utils/arm/assembler_thumb2.h
@@ -239,6 +239,7 @@
// Branch instructions.
void b(Label* label, Condition cond = AL);
+ void b(NearLabel* label, Condition cond = AL);
void bl(Label* label, Condition cond = AL);
void blx(Label* label);
void blx(Register rm, Condition cond = AL) OVERRIDE;
@@ -273,6 +274,7 @@
void Mov(Register rd, Register rm, Condition cond = AL) OVERRIDE;
void CompareAndBranchIfZero(Register r, Label* label) OVERRIDE;
+ void CompareAndBranchIfZero(Register r, NearLabel* label) OVERRIDE;
void CompareAndBranchIfNonZero(Register r, Label* label) OVERRIDE;
// Memory barriers.
@@ -431,7 +433,7 @@
void EmitVPushPop(uint32_t reg, int nregs, bool push, bool dbl, Condition cond);
- void EmitBranch(Condition cond, Label* label, bool link, bool x);
+ void EmitBranch(Condition cond, Label* label, bool link, bool x, bool is_near = false);
static int32_t EncodeBranchOffset(int32_t offset, int32_t inst);
static int DecodeBranchOffset(int32_t inst);
int32_t EncodeTstOffset(int offset, int32_t inst);
@@ -559,6 +561,7 @@
// Resolve a branch when the target is known. If this causes the
// size of the branch to change return true. Otherwise return false.
bool Resolve(uint32_t target) {
+ uint32_t old_target = target_;
target_ = target;
if (assembler_->CanRelocateBranches()) {
Size new_size = CalculateSize();
@@ -569,9 +572,12 @@
return false;
} else {
if (kIsDebugBuild) {
- Size new_size = CalculateSize();
- // Check that the size has not increased.
- DCHECK(!(new_size == k32Bit && size_ == k16Bit));
+ if (old_target == kUnresolved) {
+ // Check that the size has not increased.
+ DCHECK(!(CalculateSize() == k32Bit && size_ == k16Bit));
+ } else {
+ DCHECK(CalculateSize() == size_);
+ }
}
return false;
}
@@ -651,6 +657,10 @@
if (assembler_->IsForced32Bit() && (type_ == kUnconditional || type_ == kConditional)) {
return k32Bit;
}
+ if (IsCompareAndBranch()) {
+ // Compare and branch instructions can only be encoded on 16 bits.
+ return k16Bit;
+ }
return assembler_->CanRelocateBranches() ? k16Bit : k32Bit;
}
// When the target is resolved, we know the best encoding for it.
@@ -714,8 +724,15 @@
}
// Add an unresolved branch and return its id.
- uint16_t AddBranch(Branch::Type type, uint32_t location, Condition cond = AL) {
- branches_.push_back(new Branch(this, type, location, cond));
+ uint16_t AddBranch(Branch::Type type,
+ uint32_t location,
+ Condition cond = AL,
+ bool is_near = false) {
+ Branch* branch = new Branch(this, type, location, cond);
+ if (is_near) {
+ branch->ResetSize(Branch::k16Bit);
+ }
+ branches_.push_back(branch);
return branches_.size() - 1;
}
diff --git a/compiler/utils/assembler_test_base.h b/compiler/utils/assembler_test_base.h
index 3341151..40eb15b 100644
--- a/compiler/utils/assembler_test_base.h
+++ b/compiler/utils/assembler_test_base.h
@@ -215,9 +215,9 @@
bool success = Exec(args, error_msg);
if (!success) {
- LOG(INFO) << "Assembler command line:";
+ LOG(ERROR) << "Assembler command line:";
for (std::string arg : args) {
- LOG(INFO) << arg;
+ LOG(ERROR) << arg;
}
}
return success;
diff --git a/compiler/utils/assembler_thumb_test.cc b/compiler/utils/assembler_thumb_test.cc
index 7738627..1a2c9a9 100644
--- a/compiler/utils/assembler_thumb_test.cc
+++ b/compiler/utils/assembler_thumb_test.cc
@@ -1338,6 +1338,24 @@
delete assembler;
}
+TEST(Thumb2AssemblerTest, CompareAndBranch) {
+ arm::Thumb2Assembler* assembler = static_cast<arm::Thumb2Assembler*>(Assembler::Create(kThumb2));
+
+ arm::NearLabel label;
+ __ CompareAndBranchIfZero(arm::R0, &label);
+ __ CompareAndBranchIfZero(arm::R11, &label);
+ __ CompareAndBranchIfNonZero(arm::R0, &label);
+ __ CompareAndBranchIfNonZero(arm::R11, &label);
+ __ Bind(&label);
+
+ size_t cs = __ CodeSize();
+ std::vector<uint8_t> managed_code(cs);
+ MemoryRegion code(&managed_code[0], managed_code.size());
+ __ FinalizeInstructions(code);
+ dump(managed_code, "CompareAndBranch");
+ delete assembler;
+}
+
#undef __
} // namespace arm
} // namespace art
diff --git a/compiler/utils/assembler_thumb_test_expected.cc.inc b/compiler/utils/assembler_thumb_test_expected.cc.inc
index 3d03234..841d6a0 100644
--- a/compiler/utils/assembler_thumb_test_expected.cc.inc
+++ b/compiler/utils/assembler_thumb_test_expected.cc.inc
@@ -4822,6 +4822,16 @@
" 30: f8a4 0040 strh.w r0, [r4, #64] ; 0x40\n",
nullptr
};
+const char* CompareAndBranchResults[] = {
+ " 0: b130 cbz r0, 10 <CompareAndBranch+0x10>\n",
+ " 2: f1bb 0f00 cmp.w fp, #0\n",
+ " 6: d003 beq.n 10 <CompareAndBranch+0x10>\n",
+ " 8: b910 cbnz r0, 10 <CompareAndBranch+0x10>\n",
+ " a: f1bb 0f00 cmp.w fp, #0\n",
+ " e: d1ff bne.n 10 <CompareAndBranch+0x10>\n",
+ nullptr
+};
+
std::map<std::string, const char**> test_results;
void setup_results() {
test_results["SimpleMov"] = SimpleMovResults;
@@ -4869,4 +4879,5 @@
test_results["LoadStoreRegOffset"] = LoadStoreRegOffsetResults;
test_results["LoadStoreLiteral"] = LoadStoreLiteralResults;
test_results["LoadStoreLimits"] = LoadStoreLimitsResults;
+ test_results["CompareAndBranch"] = CompareAndBranchResults;
}
