Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

spirv-opt: A pass repairs interfaces of OpEntryPoint instructions. #4275

Merged
merged 9 commits into from
Jun 29, 2021
Prev Previous commit
Next Next commit
Change name of the pass to "remove-unused-interface-variables"
  • Loading branch information
thatname authored and s-perron committed Jun 29, 2021
commit 25982c8de833b0927e1f1b62a90305fb4f3ac3a0
10 changes: 6 additions & 4 deletions include/spirv-tools/optimizer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -514,10 +514,12 @@ Optimizer::PassToken CreateDeadInsertElimPass();
// eliminated with standard dead code elimination.
Optimizer::PassToken CreateAggressiveDCEPass();

// Interfaces of entry points may list incorrect or suboptimal global-IDs.
// For example, DXC generated SPIR-V often list more than necessary.
// This pass can traverse OpEntryPoint instructions and fix.
Optimizer::PassToken CreateInterfaceRepairPass();
// Creates a remove-unused-interface-variables pass.
// Removes variables referenced on the |OpEntryPoint| instruction that are not
// referenced in the entry point function or any function in its call tree. Note
// that this could cause the shader interface to no longer match other shader
// stages.
Optimizer::PassToken CreateRemoveUnusedInterfaceVariablesPass();

// Creates an empty pass.
// This is deprecated and will be removed.
Expand Down
4 changes: 2 additions & 2 deletions source/opt/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ set(SPIRV_TOOLS_OPT_SOURCES
instruction.h
instruction_list.h
instrument_pass.h
interface_repair_pass.h
interp_fixup_pass.h
ir_builder.h
ir_context.h
Expand Down Expand Up @@ -97,6 +96,7 @@ set(SPIRV_TOOLS_OPT_SOURCES
register_pressure.h
relax_float_ops_pass.h
remove_duplicates_pass.h
remove_unused_interface_variables_pass.h
replace_invalid_opc.h
scalar_analysis.h
scalar_analysis_nodes.h
Expand Down Expand Up @@ -167,7 +167,6 @@ set(SPIRV_TOOLS_OPT_SOURCES
instruction.cpp
instruction_list.cpp
instrument_pass.cpp
interface_repair_pass.cpp
interp_fixup_pass.cpp
ir_context.cpp
ir_loader.cpp
Expand Down Expand Up @@ -199,6 +198,7 @@ set(SPIRV_TOOLS_OPT_SOURCES
register_pressure.cpp
relax_float_ops_pass.cpp
remove_duplicates_pass.cpp
remove_unused_interface_variables_pass.cpp
replace_invalid_opc.cpp
scalar_analysis.cpp
scalar_analysis_simplification.cpp
Expand Down
8 changes: 4 additions & 4 deletions source/opt/optimizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -489,8 +489,8 @@ bool Optimizer::RegisterPassFromFlag(const std::string& flag) {
RegisterSizePasses();
} else if (pass_name == "legalize-hlsl") {
RegisterLegalizationPasses();
} else if (pass_name == "interface-repair") {
RegisterPass(CreateInterfaceRepairPass());
} else if (pass_name == "remove-unused-interface-variables") {
RegisterPass(CreateRemoveUnusedInterfaceVariablesPass());
} else if (pass_name == "graphics-robust-access") {
RegisterPass(CreateGraphicsRobustAccessPass());
} else if (pass_name == "wrap-opkill") {
Expand Down Expand Up @@ -731,9 +731,9 @@ Optimizer::PassToken CreateAggressiveDCEPass() {
MakeUnique<opt::AggressiveDCEPass>());
}

Optimizer::PassToken CreateInterfaceRepairPass() {
Optimizer::PassToken CreateRemoveUnusedInterfaceVariablesPass() {
return MakeUnique<Optimizer::PassToken::Impl>(
MakeUnique<opt::InterfaceRepairPass>());
MakeUnique<opt::RemoveUnusedInterfaceVariablesPass>());
}

Optimizer::PassToken CreatePropagateLineInfoPass() {
Expand Down
2 changes: 1 addition & 1 deletion source/opt/passes.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
#include "source/opt/inst_bindless_check_pass.h"
#include "source/opt/inst_buff_addr_check_pass.h"
#include "source/opt/inst_debug_printf_pass.h"
#include "source/opt/interface_repair_pass.h"
#include "source/opt/interp_fixup_pass.h"
#include "source/opt/licm_pass.h"
#include "source/opt/local_access_chain_convert_pass.h"
Expand All @@ -65,6 +64,7 @@
#include "source/opt/redundancy_elimination.h"
#include "source/opt/relax_float_ops_pass.h"
#include "source/opt/remove_duplicates_pass.h"
#include "source/opt/remove_unused_interface_variables_pass.h"
#include "source/opt/replace_invalid_opc.h"
#include "source/opt/scalar_replacement_pass.h"
#include "source/opt/set_spec_constant_default_value_pass.h"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,18 @@
// See the License for the specific language governing permissions and
// limitations under the License.

#include "interface_repair_pass.h"
#include "remove_unused_interface_variables_pass.h"
#include "source/spirv_constant.h"
namespace spvtools {
namespace opt {

class EntryRepairContext {
InterfaceRepairPass& parent_;
class RemoveUnusedInterfaceVariablesContext {
RemoveUnusedInterfaceVariablesPass& parent_;
Instruction& entry_;
std::unordered_set<uint32_t> used_variables_;
IRContext::ProcessFunction pfn_ = std::bind(
&EntryRepairContext::processFunction, this, std::placeholders::_1);
IRContext::ProcessFunction pfn_ =
std::bind(&RemoveUnusedInterfaceVariablesContext::processFunction, this,
std::placeholders::_1);

bool processFunction(Function* func) {
for (const auto& basic_block : *func)
Expand All @@ -43,7 +44,8 @@ class EntryRepairContext {
}

public:
EntryRepairContext(InterfaceRepairPass& parent, Instruction& entry)
RemoveUnusedInterfaceVariablesContext(
RemoveUnusedInterfaceVariablesPass& parent, Instruction& entry)
: parent_(parent), entry_(entry) {}

void CollectUsedVariables() {
Expand All @@ -52,7 +54,7 @@ class EntryRepairContext {
parent_.context()->ProcessCallTreeFromRoots(pfn_, &roots);
}

bool ShouldRepair() {
bool ShouldModify() {
std::unordered_set<uint32_t> old_variables;
for (int i = entry_.NumInOperands() - 1; i >= 3; --i) {
auto variable = entry_.GetInOperand(i).words[0];
Expand All @@ -65,7 +67,7 @@ class EntryRepairContext {
return false;
}

void Repair() {
void Modify() {
for (int i = entry_.NumInOperands() - 1; i >= 3; --i)
entry_.RemoveInOperand(i);
for (auto id : used_variables_) {
Expand All @@ -74,13 +76,14 @@ class EntryRepairContext {
}
};

InterfaceRepairPass::Status InterfaceRepairPass::Process() {
RemoveUnusedInterfaceVariablesPass::Status
RemoveUnusedInterfaceVariablesPass::Process() {
bool modified = false;
for (auto& entry : get_module()->entry_points()) {
EntryRepairContext context(*this, entry);
RemoveUnusedInterfaceVariablesContext context(*this, entry);
context.CollectUsedVariables();
if (context.ShouldRepair()) {
context.Repair();
if (context.ShouldModify()) {
context.Modify();
modified = true;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
namespace spvtools {
namespace opt {

class InterfaceRepairPass : public Pass {
const char* name() const override { return "repair-interface-pass"; }
class RemoveUnusedInterfaceVariablesPass : public Pass {
const char* name() const override { return "remove-unused-interface-variables-pass"; }
Status Process() override;
};
} // namespace opt
Expand Down
2 changes: 1 addition & 1 deletion test/opt/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ add_spvtools_unittest(TARGET opt
inst_debug_printf_test.cpp
instruction_list_test.cpp
instruction_test.cpp
interface_repair_test.cpp
interp_fixup_test.cpp
ir_builder.cpp
ir_context_test.cpp
Expand All @@ -80,6 +79,7 @@ add_spvtools_unittest(TARGET opt
propagator_test.cpp
reduce_load_size_test.cpp
redundancy_elimination_test.cpp
remove_unused_interface_variables_test.cpp
register_liveness.cpp
relax_float_ops_test.cpp
replace_invalid_opc_test.cpp
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ namespace spvtools {
namespace opt {
namespace {

using InterfaceRepairTest = PassTest<::testing::Test>;
using RemoveUnusedInterfaceVariablesTest = PassTest<::testing::Test>;

static const std::string expected = R"(OpCapability Shader
OpMemoryModel Logical GLSL450
Expand Down Expand Up @@ -72,7 +72,7 @@ OpReturn
OpFunctionEnd
)";

TEST_F(InterfaceRepairTest, RemoveUnusedVariable) {
TEST_F(RemoveUnusedInterfaceVariablesTest, RemoveUnusedVariable) {
const std::string text = R"(OpCapability Shader
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %_Z5func1v "_Z5func1v" %out_var_SV_TARGET %out_var_SV_TARGET_0
Expand Down Expand Up @@ -122,10 +122,11 @@ OpReturn
OpFunctionEnd
)";

SinglePassRunAndCheck<InterfaceRepairPass>(text, expected, true, true);
SinglePassRunAndCheck<RemoveUnusedInterfaceVariablesPass>(text, expected,
true, true);
}

TEST_F(InterfaceRepairTest, FixMissingVariable) {
TEST_F(RemoveUnusedInterfaceVariablesTest, FixMissingVariable) {
const std::string text = R"(OpCapability Shader
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %_Z5func1v "_Z5func1v"
Expand Down Expand Up @@ -175,7 +176,8 @@ OpReturn
OpFunctionEnd
)";

SinglePassRunAndCheck<InterfaceRepairPass>(text, expected, true, true);
SinglePassRunAndCheck<RemoveUnusedInterfaceVariablesPass>(text, expected,
true, true);
}

} // namespace
Expand Down
6 changes: 6 additions & 0 deletions tools/opt/opt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,12 @@ Options (in lexicographical order):)",
Removes duplicate types, decorations, capabilities and extension
instructions.)");
printf(R"(
--remove-unused-interface-variables
Removes variables referenced on the |OpEntryPoint| instruction
that are not referenced in the entry point function or any function
in its call tree. Note that this could cause the shader interface
to no longer match other shader stages.)");
printf(R"(
--replace-invalid-opcode
Replaces instructions whose opcode is valid for shader modules,
but not for the current shader stage. To have an effect, all
Expand Down