Quick: Create GC map based on compiler data.
The Quick compiler and verifier sometimes disagree on dalvik
register types (fp/core/ref) for 0/null constants and merged
registers involving 0/null constants. Since the verifier is
more lenient it can mark a register as a reference for GC
where Quick considers it a floating point register or a dead
register (which would have a ref/fp conflict if not dead).
If the compiler used an fp register to hold the zero value,
the core register or stack location used by GC based on the
verifier data can hold an invalid value.
Previously, as a workaround we stored the fp zero value also
in the stack location or core register where GC would look
for it. This wasn't precise and may have missed some cases.
To fix this properly, we now generate GC maps based on the
compiler's notion of references if register promotion is
enabled.
Bug: https://code.google.com/p/android/issues/detail?id=147187
Change-Id: Id3a2f863b16bdb8969df7004c868773084aec421
diff --git a/compiler/dex/quick/codegen_util.cc b/compiler/dex/quick/codegen_util.cc
index 029c0ca..3285195 100644
--- a/compiler/dex/quick/codegen_util.cc
+++ b/compiler/dex/quick/codegen_util.cc
@@ -16,6 +16,7 @@
#include "mir_to_lir-inl.h"
+#include "base/bit_vector-inl.h"
#include "dex/mir_graph.h"
#include "driver/compiler_driver.h"
#include "driver/compiler_options.h"
@@ -88,6 +89,8 @@
inst->u.m.def_mask = &kEncodeAll;
LIR* safepoint_pc = NewLIR0(kPseudoSafepointPC);
DCHECK(safepoint_pc->u.m.def_mask->Equals(kEncodeAll));
+ DCHECK(current_mir_ != nullptr || (current_dalvik_offset_ == 0 && safepoints_.empty()));
+ safepoints_.emplace_back(safepoint_pc, current_mir_);
}
void Mir2Lir::MarkSafepointPCAfter(LIR* after) {
@@ -102,6 +105,8 @@
InsertLIRAfter(after, safepoint_pc);
}
DCHECK(safepoint_pc->u.m.def_mask->Equals(kEncodeAll));
+ DCHECK(current_mir_ != nullptr || (current_dalvik_offset_ == 0 && safepoints_.empty()));
+ safepoints_.emplace_back(safepoint_pc, current_mir_);
}
/* Remove a LIR from the list. */
@@ -767,6 +772,61 @@
}
void Mir2Lir::CreateNativeGcMap() {
+ if (UNLIKELY((cu_->disable_opt & (1u << kPromoteRegs)) != 0u)) {
+ // If we're not promoting to physical registers, it's safe to use the verifier's notion of
+ // references. (We disable register promotion when type inference finds a type conflict and
+ // in that the case we defer to the verifier to avoid using the compiler's conflicting info.)
+ CreateNativeGcMapWithoutRegisterPromotion();
+ return;
+ }
+
+ ArenaBitVector* references = new (arena_) ArenaBitVector(arena_, mir_graph_->GetNumSSARegs(),
+ false);
+
+ // Calculate max native offset and max reference vreg.
+ MIR* prev_mir = nullptr;
+ int max_ref_vreg = -1;
+ CodeOffset max_native_offset = 0u;
+ for (const auto& entry : safepoints_) {
+ uint32_t native_offset = entry.first->offset;
+ max_native_offset = std::max(max_native_offset, native_offset);
+ MIR* mir = entry.second;
+ UpdateReferenceVRegs(mir, prev_mir, references);
+ max_ref_vreg = std::max(max_ref_vreg, references->GetHighestBitSet());
+ prev_mir = mir;
+ }
+
+ // Build the GC map.
+ uint32_t reg_width = static_cast<uint32_t>((max_ref_vreg + 8) / 8);
+ GcMapBuilder native_gc_map_builder(&native_gc_map_,
+ safepoints_.size(),
+ max_native_offset, reg_width);
+#if !defined(BYTE_ORDER) || (BYTE_ORDER != LITTLE_ENDIAN)
+ ArenaVector<uint8_t> references_buffer(arena_->Adapter());
+ references_buffer.resize(reg_width);
+#endif
+ for (const auto& entry : safepoints_) {
+ uint32_t native_offset = entry.first->offset;
+ MIR* mir = entry.second;
+ UpdateReferenceVRegs(mir, prev_mir, references);
+#if !defined(BYTE_ORDER) || (BYTE_ORDER != LITTLE_ENDIAN)
+ // Big-endian or unknown endianness, manually translate the bit vector data.
+ const auto* raw_storage = references->GetRawStorage();
+ for (size_t i = 0; i != reg_width; ++i) {
+ references_buffer[i] = static_cast<uint8_t>(
+ raw_storage[i / sizeof(raw_storage[0])] >> (8u * (i % sizeof(raw_storage[0]))));
+ }
+ native_gc_map_builder.AddEntry(native_offset, &references_buffer[0]);
+#else
+ // For little-endian, the bytes comprising the bit vector's raw storage are what we need.
+ native_gc_map_builder.AddEntry(native_offset,
+ reinterpret_cast<const uint8_t*>(references->GetRawStorage()));
+#endif
+ prev_mir = mir;
+ }
+}
+
+void Mir2Lir::CreateNativeGcMapWithoutRegisterPromotion() {
DCHECK(!encoded_mapping_table_.empty());
MappingTable mapping_table(&encoded_mapping_table_[0]);
uint32_t max_native_offset = 0;
@@ -965,6 +1025,7 @@
block_label_list_(nullptr),
promotion_map_(nullptr),
current_dalvik_offset_(0),
+ current_mir_(nullptr),
estimated_native_code_size_(0),
reg_pool_(nullptr),
live_sreg_(0),
@@ -984,6 +1045,7 @@
slow_paths_(arena->Adapter(kArenaAllocSlowPaths)),
mem_ref_type_(ResourceMask::kHeapRef),
mask_cache_(arena),
+ safepoints_(arena->Adapter()),
in_to_reg_storage_mapping_(arena) {
switch_tables_.reserve(4);
fill_array_data_.reserve(4);
@@ -1274,4 +1336,97 @@
UNREACHABLE();
}
+void Mir2Lir::InitReferenceVRegs(BasicBlock* bb, BitVector* references) {
+ // Mark the references coming from the first predecessor.
+ DCHECK(bb != nullptr);
+ DCHECK(bb->block_type == kEntryBlock || !bb->predecessors.empty());
+ BasicBlock* first_bb =
+ (bb->block_type == kEntryBlock) ? bb : mir_graph_->GetBasicBlock(bb->predecessors[0]);
+ DCHECK(first_bb != nullptr);
+ DCHECK(first_bb->data_flow_info != nullptr);
+ DCHECK(first_bb->data_flow_info->vreg_to_ssa_map_exit != nullptr);
+ const int32_t* first_vreg_to_ssa_map = first_bb->data_flow_info->vreg_to_ssa_map_exit;
+ references->ClearAllBits();
+ for (uint32_t vreg = 0, num_vregs = mir_graph_->GetNumOfCodeVRs(); vreg != num_vregs; ++vreg) {
+ int32_t sreg = first_vreg_to_ssa_map[vreg];
+ if (sreg != INVALID_SREG && mir_graph_->reg_location_[sreg].ref &&
+ !mir_graph_->IsConstantNullRef(mir_graph_->reg_location_[sreg])) {
+ references->SetBit(vreg);
+ }
+ }
+ // Unmark the references that are merging with a different value.
+ for (size_t i = 1u, num_pred = bb->predecessors.size(); i < num_pred; ++i) {
+ BasicBlock* pred_bb = mir_graph_->GetBasicBlock(bb->predecessors[i]);
+ DCHECK(pred_bb != nullptr);
+ DCHECK(pred_bb->data_flow_info != nullptr);
+ DCHECK(pred_bb->data_flow_info->vreg_to_ssa_map_exit != nullptr);
+ const int32_t* pred_vreg_to_ssa_map = pred_bb->data_flow_info->vreg_to_ssa_map_exit;
+ for (uint32_t vreg : references->Indexes()) {
+ if (first_vreg_to_ssa_map[vreg] != pred_vreg_to_ssa_map[vreg]) {
+ // NOTE: The BitVectorSet::IndexIterator will not check the pointed-to bit again,
+ // so clearing the bit has no effect on the iterator.
+ references->ClearBit(vreg);
+ }
+ }
+ }
+ if (bb->block_type != kEntryBlock && bb->first_mir_insn != nullptr &&
+ static_cast<int>(bb->first_mir_insn->dalvikInsn.opcode) == kMirOpCheckPart2) {
+ // In Mir2Lir::MethodBlockCodeGen() we have artificially moved the throwing
+ // instruction to the previous block. However, the MIRGraph data used above
+ // doesn't reflect that, so we still need to process that MIR insn here.
+ DCHECK_EQ(bb->predecessors.size(), 1u);
+ BasicBlock* pred_bb = mir_graph_->GetBasicBlock(bb->predecessors[0]);
+ DCHECK(pred_bb != nullptr);
+ DCHECK(pred_bb->last_mir_insn != nullptr);
+ UpdateReferenceVRegsLocal(nullptr, pred_bb->last_mir_insn, references);
+ }
+}
+
+bool Mir2Lir::UpdateReferenceVRegsLocal(MIR* mir, MIR* prev_mir, BitVector* references) {
+ DCHECK(mir == nullptr || mir->bb == prev_mir->bb);
+ DCHECK(prev_mir != nullptr);
+ while (prev_mir != nullptr) {
+ if (prev_mir == mir) {
+ return true;
+ }
+ const size_t num_defs = prev_mir->ssa_rep->num_defs;
+ const int32_t* defs = prev_mir->ssa_rep->defs;
+ if (num_defs == 1u && mir_graph_->reg_location_[defs[0]].ref &&
+ !mir_graph_->IsConstantNullRef(mir_graph_->reg_location_[defs[0]])) {
+ references->SetBit(mir_graph_->SRegToVReg(defs[0]));
+ } else {
+ for (size_t i = 0u; i != num_defs; ++i) {
+ references->ClearBit(mir_graph_->SRegToVReg(defs[i]));
+ }
+ }
+ prev_mir = prev_mir->next;
+ }
+ return false;
+}
+
+void Mir2Lir::UpdateReferenceVRegs(MIR* mir, MIR* prev_mir, BitVector* references) {
+ if (mir == nullptr) {
+ // Safepoint in entry sequence.
+ InitReferenceVRegs(mir_graph_->GetEntryBlock(), references);
+ return;
+ }
+ if (IsInstructionReturn(mir->dalvikInsn.opcode) ||
+ mir->dalvikInsn.opcode == Instruction::RETURN_VOID_NO_BARRIER) {
+ references->ClearAllBits();
+ if (mir->dalvikInsn.opcode == Instruction::RETURN_OBJECT) {
+ references->SetBit(mir_graph_->SRegToVReg(mir->ssa_rep->uses[0]));
+ }
+ return;
+ }
+ if (prev_mir != nullptr && mir->bb == prev_mir->bb &&
+ UpdateReferenceVRegsLocal(mir, prev_mir, references)) {
+ return;
+ }
+ BasicBlock* bb = mir_graph_->GetBasicBlock(mir->bb);
+ DCHECK(bb != nullptr);
+ InitReferenceVRegs(bb, references);
+ bool success = UpdateReferenceVRegsLocal(mir, bb->first_mir_insn, references);
+ DCHECK(success) << "MIR @0x" << std::hex << mir->offset << " not in BB#" << std::dec << mir->bb;
+}
+
} // namespace art
diff --git a/compiler/dex/quick/gen_common.cc b/compiler/dex/quick/gen_common.cc
index d613cca..b80fd74 100644
--- a/compiler/dex/quick/gen_common.cc
+++ b/compiler/dex/quick/gen_common.cc
@@ -2151,9 +2151,6 @@
RegLocation rl_result = EvalLoc(rl_dest, kAnyReg, true);
LoadConstantNoClobber(rl_result.reg, value);
StoreValue(rl_dest, rl_result);
- if (value == 0) {
- Workaround7250540(rl_dest, rl_result.reg);
- }
}
void Mir2Lir::GenConversionCall(QuickEntrypointEnum trampoline, RegLocation rl_dest,
diff --git a/compiler/dex/quick/gen_loadstore.cc b/compiler/dex/quick/gen_loadstore.cc
index db844bc..b71691f 100644
--- a/compiler/dex/quick/gen_loadstore.cc
+++ b/compiler/dex/quick/gen_loadstore.cc
@@ -37,48 +37,6 @@
}
/*
- * Temporary workaround for Issue 7250540. If we're loading a constant zero into a
- * promoted floating point register, also copy a zero into the int/ref identity of
- * that sreg.
- */
-void Mir2Lir::Workaround7250540(RegLocation rl_dest, RegStorage zero_reg) {
- if (rl_dest.fp) {
- int pmap_index = SRegToPMap(rl_dest.s_reg_low);
- const bool is_fp_promoted = promotion_map_[pmap_index].fp_location == kLocPhysReg;
- const bool is_core_promoted = promotion_map_[pmap_index].core_location == kLocPhysReg;
- if (is_fp_promoted || is_core_promoted) {
- // Now, determine if this vreg is ever used as a reference. If not, we're done.
- bool used_as_reference = false;
- int base_vreg = mir_graph_->SRegToVReg(rl_dest.s_reg_low);
- for (int i = 0; !used_as_reference && (i < mir_graph_->GetNumSSARegs()); i++) {
- if (mir_graph_->SRegToVReg(mir_graph_->reg_location_[i].s_reg_low) == base_vreg) {
- used_as_reference |= mir_graph_->reg_location_[i].ref;
- }
- }
- if (!used_as_reference) {
- return;
- }
- RegStorage temp_reg = zero_reg;
- if (!temp_reg.Valid()) {
- temp_reg = AllocTemp();
- LoadConstant(temp_reg, 0);
- }
- if (is_core_promoted) {
- // Promoted - just copy in a zero
- OpRegCopy(RegStorage::Solo32(promotion_map_[pmap_index].core_reg), temp_reg);
- } else {
- // Lives in the frame, need to store.
- ScopedMemRefType mem_ref_type(this, ResourceMask::kDalvikReg);
- StoreBaseDisp(TargetPtrReg(kSp), SRegOffset(rl_dest.s_reg_low), temp_reg, k32, kNotVolatile);
- }
- if (!zero_reg.Valid()) {
- FreeTemp(temp_reg);
- }
- }
- }
-}
-
-/*
* Load a Dalvik register into a physical register. Take care when
* using this routine, as it doesn't perform any bookkeeping regarding
* register liveness. That is the responsibility of the caller.
diff --git a/compiler/dex/quick/mir_to_lir.cc b/compiler/dex/quick/mir_to_lir.cc
index afacee0..0b480a0 100644
--- a/compiler/dex/quick/mir_to_lir.cc
+++ b/compiler/dex/quick/mir_to_lir.cc
@@ -406,6 +406,7 @@
bool Mir2Lir::GenSpecialCase(BasicBlock* bb, MIR* mir, const InlineMethod& special) {
DCHECK(special.flags & kInlineSpecial);
current_dalvik_offset_ = mir->offset;
+ DCHECK(current_mir_ == nullptr); // Safepoints attributed to prologue.
MIR* return_mir = nullptr;
bool successful = false;
EnsureInitializedArgMappingToPhysicalReg();
@@ -587,9 +588,6 @@
case Instruction::MOVE_FROM16:
case Instruction::MOVE_OBJECT_FROM16:
StoreValue(rl_dest, rl_src[0]);
- if (rl_src[0].is_const && (mir_graph_->ConstantValue(rl_src[0]) == 0)) {
- Workaround7250540(rl_dest, RegStorage::InvalidReg());
- }
break;
case Instruction::MOVE_WIDE:
@@ -1276,6 +1274,7 @@
}
current_dalvik_offset_ = mir->offset;
+ current_mir_ = mir;
int opcode = mir->dalvikInsn.opcode;
GenPrintLabel(mir);
@@ -1376,6 +1375,7 @@
LIR* Mir2Lir::LIRSlowPath::GenerateTargetLabel(int opcode) {
m2l_->SetCurrentDexPc(current_dex_pc_);
+ m2l_->current_mir_ = current_mir_;
LIR* target = m2l_->NewLIR0(opcode);
fromfast_->target = target;
return target;
diff --git a/compiler/dex/quick/mir_to_lir.h b/compiler/dex/quick/mir_to_lir.h
index 70785dc..6ee26cc 100644
--- a/compiler/dex/quick/mir_to_lir.h
+++ b/compiler/dex/quick/mir_to_lir.h
@@ -131,6 +131,7 @@
#define MAX_ASSEMBLER_RETRIES 50
class BasicBlock;
+class BitVector;
struct CallInfo;
struct CompilationUnit;
struct InlineMethod;
@@ -491,7 +492,8 @@
class LIRSlowPath : public ArenaObject<kArenaAllocSlowPaths> {
public:
LIRSlowPath(Mir2Lir* m2l, LIR* fromfast, LIR* cont = nullptr)
- : m2l_(m2l), cu_(m2l->cu_), current_dex_pc_(m2l->current_dalvik_offset_),
+ : m2l_(m2l), cu_(m2l->cu_),
+ current_dex_pc_(m2l->current_dalvik_offset_), current_mir_(m2l->current_mir_),
fromfast_(fromfast), cont_(cont) {
}
virtual ~LIRSlowPath() {}
@@ -511,6 +513,7 @@
Mir2Lir* const m2l_;
CompilationUnit* const cu_;
const DexOffset current_dex_pc_;
+ MIR* current_mir_;
LIR* const fromfast_;
LIR* const cont_;
};
@@ -670,6 +673,7 @@
bool VerifyCatchEntries();
void CreateMappingTables();
void CreateNativeGcMap();
+ void CreateNativeGcMapWithoutRegisterPromotion();
int AssignLiteralOffset(CodeOffset offset);
int AssignSwitchTablesOffset(CodeOffset offset);
int AssignFillArrayDataOffset(CodeOffset offset);
@@ -1729,6 +1733,16 @@
// See CheckRegLocationImpl.
void CheckRegLocation(RegLocation rl) const;
+ // Find the references at the beginning of a basic block (for generating GC maps).
+ void InitReferenceVRegs(BasicBlock* bb, BitVector* references);
+
+ // Update references from prev_mir to mir in the same BB. If mir is null or before
+ // prev_mir, report failure (return false) and update references to the end of the BB.
+ bool UpdateReferenceVRegsLocal(MIR* mir, MIR* prev_mir, BitVector* references);
+
+ // Update references from prev_mir to mir.
+ void UpdateReferenceVRegs(MIR* mir, MIR* prev_mir, BitVector* references);
+
public:
// TODO: add accessors for these.
LIR* literal_list_; // Constants.
@@ -1746,7 +1760,6 @@
ArenaVector<RegisterInfo*> tempreg_info_;
ArenaVector<RegisterInfo*> reginfo_map_;
ArenaVector<void*> pointer_storage_;
- CodeOffset current_code_offset_; // Working byte offset of machine instructons.
CodeOffset data_offset_; // starting offset of literal pool.
size_t total_size_; // header + code size.
LIR* block_label_list_;
@@ -1761,6 +1774,7 @@
* The low-level LIR creation utilites will pull it from here. Rework this.
*/
DexOffset current_dalvik_offset_;
+ MIR* current_mir_;
size_t estimated_native_code_size_; // Just an estimate; used to reserve code_buffer_ size.
std::unique_ptr<RegisterPool> reg_pool_;
/*
@@ -1799,6 +1813,9 @@
// to deduplicate the masks.
ResourceMaskCache mask_cache_;
+ // Record the MIR that generated a given safepoint (nullptr for prologue safepoints).
+ ArenaVector<std::pair<LIR*, MIR*>> safepoints_;
+
protected:
// ABI support
class ShortyArg {
diff --git a/test/004-ReferenceMap/stack_walk_refmap_jni.cc b/test/004-ReferenceMap/stack_walk_refmap_jni.cc
index 40be56c..76ef4a9 100644
--- a/test/004-ReferenceMap/stack_walk_refmap_jni.cc
+++ b/test/004-ReferenceMap/stack_walk_refmap_jni.cc
@@ -57,14 +57,15 @@
// We eliminate the non-live registers at a return, so only v3 is live.
// Note that it is OK for a compiler to not have a dex map at this dex PC because
// a return is not necessarily a safepoint.
- CHECK_REGS_CONTAIN_REFS(0x13U, false); // v3: y
- CHECK_REGS_CONTAIN_REFS(0x18U, true, 8, 2, 1, 0); // v8: this, v2: y, v1: x, v0: ex
- CHECK_REGS_CONTAIN_REFS(0x1aU, true, 8, 5, 2, 1, 0); // v8: this, v5: x[1], v2: y, v1: x, v0: ex
- CHECK_REGS_CONTAIN_REFS(0x1dU, true, 8, 5, 2, 1, 0); // v8: this, v5: x[1], v2: y, v1: x, v0: ex
+ CHECK_REGS_CONTAIN_REFS(0x13U, false, 3); // v3: y
+ // Note that v0: ex can be eliminated because it's a dead merge of two different exceptions.
+ CHECK_REGS_CONTAIN_REFS(0x18U, true, 8, 2, 1); // v8: this, v2: y, v1: x (dead v0: ex)
+ CHECK_REGS_CONTAIN_REFS(0x1aU, true, 8, 5, 2, 1); // v8: this, v5: x[1], v2: y, v1: x (dead v0: ex)
+ CHECK_REGS_CONTAIN_REFS(0x1dU, true, 8, 5, 2, 1); // v8: this, v5: x[1], v2: y, v1: x (dead v0: ex)
// v5 is removed from the root set because there is a "merge" operation.
// See 0015: if-nez v2, 001f.
- CHECK_REGS_CONTAIN_REFS(0x1fU, true, 8, 2, 1, 0); // v8: this, v2: y, v1: x, v0: ex
- CHECK_REGS_CONTAIN_REFS(0x21U, true, 8, 2, 1, 0); // v8: this, v2: y, v1: x, v0: ex
+ CHECK_REGS_CONTAIN_REFS(0x1fU, true, 8, 2, 1); // v8: this, v2: y, v1: x (dead v0: ex)
+ CHECK_REGS_CONTAIN_REFS(0x21U, true, 8, 2, 1); // v8: this, v2: y, v1: x (dead v0: ex)
CHECK_REGS_CONTAIN_REFS(0x27U, true, 8, 4, 2, 1); // v8: this, v4: ex, v2: y, v1: x
CHECK_REGS_CONTAIN_REFS(0x29U, true, 8, 4, 2, 1); // v8: this, v4: ex, v2: y, v1: x
CHECK_REGS_CONTAIN_REFS(0x2cU, true, 8, 4, 2, 1); // v8: this, v4: ex, v2: y, v1: x
diff --git a/test/134-reg-promotion/smali/Test.smali b/test/134-reg-promotion/smali/Test.smali
index 6a35c45..68d29dd 100644
--- a/test/134-reg-promotion/smali/Test.smali
+++ b/test/134-reg-promotion/smali/Test.smali
@@ -36,3 +36,28 @@
:end
return-void
.end method
+
+.method public static run2()V
+ .registers 4
+ new-instance v2, Ljava/lang/String;
+ invoke-direct {v2}, Ljava/lang/String;-><init>()V
+ const/4 v0, 0
+ move v1, v0
+ :start
+ invoke-static {}, LMain;->blowup()V
+ if-ne v1, v0, :end
+ const/4 v2, 1
+ invoke-static {v2}, Ljava/lang/Integer;->toString(I)Ljava/lang/String;
+ move-result-object v3
+ if-nez v3, :skip
+ const/4 v0, 0
+ :skip
+ # The Phi merging 0 with 0 hides the constant from the Quick compiler.
+ move v2, v0
+ # The call makes v2 float type.
+ invoke-static {v2}, Ljava/lang/Float;->isNaN(F)Z
+ const/4 v1, 1
+ goto :start
+ :end
+ return-void
+.end method
diff --git a/test/134-reg-promotion/src/Main.java b/test/134-reg-promotion/src/Main.java
index d45ec66..008ac58 100644
--- a/test/134-reg-promotion/src/Main.java
+++ b/test/134-reg-promotion/src/Main.java
@@ -38,5 +38,11 @@
m.invoke(null, (Object[]) null);
holder = null;
}
+ m = c.getMethod("run2", (Class[]) null);
+ for (int i = 0; i < 10; i++) {
+ holder = new char[128 * 1024][];
+ m.invoke(null, (Object[]) null);
+ holder = null;
+ }
}
}
