Adds generation for WGSL descriptor to rename source->code.

- Adds a deprecation warning for the incorrect usage.
- Adds test to verify that the warning is emitted and that the results
  are equal.
- Fixes all existing tests to use the newer code path.

Change-Id: I01bf85137ad1c66966e1aa0259ae03fc5247ba25
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/130320
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Loko Kung <lokokung@google.com>
Reviewed-by: Austin Eng <enga@chromium.org>
This commit is contained in:
Loko Kung 2023-05-03 02:23:25 +00:00 committed by Dawn LUCI CQ
parent fed6b6b35b
commit b27e2bfa71
14 changed files with 67 additions and 21 deletions

View File

@ -2408,8 +2408,8 @@
"chained": "in",
"chain roots": ["shader module descriptor"],
"members": [
{"name": "source", "type": "char", "annotation": "const*", "length": "strlen", "tags": ["dawn", "emscripten"]},
{"name": "code", "type": "char", "annotation": "const*", "length": "strlen", "tags": ["upstream"]}
{"name": "source", "type": "char", "annotation": "const*", "length": "strlen", "default": "nullptr", "optional": true, "tags": ["dawn", "emscripten"]},
{"name": "code", "type": "char", "annotation": "const*", "length": "strlen", "default": "nullptr", "optional": true}
]
},
"dawn shader module SPIRV options descriptor": {

View File

@ -163,6 +163,8 @@
{% if member.optional %}
bool has_{{memberName}} = record.{{memberName}} != nullptr;
if (has_{{memberName}})
{% else %}
ASSERT(record.{{memberName}} != nullptr);
{% endif %}
{
result += Align(std::strlen(record.{{memberName}}), kWireBufferAlignment);

View File

@ -125,7 +125,7 @@ ResultOrError<Ref<RenderPipelineBase>> GetOrCreateRG8ToDepth16UnormPipeline(Devi
ShaderModuleWGSLDescriptor wgslDesc = {};
ShaderModuleDescriptor shaderModuleDesc = {};
shaderModuleDesc.nextInChain = &wgslDesc;
wgslDesc.source = kBlitRG8ToDepthShaders;
wgslDesc.code = kBlitRG8ToDepthShaders;
Ref<ShaderModuleBase> shaderModule;
DAWN_TRY_ASSIGN(shaderModule, device->CreateShaderModule(&shaderModuleDesc));
@ -176,7 +176,7 @@ ResultOrError<InternalPipelineStore::BlitR8ToStencilPipelines> GetOrCreateR8ToSt
ShaderModuleWGSLDescriptor wgslDesc = {};
ShaderModuleDescriptor shaderModuleDesc = {};
shaderModuleDesc.nextInChain = &wgslDesc;
wgslDesc.source = kBlitStencilShaders;
wgslDesc.code = kBlitStencilShaders;
Ref<ShaderModuleBase> shaderModule;
DAWN_TRY_ASSIGN(shaderModule, device->CreateShaderModule(&shaderModuleDesc));

View File

@ -63,7 +63,7 @@ ResultOrError<Ref<RenderPipelineBase>> GetOrCreateDepthBlitPipeline(DeviceBase*
ShaderModuleWGSLDescriptor wgslDesc = {};
ShaderModuleDescriptor shaderModuleDesc = {};
shaderModuleDesc.nextInChain = &wgslDesc;
wgslDesc.source = kBlitToDepthShaders;
wgslDesc.code = kBlitToDepthShaders;
Ref<ShaderModuleBase> shaderModule;
DAWN_TRY_ASSIGN(shaderModule, device->CreateShaderModule(&shaderModuleDesc));

View File

@ -270,7 +270,7 @@ MaybeError DeviceBase::Initialize(Ref<QueueBase> defaultQueue) {
)";
ShaderModuleDescriptor descriptor;
ShaderModuleWGSLDescriptor wgslDesc;
wgslDesc.source = kEmptyFragmentShader;
wgslDesc.code = kEmptyFragmentShader;
descriptor.nextInChain = &wgslDesc;
DAWN_TRY_ASSIGN(mInternalPipelineStore->placeholderFragmentShader,

View File

@ -955,7 +955,7 @@ MaybeError ValidateAndParseShaderModule(DeviceBase* device,
DAWN_INVALID_IF(!result.success, "Tint WGSL failure: Generator: %s", result.error);
newWgslCode = std::move(result.wgsl);
newWgslDesc.source = newWgslCode.c_str();
newWgslDesc.code = newWgslCode.c_str();
spirvDesc = nullptr;
wgslDesc = &newWgslDesc;
@ -980,11 +980,16 @@ MaybeError ValidateAndParseShaderModule(DeviceBase* device,
ASSERT(wgslDesc != nullptr);
auto tintSource = std::make_unique<TintSource>("", wgslDesc->source);
const char* code = wgslDesc->code ? wgslDesc->code : wgslDesc->source;
DAWN_INVALID_IF(code == nullptr,
"At least one of ShaderModuleWGSLDescriptor.source or "
"ShaderModuleWGSLDescriptor.code must be set.");
auto tintSource = std::make_unique<TintSource>("", code);
if (device->IsToggleEnabled(Toggle::DumpShaders)) {
std::ostringstream dumpedMsg;
dumpedMsg << "// Dumped WGSL:" << std::endl << wgslDesc->source;
dumpedMsg << "// Dumped WGSL:" << std::endl << code;
device->EmitLog(WGPULoggingType_Info, dumpedMsg.str().c_str());
}
@ -1101,7 +1106,14 @@ ShaderModuleBase::ShaderModuleBase(DeviceBase* device,
mOriginalSpirv.assign(spirvDesc->code, spirvDesc->code + spirvDesc->codeSize);
} else if (wgslDesc) {
mType = Type::Wgsl;
mWgsl = std::string(wgslDesc->source);
if (wgslDesc->code) {
mWgsl = std::string(wgslDesc->code);
} else {
device->EmitDeprecationWarning(
"ShaderModuleWGSLDescriptor.source is deprecated, use "
"ShaderModuleWGSLDescriptor.code instead.");
mWgsl = std::string(wgslDesc->source);
}
}
}

View File

@ -36,7 +36,7 @@ namespace dawn::native::utils {
ResultOrError<Ref<ShaderModuleBase>> CreateShaderModule(DeviceBase* device, const char* source) {
ShaderModuleWGSLDescriptor wgslDesc;
wgslDesc.source = source;
wgslDesc.code = source;
ShaderModuleDescriptor descriptor;
descriptor.nextInChain = &wgslDesc;
return device->CreateShaderModule(&descriptor);

View File

@ -618,7 +618,7 @@ TEST_F(DestroyObjectTests, ShaderModuleNativeExplicit) {
TEST_F(DestroyObjectTests, ShaderModuleImplicit) {
ShaderModuleWGSLDescriptor wgslDesc = {};
wgslDesc.source = kVertexShader.data();
wgslDesc.code = kVertexShader.data();
ShaderModuleDescriptor desc = {};
desc.nextInChain = &wgslDesc;
@ -816,7 +816,7 @@ TEST_F(DestroyObjectTests, DestroyObjectsApiExplicit) {
wgpu::ShaderModule csModule;
{
ShaderModuleWGSLDescriptor wgslDesc = {};
wgslDesc.source = kComputeShader.data();
wgslDesc.code = kComputeShader.data();
ShaderModuleDescriptor desc = {};
desc.nextInChain = &wgslDesc;
@ -830,7 +830,7 @@ TEST_F(DestroyObjectTests, DestroyObjectsApiExplicit) {
wgpu::ShaderModule vsModule;
{
ShaderModuleWGSLDescriptor wgslDesc = {};
wgslDesc.source = kVertexShader.data();
wgslDesc.code = kVertexShader.data();
ShaderModuleDescriptor desc = {};
desc.nextInChain = &wgslDesc;

View File

@ -43,7 +43,7 @@ Ref<ShaderModuleMock> ShaderModuleMock::Create(DeviceMock* device,
// static
Ref<ShaderModuleMock> ShaderModuleMock::Create(DeviceMock* device, const char* source) {
ShaderModuleWGSLDescriptor wgslDesc = {};
wgslDesc.source = source;
wgslDesc.code = source;
ShaderModuleDescriptor desc = {};
desc.nextInChain = &wgslDesc;
return Create(device, &desc);

View File

@ -615,7 +615,7 @@ TEST_F(LabelTest, ShaderModule) {
})";
wgpu::ShaderModuleWGSLDescriptor wgslDesc;
wgslDesc.source = source;
wgslDesc.code = source;
wgpu::ShaderModuleDescriptor descriptor;
descriptor.nextInChain = &wgslDesc;

View File

@ -37,7 +37,7 @@ TEST_F(MultipleDeviceTest, ValidatesSameDevice) {
// objects from a different device.
TEST_F(MultipleDeviceTest, ValidatesSameDeviceCreatePipelineAsync) {
wgpu::ShaderModuleWGSLDescriptor wgslDesc = {};
wgslDesc.source = R"(
wgslDesc.code = R"(
@compute @workgroup_size(1, 1, 1) fn main() {
}
)";

View File

@ -205,7 +205,7 @@ TEST_F(ShaderModuleValidationTest, MultipleChainedDescriptor_WgslAndSpirv) {
spirv_desc.code = &code;
spirv_desc.codeSize = 1;
wgpu::ShaderModuleWGSLDescriptor wgsl_desc = {};
wgsl_desc.source = "";
wgsl_desc.code = "";
wgsl_desc.nextInChain = &spirv_desc;
desc.nextInChain = &wgsl_desc;
ASSERT_DEVICE_ERROR(device.CreateShaderModule(&desc),
@ -219,7 +219,7 @@ TEST_F(ShaderModuleValidationTest, MultipleChainedDescriptor_WgslAndDawnSpirvOpt
wgpu::DawnShaderModuleSPIRVOptionsDescriptor spirv_options_desc = {};
wgpu::ShaderModuleWGSLDescriptor wgsl_desc = {};
wgsl_desc.nextInChain = &spirv_options_desc;
wgsl_desc.source = "";
wgsl_desc.code = "";
desc.nextInChain = &wgsl_desc;
ASSERT_DEVICE_ERROR(
device.CreateShaderModule(&desc),
@ -692,3 +692,35 @@ enable f16;
@compute @workgroup_size(1) fn main() {})"));
}
// Test that passing a WGSL extension without setting the shader string should fail.
TEST_F(ShaderModuleValidationTest, WgslNullptrShader) {
wgpu::ShaderModuleWGSLDescriptor wgslDesc = {};
wgpu::ShaderModuleDescriptor descriptor = {};
descriptor.nextInChain = &wgslDesc;
ASSERT_DEVICE_ERROR(device.CreateShaderModule(&descriptor));
}
// Tests that WGSL extension with deprecated source member still works but emits warning.
TEST_F(ShaderModuleValidationTest, SourceToCodeMemberDeprecation) {
// This test works assuming ShaderModule is backed by a dawn::native::ShaderModuleBase, which
// is not the case on the wire.
DAWN_SKIP_TEST_IF(UsesWire());
wgpu::ShaderModuleWGSLDescriptor wgslDesc = {};
wgpu::ShaderModuleDescriptor descriptor = {};
descriptor.nextInChain = &wgslDesc;
wgpu::ShaderModule sourceShader;
wgslDesc.source = "@compute @workgroup_size(1) fn main() {}";
// Note that there are actually 2 warnings emitted because 1 is for the blueprint and one is for
// the actual shader.
EXPECT_DEPRECATION_WARNINGS(sourceShader = device.CreateShaderModule(&descriptor), 2);
wgslDesc.source = nullptr;
wgslDesc.code = "@compute @workgroup_size(1) fn main() {}";
wgpu::ShaderModule codeShader = device.CreateShaderModule(&descriptor);
EXPECT_TRUE(dawn::native::ShaderModuleBase::EqualityFunc()(
dawn::native::FromAPI(sourceShader.Get()), dawn::native::FromAPI(codeShader.Get())));
}

View File

@ -144,7 +144,7 @@ TEST_F(WireDeviceLifetimeTests, DeviceDroppedFromWireThenLoggingCallback) {
wgpu::ShaderModuleDescriptor shaderModuleDesc = {};
wgpu::ShaderModuleWGSLDescriptor wgslDesc = {};
shaderModuleDesc.nextInChain = &wgslDesc;
wgslDesc.source = "@compute @workgroup_size(64) fn main() {}";
wgslDesc.code = "@compute @workgroup_size(64) fn main() {}";
// Create a shader module so the transformed shaders are dumped.
device.CreateShaderModule(&shaderModuleDesc);

View File

@ -84,7 +84,7 @@ wgpu::ShaderModule CreateShaderModuleFromASM(
wgpu::ShaderModule CreateShaderModule(const wgpu::Device& device, const char* source) {
wgpu::ShaderModuleWGSLDescriptor wgslDesc;
wgslDesc.source = source;
wgslDesc.code = source;
wgpu::ShaderModuleDescriptor descriptor;
descriptor.nextInChain = &wgslDesc;
return device.CreateShaderModule(&descriptor);