Refactor D3D12 pipeline creation to better propagate errors
BUG=dawn:301
Change-Id: Ia7982cfe40abb28ab786c8941e269bded11468ee
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/14282
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
diff --git a/src/dawn_native/d3d12/ComputePipelineD3D12.cpp b/src/dawn_native/d3d12/ComputePipelineD3D12.cpp
index f1b9491..1fc81d6 100644
--- a/src/dawn_native/d3d12/ComputePipelineD3D12.cpp
+++ b/src/dawn_native/d3d12/ComputePipelineD3D12.cpp
@@ -22,8 +22,17 @@
namespace dawn_native { namespace d3d12 {
- ComputePipeline::ComputePipeline(Device* device, const ComputePipelineDescriptor* descriptor)
- : ComputePipelineBase(device, descriptor) {
+ ResultOrError<ComputePipeline*> ComputePipeline::Create(
+ Device* device,
+ const ComputePipelineDescriptor* descriptor) {
+ std::unique_ptr<ComputePipeline> pipeline =
+ std::make_unique<ComputePipeline>(device, descriptor);
+ DAWN_TRY(pipeline->Initialize(descriptor));
+ return pipeline.release();
+ }
+
+ MaybeError ComputePipeline::Initialize(const ComputePipelineDescriptor* descriptor) {
+ Device* device = ToBackend(GetDevice());
uint32_t compileFlags = 0;
#if defined(_DEBUG)
// Enable better shader debugging with the graphics debugging tools.
@@ -33,7 +42,8 @@
compileFlags |= D3DCOMPILE_PACK_MATRIX_ROW_MAJOR;
ShaderModule* module = ToBackend(descriptor->computeStage.module);
- const std::string hlslSource = module->GetHLSLSource(ToBackend(GetLayout()));
+ std::string hlslSource;
+ DAWN_TRY_ASSIGN(hlslSource, module->GetHLSLSource(ToBackend(GetLayout())));
ComPtr<ID3DBlob> compiledShader;
ComPtr<ID3DBlob> errors;
@@ -53,6 +63,7 @@
device->GetD3D12Device()->CreateComputePipelineState(&d3dDesc,
IID_PPV_ARGS(&mPipelineState));
+ return {};
}
ComputePipeline::~ComputePipeline() {
diff --git a/src/dawn_native/d3d12/ComputePipelineD3D12.h b/src/dawn_native/d3d12/ComputePipelineD3D12.h
index 7b1af9f..9f38bbe 100644
--- a/src/dawn_native/d3d12/ComputePipelineD3D12.h
+++ b/src/dawn_native/d3d12/ComputePipelineD3D12.h
@@ -25,12 +25,16 @@
class ComputePipeline : public ComputePipelineBase {
public:
- ComputePipeline(Device* device, const ComputePipelineDescriptor* descriptor);
+ static ResultOrError<ComputePipeline*> Create(Device* device,
+ const ComputePipelineDescriptor* descriptor);
+ ComputePipeline() = delete;
~ComputePipeline();
ComPtr<ID3D12PipelineState> GetPipelineState();
private:
+ using ComputePipelineBase::ComputePipelineBase;
+ MaybeError Initialize(const ComputePipelineDescriptor* descriptor);
ComPtr<ID3D12PipelineState> mPipelineState;
};
diff --git a/src/dawn_native/d3d12/DeviceD3D12.cpp b/src/dawn_native/d3d12/DeviceD3D12.cpp
index 0e1ea47..204e55c 100644
--- a/src/dawn_native/d3d12/DeviceD3D12.cpp
+++ b/src/dawn_native/d3d12/DeviceD3D12.cpp
@@ -256,7 +256,7 @@
}
ResultOrError<ComputePipelineBase*> Device::CreateComputePipelineImpl(
const ComputePipelineDescriptor* descriptor) {
- return new ComputePipeline(this, descriptor);
+ return ComputePipeline::Create(this, descriptor);
}
ResultOrError<PipelineLayoutBase*> Device::CreatePipelineLayoutImpl(
const PipelineLayoutDescriptor* descriptor) {
diff --git a/src/dawn_native/d3d12/RenderPipelineD3D12.cpp b/src/dawn_native/d3d12/RenderPipelineD3D12.cpp
index 89c9ed5..830860b 100644
--- a/src/dawn_native/d3d12/RenderPipelineD3D12.cpp
+++ b/src/dawn_native/d3d12/RenderPipelineD3D12.cpp
@@ -337,7 +337,8 @@
break;
}
- const std::string hlslSource = module->GetHLSLSource(ToBackend(GetLayout()));
+ std::string hlslSource;
+ DAWN_TRY_ASSIGN(hlslSource, module->GetHLSLSource(ToBackend(GetLayout())));
const PlatformFunctions* functions = device->GetFunctions();
if (FAILED(functions->d3dCompile(hlslSource.c_str(), hlslSource.length(), nullptr,
diff --git a/src/dawn_native/d3d12/RenderPipelineD3D12.h b/src/dawn_native/d3d12/RenderPipelineD3D12.h
index affd5fe..7340872 100644
--- a/src/dawn_native/d3d12/RenderPipelineD3D12.h
+++ b/src/dawn_native/d3d12/RenderPipelineD3D12.h
@@ -27,6 +27,7 @@
public:
static ResultOrError<RenderPipeline*> Create(Device* device,
const RenderPipelineDescriptor* descriptor);
+ RenderPipeline() = delete;
~RenderPipeline();
D3D12_PRIMITIVE_TOPOLOGY GetD3D12PrimitiveTopology() const;
diff --git a/src/dawn_native/d3d12/ShaderModuleD3D12.cpp b/src/dawn_native/d3d12/ShaderModuleD3D12.cpp
index c542586..0e5c2ef 100644
--- a/src/dawn_native/d3d12/ShaderModuleD3D12.cpp
+++ b/src/dawn_native/d3d12/ShaderModuleD3D12.cpp
@@ -68,7 +68,7 @@
return {};
}
- const std::string ShaderModule::GetHLSLSource(PipelineLayout* layout) {
+ ResultOrError<std::string> ShaderModule::GetHLSLSource(PipelineLayout* layout) {
std::unique_ptr<spirv_cross::CompilerHLSL> compiler_impl;
spirv_cross::CompilerHLSL* compiler;
if (!GetDevice()->IsToggleEnabled(Toggle::UseSpvc)) {
@@ -102,9 +102,13 @@
if (bindingInfo.used) {
uint32_t bindingOffset = bindingOffsets[binding];
if (GetDevice()->IsToggleEnabled(Toggle::UseSpvc)) {
- mSpvcContext.SetDecoration(bindingInfo.id, SHADERC_SPVC_DECORATION_BINDING,
- bindingOffset);
- // TODO(dawn:301): Check status & have some sort of meaningful error path
+ if (mSpvcContext.SetDecoration(
+ bindingInfo.id, SHADERC_SPVC_DECORATION_BINDING, bindingOffset) !=
+ shaderc_spvc_status_success) {
+ return DAWN_VALIDATION_ERROR(
+ "Unable to set decorating binding before generating HLSL shader w/ "
+ "spvc");
+ }
} else {
compiler->set_decoration(bindingInfo.id, spv::DecorationBinding,
bindingOffset);
@@ -114,9 +118,12 @@
}
if (GetDevice()->IsToggleEnabled(Toggle::UseSpvc)) {
shaderc_spvc::CompilationResult result;
- mSpvcContext.CompileShader(&result);
- // TODO(dawn:301): Check status & have some sort of meaningful error path
- return result.GetStringOutput();
+ if (mSpvcContext.CompileShader(&result) != shaderc_spvc_status_success) {
+ return DAWN_VALIDATION_ERROR("Unable to generate HLSL shader w/ spvc");
+ }
+ std::string result_string =
+ result.GetStringOutput(); // Stripping const for ResultOrError
+ return result_string;
} else {
return compiler->compile();
}
diff --git a/src/dawn_native/d3d12/ShaderModuleD3D12.h b/src/dawn_native/d3d12/ShaderModuleD3D12.h
index bcec904..0b4aeff 100644
--- a/src/dawn_native/d3d12/ShaderModuleD3D12.h
+++ b/src/dawn_native/d3d12/ShaderModuleD3D12.h
@@ -27,7 +27,7 @@
static ResultOrError<ShaderModule*> Create(Device* device,
const ShaderModuleDescriptor* descriptor);
- const std::string GetHLSLSource(PipelineLayout* layout);
+ ResultOrError<std::string> GetHLSLSource(PipelineLayout* layout);
private:
ShaderModule(Device* device, const ShaderModuleDescriptor* descriptor);
