From 645ee1d9e09172e177399c87eb3c8343e3b04a7b Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Wed, 10 Aug 2016 10:02:28 -0400 Subject: [PATCH] Create an iterator class for in-memory representation. --- source/opt/function.h | 13 +- source/opt/ir_loader.cpp | 8 +- source/opt/iterator.h | 171 +++++++++++++++++++++++++ source/opt/module.h | 81 +++++++++--- source/opt/passes.cpp | 2 +- source/opt/type_manager.cpp | 5 +- test/opt/CMakeLists.txt | 5 + test/opt/test_iterator.cpp | 228 +++++++++++++++++++++++++++++++++ test/opt/test_pass_manager.cpp | 2 +- 9 files changed, 486 insertions(+), 29 deletions(-) create mode 100644 source/opt/iterator.h create mode 100644 test/opt/test_iterator.cpp diff --git a/source/opt/function.h b/source/opt/function.h index 424976856..70c7ff132 100644 --- a/source/opt/function.h +++ b/source/opt/function.h @@ -34,6 +34,7 @@ #include "basic_block.h" #include "instruction.h" +#include "iterator.h" namespace spvtools { namespace ir { @@ -43,6 +44,9 @@ class Module; // A SPIR-V function. class Function { public: + using iterator = UptrVectorIterator; + using const_iterator = UptrVectorIterator; + // Creates a function instance declared by the given instruction |def_inst|. Function(std::unique_ptr def_inst) : module_(nullptr), @@ -55,11 +59,10 @@ class Function { inline void AddParameter(std::unique_ptr p); // Appends a basic block to this function. inline void AddBasicBlock(std::unique_ptr b); - - const std::vector>& basic_blocks() const { - return blocks_; - } - std::vector>& basic_blocks() { return blocks_; } + iterator begin() { return iterator(&blocks_, blocks_.begin()); } + iterator end() { return iterator(&blocks_, blocks_.end()); } + const_iterator cbegin() { return const_iterator(&blocks_, blocks_.cbegin()); } + const_iterator cend() { return const_iterator(&blocks_, blocks_.cend()); } // Runs the given function |f| on each instruction in this basic block. void ForEachInst(const std::function& f); diff --git a/source/opt/ir_loader.cpp b/source/opt/ir_loader.cpp index 3f8153c7f..d1a50c1a3 100644 --- a/source/opt/ir_loader.cpp +++ b/source/opt/ir_loader.cpp @@ -106,11 +106,9 @@ void IrLoader::AddInstruction(const spv_parsed_instruction_t* inst) { // Resolves internal references among the module, functions, basic blocks, etc. // This function should be called after adding all instructions. void IrLoader::EndModule() { - for (auto& function : module_->functions()) { - for (auto& bb : function->basic_blocks()) { - bb->SetParent(function.get()); - } - function->SetParent(module_); + for (auto& function : *module_) { + for (auto& bb : function) bb.SetParent(&function); + function.SetParent(module_); } } diff --git a/source/opt/iterator.h b/source/opt/iterator.h new file mode 100644 index 000000000..bb52171fc --- /dev/null +++ b/source/opt/iterator.h @@ -0,0 +1,171 @@ +// Copyright (c) 2016 Google Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a +// copy of this software and/or associated documentation files (the +// "Materials"), to deal in the Materials without restriction, including +// without limitation the rights to use, copy, modify, merge, publish, +// distribute, sublicense, and/or sell copies of the Materials, and to +// permit persons to whom the Materials are furnished to do so, subject to +// the following conditions: +// +// The above copyright notice and this permission notice shall be included +// in all copies or substantial portions of the Materials. +// +// MODIFICATIONS TO THIS FILE MAY MEAN IT NO LONGER ACCURATELY REFLECTS +// KHRONOS STANDARDS. THE UNMODIFIED, NORMATIVE VERSIONS OF KHRONOS +// SPECIFICATIONS AND HEADER INFORMATION ARE LOCATED AT +// https://www.khronos.org/registry/ +// +// THE MATERIALS ARE PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, +// EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. +// IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY +// CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, +// TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE +// MATERIALS OR THE USE OR OTHER DEALINGS IN THE MATERIALS. + +#ifndef LIBSPIRV_OPT_ITERATOR_H_ +#define LIBSPIRV_OPT_ITERATOR_H_ + +#include +#include +#include + +namespace spvtools { +namespace ir { + +// An ad hoc iterator class for std::vector>. The +// purpose of this iterator class is to provide transparent access to those +// std::unique_ptr managed elements in the vector, behaving like we are using +// std::vector<|ValueType|>. +template +class UptrVectorIterator { + public: + using pointer = + typename std::conditional::type; + using reference = + typename std::conditional::type; + + // Type aliases. We need to apply constness properly if |IsConst| is true. + using Uptr = std::unique_ptr; + using UptrVector = typename std::conditional, + std::vector>::type; + using UnderlyingIterator = + typename std::conditional::type; + + // Creates a new iterator from the given |container| and its raw iterator + // |it|. + UptrVectorIterator(UptrVector* container, const UnderlyingIterator& it) + : container_(container), iterator_(it) {} + UptrVectorIterator(const UptrVectorIterator&) = default; + UptrVectorIterator& operator=(const UptrVectorIterator&) = default; + + inline UptrVectorIterator& operator++(); + inline UptrVectorIterator operator++(int); + inline UptrVectorIterator& operator--(); + inline UptrVectorIterator operator--(int); + + reference operator*() const { return **iterator_; } + pointer operator->() { return (*iterator_).get(); } + reference operator[](ptrdiff_t index) { return **(iterator_ + index); } + + inline bool operator==(const UptrVectorIterator& that) const; + inline bool operator!=(const UptrVectorIterator& that) const; + + inline ptrdiff_t operator-(const UptrVectorIterator& that) const; + inline bool operator<(const UptrVectorIterator& that) const; + + // Inserts the given |value| to the position pointed to by this iterator + // and returns an iterator to the newly iserted |value|. + // If the underlying vector changes capacity, all previous iterators will be + // invalidated. Otherwise, those previous iterators pointing to after the + // insertion point will be invalidated. + inline UptrVectorIterator InsertBefore(Uptr value); + + private: + UptrVector* container_; // The container we are manipulating. + UnderlyingIterator iterator_; // The raw iterator from the container. +}; + +// Handy class for a (begin, end) iterator pair. +template +class IteratorRange { + public: + IteratorRange(IteratorType b, IteratorType e) : begin_(b), end_(e) {} + + IteratorType begin() const { return begin_; } + IteratorType end() const { return end_; } + + bool empty() const { return begin_ == end_; } + size_t size() const { return end_ - begin_; } + + private: + IteratorType begin_; + IteratorType end_; +}; + +template +inline UptrVectorIterator& UptrVectorIterator::operator++() { + ++iterator_; + return *this; +} + +template +inline UptrVectorIterator UptrVectorIterator::operator++(int) { + auto it = *this; + ++(*this); + return it; +} + +template +inline UptrVectorIterator& UptrVectorIterator::operator--() { + --iterator_; + return *this; +} + +template +inline UptrVectorIterator UptrVectorIterator::operator--(int) { + auto it = *this; + --(*this); + return it; +} + +template +inline bool UptrVectorIterator::operator==( + const UptrVectorIterator& that) const { + return container_ == that.container_ && iterator_ == that.iterator_; +} + +template +inline bool UptrVectorIterator::operator!=( + const UptrVectorIterator& that) const { + return !(*this == that); +} + +template +inline ptrdiff_t UptrVectorIterator::operator-( + const UptrVectorIterator& that) const { + assert(container_ == that.container_); + return iterator_ - that.iterator_; +} + +template +inline bool UptrVectorIterator::operator<( + const UptrVectorIterator& that) const { + assert(container_ == that.container_); + return iterator_ < that.iterator_; +} + +template +inline UptrVectorIterator UptrVectorIterator::InsertBefore( + Uptr value) { + auto index = iterator_ - container_->begin(); + container_->insert(iterator_, std::move(value)); + return UptrVectorIterator(container_, container_->begin() + index); +} + +} // namespace ir +} // namespace spvtools + +#endif // LIBSPIRV_OPT_ITERATOR_H_ diff --git a/source/opt/module.h b/source/opt/module.h index a46329674..ac93acfc6 100644 --- a/source/opt/module.h +++ b/source/opt/module.h @@ -34,6 +34,7 @@ #include "function.h" #include "instruction.h" +#include "iterator.h" namespace spvtools { namespace ir { @@ -51,6 +52,11 @@ struct ModuleHeader { // serves as the backbone of optimization transformations. class Module { public: + using iterator = UptrVectorIterator; + using const_iterator = UptrVectorIterator; + using inst_iterator = UptrVectorIterator; + using const_inst_iterator = UptrVectorIterator; + // Creates an empty module with zero'd header. Module() : header_({}) {} @@ -85,22 +91,29 @@ class Module { // module. std::vector GetTypes(); std::vector GetTypes() const; - // Returns the constant-defining instructions. + // Returns a vector of pointers to constant-creation instructions in this + // module. std::vector GetConstants(); - const std::vector>& debugs() const { - return debugs_; - } - std::vector>& debugs() { return debugs_; } - const std::vector>& annotations() const { - return annotations_; - } - std::vector>& annotations() { - return annotations_; - } - const std::vector>& functions() const { - return functions_; - } - std::vector>& functions() { return functions_; } + + // Iterators for debug instructions (excluding OpLine & OpNoLine) contained in + // this module. + inline inst_iterator debug_begin(); + inline inst_iterator debug_end(); + inline IteratorRange debugs(); + inline IteratorRange debugs() const; + + // Clears all debug instructions (excluding OpLine & OpNoLine). + void debug_clear() { debugs_.clear(); } + + // Iterators for annotation instructions contained in this module. + IteratorRange annotations(); + IteratorRange annotations() const; + + // Iterators for functions contained in this module. + iterator begin() { return iterator(&functions_, functions_.begin()); } + iterator end() { return iterator(&functions_, functions_.end()); } + inline const_iterator cbegin() const; + inline const_iterator cend() const; // Invokes function |f| on all instructions in this module. void ForEachInst(const std::function& f); @@ -176,6 +189,44 @@ inline void Module::AddFunction(std::unique_ptr f) { functions_.emplace_back(std::move(f)); } +inline Module::inst_iterator Module::debug_begin() { + return inst_iterator(&debugs_, debugs_.begin()); +} +inline Module::inst_iterator Module::debug_end() { + return inst_iterator(&debugs_, debugs_.end()); +} + +inline IteratorRange Module::debugs() { + return IteratorRange(inst_iterator(&debugs_, debugs_.begin()), + inst_iterator(&debugs_, debugs_.end())); +} + +inline IteratorRange Module::debugs() const { + return IteratorRange( + const_inst_iterator(&debugs_, debugs_.cbegin()), + const_inst_iterator(&debugs_, debugs_.cend())); +} + +inline IteratorRange Module::annotations() { + return IteratorRange( + inst_iterator(&annotations_, annotations_.begin()), + inst_iterator(&annotations_, annotations_.end())); +} + +inline IteratorRange Module::annotations() const { + return IteratorRange( + const_inst_iterator(&annotations_, annotations_.cbegin()), + const_inst_iterator(&annotations_, annotations_.cend())); +} + +inline Module::const_iterator Module::cbegin() const { + return const_iterator(&functions_, functions_.cbegin()); +} + +inline Module::const_iterator Module::cend() const { + return const_iterator(&functions_, functions_.cend()); +} + } // namespace ir } // namespace spvtools diff --git a/source/opt/passes.cpp b/source/opt/passes.cpp index 63260d1c6..edc597696 100644 --- a/source/opt/passes.cpp +++ b/source/opt/passes.cpp @@ -39,7 +39,7 @@ namespace opt { bool StripDebugInfoPass::Process(ir::Module* module) { bool modified = !module->debugs().empty(); - module->debugs().clear(); + module->debug_clear(); module->ForEachInst([&modified](ir::Instruction* inst) { modified |= !inst->dbg_line_insts().empty(); diff --git a/source/opt/type_manager.cpp b/source/opt/type_manager.cpp index 351791c16..96229915b 100644 --- a/source/opt/type_manager.cpp +++ b/source/opt/type_manager.cpp @@ -53,10 +53,11 @@ ForwardPointer* TypeManager::GetForwardPointer(uint32_t index) const { void TypeManager::AnalyzeTypes(const spvtools::ir::Module& module) { for (const auto* inst : module.GetTypes()) RecordIfTypeDefinition(*inst); - for (const auto& inst : module.annotations()) AttachIfTypeDecoration(*inst); + for (const auto& inst : module.annotations()) AttachIfTypeDecoration(inst); } -Type* TypeManager::RecordIfTypeDefinition(const spvtools::ir::Instruction& inst) { +Type* TypeManager::RecordIfTypeDefinition( + const spvtools::ir::Instruction& inst) { if (!spvtools::ir::IsTypeInst(inst.opcode())) return nullptr; Type* type = nullptr; diff --git a/test/opt/CMakeLists.txt b/test/opt/CMakeLists.txt index 1167e0d0d..fc3d1028b 100644 --- a/test/opt/CMakeLists.txt +++ b/test/opt/CMakeLists.txt @@ -73,3 +73,8 @@ add_spvtools_unittest(TARGET type_manager SRCS test_type_manager.cpp LIBS SPIRV-Tools-opt ${SPIRV_TOOLS} ) + +add_spvtools_unittest(TARGET iterator + SRCS test_iterator.cpp + LIBS SPIRV-Tools-opt ${SPIRV_TOOLS} +) diff --git a/test/opt/test_iterator.cpp b/test/opt/test_iterator.cpp new file mode 100644 index 000000000..c67886f55 --- /dev/null +++ b/test/opt/test_iterator.cpp @@ -0,0 +1,228 @@ +// Copyright (c) 2016 Google Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a +// copy of this software and/or associated documentation files (the +// "Materials"), to deal in the Materials without restriction, including +// without limitation the rights to use, copy, modify, merge, publish, +// distribute, sublicense, and/or sell copies of the Materials, and to +// permit persons to whom the Materials are furnished to do so, subject to +// the following conditions: +// +// The above copyright notice and this permission notice shall be included +// in all copies or substantial portions of the Materials. +// +// MODIFICATIONS TO THIS FILE MAY MEAN IT NO LONGER ACCURATELY REFLECTS +// KHRONOS STANDARDS. THE UNMODIFIED, NORMATIVE VERSIONS OF KHRONOS +// SPECIFICATIONS AND HEADER INFORMATION ARE LOCATED AT +// https://www.khronos.org/registry/ +// +// THE MATERIALS ARE PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, +// EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. +// IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY +// CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, +// TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE +// MATERIALS OR THE USE OR OTHER DEALINGS IN THE MATERIALS. + +#include +#include + +#include "gmock/gmock.h" + +#include "opt/iterator.h" + +namespace { + +using namespace spvtools; +using ::testing::ContainerEq; + +TEST(Iterator, IncrementDeref) { + const int count = 100; + std::vector> data; + for (int i = 0; i < count; ++i) { + data.emplace_back(new int(i)); + } + + ir::UptrVectorIterator it(&data, data.begin()); + ir::UptrVectorIterator end(&data, data.end()); + + EXPECT_EQ(*data[0], *it); + for (int i = 1; i < count; ++i) { + EXPECT_NE(end, it); + EXPECT_EQ(*data[i], *(++it)); + } + EXPECT_EQ(end, ++it); +} + +TEST(Iterator, DecrementDeref) { + const int count = 100; + std::vector> data; + for (int i = 0; i < count; ++i) { + data.emplace_back(new int(i)); + } + + ir::UptrVectorIterator begin(&data, data.begin()); + ir::UptrVectorIterator it(&data, data.end()); + + for (int i = count - 1; i >= 0; --i) { + EXPECT_NE(begin, it); + EXPECT_EQ(*data[i], *(--it)); + } + EXPECT_EQ(begin, it); +} + +TEST(Iterator, PostIncrementDeref) { + const int count = 100; + std::vector> data; + for (int i = 0; i < count; ++i) { + data.emplace_back(new int(i)); + } + + ir::UptrVectorIterator it(&data, data.begin()); + ir::UptrVectorIterator end(&data, data.end()); + + for (int i = 0; i < count; ++i) { + EXPECT_NE(end, it); + EXPECT_EQ(*data[i], *(it++)); + } + EXPECT_EQ(end, it); +} + +TEST(Iterator, PostDecrementDeref) { + const int count = 100; + std::vector> data; + for (int i = 0; i < count; ++i) { + data.emplace_back(new int(i)); + } + + ir::UptrVectorIterator begin(&data, data.begin()); + ir::UptrVectorIterator end(&data, data.end()); + ir::UptrVectorIterator it(&data, data.end()); + + EXPECT_EQ(end, it--); + for (int i = count - 1; i >= 1; --i) { + EXPECT_EQ(*data[i], *(it--)); + } + // Decrementing .begin() is undefined behavior. + EXPECT_EQ(*data[0], *it); +} + +TEST(Iterator, Access) { + const int count = 100; + std::vector> data; + for (int i = 0; i < count; ++i) { + data.emplace_back(new int(i)); + } + + ir::UptrVectorIterator it(&data, data.begin()); + + for (int i = 0; i < count; ++i) EXPECT_EQ(*data[i], it[i]); +} + +TEST(Iterator, Comparison) { + const int count = 100; + std::vector> data; + for (int i = 0; i < count; ++i) { + data.emplace_back(new int(i)); + } + + ir::UptrVectorIterator it(&data, data.begin()); + ir::UptrVectorIterator end(&data, data.end()); + + for (int i = 0; i < count; ++i, ++it) EXPECT_TRUE(it < end); + EXPECT_EQ(end, it); +} + +TEST(Iterator, InsertBeginEnd) { + const int count = 100; + + std::vector> data; + std::vector expected; + std::vector actual; + + for (int i = 0; i < count; ++i) { + data.emplace_back(new int(i)); + expected.push_back(i); + } + + // Insert at the beginning + expected.insert(expected.begin(), -100); + ir::UptrVectorIterator begin(&data, data.begin()); + auto insert_point = begin.InsertBefore(std::unique_ptr(new int(-100))); + for (int i = 0; i < count + 1; ++i) { + actual.push_back(*(insert_point++)); + } + EXPECT_THAT(actual, ContainerEq(expected)); + + // Insert at the end + expected.push_back(-42); + expected.push_back(-36); + expected.push_back(-77); + ir::UptrVectorIterator end(&data, data.end()); + end = end.InsertBefore(std::unique_ptr(new int(-77))); + end = end.InsertBefore(std::unique_ptr(new int(-36))); + end = end.InsertBefore(std::unique_ptr(new int(-42))); + + actual.clear(); + begin = ir::UptrVectorIterator(&data, data.begin()); + for (int i = 0; i < count + 4; ++i) { + actual.push_back(*(begin++)); + } + EXPECT_THAT(actual, ContainerEq(expected)); +} + +TEST(Iterator, InsertMiddle) { + const int count = 100; + + std::vector> data; + std::vector expected; + std::vector actual; + + for (int i = 0; i < count; ++i) { + data.emplace_back(new int(i)); + expected.push_back(i); + } + + const int insert_pos = 42; + expected.insert(expected.begin() + insert_pos, -100); + expected.insert(expected.begin() + insert_pos, -42); + + ir::UptrVectorIterator it(&data, data.begin()); + for (int i = 0; i < insert_pos; ++i) ++it; + it = it.InsertBefore(std::unique_ptr(new int(-100))); + it = it.InsertBefore(std::unique_ptr(new int(-42))); + auto begin = ir::UptrVectorIterator(&data, data.begin()); + for (int i = 0; i < count + 2; ++i) { + actual.push_back(*(begin++)); + } + EXPECT_THAT(actual, ContainerEq(expected)); +} + +TEST(IteratorRange, Interface) { + const uint32_t count = 100; + + std::vector> data; + + for (uint32_t i = 0; i < count; ++i) { + data.emplace_back(new uint32_t(i)); + } + + auto b = ir::UptrVectorIterator(&data, data.begin()); + auto e = ir::UptrVectorIterator(&data, data.end()); + auto range = ir::IteratorRange(b, e); + + EXPECT_EQ(b, range.begin()); + EXPECT_EQ(e, range.end()); + EXPECT_FALSE(range.empty()); + EXPECT_EQ(count, range.size()); + EXPECT_EQ(0u, *range.begin()); + EXPECT_EQ(99u, *(--range.end())); + + // IteratorRange itself is immutable. + ++b, --e; + EXPECT_EQ(count, range.size()); + ++range.begin(), --range.end(); + EXPECT_EQ(count, range.size()); +} + +} // anonymous namespace diff --git a/test/opt/test_pass_manager.cpp b/test/opt/test_pass_manager.cpp index b5268ab02..5c48f8f62 100644 --- a/test/opt/test_pass_manager.cpp +++ b/test/opt/test_pass_manager.cpp @@ -65,7 +65,7 @@ class DuplicateInstPass : public opt::Pass { const char* name() const override { return "DuplicateInst"; } bool Process(ir::Module* module) override { std::unique_ptr inst( - new ir::Instruction(*module->debugs().back())); + new ir::Instruction(*(--module->debug_end()))); module->AddDebugInst(std::move(inst)); return true; }