spirv-opt: Set parent when adding basic block (#4021)
Ensures that the parent of a block is set in Function::AddBasicBlock.
Removes various now unnecessary calls to BasicBlock::SetParent.
Fixes #3912.
diff --git a/source/fuzz/force_render_red.cpp b/source/fuzz/force_render_red.cpp
index ed60bd0..fd0587a 100644
--- a/source/fuzz/force_render_red.cpp
+++ b/source/fuzz/force_render_red.cpp
@@ -212,7 +212,6 @@
auto new_exit_block = MakeUnique<opt::BasicBlock>(std::move(label));
new_exit_block->AddInstruction(MakeUnique<opt::Instruction>(
ir_context.get(), SpvOpReturn, 0, 0, opt::Instruction::OperandList()));
- new_exit_block->SetParent(entry_point_function);
entry_point_function->AddBasicBlock(std::move(new_exit_block));
}
diff --git a/source/fuzz/transformation_add_dead_block.cpp b/source/fuzz/transformation_add_dead_block.cpp
index 3d77a80..5dce356 100644
--- a/source/fuzz/transformation_add_dead_block.cpp
+++ b/source/fuzz/transformation_add_dead_block.cpp
@@ -153,7 +153,6 @@
: successor_block_id}}});
// Add the new block to the enclosing function.
- new_block->SetParent(enclosing_function);
enclosing_function->InsertBasicBlockAfter(std::move(new_block),
existing_block);
diff --git a/source/fuzz/transformation_add_early_terminator_wrapper.cpp b/source/fuzz/transformation_add_early_terminator_wrapper.cpp
index 0aa1214..9f86070 100644
--- a/source/fuzz/transformation_add_early_terminator_wrapper.cpp
+++ b/source/fuzz/transformation_add_early_terminator_wrapper.cpp
@@ -84,7 +84,6 @@
// Add the basic block to the function as the sole block, and add the function
// to the module.
- basic_block->SetParent(function.get());
function->AddBasicBlock(std::move(basic_block));
function->SetFunctionEnd(MakeUnique<opt::Instruction>(
ir_context, SpvOpFunctionEnd, 0, 0, opt::Instruction::OperandList()));
diff --git a/source/fuzz/transformation_add_function.cpp b/source/fuzz/transformation_add_function.cpp
index 214f741..9799373 100644
--- a/source/fuzz/transformation_add_function.cpp
+++ b/source/fuzz/transformation_add_function.cpp
@@ -276,12 +276,10 @@
// Invariant: we should always be at a label instruction at this point.
assert(message_.instruction(instruction_index).opcode() == SpvOpLabel);
- // Make a basic block using the label instruction, with the new function
- // as its parent.
+ // Make a basic block using the label instruction.
std::unique_ptr<opt::BasicBlock> block =
MakeUnique<opt::BasicBlock>(InstructionFromMessage(
ir_context, message_.instruction(instruction_index)));
- block->SetParent(new_function.get());
// Consider successive instructions until we hit another label or the end
// of the function, adding each such instruction to the block.
diff --git a/source/fuzz/transformation_inline_function.cpp b/source/fuzz/transformation_inline_function.cpp
index f58b123..f997491 100644
--- a/source/fuzz/transformation_inline_function.cpp
+++ b/source/fuzz/transformation_inline_function.cpp
@@ -183,7 +183,6 @@
auto* cloned_block = block.Clone(ir_context);
cloned_block = caller_function->InsertBasicBlockBefore(
std::unique_ptr<opt::BasicBlock>(cloned_block), successor_block);
- cloned_block->SetParent(caller_function);
cloned_block->GetLabel()->SetResultId(result_id_map.at(cloned_block->id()));
fuzzerutil::UpdateModuleIdBound(ir_context, cloned_block->id());
diff --git a/source/fuzz/transformation_merge_function_returns.cpp b/source/fuzz/transformation_merge_function_returns.cpp
index 90578a2..c7cb557 100644
--- a/source/fuzz/transformation_merge_function_returns.cpp
+++ b/source/fuzz/transformation_merge_function_returns.cpp
@@ -579,7 +579,6 @@
}
// Insert the new return block at the end of the function.
- outer_return_block->SetParent(function);
function->AddBasicBlock(std::move(outer_return_block));
// All analyses must be invalidated because the structure of the module was
diff --git a/source/fuzz/transformation_outline_function.cpp b/source/fuzz/transformation_outline_function.cpp
index 26f9e0b..643fd69 100644
--- a/source/fuzz/transformation_outline_function.cpp
+++ b/source/fuzz/transformation_outline_function.cpp
@@ -778,7 +778,6 @@
MakeUnique<opt::BasicBlock>(MakeUnique<opt::Instruction>(
ir_context, SpvOpLabel, 0, message_.new_function_region_entry_block(),
opt::Instruction::OperandList()));
- outlined_region_entry_block->SetParent(outlined_function);
if (&original_region_entry_block == &original_region_exit_block) {
outlined_region_exit_block = outlined_region_entry_block.get();
@@ -815,8 +814,6 @@
outlined_region_exit_block = cloned_block.get();
}
- cloned_block->SetParent(outlined_function);
-
// Redirect any OpPhi operands whose predecessors are the original region
// entry block to become the new function entry block.
cloned_block->ForEachPhiInst([this](opt::Instruction* phi_inst) {
diff --git a/source/opt/basic_block.cpp b/source/opt/basic_block.cpp
index 3608448..b7e122c 100644
--- a/source/opt/basic_block.cpp
+++ b/source/opt/basic_block.cpp
@@ -248,7 +248,8 @@
function_->InsertBasicBlockAfter(std::move(new_block_temp), this);
new_block->insts_.Splice(new_block->end(), &insts_, iter, end());
- new_block->SetParent(GetParent());
+ assert(new_block->GetParent() == GetParent() &&
+ "The parent should already be set appropriately.");
context->AnalyzeDefUse(new_block->GetLabelInst());
diff --git a/source/opt/eliminate_dead_members_pass.cpp b/source/opt/eliminate_dead_members_pass.cpp
index 5b8f4ec..ab28932 100644
--- a/source/opt/eliminate_dead_members_pass.cpp
+++ b/source/opt/eliminate_dead_members_pass.cpp
@@ -20,7 +20,7 @@
namespace {
const uint32_t kRemovedMember = 0xFFFFFFFF;
const uint32_t kSpecConstOpOpcodeIdx = 0;
-}
+} // namespace
namespace spvtools {
namespace opt {
diff --git a/source/opt/function.cpp b/source/opt/function.cpp
index 52054ea..38c6695 100644
--- a/source/opt/function.cpp
+++ b/source/opt/function.cpp
@@ -42,7 +42,6 @@
clone->blocks_.reserve(blocks_.size());
for (const auto& b : blocks_) {
std::unique_ptr<BasicBlock> bb(b->Clone(ctx));
- bb->SetParent(clone);
clone->AddBasicBlock(std::move(bb));
}
diff --git a/source/opt/function.h b/source/opt/function.h
index 4b20dcb..9e1c727 100644
--- a/source/opt/function.h
+++ b/source/opt/function.h
@@ -213,6 +213,7 @@
inline void Function::AddBasicBlock(std::unique_ptr<BasicBlock> b,
iterator ip) {
+ b->SetParent(this);
ip.InsertBefore(std::move(b));
}
diff --git a/source/opt/inst_buff_addr_check_pass.cpp b/source/opt/inst_buff_addr_check_pass.cpp
index fa6c2c6..06acc7e 100644
--- a/source/opt/inst_buff_addr_check_pass.cpp
+++ b/source/opt/inst_buff_addr_check_pass.cpp
@@ -202,7 +202,6 @@
(void)builder.AddInstruction(MakeUnique<Instruction>(
context(), SpvOpBranch, 0, 0,
std::initializer_list<Operand>{{SPV_OPERAND_TYPE_ID, {hdr_blk_id}}}));
- first_blk_ptr->SetParent(&*input_func);
input_func->AddBasicBlock(std::move(first_blk_ptr));
// Linear search loop header block
// TODO(greg-lunarg): Implement binary search
@@ -246,7 +245,6 @@
(void)builder.AddInstruction(MakeUnique<Instruction>(
context(), SpvOpBranch, 0, 0,
std::initializer_list<Operand>{{SPV_OPERAND_TYPE_ID, {cont_blk_id}}}));
- hdr_blk_ptr->SetParent(&*input_func);
input_func->AddBasicBlock(std::move(hdr_blk_ptr));
// Continue/Work Block. Read next buffer pointer and break if greater
// than ref_ptr arg.
@@ -272,7 +270,6 @@
(void)builder.AddConditionalBranch(uptr_test_inst->result_id(),
bound_test_blk_id, hdr_blk_id,
kInvalidId, SpvSelectionControlMaskNone);
- cont_blk_ptr->SetParent(&*input_func);
input_func->AddBasicBlock(std::move(cont_blk_ptr));
// Bounds test block. Read length of selected buffer and test that
// all len arg bytes are in buffer.
@@ -333,7 +330,6 @@
std::initializer_list<Operand>{
{SPV_OPERAND_TYPE_ID, {len_test_inst->result_id()}}}));
// Close block
- bound_test_blk_ptr->SetParent(&*input_func);
input_func->AddBasicBlock(std::move(bound_test_blk_ptr));
// Close function and add function to module
std::unique_ptr<Instruction> func_end_inst(
diff --git a/source/opt/instrument_pass.cpp b/source/opt/instrument_pass.cpp
index e7d9778..ed34fb0 100644
--- a/source/opt/instrument_pass.cpp
+++ b/source/opt/instrument_pass.cpp
@@ -758,7 +758,6 @@
write_blk_id, merge_blk_id, merge_blk_id,
SpvSelectionControlMaskNone);
// Close safety test block and gen write block
- new_blk_ptr->SetParent(&*output_func);
output_func->AddBasicBlock(std::move(new_blk_ptr));
new_blk_ptr = MakeUnique<BasicBlock>(std::move(write_label));
builder.SetInsertPoint(&*new_blk_ptr);
@@ -773,13 +772,11 @@
}
// Close write block and gen merge block
(void)builder.AddBranch(merge_blk_id);
- new_blk_ptr->SetParent(&*output_func);
output_func->AddBasicBlock(std::move(new_blk_ptr));
new_blk_ptr = MakeUnique<BasicBlock>(std::move(merge_label));
builder.SetInsertPoint(&*new_blk_ptr);
// Close merge block and function and add function to module
(void)builder.AddNullaryOp(0, SpvOpReturn);
- new_blk_ptr->SetParent(&*output_func);
output_func->AddBasicBlock(std::move(new_blk_ptr));
std::unique_ptr<Instruction> func_end_inst(
new Instruction(get_module()->context(), SpvOpFunctionEnd, 0, 0, {}));
@@ -860,7 +857,6 @@
context(), SpvOpReturnValue, 0, 0,
std::initializer_list<Operand>{{SPV_OPERAND_TYPE_ID, {last_value_id}}}));
// Close block and function and add function to module
- new_blk_ptr->SetParent(&*input_func);
input_func->AddBasicBlock(std::move(new_blk_ptr));
std::unique_ptr<Instruction> func_end_inst(
new Instruction(get_module()->context(), SpvOpFunctionEnd, 0, 0, {}));
diff --git a/source/opt/loop_peeling.cpp b/source/opt/loop_peeling.cpp
index 071c27c..34f0a8d 100644
--- a/source/opt/loop_peeling.cpp
+++ b/source/opt/loop_peeling.cpp
@@ -351,7 +351,6 @@
std::unique_ptr<BasicBlock> new_bb =
MakeUnique<BasicBlock>(std::unique_ptr<Instruction>(new Instruction(
context_, SpvOpLabel, 0, context_->TakeNextId(), {})));
- new_bb->SetParent(loop_utils_.GetFunction());
// Update the loop descriptor.
Loop* in_loop = (*loop_utils_.GetLoopDescriptor())[bb];
if (in_loop) {
diff --git a/source/opt/merge_return_pass.cpp b/source/opt/merge_return_pass.cpp
index b43eb31..f160104 100644
--- a/source/opt/merge_return_pass.cpp
+++ b/source/opt/merge_return_pass.cpp
@@ -182,7 +182,8 @@
context()->AnalyzeDefUse(final_return_block_->GetLabelInst());
context()->set_instr_block(final_return_block_->GetLabelInst(),
final_return_block_);
- final_return_block_->SetParent(function_);
+ assert(final_return_block_->GetParent() == function_ &&
+ "The function should have been set when the block was created.");
}
void MergeReturnPass::CreateReturn(BasicBlock* block) {
diff --git a/source/opt/wrap_opkill.cpp b/source/opt/wrap_opkill.cpp
index ae1000c..51432a7 100644
--- a/source/opt/wrap_opkill.cpp
+++ b/source/opt/wrap_opkill.cpp
@@ -164,7 +164,6 @@
bb->AddInstruction(std::move(kill_inst));
// Add the bb to the function
- bb->SetParent((*killing_func).get());
(*killing_func)->AddBasicBlock(std::move(bb));
// Add the function to the module.
