Skip to content

Commit 5d3c8b7

Browse files
authored
opt: Add OpEntryPoint to DescriptorScalarReplacement pass (KhronosGroup#5553)
Add OpEntryPoint to the list of instructions processed by the DescriptorScalarReplacement pass. This is necessary for SPIR-V 1.4 and above where global variables must be included in the interface. Fixes microsoft/DirectXShaderCompiler#5962
1 parent de65e81 commit 5d3c8b7

File tree

3 files changed

+120
-2
lines changed

3 files changed

+120
-2
lines changed

source/opt/desc_sroa.cpp

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,10 @@ Pass::Status DescriptorScalarReplacement::Process() {
5454
bool DescriptorScalarReplacement::ReplaceCandidate(Instruction* var) {
5555
std::vector<Instruction*> access_chain_work_list;
5656
std::vector<Instruction*> load_work_list;
57+
std::vector<Instruction*> entry_point_work_list;
5758
bool failed = !get_def_use_mgr()->WhileEachUser(
58-
var->result_id(),
59-
[this, &access_chain_work_list, &load_work_list](Instruction* use) {
59+
var->result_id(), [this, &access_chain_work_list, &load_work_list,
60+
&entry_point_work_list](Instruction* use) {
6061
if (use->opcode() == spv::Op::OpName) {
6162
return true;
6263
}
@@ -73,6 +74,9 @@ bool DescriptorScalarReplacement::ReplaceCandidate(Instruction* var) {
7374
case spv::Op::OpLoad:
7475
load_work_list.push_back(use);
7576
return true;
77+
case spv::Op::OpEntryPoint:
78+
entry_point_work_list.push_back(use);
79+
return true;
7680
default:
7781
context()->EmitErrorMessage(
7882
"Variable cannot be replaced: invalid instruction", use);
@@ -95,6 +99,11 @@ bool DescriptorScalarReplacement::ReplaceCandidate(Instruction* var) {
9599
return false;
96100
}
97101
}
102+
for (Instruction* use : entry_point_work_list) {
103+
if (!ReplaceEntryPoint(var, use)) {
104+
return false;
105+
}
106+
}
98107
return true;
99108
}
100109

@@ -147,6 +156,42 @@ bool DescriptorScalarReplacement::ReplaceAccessChain(Instruction* var,
147156
return true;
148157
}
149158

159+
bool DescriptorScalarReplacement::ReplaceEntryPoint(Instruction* var,
160+
Instruction* use) {
161+
// Build a new |OperandList| for |use| that removes |var| and adds its
162+
// replacement variables.
163+
Instruction::OperandList new_operands;
164+
165+
// Copy all operands except |var|.
166+
bool found = false;
167+
for (uint32_t idx = 0; idx < use->NumOperands(); idx++) {
168+
Operand& op = use->GetOperand(idx);
169+
if (op.type == SPV_OPERAND_TYPE_ID && op.words[0] == var->result_id()) {
170+
found = true;
171+
} else {
172+
new_operands.emplace_back(op);
173+
}
174+
}
175+
176+
if (!found) {
177+
context()->EmitErrorMessage(
178+
"Variable cannot be replaced: invalid instruction", use);
179+
return false;
180+
}
181+
182+
// Add all new replacement variables.
183+
uint32_t num_replacement_vars =
184+
descsroautil::GetNumberOfElementsForArrayOrStruct(context(), var);
185+
for (uint32_t i = 0; i < num_replacement_vars; i++) {
186+
new_operands.push_back(
187+
{SPV_OPERAND_TYPE_ID, {GetReplacementVariable(var, i)}});
188+
}
189+
190+
use->ReplaceOperands(new_operands);
191+
context()->UpdateDefUse(use);
192+
return true;
193+
}
194+
150195
uint32_t DescriptorScalarReplacement::GetReplacementVariable(Instruction* var,
151196
uint32_t idx) {
152197
auto replacement_vars = replacement_variables_.find(var);

source/opt/desc_sroa.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,11 @@ class DescriptorScalarReplacement : public Pass {
6464
// otherwise.
6565
bool ReplaceLoadedValue(Instruction* var, Instruction* value);
6666

67+
// Replaces the given composite variable |var| in the OpEntryPoint with the
68+
// new replacement variables, one for each element of the array |var|. Returns
69+
// |true| if successful, and |false| otherwise.
70+
bool ReplaceEntryPoint(Instruction* var, Instruction* use);
71+
6772
// Replaces the given OpCompositeExtract |extract| and all of its references
6873
// with an OpLoad of a replacement variable. |var| is the variable with
6974
// composite type whose value is being used by |extract|. Assumes that

test/opt/desc_sroa_test.cpp

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -918,6 +918,74 @@ TEST_F(DescriptorScalarReplacementTest, DecorateStringForReflect) {
918918
SinglePassRunAndMatch<DescriptorScalarReplacement>(shader, true);
919919
}
920920

921+
TEST_F(DescriptorScalarReplacementTest, ExpandArrayInOpEntryPoint) {
922+
const std::string text = R"(; SPIR-V
923+
; Version: 1.6
924+
; Bound: 31
925+
; Schema: 0
926+
OpCapability Shader
927+
OpMemoryModel Logical GLSL450
928+
929+
; CHECK: OpEntryPoint GLCompute %main "main" %output_0_ %output_1_
930+
931+
OpEntryPoint GLCompute %main "main" %output
932+
OpExecutionMode %main LocalSize 1 1 1
933+
OpSource HLSL 670
934+
OpName %type_RWByteAddressBuffer "type.RWByteAddressBuffer"
935+
OpName %output "output"
936+
OpName %main "main"
937+
OpName %src_main "src.main"
938+
OpName %bb_entry "bb.entry"
939+
940+
; CHECK: OpDecorate %output_1_ DescriptorSet 0
941+
; CHECK: OpDecorate %output_1_ Binding 1
942+
; CHECK: OpDecorate %output_0_ DescriptorSet 0
943+
; CHECK: OpDecorate %output_0_ Binding 0
944+
945+
OpDecorate %output DescriptorSet 0
946+
OpDecorate %output Binding 0
947+
948+
OpDecorate %_runtimearr_uint ArrayStride 4
949+
OpMemberDecorate %type_RWByteAddressBuffer 0 Offset 0
950+
OpDecorate %type_RWByteAddressBuffer Block
951+
%int = OpTypeInt 32 1
952+
%int_1 = OpConstant %int 1
953+
%uint = OpTypeInt 32 0
954+
%uint_0 = OpConstant %uint 0
955+
%uint_2 = OpConstant %uint 2
956+
%uint_32 = OpConstant %uint 32
957+
%_runtimearr_uint = OpTypeRuntimeArray %uint
958+
%type_RWByteAddressBuffer = OpTypeStruct %_runtimearr_uint
959+
%_arr_type_RWByteAddressBuffer_uint_2 = OpTypeArray %type_RWByteAddressBuffer %uint_2
960+
%_ptr_StorageBuffer__arr_type_RWByteAddressBuffer_uint_2 = OpTypePointer StorageBuffer %_arr_type_RWByteAddressBuffer_uint_2
961+
%void = OpTypeVoid
962+
%23 = OpTypeFunction %void
963+
%_ptr_StorageBuffer_type_RWByteAddressBuffer = OpTypePointer StorageBuffer %type_RWByteAddressBuffer
964+
%_ptr_StorageBuffer_uint = OpTypePointer StorageBuffer %uint
965+
966+
; CHECK: %output_1_ = OpVariable %_ptr_StorageBuffer_type_RWByteAddressBuffer StorageBuffer
967+
; CHECK: %output_0_ = OpVariable %_ptr_StorageBuffer_type_RWByteAddressBuffer StorageBuffer
968+
969+
%output = OpVariable %_ptr_StorageBuffer__arr_type_RWByteAddressBuffer_uint_2 StorageBuffer
970+
971+
%main = OpFunction %void None %23
972+
%26 = OpLabel
973+
%27 = OpFunctionCall %void %src_main
974+
OpReturn
975+
OpFunctionEnd
976+
%src_main = OpFunction %void None %23
977+
%bb_entry = OpLabel
978+
%28 = OpAccessChain %_ptr_StorageBuffer_type_RWByteAddressBuffer %output %int_1
979+
%29 = OpShiftRightLogical %uint %uint_0 %uint_2
980+
%30 = OpAccessChain %_ptr_StorageBuffer_uint %28 %uint_0 %29
981+
OpStore %30 %uint_32
982+
OpReturn
983+
OpFunctionEnd
984+
)";
985+
986+
SinglePassRunAndMatch<DescriptorScalarReplacement>(text, false);
987+
}
988+
921989
} // namespace
922990
} // namespace opt
923991
} // namespace spvtools

0 commit comments

Comments
 (0)