Remove the module from opt::Function. (#1717)
The function class provides a {Set|Get}Parent call in order to provide
the context to the LoopDescriptor methods. This CL removes the module
from Function and provides the needed context directly to LoopDescriptor
on creation.
diff --git a/source/link/linker.cpp b/source/link/linker.cpp
index de79376..4a7e806 100644
--- a/source/link/linker.cpp
+++ b/source/link/linker.cpp
@@ -361,7 +361,6 @@
for (const auto& module : input_modules) {
for (const auto& func : *module) {
std::unique_ptr<opt::Function> cloned_func(func.Clone(linked_context));
- cloned_func->SetParent(linked_module);
linked_module->AddFunction(std::move(cloned_func));
}
}
diff --git a/source/opt/dominator_analysis.h b/source/opt/dominator_analysis.h
index f4471d6..c4e53ca 100644
--- a/source/opt/dominator_analysis.h
+++ b/source/opt/dominator_analysis.h
@@ -19,7 +19,6 @@
#include <map>
#include "dominator_tree.h"
-#include "module.h"
namespace spvtools {
namespace opt {
diff --git a/source/opt/dominator_tree.h b/source/opt/dominator_tree.h
index 2656388..c25f0b7 100644
--- a/source/opt/dominator_tree.h
+++ b/source/opt/dominator_tree.h
@@ -22,7 +22,6 @@
#include <vector>
#include "cfg.h"
-#include "module.h"
#include "tree_iterator.h"
namespace spvtools {
diff --git a/source/opt/function.h b/source/opt/function.h
index 6830392..ff43430 100644
--- a/source/opt/function.h
+++ b/source/opt/function.h
@@ -53,10 +53,6 @@
Instruction& DefInst() { return *def_inst_; }
const Instruction& DefInst() const { return *def_inst_; }
- // Sets the enclosing module for this function.
- void SetParent(Module* module) { module_ = module; }
- // Gets the enclosing module for this function
- Module* GetParent() const { return module_; }
// Appends a parameter to this function.
inline void AddParameter(std::unique_ptr<Instruction> p);
// Appends a basic block to this function.
@@ -129,8 +125,6 @@
std::string PrettyPrint(uint32_t options = 0u) const;
private:
- // The enclosing module.
- Module* module_;
// The OpFunction instruction that begins the definition of this function.
std::unique_ptr<Instruction> def_inst_;
// All parameters to this function.
@@ -145,7 +139,7 @@
std::ostream& operator<<(std::ostream& str, const Function& func);
inline Function::Function(std::unique_ptr<Instruction> def_inst)
- : module_(nullptr), def_inst_(std::move(def_inst)), end_inst_() {}
+ : def_inst_(std::move(def_inst)), end_inst_() {}
inline void Function::AddParameter(std::unique_ptr<Instruction> p) {
params_.emplace_back(std::move(p));
diff --git a/source/opt/ir_context.cpp b/source/opt/ir_context.cpp
index 68d5415..5594ad3 100644
--- a/source/opt/ir_context.cpp
+++ b/source/opt/ir_context.cpp
@@ -565,7 +565,8 @@
std::unordered_map<const opt::Function*, opt::LoopDescriptor>::iterator it =
loop_descriptors_.find(f);
if (it == loop_descriptors_.end()) {
- return &loop_descriptors_.emplace(std::make_pair(f, opt::LoopDescriptor(f)))
+ return &loop_descriptors_
+ .emplace(std::make_pair(f, opt::LoopDescriptor(this, f)))
.first->second;
}
diff --git a/source/opt/ir_loader.cpp b/source/opt/ir_loader.cpp
index eb9a055..b12ebbe 100644
--- a/source/opt/ir_loader.cpp
+++ b/source/opt/ir_loader.cpp
@@ -153,7 +153,6 @@
}
for (auto& function : *module_) {
for (auto& bb : function) bb.SetParent(&function);
- function.SetParent(module_);
}
}
diff --git a/source/opt/loop_descriptor.cpp b/source/opt/loop_descriptor.cpp
index 003f364..dce7015 100644
--- a/source/opt/loop_descriptor.cpp
+++ b/source/opt/loop_descriptor.cpp
@@ -490,16 +490,14 @@
ordered_loop_blocks->push_back(loop_merge_);
}
-LoopDescriptor::LoopDescriptor(const Function* f)
+LoopDescriptor::LoopDescriptor(IRContext* context, const Function* f)
: loops_(), dummy_top_loop_(nullptr) {
- PopulateList(f);
+ PopulateList(context, f);
}
LoopDescriptor::~LoopDescriptor() { ClearLoops(); }
-void LoopDescriptor::PopulateList(const Function* f) {
- IRContext* context = f->GetParent()->context();
-
+void LoopDescriptor::PopulateList(IRContext* context, const Function* f) {
opt::DominatorAnalysis* dom_analysis = context->GetDominatorAnalysis(f);
ClearLoops();
diff --git a/source/opt/loop_descriptor.h b/source/opt/loop_descriptor.h
index f0dd085..e700033 100644
--- a/source/opt/loop_descriptor.h
+++ b/source/opt/loop_descriptor.h
@@ -427,7 +427,7 @@
using const_pre_iterator = opt::TreeDFIterator<const Loop>;
// Creates a loop object for all loops found in |f|.
- explicit LoopDescriptor(const Function* f);
+ LoopDescriptor(IRContext* context, const Function* f);
// Disable copy constructor, to avoid double-free on destruction.
LoopDescriptor(const LoopDescriptor&) = delete;
@@ -542,7 +542,7 @@
using LoopsToAddContainerType = std::vector<std::pair<Loop*, Loop*>>;
// Creates loop descriptors for the function |f|.
- void PopulateList(const Function* f);
+ void PopulateList(IRContext* context, const Function* f);
// Returns the inner most loop that contains the basic block id |block_id|.
inline Loop* FindLoopForBasicBlock(uint32_t block_id) const {
diff --git a/test/opt/loop_optimizations/lcssa.cpp b/test/opt/loop_optimizations/lcssa.cpp
index 5ecb899..0f0bd0b 100644
--- a/test/opt/loop_optimizations/lcssa.cpp
+++ b/test/opt/loop_optimizations/lcssa.cpp
@@ -143,7 +143,7 @@
EXPECT_NE(nullptr, module) << "Assembling failed for shader:\n"
<< text << std::endl;
const Function* f = spvtest::GetFunction(module, 2);
- LoopDescriptor ld{f};
+ LoopDescriptor ld{context.get(), f};
Loop* loop = ld[17];
EXPECT_FALSE(loop->IsLCSSA());
@@ -229,7 +229,7 @@
EXPECT_NE(nullptr, module) << "Assembling failed for shader:\n"
<< text << std::endl;
const Function* f = spvtest::GetFunction(module, 2);
- LoopDescriptor ld{f};
+ LoopDescriptor ld{context.get(), f};
Loop* loop = ld[17];
EXPECT_FALSE(loop->IsLCSSA());
@@ -328,7 +328,7 @@
EXPECT_NE(nullptr, module) << "Assembling failed for shader:\n"
<< text << std::endl;
const Function* f = spvtest::GetFunction(module, 2);
- LoopDescriptor ld{f};
+ LoopDescriptor ld{context.get(), f};
Loop* loop = ld[16];
EXPECT_FALSE(loop->IsLCSSA());
@@ -421,7 +421,7 @@
EXPECT_NE(nullptr, module) << "Assembling failed for shader:\n"
<< text << std::endl;
const Function* f = spvtest::GetFunction(module, 2);
- LoopDescriptor ld{f};
+ LoopDescriptor ld{context.get(), f};
Loop* loop = ld[19];
EXPECT_FALSE(loop->IsLCSSA());
@@ -516,7 +516,7 @@
EXPECT_NE(nullptr, module) << "Assembling failed for shader:\n"
<< text << std::endl;
const Function* f = spvtest::GetFunction(module, 2);
- LoopDescriptor ld{f};
+ LoopDescriptor ld{context.get(), f};
Loop* loop = ld[19];
EXPECT_FALSE(loop->IsLCSSA());
@@ -599,7 +599,7 @@
EXPECT_NE(nullptr, module) << "Assembling failed for shader:\n"
<< text << std::endl;
const Function* f = spvtest::GetFunction(module, 2);
- LoopDescriptor ld{f};
+ LoopDescriptor ld{context.get(), f};
Loop* loop = ld[12];
EXPECT_FALSE(loop->IsLCSSA());
diff --git a/test/opt/loop_optimizations/loop_descriptions.cpp b/test/opt/loop_optimizations/loop_descriptions.cpp
index 94aff52..c55f126 100644
--- a/test/opt/loop_optimizations/loop_descriptions.cpp
+++ b/test/opt/loop_optimizations/loop_descriptions.cpp
@@ -292,7 +292,7 @@
EXPECT_NE(nullptr, module) << "Assembling failed for shader:\n"
<< text << std::endl;
const Function* f = spvtest::GetFunction(module, 4);
- LoopDescriptor ld{f};
+ LoopDescriptor ld{context.get(), f};
EXPECT_EQ(ld.NumLoops(), 0u);
}
@@ -368,7 +368,7 @@
EXPECT_NE(nullptr, module) << "Assembling failed for shader:\n"
<< text << std::endl;
const Function* f = spvtest::GetFunction(module, 2);
- LoopDescriptor ld{f};
+ LoopDescriptor ld{context.get(), f};
EXPECT_EQ(ld.NumLoops(), 1u);
diff --git a/test/opt/loop_optimizations/unroll_simple.cpp b/test/opt/loop_optimizations/unroll_simple.cpp
index a63252c..26cb07f 100644
--- a/test/opt/loop_optimizations/unroll_simple.cpp
+++ b/test/opt/loop_optimizations/unroll_simple.cpp
@@ -2941,7 +2941,7 @@
EXPECT_NE(nullptr, module) << "Assembling failed for shader:\n"
<< text << std::endl;
const Function* f = spvtest::GetFunction(module, 2);
- LoopDescriptor ld{f};
+ LoopDescriptor ld{context.get(), f};
EXPECT_EQ(ld.NumLoops(), 2u);