[glsl] Match parameter order in CombineSamplers
The code to generate parameters processes all pairs that have a
sampler first, then all pairs that do not have a sampler. Do the same
when generating arguments for function calls to ensure that the order
matches.
Change-Id: I22cda5553134ff9f8a13a111c7f146ba366fa033
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/166180
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
diff --git a/src/tint/lang/glsl/writer/ast_raise/combine_samplers.cc b/src/tint/lang/glsl/writer/ast_raise/combine_samplers.cc
index b056310..819241a 100644
--- a/src/tint/lang/glsl/writer/ast_raise/combine_samplers.cc
+++ b/src/tint/lang/glsl/writer/ast_raise/combine_samplers.cc
@@ -358,32 +358,17 @@
}
// Replace all function calls.
if (auto* callee = call->Target()->As<sem::Function>()) {
- for (auto pair : callee->TextureSamplerPairs()) {
- // Global pairs used by the callee do not require a function
- // parameter at the call site.
- if (IsGlobal(pair)) {
- continue;
- }
- // Texture-only pairs do not require a function parameter if they've been
- // replaced by a real pair.
- if (!pair.second && FindFullTextureSamplerPair(pair.first, callee)) {
- continue;
- }
-
- const sem::Variable* texture_var = pair.first;
- const sem::Variable* sampler_var = pair.second;
- if (auto* param = texture_var->As<sem::Parameter>()) {
+ auto make_arg = [&](const sem::Variable* texture_var,
+ const sem::Variable* sampler_var) {
+ if (auto* param = tint::As<sem::Parameter>(texture_var)) {
const sem::ValueExpression* texture = call->Arguments()[param->Index()];
texture_var =
texture->UnwrapLoad()->As<sem::VariableUser>()->Variable();
}
- if (sampler_var) {
- if (auto* param = sampler_var->As<sem::Parameter>()) {
- const sem::ValueExpression* sampler =
- call->Arguments()[param->Index()];
- sampler_var =
- sampler->UnwrapLoad()->As<sem::VariableUser>()->Variable();
- }
+ if (auto* param = tint::As<sem::Parameter>(sampler_var)) {
+ const sem::ValueExpression* sampler = call->Arguments()[param->Index()];
+ sampler_var =
+ sampler->UnwrapLoad()->As<sem::VariableUser>()->Variable();
}
sem::VariablePair new_pair(texture_var, sampler_var);
// If both texture and sampler are (now) global, pass that
@@ -394,8 +379,37 @@
? global_combined_texture_samplers_[new_pair]
: function_combined_texture_samplers_[call->Stmt()->Function()]
[new_pair];
- auto* arg = ctx.dst->Expr(var->name->symbol);
- args.Push(arg);
+ return ctx.dst->Expr(var->name->symbol);
+ };
+
+ for (auto pair : callee->TextureSamplerPairs()) {
+ if (!pair.second) {
+ continue;
+ }
+ // Global pairs used by the callee do not require a function
+ // parameter at the call site.
+ if (IsGlobal(pair)) {
+ continue;
+ }
+
+ args.Push(make_arg(pair.first, pair.second));
+ }
+ for (auto pair : callee->TextureSamplerPairs()) {
+ if (pair.second) {
+ continue;
+ }
+ // Global pairs used by the callee do not require a function
+ // parameter at the call site.
+ if (IsGlobal(pair)) {
+ continue;
+ }
+ // Texture-only pairs do not require a function parameter if they've been
+ // replaced by a real pair.
+ if (FindFullTextureSamplerPair(pair.first, callee)) {
+ continue;
+ }
+
+ args.Push(make_arg(pair.first, nullptr));
}
// Append all of the remaining non-texture and non-sampler
// parameters.
diff --git a/src/tint/lang/glsl/writer/ast_raise/combine_samplers_test.cc b/src/tint/lang/glsl/writer/ast_raise/combine_samplers_test.cc
index 33fd6bc..78273c1 100644
--- a/src/tint/lang/glsl/writer/ast_raise/combine_samplers_test.cc
+++ b/src/tint/lang/glsl/writer/ast_raise/combine_samplers_test.cc
@@ -1170,5 +1170,49 @@
EXPECT_EQ(expect, str(got));
}
+TEST_F(CombineSamplersTest, UnusedTextureAndSamplerFunctionParameter_Nested) {
+ auto* src = R"(
+@group(0) @binding(0) var t : texture_2d<f32>;
+
+@group(0) @binding(1) var s : sampler;
+
+fn f_nested(tex: texture_2d<f32>, sampler1: sampler) -> u32 {
+ return 1u;
+}
+
+fn f(tex: texture_2d<f32>, sampler1: sampler) -> u32 {
+ return f_nested(tex, sampler1);
+}
+
+fn main() {
+ _ = f(t, s);
+}
+)";
+ auto* expect =
+ R"(
+fn f_nested(sampler1_1 : sampler, tex_1 : texture_2d<f32>) -> u32 {
+ return 1u;
+}
+
+fn f(sampler1_2 : sampler, tex_2 : texture_2d<f32>) -> u32 {
+ return f_nested(sampler1_2, tex_2);
+}
+
+@group(0) @binding(0) @internal(disable_validation__binding_point_collision) var s_1 : sampler;
+
+@group(0) @binding(0) @internal(disable_validation__binding_point_collision) var t_1 : texture_2d<f32>;
+
+fn main() {
+ _ = f(s_1, t_1);
+}
+)";
+
+ ast::transform::DataMap data;
+ data.Add<CombineSamplers::BindingInfo>(CombineSamplers::BindingMap(), BindingPoint());
+ auto got = Run<CombineSamplers>(src, data);
+
+ EXPECT_EQ(expect, str(got));
+}
+
} // namespace
} // namespace tint::glsl::writer
