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/graph_test.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/graph_test.cc')
-rw-r--r-- | compiler/optimizing/graph_test.cc | 66 |
1 files changed, 44 insertions, 22 deletions
diff --git a/compiler/optimizing/graph_test.cc b/compiler/optimizing/graph_test.cc index b5d712736f..f391bcbcbe 100644 --- a/compiler/optimizing/graph_test.cc +++ b/compiler/optimizing/graph_test.cc @@ -92,16 +92,20 @@ TEST_F(GraphTest, IfSuccessorSimpleJoinBlock1) { if_block->AddSuccessor(return_block); return_block->AddSuccessor(exit_block); - ASSERT_EQ(if_block->GetLastInstruction()->AsIf()->IfTrueSuccessor(), if_true); - ASSERT_EQ(if_block->GetLastInstruction()->AsIf()->IfFalseSuccessor(), return_block); + // TODO: Remove "OrNull". + ASSERT_EQ(if_block->GetLastInstruction()->AsIfOrNull()->IfTrueSuccessor(), if_true); + // TODO: Remove "OrNull". + ASSERT_EQ(if_block->GetLastInstruction()->AsIfOrNull()->IfFalseSuccessor(), return_block); graph->SimplifyCFG(); // Ensure we still have the same if true block. - ASSERT_EQ(if_block->GetLastInstruction()->AsIf()->IfTrueSuccessor(), if_true); + // TODO: Remove "OrNull". + ASSERT_EQ(if_block->GetLastInstruction()->AsIfOrNull()->IfTrueSuccessor(), if_true); // Ensure the critical edge has been removed. - HBasicBlock* false_block = if_block->GetLastInstruction()->AsIf()->IfFalseSuccessor(); + // TODO: Remove "OrNull". + HBasicBlock* false_block = if_block->GetLastInstruction()->AsIfOrNull()->IfFalseSuccessor(); ASSERT_NE(false_block, return_block); // Ensure the new block branches to the join block. @@ -124,16 +128,20 @@ TEST_F(GraphTest, IfSuccessorSimpleJoinBlock2) { if_block->AddSuccessor(if_false); return_block->AddSuccessor(exit_block); - ASSERT_EQ(if_block->GetLastInstruction()->AsIf()->IfTrueSuccessor(), return_block); - ASSERT_EQ(if_block->GetLastInstruction()->AsIf()->IfFalseSuccessor(), if_false); + // TODO: Remove "OrNull". + ASSERT_EQ(if_block->GetLastInstruction()->AsIfOrNull()->IfTrueSuccessor(), return_block); + // TODO: Remove "OrNull". + ASSERT_EQ(if_block->GetLastInstruction()->AsIfOrNull()->IfFalseSuccessor(), if_false); graph->SimplifyCFG(); // Ensure we still have the same if true block. - ASSERT_EQ(if_block->GetLastInstruction()->AsIf()->IfFalseSuccessor(), if_false); + // TODO: Remove "OrNull". + ASSERT_EQ(if_block->GetLastInstruction()->AsIfOrNull()->IfFalseSuccessor(), if_false); // Ensure the critical edge has been removed. - HBasicBlock* true_block = if_block->GetLastInstruction()->AsIf()->IfTrueSuccessor(); + // TODO: Remove "OrNull". + HBasicBlock* true_block = if_block->GetLastInstruction()->AsIfOrNull()->IfTrueSuccessor(); ASSERT_NE(true_block, return_block); // Ensure the new block branches to the join block. @@ -154,13 +162,16 @@ TEST_F(GraphTest, IfSuccessorMultipleBackEdges1) { if_block->AddSuccessor(return_block); return_block->AddSuccessor(exit_block); - ASSERT_EQ(if_block->GetLastInstruction()->AsIf()->IfTrueSuccessor(), if_block); - ASSERT_EQ(if_block->GetLastInstruction()->AsIf()->IfFalseSuccessor(), return_block); + // TODO: Remove "OrNull". + ASSERT_EQ(if_block->GetLastInstruction()->AsIfOrNull()->IfTrueSuccessor(), if_block); + // TODO: Remove "OrNull". + ASSERT_EQ(if_block->GetLastInstruction()->AsIfOrNull()->IfFalseSuccessor(), return_block); graph->BuildDominatorTree(); // Ensure we still have the same if false block. - ASSERT_EQ(if_block->GetLastInstruction()->AsIf()->IfFalseSuccessor(), return_block); + // TODO: Remove "OrNull". + ASSERT_EQ(if_block->GetLastInstruction()->AsIfOrNull()->IfFalseSuccessor(), return_block); // Ensure there is only one back edge. ASSERT_EQ(if_block->GetPredecessors().size(), 2u); @@ -169,7 +180,8 @@ TEST_F(GraphTest, IfSuccessorMultipleBackEdges1) { // Ensure the new block is the back edge. ASSERT_EQ(if_block->GetPredecessors()[1], - if_block->GetLastInstruction()->AsIf()->IfTrueSuccessor()); + // TODO: Remove "OrNull". + if_block->GetLastInstruction()->AsIfOrNull()->IfTrueSuccessor()); } // Test that the successors of an if block stay consistent after a SimplifyCFG. @@ -186,13 +198,16 @@ TEST_F(GraphTest, IfSuccessorMultipleBackEdges2) { if_block->AddSuccessor(if_block); return_block->AddSuccessor(exit_block); - ASSERT_EQ(if_block->GetLastInstruction()->AsIf()->IfTrueSuccessor(), return_block); - ASSERT_EQ(if_block->GetLastInstruction()->AsIf()->IfFalseSuccessor(), if_block); + // TODO: Remove "OrNull". + ASSERT_EQ(if_block->GetLastInstruction()->AsIfOrNull()->IfTrueSuccessor(), return_block); + // TODO: Remove "OrNull". + ASSERT_EQ(if_block->GetLastInstruction()->AsIfOrNull()->IfFalseSuccessor(), if_block); graph->BuildDominatorTree(); // Ensure we still have the same if true block. - ASSERT_EQ(if_block->GetLastInstruction()->AsIf()->IfTrueSuccessor(), return_block); + // TODO: Remove "OrNull". + ASSERT_EQ(if_block->GetLastInstruction()->AsIfOrNull()->IfTrueSuccessor(), return_block); // Ensure there is only one back edge. ASSERT_EQ(if_block->GetPredecessors().size(), 2u); @@ -201,7 +216,8 @@ TEST_F(GraphTest, IfSuccessorMultipleBackEdges2) { // Ensure the new block is the back edge. ASSERT_EQ(if_block->GetPredecessors()[1], - if_block->GetLastInstruction()->AsIf()->IfFalseSuccessor()); + // TODO: Remove "OrNull". + if_block->GetLastInstruction()->AsIfOrNull()->IfFalseSuccessor()); } // Test that the successors of an if block stay consistent after a SimplifyCFG. @@ -222,12 +238,15 @@ TEST_F(GraphTest, IfSuccessorMultiplePreHeaders1) { if_block->AddSuccessor(return_block); - ASSERT_EQ(if_block->GetLastInstruction()->AsIf()->IfTrueSuccessor(), loop_block); - ASSERT_EQ(if_block->GetLastInstruction()->AsIf()->IfFalseSuccessor(), return_block); + // TODO: Remove "OrNull". + ASSERT_EQ(if_block->GetLastInstruction()->AsIfOrNull()->IfTrueSuccessor(), loop_block); + // TODO: Remove "OrNull". + ASSERT_EQ(if_block->GetLastInstruction()->AsIfOrNull()->IfFalseSuccessor(), return_block); graph->BuildDominatorTree(); - HIf* if_instr = if_block->GetLastInstruction()->AsIf(); + // TODO: Remove "OrNull". + HIf* if_instr = if_block->GetLastInstruction()->AsIfOrNull(); // Ensure we still have the same if false block. ASSERT_EQ(if_instr->IfFalseSuccessor(), return_block); @@ -257,12 +276,15 @@ TEST_F(GraphTest, IfSuccessorMultiplePreHeaders2) { if_block->AddSuccessor(return_block); if_block->AddSuccessor(loop_block); - ASSERT_EQ(if_block->GetLastInstruction()->AsIf()->IfTrueSuccessor(), return_block); - ASSERT_EQ(if_block->GetLastInstruction()->AsIf()->IfFalseSuccessor(), loop_block); + // TODO: Remove "OrNull". + ASSERT_EQ(if_block->GetLastInstruction()->AsIfOrNull()->IfTrueSuccessor(), return_block); + // TODO: Remove "OrNull". + ASSERT_EQ(if_block->GetLastInstruction()->AsIfOrNull()->IfFalseSuccessor(), loop_block); graph->BuildDominatorTree(); - HIf* if_instr = if_block->GetLastInstruction()->AsIf(); + // TODO: Remove "OrNull". + HIf* if_instr = if_block->GetLastInstruction()->AsIfOrNull(); // Ensure we still have the same if true block. ASSERT_EQ(if_instr->IfTrueSuccessor(), return_block); |