dawn/hlsl: revert dumping HLSL if compilation fails
This CL effectively reverts https://dawn-review.googlesource.com/c/dawn/+/130460
Unfortunately, this change never actually worked since if
`d3d::CompileShader` fails to compile, the output `compiledShader` would
remain invalid, so attempting to dump its contents would not work.
Furthermore, we would actually crash the GPU process because we were
dereferencing `compiledShader` when it was still invalid.
This CL reverts the original change, so if compilation fails, we don't
crash, and we output the compilation error without dumping HLSL. I will
follow up with a working implementation.
Bug: 42240637
Bug: 381065163
Change-Id: I8d10980863f746bc886e3af4d1b0b26dfdb9671a
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/216994
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
diff --git a/src/dawn/native/d3d/PlatformFunctions.h b/src/dawn/native/d3d/PlatformFunctions.h
index 889e6ba..e886f86 100644
--- a/src/dawn/native/d3d/PlatformFunctions.h
+++ b/src/dawn/native/d3d/PlatformFunctions.h
@@ -60,7 +60,7 @@
_COM_Outptr_ void** ppFactory);
PFN_CREATE_DXGI_FACTORY2 createDxgiFactory2 = nullptr;
- // Functions from d3d3compiler.dll
+ // Functions from D3DCompiler_47.dll
pD3DCompile d3dCompile = nullptr;
pD3DDisassemble d3dDisassemble = nullptr;
diff --git a/src/dawn/native/d3d/ShaderUtils.cpp b/src/dawn/native/d3d/ShaderUtils.cpp
index ad21be2..b676ee7 100644
--- a/src/dawn/native/d3d/ShaderUtils.cpp
+++ b/src/dawn/native/d3d/ShaderUtils.cpp
@@ -393,28 +393,24 @@
const CompiledShader& compiledShader,
uint32_t compileFlags) {
std::ostringstream dumpedMsg;
- // The HLSL may be empty if compilation failed.
- if (!compiledShader.hlslSource.empty()) {
- dumpedMsg << "/* Dumped generated HLSL */\n" << compiledShader.hlslSource << "\n";
- }
+ DAWN_ASSERT(!compiledShader.hlslSource.empty());
+ dumpedMsg << "/* Dumped generated HLSL */\n" << compiledShader.hlslSource << "\n";
- // The blob may be empty if FXC compilation failed.
const Blob& shaderBlob = compiledShader.shaderBlob;
- if (!shaderBlob.Empty()) {
- dumpedMsg << "/* FXC compile flags */\n" << CompileFlagsToString(compileFlags) << "\n";
- dumpedMsg << "/* Dumped disassembled DXBC */\n";
- ComPtr<ID3DBlob> disassembly;
- UINT flags =
- // Some literals are printed as floats with precision(6) which is not enough
- // precision for values very close to 0, so always print literals as hex values.
- D3D_DISASM_PRINT_HEX_LITERALS;
- if (FAILED(device->GetFunctions()->d3dDisassemble(shaderBlob.Data(), shaderBlob.Size(),
- flags, nullptr, &disassembly))) {
- dumpedMsg << "D3D disassemble failed\n";
- } else {
- dumpedMsg << std::string_view(static_cast<const char*>(disassembly->GetBufferPointer()),
- disassembly->GetBufferSize());
- }
+ DAWN_ASSERT(!shaderBlob.Empty());
+ dumpedMsg << "/* FXC compile flags */\n" << CompileFlagsToString(compileFlags) << "\n";
+ dumpedMsg << "/* Dumped disassembled DXBC */\n";
+ ComPtr<ID3DBlob> disassembly;
+ UINT flags =
+ // Some literals are printed as floats with precision(6) which is not enough
+ // precision for values very close to 0, so always print literals as hex values.
+ D3D_DISASM_PRINT_HEX_LITERALS;
+ if (FAILED(device->GetFunctions()->d3dDisassemble(shaderBlob.Data(), shaderBlob.Size(), flags,
+ nullptr, &disassembly))) {
+ dumpedMsg << "D3D disassemble failed\n";
+ } else {
+ dumpedMsg << std::string_view(static_cast<const char*>(disassembly->GetBufferPointer()),
+ disassembly->GetBufferSize());
}
std::string logMessage = dumpedMsg.str();
diff --git a/src/dawn/native/d3d11/ShaderModuleD3D11.cpp b/src/dawn/native/d3d11/ShaderModuleD3D11.cpp
index b0d4f72..41c42c0 100644
--- a/src/dawn/native/d3d11/ShaderModuleD3D11.cpp
+++ b/src/dawn/native/d3d11/ShaderModuleD3D11.cpp
@@ -252,20 +252,13 @@
req.hlsl.tintOptions.polyfill_pack_unpack_4x8 = true;
CacheResult<d3d::CompiledShader> compiledShader;
- MaybeError compileError = [&]() -> MaybeError {
- DAWN_TRY_LOAD_OR_RUN(compiledShader, device, std::move(req), d3d::CompiledShader::FromBlob,
- d3d::CompileShader, "D3D11.CompileShader");
- return {};
- }();
+ DAWN_TRY_LOAD_OR_RUN(compiledShader, device, std::move(req), d3d::CompiledShader::FromBlob,
+ d3d::CompileShader, "D3D11.CompileShader");
if (device->IsToggleEnabled(Toggle::DumpShaders)) {
d3d::DumpFXCCompiledShader(device, *compiledShader, compileFlags);
}
- if (compileError.IsError()) {
- return {compileError.AcquireError()};
- }
-
device->GetBlobCache()->EnsureStored(compiledShader);
// Clear the hlslSource. It is only used for logging and should not be used
diff --git a/src/dawn/native/d3d12/ShaderModuleD3D12.cpp b/src/dawn/native/d3d12/ShaderModuleD3D12.cpp
index 620580c..69bbfea 100644
--- a/src/dawn/native/d3d12/ShaderModuleD3D12.cpp
+++ b/src/dawn/native/d3d12/ShaderModuleD3D12.cpp
@@ -59,39 +59,34 @@
const dawn::native::d3d::CompiledShader& compiledShader,
uint32_t compileFlags) {
std::ostringstream dumpedMsg;
- // The HLSL may be empty if compilation failed.
- if (!compiledShader.hlslSource.empty()) {
- dumpedMsg << "/* Dumped generated HLSL */\n" << compiledShader.hlslSource << "\n";
- }
+ DAWN_ASSERT(!compiledShader.hlslSource.empty());
+ dumpedMsg << "/* Dumped generated HLSL */\n" << compiledShader.hlslSource << "\n";
- // The blob may be empty if DXC compilation failed.
const Blob& shaderBlob = compiledShader.shaderBlob;
- if (!shaderBlob.Empty()) {
- dumpedMsg << "/* DXC compile flags */\n"
- << dawn::native::d3d::CompileFlagsToString(compileFlags) << "\n";
- dumpedMsg << "/* Dumped disassembled DXIL */\n";
- DxcBuffer dxcBuffer;
- dxcBuffer.Encoding = DXC_CP_UTF8;
- dxcBuffer.Ptr = shaderBlob.Data();
- dxcBuffer.Size = shaderBlob.Size();
+ DAWN_ASSERT(!shaderBlob.Empty());
+ dumpedMsg << "/* DXC compile flags */\n"
+ << dawn::native::d3d::CompileFlagsToString(compileFlags) << "\n";
+ dumpedMsg << "/* Dumped disassembled DXIL */\n";
+ DxcBuffer dxcBuffer;
+ dxcBuffer.Encoding = DXC_CP_UTF8;
+ dxcBuffer.Ptr = shaderBlob.Data();
+ dxcBuffer.Size = shaderBlob.Size();
- ComPtr<IDxcResult> dxcResult;
- device->GetDxcCompiler()->Disassemble(&dxcBuffer, IID_PPV_ARGS(&dxcResult));
+ ComPtr<IDxcResult> dxcResult;
+ device->GetDxcCompiler()->Disassemble(&dxcBuffer, IID_PPV_ARGS(&dxcResult));
- ComPtr<IDxcBlobEncoding> disassembly;
- if (dxcResult && dxcResult->HasOutput(DXC_OUT_DISASSEMBLY) &&
- SUCCEEDED(
- dxcResult->GetOutput(DXC_OUT_DISASSEMBLY, IID_PPV_ARGS(&disassembly), nullptr))) {
- dumpedMsg << std::string_view(static_cast<const char*>(disassembly->GetBufferPointer()),
- disassembly->GetBufferSize());
- } else {
- dumpedMsg << "DXC disassemble failed\n";
- ComPtr<IDxcBlobEncoding> errors;
- if (dxcResult && dxcResult->HasOutput(DXC_OUT_ERRORS) &&
- SUCCEEDED(dxcResult->GetOutput(DXC_OUT_ERRORS, IID_PPV_ARGS(&errors), nullptr))) {
- dumpedMsg << std::string_view(static_cast<const char*>(errors->GetBufferPointer()),
- errors->GetBufferSize());
- }
+ ComPtr<IDxcBlobEncoding> disassembly;
+ if (dxcResult && dxcResult->HasOutput(DXC_OUT_DISASSEMBLY) &&
+ SUCCEEDED(dxcResult->GetOutput(DXC_OUT_DISASSEMBLY, IID_PPV_ARGS(&disassembly), nullptr))) {
+ dumpedMsg << std::string_view(static_cast<const char*>(disassembly->GetBufferPointer()),
+ disassembly->GetBufferSize());
+ } else {
+ dumpedMsg << "DXC disassemble failed\n";
+ ComPtr<IDxcBlobEncoding> errors;
+ if (dxcResult && dxcResult->HasOutput(DXC_OUT_ERRORS) &&
+ SUCCEEDED(dxcResult->GetOutput(DXC_OUT_ERRORS, IID_PPV_ARGS(&errors), nullptr))) {
+ dumpedMsg << std::string_view(static_cast<const char*>(errors->GetBufferPointer()),
+ errors->GetBufferSize());
}
}
@@ -381,11 +376,8 @@
req.hlsl.limits = LimitsForCompilationRequest::Create(limits.v1);
CacheResult<d3d::CompiledShader> compiledShader;
- MaybeError compileError = [&]() -> MaybeError {
- DAWN_TRY_LOAD_OR_RUN(compiledShader, device, std::move(req), d3d::CompiledShader::FromBlob,
- d3d::CompileShader, "D3D12.CompileShader");
- return {};
- }();
+ DAWN_TRY_LOAD_OR_RUN(compiledShader, device, std::move(req), d3d::CompiledShader::FromBlob,
+ d3d::CompileShader, "D3D12.CompileShader");
if (device->IsToggleEnabled(Toggle::DumpShaders)) {
if (device->IsToggleEnabled(Toggle::UseDXC)) {
@@ -395,10 +387,6 @@
}
}
- if (compileError.IsError()) {
- return {compileError.AcquireError()};
- }
-
device->GetBlobCache()->EnsureStored(compiledShader);
// Clear the hlslSource. It is only used for logging and should not be used
