GLSL: run OFI if any stage uses firstInstance.
Run the OffsetFirstIndex transform if either the vertex or fragment
stage uses instance_index.
This requires cooperation from Dawn, since Tint is unaware of the
other shader stages being compiled.
Bug: dawn:2185
Change-Id: Id6f019da2b9acb4bde0cca888920dc6e969109b0
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/173741
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Stephen White <senorblanco@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Kokoro: Stephen White <senorblanco@chromium.org>
diff --git a/src/dawn/native/opengl/ComputePipelineGL.cpp b/src/dawn/native/opengl/ComputePipelineGL.cpp
index eecab24..6dfa69a8 100644
--- a/src/dawn/native/opengl/ComputePipelineGL.cpp
+++ b/src/dawn/native/opengl/ComputePipelineGL.cpp
@@ -46,8 +46,8 @@
}
MaybeError ComputePipeline::InitializeImpl() {
- DAWN_TRY(
- InitializeBase(ToBackend(GetDevice())->GetGL(), ToBackend(GetLayout()), GetAllStages()));
+ DAWN_TRY(InitializeBase(ToBackend(GetDevice())->GetGL(), ToBackend(GetLayout()), GetAllStages(),
+ /* usesInstanceIndex */ false));
return {};
}
diff --git a/src/dawn/native/opengl/PipelineGL.cpp b/src/dawn/native/opengl/PipelineGL.cpp
index 6c5d12b..08c4ebe 100644
--- a/src/dawn/native/opengl/PipelineGL.cpp
+++ b/src/dawn/native/opengl/PipelineGL.cpp
@@ -52,7 +52,8 @@
MaybeError PipelineGL::InitializeBase(const OpenGLFunctions& gl,
const PipelineLayout* layout,
- const PerStage<ProgrammableStage>& stages) {
+ const PerStage<ProgrammableStage>& stages,
+ bool usesInstanceIndex) {
mProgram = gl.CreateProgram();
// Compute the set of active stages.
@@ -70,10 +71,11 @@
for (SingleShaderStage stage : IterateStages(activeStages)) {
const ShaderModule* module = ToBackend(stages[stage].module.Get());
GLuint shader;
- DAWN_TRY_ASSIGN(shader, module->CompileShader(
- gl, stages[stage], stage, &combinedSamplers[stage], layout,
- &needsPlaceholderSampler, &mNeedsTextureBuiltinUniformBuffer,
- &mBindingPointEmulatedBuiltins));
+ DAWN_TRY_ASSIGN(shader, module->CompileShader(gl, stages[stage], stage, usesInstanceIndex,
+ &combinedSamplers[stage], layout,
+ &needsPlaceholderSampler,
+ &mNeedsTextureBuiltinUniformBuffer,
+ &mBindingPointEmulatedBuiltins));
gl.AttachShader(mProgram, shader);
glShaders.push_back(shader);
}
diff --git a/src/dawn/native/opengl/PipelineGL.h b/src/dawn/native/opengl/PipelineGL.h
index b2a6591..60fc43f 100644
--- a/src/dawn/native/opengl/PipelineGL.h
+++ b/src/dawn/native/opengl/PipelineGL.h
@@ -73,7 +73,8 @@
void ApplyNow(const OpenGLFunctions& gl);
MaybeError InitializeBase(const OpenGLFunctions& gl,
const PipelineLayout* layout,
- const PerStage<ProgrammableStage>& stages);
+ const PerStage<ProgrammableStage>& stages,
+ bool usesInstanceIndex);
void DeleteProgram(const OpenGLFunctions& gl);
private:
diff --git a/src/dawn/native/opengl/RenderPipelineGL.cpp b/src/dawn/native/opengl/RenderPipelineGL.cpp
index aea3743..5819a32 100644
--- a/src/dawn/native/opengl/RenderPipelineGL.cpp
+++ b/src/dawn/native/opengl/RenderPipelineGL.cpp
@@ -256,8 +256,8 @@
mGlPrimitiveTopology(GLPrimitiveTopology(GetPrimitiveTopology())) {}
MaybeError RenderPipeline::InitializeImpl() {
- DAWN_TRY(
- InitializeBase(ToBackend(GetDevice())->GetGL(), ToBackend(GetLayout()), GetAllStages()));
+ DAWN_TRY(InitializeBase(ToBackend(GetDevice())->GetGL(), ToBackend(GetLayout()), GetAllStages(),
+ UsesInstanceIndex()));
CreateVAOForVertexState();
return {};
}
diff --git a/src/dawn/native/opengl/ShaderModuleGL.cpp b/src/dawn/native/opengl/ShaderModuleGL.cpp
index 77ad5ed..b4659b5 100644
--- a/src/dawn/native/opengl/ShaderModuleGL.cpp
+++ b/src/dawn/native/opengl/ShaderModuleGL.cpp
@@ -167,6 +167,7 @@
const OpenGLFunctions& gl,
const ProgrammableStage& programmableStage,
SingleShaderStage stage,
+ bool usesInstanceIndex,
CombinedSamplerInfo* combinedSamplers,
const PipelineLayout* layout,
bool* needsPlaceholderSampler,
@@ -287,6 +288,12 @@
version.GetMajor(), version.GetMinor());
req.tintOptions.disable_robustness = false;
+
+ if (usesInstanceIndex) {
+ req.tintOptions.first_instance_offset =
+ 4 * PipelineLayout::PushConstantLocation::FirstInstance;
+ }
+
req.disableSymbolRenaming = GetDevice()->IsToggleEnabled(Toggle::DisableSymbolRenaming);
req.interstageVariables = {};
@@ -418,9 +425,6 @@
/* fullSubgroups */ {}));
}
- r.tintOptions.first_instance_offset =
- 4 * PipelineLayout::PushConstantLocation::FirstInstance;
-
auto result = tint::glsl::writer::Generate(program, r.tintOptions, remappedEntryPoint);
DAWN_INVALID_IF(result != tint::Success, "An error occurred while generating GLSL:\n%s",
result.Failure().reason.Str());
diff --git a/src/dawn/native/opengl/ShaderModuleGL.h b/src/dawn/native/opengl/ShaderModuleGL.h
index a452b2fc..1a3b3a1 100644
--- a/src/dawn/native/opengl/ShaderModuleGL.h
+++ b/src/dawn/native/opengl/ShaderModuleGL.h
@@ -89,6 +89,7 @@
ResultOrError<GLuint> CompileShader(const OpenGLFunctions& gl,
const ProgrammableStage& programmableStage,
SingleShaderStage stage,
+ bool usesInstanceIndex,
CombinedSamplerInfo* combinedSamplers,
const PipelineLayout* layout,
bool* needsPlaceholderSampler,
diff --git a/src/tint/cmd/tint/main.cc b/src/tint/cmd/tint/main.cc
index 81a9e5b..85d8d3e 100644
--- a/src/tint/cmd/tint/main.cc
+++ b/src/tint/cmd/tint/main.cc
@@ -1067,9 +1067,13 @@
gen_options.texture_builtins_from_uniform = std::move(textureBuiltinsFromUniform);
- // Place the first_instance push constant member after user-defined push constants (if any).
- gen_options.first_instance_offset =
- inspector.GetEntryPoint(entry_point_name).push_constant_size;
+ auto entry_point = inspector.GetEntryPoint(entry_point_name);
+
+ if (entry_point.instance_index_used) {
+ // Place the first_instance push constant member after user-defined push constants (if
+ // any).
+ gen_options.first_instance_offset = entry_point.push_constant_size;
+ }
auto result = tint::glsl::writer::Generate(prg, gen_options, entry_point_name);
if (result != tint::Success) {
diff --git a/src/tint/lang/wgsl/ast/transform/offset_first_index.cc b/src/tint/lang/wgsl/ast/transform/offset_first_index.cc
index 04bdd9a..143b43f 100644
--- a/src/tint/lang/wgsl/ast/transform/offset_first_index.cc
+++ b/src/tint/lang/wgsl/ast/transform/offset_first_index.cc
@@ -53,15 +53,6 @@
constexpr char kFirstVertexName[] = "first_vertex";
constexpr char kFirstInstanceName[] = "first_instance";
-bool ShouldRun(const Program& program) {
- for (auto* fn : program.AST().Functions()) {
- if (fn->PipelineStage() == PipelineStage::kVertex) {
- return true;
- }
- }
- return false;
-}
-
} // namespace
OffsetFirstIndex::OffsetFirstIndex() = default;
@@ -70,14 +61,13 @@
Transform::ApplyResult OffsetFirstIndex::Apply(const Program& src,
const DataMap& inputs,
DataMap&) const {
- if (!ShouldRun(src)) {
- return SkipTransform;
- }
-
const Config* cfg = inputs.Get<Config>();
if (!cfg) {
return SkipTransform;
}
+ if (!cfg->first_vertex_offset.has_value() && !cfg->first_instance_offset.has_value()) {
+ return SkipTransform;
+ }
ProgramBuilder b;
program::CloneContext ctx{&b, &src, /* auto_clone_symbols */ true};
@@ -86,9 +76,6 @@
std::unordered_map<const sem::Variable*, const char*> builtin_vars;
std::unordered_map<const core::type::StructMember*, const char*> builtin_members;
- bool has_vertex_index = false;
- bool has_instance_index = false;
-
// Traverse the AST scanning for builtin accesses via variables (includes
// parameters) or structure member accesses.
for (auto* node : ctx.src->ASTNodes().Objects()) {
@@ -100,13 +87,11 @@
cfg->first_vertex_offset.has_value()) {
auto* sem_var = ctx.src->Sem().Get(var);
builtin_vars.emplace(sem_var, kFirstVertexName);
- has_vertex_index = true;
}
if (builtin == core::BuiltinValue::kInstanceIndex && cfg &&
cfg->first_instance_offset.has_value()) {
auto* sem_var = ctx.src->Sem().Get(var);
builtin_vars.emplace(sem_var, kFirstInstanceName);
- has_instance_index = true;
}
}
}
@@ -119,23 +104,17 @@
cfg->first_vertex_offset.has_value()) {
auto* sem_mem = ctx.src->Sem().Get(member);
builtin_members.emplace(sem_mem, kFirstVertexName);
- has_vertex_index = true;
}
if (builtin == core::BuiltinValue::kInstanceIndex && cfg &&
cfg->first_instance_offset.has_value()) {
auto* sem_mem = ctx.src->Sem().Get(member);
builtin_members.emplace(sem_mem, kFirstInstanceName);
- has_instance_index = true;
}
}
}
}
}
- if (!has_vertex_index && !has_instance_index) {
- return SkipTransform;
- }
-
Vector<const ast::StructMember*, 8> members;
const ast::Variable* push_constants_var = nullptr;
@@ -159,11 +138,11 @@
}
// Add push constant members and calculate byte offsets
- if (has_vertex_index) {
+ if (cfg->first_vertex_offset.has_value()) {
members.Push(b.Member(kFirstVertexName, b.ty.u32(),
Vector{b.MemberOffset(AInt(*cfg->first_vertex_offset))}));
}
- if (has_instance_index) {
+ if (cfg->first_instance_offset.has_value()) {
members.Push(b.Member(kFirstInstanceName, b.ty.u32(),
Vector{b.MemberOffset(AInt(*cfg->first_instance_offset))}));
}
diff --git a/src/tint/lang/wgsl/ast/transform/offset_first_index_test.cc b/src/tint/lang/wgsl/ast/transform/offset_first_index_test.cc
index 7d026ed..09dd505 100644
--- a/src/tint/lang/wgsl/ast/transform/offset_first_index_test.cc
+++ b/src/tint/lang/wgsl/ast/transform/offset_first_index_test.cc
@@ -41,12 +41,49 @@
TEST_F(OffsetFirstIndexTest, ShouldRunEmptyModule) {
auto* src = R"()";
- DataMap config;
- config.Add<OffsetFirstIndex::Config>(0, 4);
- EXPECT_FALSE(ShouldRun<OffsetFirstIndex>(src, config));
+ EXPECT_FALSE(ShouldRun<OffsetFirstIndex>(src));
}
-TEST_F(OffsetFirstIndexTest, ShouldRunFragmentStage) {
+TEST_F(OffsetFirstIndexTest, ShouldRunNoVertexIndexNoInstanceIndex) {
+ auto* src = R"(
+@fragment
+fn entry() {
+ return;
+}
+)";
+
+ DataMap config;
+ config.Add<OffsetFirstIndex::Config>(std::nullopt, std::nullopt);
+ EXPECT_FALSE(ShouldRun<OffsetFirstIndex>(src, std::move(config)));
+}
+
+TEST_F(OffsetFirstIndexTest, ShouldRunNoVertexIndex) {
+ auto* src = R"(
+@fragment
+fn entry() {
+ return;
+}
+)";
+
+ DataMap config;
+ config.Add<OffsetFirstIndex::Config>(std::nullopt, 0);
+ EXPECT_TRUE(ShouldRun<OffsetFirstIndex>(src, std::move(config)));
+}
+
+TEST_F(OffsetFirstIndexTest, ShouldRunNoInstanceIndex) {
+ auto* src = R"(
+@fragment
+fn entry() {
+ return;
+}
+)";
+
+ DataMap config;
+ config.Add<OffsetFirstIndex::Config>(0, std::nullopt);
+ EXPECT_TRUE(ShouldRun<OffsetFirstIndex>(src, std::move(config)));
+}
+
+TEST_F(OffsetFirstIndexTest, ShouldRun) {
auto* src = R"(
@fragment
fn entry() {
@@ -56,20 +93,7 @@
DataMap config;
config.Add<OffsetFirstIndex::Config>(0, 4);
- EXPECT_FALSE(ShouldRun<OffsetFirstIndex>(src, config));
-}
-
-TEST_F(OffsetFirstIndexTest, ShouldRunVertexStage) {
- auto* src = R"(
-@vertex
-fn entry() -> @builtin(position) vec4<f32> {
- return vec4<f32>();
-}
-)";
-
- DataMap config;
- config.Add<OffsetFirstIndex::Config>(0, 4);
- EXPECT_FALSE(ShouldRun<OffsetFirstIndex>(src, config));
+ EXPECT_TRUE(ShouldRun<OffsetFirstIndex>(src, std::move(config)));
}
TEST_F(OffsetFirstIndexTest, ShouldRunVertexStageWithVertexIndex) {
@@ -82,7 +106,7 @@
DataMap config;
config.Add<OffsetFirstIndex::Config>(0, 4);
- EXPECT_TRUE(ShouldRun<OffsetFirstIndex>(src, config));
+ EXPECT_TRUE(ShouldRun<OffsetFirstIndex>(src, std::move(config)));
}
TEST_F(OffsetFirstIndexTest, ShouldRunVertexStageWithInstanceIndex) {
@@ -95,16 +119,14 @@
DataMap config;
config.Add<OffsetFirstIndex::Config>(0, 4);
- EXPECT_TRUE(ShouldRun<OffsetFirstIndex>(src, config));
+ EXPECT_TRUE(ShouldRun<OffsetFirstIndex>(src, std::move(config)));
}
TEST_F(OffsetFirstIndexTest, EmptyModule) {
auto* src = "";
auto* expect = "";
- DataMap config;
- config.Add<OffsetFirstIndex::Config>(0, 4);
- auto got = Run<OffsetFirstIndex>(src, std::move(config));
+ auto got = Run<OffsetFirstIndex>(src);
EXPECT_EQ(expect, str(got));
}
@@ -118,9 +140,7 @@
)";
auto* expect = src;
- DataMap config;
- config.Add<OffsetFirstIndex::Config>(0, 4);
- auto got = Run<OffsetFirstIndex>(src, std::move(config));
+ auto got = Run<OffsetFirstIndex>(src);
EXPECT_EQ(expect, str(got));
}
@@ -160,7 +180,7 @@
)";
DataMap config;
- config.Add<OffsetFirstIndex::Config>(0, 4);
+ config.Add<OffsetFirstIndex::Config>(0, std::nullopt);
auto got = Run<OffsetFirstIndex>(src, std::move(config));
EXPECT_EQ(expect, str(got));
@@ -201,7 +221,7 @@
)";
DataMap config;
- config.Add<OffsetFirstIndex::Config>(0, 4);
+ config.Add<OffsetFirstIndex::Config>(0, std::nullopt);
auto got = Run<OffsetFirstIndex>(src, std::move(config));
EXPECT_EQ(expect, str(got));
@@ -244,7 +264,7 @@
)";
DataMap config;
- config.Add<OffsetFirstIndex::Config>(0, 4);
+ config.Add<OffsetFirstIndex::Config>(std::nullopt, 4);
auto got = Run<OffsetFirstIndex>(src, std::move(config));
EXPECT_EQ(expect, str(got));
@@ -287,7 +307,7 @@
)";
DataMap config;
- config.Add<OffsetFirstIndex::Config>(0, 4);
+ config.Add<OffsetFirstIndex::Config>(std::nullopt, 4);
auto got = Run<OffsetFirstIndex>(src, std::move(config));
EXPECT_EQ(expect, str(got));
@@ -577,7 +597,7 @@
)";
DataMap config;
- config.Add<OffsetFirstIndex::Config>(0, 4);
+ config.Add<OffsetFirstIndex::Config>(0, std::nullopt);
auto got = Run<OffsetFirstIndex>(src, std::move(config));
EXPECT_EQ(expect, str(got));
@@ -614,7 +634,7 @@
)";
DataMap config;
- config.Add<OffsetFirstIndex::Config>(0, 4);
+ config.Add<OffsetFirstIndex::Config>(0, std::nullopt);
auto got = Run<OffsetFirstIndex>(src, std::move(config));
EXPECT_EQ(expect, str(got));
@@ -663,7 +683,7 @@
)";
DataMap config;
- config.Add<OffsetFirstIndex::Config>(0, 4);
+ config.Add<OffsetFirstIndex::Config>(0, std::nullopt);
auto got = Run<OffsetFirstIndex>(src, std::move(config));
EXPECT_EQ(expect, str(got));
@@ -712,7 +732,7 @@
)";
DataMap config;
- config.Add<OffsetFirstIndex::Config>(0, 4);
+ config.Add<OffsetFirstIndex::Config>(0, std::nullopt);
auto got = Run<OffsetFirstIndex>(src, std::move(config));
EXPECT_EQ(expect, str(got));
