Make BuildComputePipeline fail if the shader does not pass validation

Validation might fail e.g. because the shader entry point is missing. This change allows the error to bubble up through the call stack until vkCreateComputePipelines, instead of killing the app later on with exit code 1. BuildComputePipeline is converted to early-return style.
GPUOpen-Drivers · Dec 12, 2023 · f8feeaa · f8feeaa
1 parent 0dac4f3
commit f8feeaa
Showing 1 changed file with 25 additions and 22 deletions.
diff --git a/llpc/context/llpcCompiler.cpp b/llpc/context/llpcCompiler.cpp
@@ -2289,13 +2289,15 @@ Result Compiler::BuildComputePipeline(const ComputePipelineBuildInfo *pipelineIn
   const bool buildUsingRelocatableElf = relocatableElfRequested && canUseRelocatableComputeShaderElf(pipelineInfo);
 
   Result result = validatePipelineShaderInfo(&pipelineInfo->cs);
+  if (result != Result::Success)
+    return result;
 
   MetroHash::Hash cacheHash = {};
   MetroHash::Hash pipelineHash = {};
   cacheHash = PipelineDumper::generateHashForComputePipeline(pipelineInfo, true);
   pipelineHash = PipelineDumper::generateHashForComputePipeline(pipelineInfo, false);
 
-  if (result == Result::Success && EnableOuts()) {
+  if (EnableOuts()) {
     const ShaderModuleData *moduleData = reinterpret_cast<const ShaderModuleData *>(pipelineInfo->cs.pModuleData);
     auto moduleHash = reinterpret_cast<const MetroHash::Hash *>(&moduleData->hash[0]);
     LLPC_OUTS("\n===============================================================================\n");
@@ -2310,8 +2312,7 @@ Result Compiler::BuildComputePipeline(const ComputePipelineBuildInfo *pipelineIn
     LLPC_OUTS("\n");
   }
 
-  if (result == Result::Success)
-    dumpCompilerOptions(pipelineDumpFile);
+  dumpCompilerOptions(pipelineDumpFile);
 
   std::optional<CacheAccessor> cacheAccessor;
   if (cl::CacheFullPipelines) {
@@ -2325,40 +2326,42 @@ Result Compiler::BuildComputePipeline(const ComputePipelineBuildInfo *pipelineIn
     result = buildComputePipelineInternal(&computeContext, pipelineInfo, buildUsingRelocatableElf, &candidateElf,
                                           &pipelineOut->stageCacheAccess);
 
-    if (result == Result::Success) {
-      elfBin.codeSize = candidateElf.size();
-      elfBin.pCode = candidateElf.data();
-    }
     if (cacheAccessor && pipelineOut->pipelineCacheAccess == CacheAccessInfo::CacheNotChecked)
       pipelineOut->pipelineCacheAccess = CacheAccessInfo::CacheMiss;
+
+    if (result != Result::Success) {
+      return result;
+    }
+    elfBin.codeSize = candidateElf.size();
+    elfBin.pCode = candidateElf.data();
   } else {
     LLPC_OUTS("Cache hit for compute pipeline.\n");
     elfBin = cacheAccessor->getElfFromCache();
     pipelineOut->pipelineCacheAccess = CacheAccessInfo::InternalCacheHit;
   }
 
-  if (result == Result::Success) {
-    void *allocBuf = nullptr;
-    if (pipelineInfo->pfnOutputAlloc) {
-      allocBuf = pipelineInfo->pfnOutputAlloc(pipelineInfo->pInstance, pipelineInfo->pUserData, elfBin.codeSize);
-      if (allocBuf) {
-        uint8_t *code = static_cast<uint8_t *>(allocBuf);
-        memcpy(code, elfBin.pCode, elfBin.codeSize);
+  void *allocBuf = nullptr;
+  if (pipelineInfo->pfnOutputAlloc) {
+    allocBuf = pipelineInfo->pfnOutputAlloc(pipelineInfo->pInstance, pipelineInfo->pUserData, elfBin.codeSize);
+    if (allocBuf) {
+      uint8_t *code = static_cast<uint8_t *>(allocBuf);
+      memcpy(code, elfBin.pCode, elfBin.codeSize);
 
-        pipelineOut->pipelineBin.codeSize = elfBin.codeSize;
-        pipelineOut->pipelineBin.pCode = code;
-      } else
-        result = Result::ErrorOutOfMemory;
+      pipelineOut->pipelineBin.codeSize = elfBin.codeSize;
+      pipelineOut->pipelineBin.pCode = code;
     } else {
-      // Allocator is not specified
-      result = Result::ErrorInvalidPointer;
+      return Result::ErrorOutOfMemory;
     }
+  } else {
+    // Allocator is not specified
+    return Result::ErrorInvalidPointer;
   }
 
-  if (cacheAccessor && !cacheAccessor->isInCache() && result == Result::Success) {
+  if (cacheAccessor && !cacheAccessor->isInCache()) {
     cacheAccessor->setElfInCache(elfBin);
   }
-  return result;
+
+  return Result::Success;
 }
 
 // =====================================================================================================================