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>

src/dawn_native/d3d12/ComputePipelineD3D12.cpp

@@ -22,8 +22,17 @@
 
 namespace dawn_native { namespace d3d12 {
 
-    ComputePipeline::ComputePipeline(Device* device, const ComputePipelineDescriptor* descriptor)
+    ResultOrError<ComputePipeline*> ComputePipeline::Create(
-        : ComputePipelineBase(device, descriptor) {
+        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 @@ namespace dawn_native { namespace d3d12 {
         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 @@ namespace dawn_native { namespace d3d12 {
 
         device->GetD3D12Device()->CreateComputePipelineState(&d3dDesc,
                                                              IID_PPV_ARGS(&mPipelineState));
+        return {};
     }
 
     ComputePipeline::~ComputePipeline() {

src/dawn_native/d3d12/ComputePipelineD3D12.h

@@ -25,12 +25,16 @@ namespace dawn_native { namespace d3d12 {
 
     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;
     };
 

src/dawn_native/d3d12/DeviceD3D12.cpp

@@ -256,7 +256,7 @@ namespace dawn_native { namespace d3d12 {
     }
     ResultOrError<ComputePipelineBase*> Device::CreateComputePipelineImpl(
         const ComputePipelineDescriptor* descriptor) {
-        return new ComputePipeline(this, descriptor);
+        return ComputePipeline::Create(this, descriptor);
     }
     ResultOrError<PipelineLayoutBase*> Device::CreatePipelineLayoutImpl(
         const PipelineLayoutDescriptor* descriptor) {

src/dawn_native/d3d12/RenderPipelineD3D12.cpp

@@ -337,7 +337,8 @@ namespace dawn_native { namespace d3d12 {
                     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,

src/dawn_native/d3d12/RenderPipelineD3D12.h

@@ -27,6 +27,7 @@ namespace dawn_native { namespace d3d12 {
       public:
         static ResultOrError<RenderPipeline*> Create(Device* device,
                                                      const RenderPipelineDescriptor* descriptor);
+        RenderPipeline() = delete;
         ~RenderPipeline();
 
         D3D12_PRIMITIVE_TOPOLOGY GetD3D12PrimitiveTopology() const;

src/dawn_native/d3d12/ShaderModuleD3D12.cpp

@@ -68,7 +68,7 @@ namespace dawn_native { namespace d3d12 {
         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 @@ namespace dawn_native { namespace d3d12 {
                 if (bindingInfo.used) {
                     uint32_t bindingOffset = bindingOffsets[binding];
                     if (GetDevice()->IsToggleEnabled(Toggle::UseSpvc)) {
-                        mSpvcContext.SetDecoration(bindingInfo.id, SHADERC_SPVC_DECORATION_BINDING,
+                        if (mSpvcContext.SetDecoration(
-                                                   bindingOffset);
+                                bindingInfo.id, SHADERC_SPVC_DECORATION_BINDING, bindingOffset) !=
-                        // TODO(dawn:301): Check status & have some sort of meaningful error path
+                            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 @@ namespace dawn_native { namespace d3d12 {
         }
         if (GetDevice()->IsToggleEnabled(Toggle::UseSpvc)) {
             shaderc_spvc::CompilationResult result;
-            mSpvcContext.CompileShader(&result);
+            if (mSpvcContext.CompileShader(&result) != shaderc_spvc_status_success) {
-            // TODO(dawn:301): Check status & have some sort of meaningful error path
+                return DAWN_VALIDATION_ERROR("Unable to generate HLSL shader w/ spvc");
-            return result.GetStringOutput();
+            }
+            std::string result_string =
+                result.GetStringOutput();  // Stripping const for ResultOrError
+            return result_string;
         } else {
             return compiler->compile();
         }

src/dawn_native/d3d12/ShaderModuleD3D12.h

@@ -27,7 +27,7 @@ namespace dawn_native { namespace d3d12 {
         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);