diff options
author | 2023-04-05 10:33:07 +0000 | |
---|---|---|
committer | 2023-04-27 10:52:39 +0000 | |
commit | 79dc217688a774fc532584f6551a0aec8b45bc4a (patch) | |
tree | 5abfe4bd90364e66b593088ab4d1b407b51dada5 /compiler/optimizing/ssa_builder.cc | |
parent | d60aff547dedefc35265ce57707d406e8ccc4dc6 (diff) |
Optimizing: Rename `As##type` to `As##type##OrNull`.
The null type check in the current implementation of
`HInstruction::As##type()` often cannot be optimized away
by clang++. It is therefore beneficial to have two functions
HInstruction::As##type()
HInstruction::As##type##OrNull()
where the first function never returns null but the second
one can return null. The additional text "OrNull" shall also
flag the possibility of yielding null to the developer which
may help avoid bugs similar to what we have seen previously.
This requires renaming the existing function that can return
null and introducing new function that cannot. However,
defining the new function `HInstruction::As##type()` in the
same change as renaming the old one would risk introducing
bugs by missing a rename. Therefore we simply rename the old
function here and the new function shall be introduced in a
separate change with all behavioral changes being explicit.
Test: m test-art-host-gtest
Test: testrunner.py --host --optimizing
Test: buildbot-build.sh --target
Bug: 181943478
Change-Id: I4defd85038e28fe3506903ba3f33f723682b3298
Diffstat (limited to 'compiler/optimizing/ssa_builder.cc')
-rw-r--r-- | compiler/optimizing/ssa_builder.cc | 86 |
1 files changed, 57 insertions, 29 deletions
diff --git a/compiler/optimizing/ssa_builder.cc b/compiler/optimizing/ssa_builder.cc index 08ccbeee0d..09ca850eee 100644 --- a/compiler/optimizing/ssa_builder.cc +++ b/compiler/optimizing/ssa_builder.cc @@ -56,7 +56,8 @@ void SsaBuilder::FixNullConstantType() { // Both type propagation and redundant phi elimination ensure `int_operand` // can only be the 0 constant. DCHECK(int_operand->IsIntConstant()) << int_operand->DebugName(); - DCHECK_EQ(0, int_operand->AsIntConstant()->GetValue()); + // TODO: Remove "OrNull". + DCHECK_EQ(0, int_operand->AsIntConstantOrNull()->GetValue()); equality_instr->ReplaceInput(graph_->GetNullConstant(), int_operand == right ? 1 : 0); } } @@ -66,7 +67,8 @@ void SsaBuilder::EquivalentPhisCleanup() { // The order doesn't matter here. for (HBasicBlock* block : graph_->GetReversePostOrder()) { for (HInstructionIterator it(block->GetPhis()); !it.Done(); it.Advance()) { - HPhi* phi = it.Current()->AsPhi(); + // TODO: Remove "OrNull". + HPhi* phi = it.Current()->AsPhiOrNull(); HPhi* next = phi->GetNextEquivalentPhiWithSameType(); if (next != nullptr) { // Make sure we do not replace a live phi with a dead phi. A live phi @@ -88,18 +90,21 @@ void SsaBuilder::EquivalentPhisCleanup() { void SsaBuilder::FixEnvironmentPhis() { for (HBasicBlock* block : graph_->GetReversePostOrder()) { for (HInstructionIterator it_phis(block->GetPhis()); !it_phis.Done(); it_phis.Advance()) { - HPhi* phi = it_phis.Current()->AsPhi(); + // TODO: Remove "OrNull". + HPhi* phi = it_phis.Current()->AsPhiOrNull(); // If the phi is not dead, or has no environment uses, there is nothing to do. if (!phi->IsDead() || !phi->HasEnvironmentUses()) continue; HInstruction* next = phi->GetNext(); if (!phi->IsVRegEquivalentOf(next)) continue; - if (next->AsPhi()->IsDead()) { + // TODO: Remove "OrNull". + if (next->AsPhiOrNull()->IsDead()) { // If the phi equivalent is dead, check if there is another one. next = next->GetNext(); if (!phi->IsVRegEquivalentOf(next)) continue; // There can be at most two phi equivalents. DCHECK(!phi->IsVRegEquivalentOf(next->GetNext())); - if (next->AsPhi()->IsDead()) continue; + // TODO: Remove "OrNull". + if (next->AsPhiOrNull()->IsDead()) continue; } // We found a live phi equivalent. Update the environment uses of `phi` with it. phi->ReplaceWith(next); @@ -113,12 +118,15 @@ static void AddDependentInstructionsToWorklist(HInstruction* instruction, // live phi users, and transitively users of those users, therefore need to be // marked dead/conflicting too, so we add them to the worklist. Otherwise we // add users whose type does not match and needs to be updated. - bool add_all_live_phis = instruction->IsPhi() && instruction->AsPhi()->IsDead(); + // TODO: Remove "OrNull". + bool add_all_live_phis = instruction->IsPhi() && instruction->AsPhiOrNull()->IsDead(); for (const HUseListNode<HInstruction*>& use : instruction->GetUses()) { HInstruction* user = use.GetUser(); - if (user->IsPhi() && user->AsPhi()->IsLive()) { + // TODO: Remove "OrNull". + if (user->IsPhi() && user->AsPhiOrNull()->IsLive()) { if (add_all_live_phis || user->GetType() != instruction->GetType()) { - worklist->push_back(user->AsPhi()); + // TODO: Remove "OrNull". + worklist->push_back(user->AsPhiOrNull()); } } } @@ -130,7 +138,8 @@ static bool TypePhiFromInputs(HPhi* phi) { DataType::Type common_type = phi->GetType(); for (HInstruction* input : phi->GetInputs()) { - if (input->IsPhi() && input->AsPhi()->IsDead()) { + // TODO: Remove "OrNull". + if (input->IsPhi() && input->AsPhiOrNull()->IsDead()) { // Phis are constructed live so if an input is a dead phi, it must have // been made dead due to type conflict. Mark this phi conflicting too. return false; @@ -204,7 +213,8 @@ bool SsaBuilder::TypeInputsOfPhi(HPhi* phi, ScopedArenaVector<HPhi*>* worklist) phi->ReplaceInput(equivalent, i); if (equivalent->IsPhi()) { - worklist->push_back(equivalent->AsPhi()); + // TODO: Remove "OrNull". + worklist->push_back(equivalent->AsPhiOrNull()); } } } @@ -241,7 +251,8 @@ void SsaBuilder::RunPrimitiveTypePropagation() { for (HBasicBlock* block : graph_->GetReversePostOrder()) { if (block->IsLoopHeader()) { for (HInstructionIterator phi_it(block->GetPhis()); !phi_it.Done(); phi_it.Advance()) { - HPhi* phi = phi_it.Current()->AsPhi(); + // TODO: Remove "OrNull". + HPhi* phi = phi_it.Current()->AsPhiOrNull(); if (phi->IsLive()) { worklist.push_back(phi); } @@ -253,7 +264,8 @@ void SsaBuilder::RunPrimitiveTypePropagation() { // doing a reverse post-order visit, therefore either the phi users are // non-loop phi and will be visited later in the visit, or are loop-phis, // and they are already in the work list. - HPhi* phi = phi_it.Current()->AsPhi(); + // TODO: Remove "OrNull". + HPhi* phi = phi_it.Current()->AsPhiOrNull(); if (phi->IsLive()) { UpdatePrimitiveType(phi, &worklist); } @@ -283,7 +295,8 @@ static HArrayGet* FindFloatOrDoubleEquivalentOfArrayGet(HArrayGet* aget) { DCHECK(DataType::IsIntOrLongType(type)); HInstruction* next = aget->GetNext(); if (next != nullptr && next->IsArrayGet()) { - HArrayGet* next_aget = next->AsArrayGet(); + // TODO: Remove "OrNull". + HArrayGet* next_aget = next->AsArrayGetOrNull(); if (next_aget->IsEquivalentOf(aget)) { return next_aget; } @@ -395,7 +408,8 @@ bool SsaBuilder::FixAmbiguousArrayOps() { if (equivalent->IsPhi()) { // Returned equivalent is a phi which may not have had its inputs // replaced yet. We need to run primitive type propagation on it. - worklist.push_back(equivalent->AsPhi()); + // TODO: Remove "OrNull". + worklist.push_back(equivalent->AsPhiOrNull()); } } // Refine the side effects of this floating point aset. Note that we do this even if @@ -442,7 +456,8 @@ bool SsaBuilder::ReplaceUninitializedStringPhis() { return false; } DCHECK(str->IsNewInstance()); - AddUninitializedString(str->AsNewInstance()); + // TODO: Remove "OrNull". + AddUninitializedString(str->AsNewInstanceOrNull()); str->ReplaceUsesDominatedBy(invoke, invoke); str->ReplaceEnvUsesDominatedBy(invoke, invoke); invoke->RemoveInputAt(invoke->InputCount() - 1); @@ -478,11 +493,13 @@ void SsaBuilder::RemoveRedundantUninitializedStrings() { // class is always initialized at the point of running Java code, we can remove // that check. if (input->IsClinitCheck()) { - load_class = input->InputAt(0)->AsLoadClass(); + // TODO: Remove "OrNull". + load_class = input->InputAt(0)->AsLoadClassOrNull(); input->ReplaceWith(load_class); input->GetBlock()->RemoveInstruction(input); } else { - load_class = input->AsLoadClass(); + // TODO: Remove "OrNull". + load_class = input->AsLoadClassOrNull(); DCHECK(new_instance->IsStringAlloc()); DCHECK(!load_class->NeedsAccessCheck()) << "String class is always accessible"; } @@ -503,7 +520,8 @@ static bool HasPhiEquivalentAtLoopEntry(HGraph* graph) { for (HBasicBlock* block : graph->GetReversePostOrder()) { if (block->IsLoopHeader()) { for (HInstructionIterator it(block->GetPhis()); !it.Done(); it.Advance()) { - if (it.Current()->AsPhi()->HasEquivalentPhi()) { + // TODO: Remove "OrNull". + if (it.Current()->AsPhiOrNull()->HasEquivalentPhi()) { return true; } } @@ -604,7 +622,7 @@ GraphAnalysisResult SsaBuilder::BuildSsa() { */ HFloatConstant* SsaBuilder::GetFloatEquivalent(HIntConstant* constant) { // We place the floating point constant next to this constant. - HFloatConstant* result = constant->GetNext()->AsFloatConstant(); + HFloatConstant* result = constant->GetNext()->AsFloatConstantOrNull(); if (result == nullptr) { float value = bit_cast<float, int32_t>(constant->GetValue()); result = new (graph_->GetAllocator()) HFloatConstant(value); @@ -626,7 +644,7 @@ HFloatConstant* SsaBuilder::GetFloatEquivalent(HIntConstant* constant) { */ HDoubleConstant* SsaBuilder::GetDoubleEquivalent(HLongConstant* constant) { // We place the floating point constant next to this constant. - HDoubleConstant* result = constant->GetNext()->AsDoubleConstant(); + HDoubleConstant* result = constant->GetNext()->AsDoubleConstantOrNull(); if (result == nullptr) { double value = bit_cast<double, int64_t>(constant->GetValue()); result = new (graph_->GetAllocator()) HDoubleConstant(value); @@ -653,14 +671,16 @@ HPhi* SsaBuilder::GetFloatDoubleOrReferenceEquivalentOfPhi(HPhi* phi, DataType:: // We place the floating point /reference phi next to this phi. HInstruction* next = phi->GetNext(); if (next != nullptr && - next->AsPhi()->GetRegNumber() == phi->GetRegNumber() && + // TODO: Remove "OrNull". + next->AsPhiOrNull()->GetRegNumber() == phi->GetRegNumber() && next->GetType() != type) { // Move to the next phi to see if it is the one we are looking for. next = next->GetNext(); } if (next == nullptr || - (next->AsPhi()->GetRegNumber() != phi->GetRegNumber()) || + // TODO: Remove "OrNull". + (next->AsPhiOrNull()->GetRegNumber() != phi->GetRegNumber()) || (next->GetType() != type)) { ArenaAllocator* allocator = graph_->GetAllocator(); HInputsRef inputs = phi->GetInputs(); @@ -677,7 +697,8 @@ HPhi* SsaBuilder::GetFloatDoubleOrReferenceEquivalentOfPhi(HPhi* phi, DataType:: } else { // An existing equivalent was found. If it is dead, conflict was previously // identified and we return nullptr instead. - HPhi* next_phi = next->AsPhi(); + // TODO: Remove "OrNull". + HPhi* next_phi = next->AsPhiOrNull(); DCHECK_EQ(next_phi->GetType(), type); return next_phi->IsLive() ? next_phi : nullptr; } @@ -710,23 +731,30 @@ HArrayGet* SsaBuilder::GetFloatOrDoubleEquivalentOfArrayGet(HArrayGet* aget) { HInstruction* SsaBuilder::GetFloatOrDoubleEquivalent(HInstruction* value, DataType::Type type) { if (value->IsArrayGet()) { - return GetFloatOrDoubleEquivalentOfArrayGet(value->AsArrayGet()); + // TODO: Remove "OrNull". + return GetFloatOrDoubleEquivalentOfArrayGet(value->AsArrayGetOrNull()); } else if (value->IsLongConstant()) { - return GetDoubleEquivalent(value->AsLongConstant()); + // TODO: Remove "OrNull". + return GetDoubleEquivalent(value->AsLongConstantOrNull()); } else if (value->IsIntConstant()) { - return GetFloatEquivalent(value->AsIntConstant()); + // TODO: Remove "OrNull". + return GetFloatEquivalent(value->AsIntConstantOrNull()); } else if (value->IsPhi()) { - return GetFloatDoubleOrReferenceEquivalentOfPhi(value->AsPhi(), type); + // TODO: Remove "OrNull". + return GetFloatDoubleOrReferenceEquivalentOfPhi(value->AsPhiOrNull(), type); } else { return nullptr; } } HInstruction* SsaBuilder::GetReferenceTypeEquivalent(HInstruction* value) { - if (value->IsIntConstant() && value->AsIntConstant()->GetValue() == 0) { + // TODO: Remove "OrNull". + if (value->IsIntConstant() && value->AsIntConstantOrNull()->GetValue() == 0) { return graph_->GetNullConstant(); } else if (value->IsPhi()) { - return GetFloatDoubleOrReferenceEquivalentOfPhi(value->AsPhi(), DataType::Type::kReference); + // TODO: Remove "OrNull". + return GetFloatDoubleOrReferenceEquivalentOfPhi( + value->AsPhiOrNull(), DataType::Type::kReference); } else { return nullptr; } |