tint::transform::VertexPulling: require SingleEntryPoint

This change the vertex pulling transform to look for the single vertex
entry point in the module, instead of taking the entry point name in the
config. This is necessary because the renamer needs to run before
VertexPulling so that builtins like min() don't end up referring to the
input WGSL. Putting the renamer before VertexPulling makes the config
entry point name no longer match.

Bug: dawn:1583
Change-Id: I4c96eb83518e0d6fe8ce23b37e238f4a890eeb2f
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/107080
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>
This commit is contained in:
Corentin Wallez 2022-10-26 12:47:30 +00:00 committed by Dawn LUCI CQ
parent 4faf3581b8
commit 63463c2f77
6 changed files with 23 additions and 32 deletions

View File

@ -154,10 +154,8 @@ tint::transform::MultiplanarExternalTexture::BindingsMap BuildExternalTextureTra
tint::transform::VertexPulling::Config BuildVertexPullingTransformConfig(
const RenderPipelineBase& renderPipeline,
const std::string_view& entryPoint,
BindGroupIndex pullingBufferBindingSet) {
tint::transform::VertexPulling::Config cfg;
cfg.entry_point_name = entryPoint;
cfg.pulling_group = static_cast<uint32_t>(pullingBufferBindingSet);
cfg.vertex_state.resize(renderPipeline.GetVertexBufferCount());

View File

@ -45,7 +45,6 @@ tint::transform::MultiplanarExternalTexture::BindingsMap BuildExternalTextureTra
tint::transform::VertexPulling::Config BuildVertexPullingTransformConfig(
const RenderPipelineBase& renderPipeline,
const std::string_view& entryPoint,
BindGroupIndex pullingBufferBindingSet);
tint::transform::SubstituteOverride::Config BuildSubstituteOverridesTransformConfig(

View File

@ -141,8 +141,8 @@ ResultOrError<CacheResult<MslCompilation>> TranslateToMSL(
std::optional<tint::transform::VertexPulling::Config> vertexPullingTransformConfig;
if (stage == SingleShaderStage::Vertex &&
device->IsToggleEnabled(Toggle::MetalEnableVertexPulling)) {
vertexPullingTransformConfig = BuildVertexPullingTransformConfig(
*renderPipeline, programmableStage.entryPoint.c_str(), kPullingBufferBindingSet);
vertexPullingTransformConfig =
BuildVertexPullingTransformConfig(*renderPipeline, kPullingBufferBindingSet);
for (VertexBufferSlot slot : IterateBitSet(renderPipeline->GetVertexBufferSlotsUsed())) {
uint32_t metalIndex = renderPipeline->GetMtlVertexBufferIndex(slot);

View File

@ -882,8 +882,18 @@ void VertexPulling::Run(CloneContext& ctx, const DataMap& inputs, DataMap&) cons
}
// Find entry point
auto* func = ctx.src->AST().Functions().Find(ctx.src->Symbols().Get(cfg.entry_point_name),
ast::PipelineStage::kVertex);
const ast::Function* func = nullptr;
for (auto* fn : ctx.src->AST().Functions()) {
if (fn->PipelineStage() == ast::PipelineStage::kVertex) {
if (func != nullptr) {
ctx.dst->Diagnostics().add_error(
diag::System::Transform,
"VertexPulling found more than one vertex entry point");
return;
}
func = fn;
}
}
if (func == nullptr) {
ctx.dst->Diagnostics().add_error(diag::System::Transform,
"Vertex stage entry point not found");

View File

@ -135,6 +135,8 @@ using VertexStateDescriptor = std::vector<VertexBufferLayoutDescriptor>;
/// code, but these are types that the data may arrive as. We need to convert
/// these smaller types into the base types such as `f32` and `u32` for the
/// shader to use.
///
/// The SingleEntryPoint transform must have run before VertexPulling.
class VertexPulling final : public Castable<VertexPulling, Transform> {
public:
/// Configuration options for the transform
@ -152,9 +154,6 @@ class VertexPulling final : public Castable<VertexPulling, Transform> {
/// @returns this Config
Config& operator=(const Config&);
/// The entry point to add assignments into
std::string entry_point_name;
/// The vertex state descriptor, containing info about attributes
VertexStateDescriptor vertex_state;
@ -163,7 +162,7 @@ class VertexPulling final : public Castable<VertexPulling, Transform> {
uint32_t pulling_group = 4u;
/// Reflect the fields of this class so that it can be used by tint::ForeachField()
TINT_REFLECT(entry_point_name, vertex_state, pulling_group);
TINT_REFLECT(vertex_state, pulling_group);
};
/// Constructor

View File

@ -35,18 +35,21 @@ TEST_F(VertexPullingTest, Error_NoEntryPoint) {
EXPECT_EQ(expect, str(got));
}
TEST_F(VertexPullingTest, Error_InvalidEntryPoint) {
TEST_F(VertexPullingTest, Error_MultipleEntryPoint) {
auto* src = R"(
@vertex
fn main() -> @builtin(position) vec4<f32> {
return vec4<f32>();
}
@vertex
fn main2() -> @builtin(position) vec4<f32> {
return vec4<f32>();
}
)";
auto* expect = "error: Vertex stage entry point not found";
auto* expect = "error: VertexPulling found more than one vertex entry point";
VertexPulling::Config cfg;
cfg.entry_point_name = "_";
DataMap data;
data.Add<VertexPulling::Config>(cfg);
@ -64,7 +67,6 @@ fn main() {}
auto* expect = "error: Vertex stage entry point not found";
VertexPulling::Config cfg;
cfg.entry_point_name = "main";
DataMap data;
data.Add<VertexPulling::Config>(cfg);
@ -87,7 +89,6 @@ fn main(@location(0) var_a : f32) -> @builtin(position) vec4<f32> {
VertexPulling::Config cfg;
cfg.vertex_state = {{{15, VertexStepMode::kVertex, {{VertexFormat::kFloat32, 0, 0}}}}};
cfg.entry_point_name = "main";
DataMap data;
data.Add<VertexPulling::Config>(cfg);
@ -116,7 +117,6 @@ fn main() -> @builtin(position) vec4<f32> {
)";
VertexPulling::Config cfg;
cfg.entry_point_name = "main";
DataMap data;
data.Add<VertexPulling::Config>(cfg);
@ -153,7 +153,6 @@ fn main(@builtin(vertex_index) tint_pulling_vertex_index : u32) -> @builtin(posi
VertexPulling::Config cfg;
cfg.vertex_state = {{{4, VertexStepMode::kVertex, {{VertexFormat::kFloat32, 0, 0}}}}};
cfg.entry_point_name = "main";
DataMap data;
data.Add<VertexPulling::Config>(cfg);
@ -190,7 +189,6 @@ fn main(@builtin(instance_index) tint_pulling_instance_index : u32) -> @builtin(
VertexPulling::Config cfg;
cfg.vertex_state = {{{4, VertexStepMode::kInstance, {{VertexFormat::kFloat32, 0, 0}}}}};
cfg.entry_point_name = "main";
DataMap data;
data.Add<VertexPulling::Config>(cfg);
@ -228,7 +226,6 @@ fn main(@builtin(vertex_index) tint_pulling_vertex_index : u32) -> @builtin(posi
VertexPulling::Config cfg;
cfg.vertex_state = {{{4, VertexStepMode::kVertex, {{VertexFormat::kFloat32, 0, 0}}}}};
cfg.pulling_group = 5;
cfg.entry_point_name = "main";
DataMap data;
data.Add<VertexPulling::Config>(cfg);
@ -274,7 +271,6 @@ fn main(@builtin(vertex_index) tint_pulling_vertex_index : u32) -> @builtin(posi
VertexPulling::Config cfg;
cfg.vertex_state = {{{4, VertexStepMode::kVertex, {{VertexFormat::kFloat32, 0, 0}}}}};
cfg.entry_point_name = "main";
DataMap data;
data.Add<VertexPulling::Config>(cfg);
@ -332,7 +328,6 @@ fn main(@builtin(vertex_index) custom_vertex_index : u32, @builtin(instance_inde
{{VertexFormat::kFloat32, 0, 1}},
},
}};
cfg.entry_point_name = "main";
DataMap data;
data.Add<VertexPulling::Config>(cfg);
@ -411,7 +406,6 @@ fn main(tint_symbol_1 : tint_symbol) -> @builtin(position) vec4<f32> {
{{VertexFormat::kFloat32, 0, 1}},
},
}};
cfg.entry_point_name = "main";
DataMap data;
data.Add<VertexPulling::Config>(cfg);
@ -490,7 +484,6 @@ struct Inputs {
{{VertexFormat::kFloat32, 0, 1}},
},
}};
cfg.entry_point_name = "main";
DataMap data;
data.Add<VertexPulling::Config>(cfg);
@ -566,7 +559,6 @@ fn main(indices : Indices) -> @builtin(position) vec4<f32> {
{{VertexFormat::kFloat32, 0, 1}},
},
}};
cfg.entry_point_name = "main";
DataMap data;
data.Add<VertexPulling::Config>(cfg);
@ -642,7 +634,6 @@ struct Indices {
{{VertexFormat::kFloat32, 0, 1}},
},
}};
cfg.entry_point_name = "main";
DataMap data;
data.Add<VertexPulling::Config>(cfg);
@ -684,7 +675,6 @@ fn main(@builtin(vertex_index) tint_pulling_vertex_index : u32) -> @builtin(posi
cfg.vertex_state = {{{16,
VertexStepMode::kVertex,
{{VertexFormat::kFloat32, 0, 0}, {VertexFormat::kFloat32x4, 0, 1}}}}};
cfg.entry_point_name = "main";
DataMap data;
data.Add<VertexPulling::Config>(cfg);
@ -738,7 +728,6 @@ fn main(@builtin(vertex_index) tint_pulling_vertex_index : u32) -> @builtin(posi
{12, VertexStepMode::kVertex, {{VertexFormat::kFloat32x3, 0, 1}}},
{16, VertexStepMode::kVertex, {{VertexFormat::kFloat32x4, 0, 2}}},
}};
cfg.entry_point_name = "main";
DataMap data;
data.Add<VertexPulling::Config>(cfg);
@ -788,7 +777,6 @@ fn main(@builtin(vertex_index) tint_pulling_vertex_index_1 : u32) -> @builtin(po
cfg.vertex_state = {{{16,
VertexStepMode::kVertex,
{{VertexFormat::kFloat32, 0, 0}, {VertexFormat::kFloat32x4, 0, 1}}}}};
cfg.entry_point_name = "main";
DataMap data;
data.Add<VertexPulling::Config>(cfg);
@ -933,7 +921,6 @@ fn main(@builtin(vertex_index) tint_pulling_vertex_index : u32) -> @builtin(posi
{VertexFormat::kSint32, 64, 26}, {VertexFormat::kSint32x2, 64, 27},
{VertexFormat::kSint32x3, 64, 28}, {VertexFormat::kSint32x4, 64, 29},
}}}};
cfg.entry_point_name = "main";
DataMap data;
data.Add<VertexPulling::Config>(cfg);
@ -1079,7 +1066,6 @@ fn main(@builtin(vertex_index) tint_pulling_vertex_index : u32) -> @builtin(posi
{VertexFormat::kSint32, 63, 26}, {VertexFormat::kSint32x2, 63, 27},
{VertexFormat::kSint32x3, 63, 28}, {VertexFormat::kSint32x4, 63, 29},
}}}};
cfg.entry_point_name = "main";
DataMap data;
data.Add<VertexPulling::Config>(cfg);
@ -1224,7 +1210,6 @@ fn main(@builtin(vertex_index) tint_pulling_vertex_index : u32) -> @builtin(posi
{VertexFormat::kSint32, 64, 26}, {VertexFormat::kSint32x2, 64, 27},
{VertexFormat::kSint32x3, 64, 28}, {VertexFormat::kSint32x4, 64, 29},
}}}};
cfg.entry_point_name = "main";
DataMap data;
data.Add<VertexPulling::Config>(cfg);