[llvm-branch-commits] [llvm] [DirectX] Lower `@llvm.dx.typedBufferStore` to DXIL ops (PR #104253)

[Justin Bogner via llvm-branch-commits]

https://github.com/bogner updated https://github.com/llvm/llvm-project/pull/104253

>From d49a863313c30d3e75205dbffddc8f81980ed25b Mon Sep 17 00:00:00 2001
From: Justin Bogner <mail at justinbogner.com>
Date: Mon, 9 Sep 2024 17:55:15 -0700
Subject: [PATCH] Propagate HasErrors through return values

Created using spr 1.3.5-bogner
---
 llvm/lib/Target/DirectX/DXILOpLowering.cpp | 49 ++++++++++++----------
 1 file changed, 26 insertions(+), 23 deletions(-)

diff --git a/llvm/lib/Target/DirectX/DXILOpLowering.cpp b/llvm/lib/Target/DirectX/DXILOpLowering.cpp
index 02909c41016c72..f968cab1dccf1e 100644
--- a/llvm/lib/Target/DirectX/DXILOpLowering.cpp
+++ b/llvm/lib/Target/DirectX/DXILOpLowering.cpp
@@ -78,13 +78,15 @@ class OpLowerer {
   DXILOpBuilder OpBuilder;
   DXILResourceMap &DRM;
   SmallVector<CallInst *> CleanupCasts;
-  bool HasErrors = false;
 
 public:
   OpLowerer(Module &M, DXILResourceMap &DRM) : M(M), OpBuilder(M), DRM(DRM) {}
 
-  void replaceFunction(Function &F,
-                       llvm::function_ref<Error(CallInst *CI)> ReplaceCall) {
+  /// Replace every call to \c F using \c ReplaceCall, and then erase \c F. If
+  /// there is an error replacing a call, we emit a diagnostic and return true.
+  [[nodiscard]] bool
+  replaceFunction(Function &F,
+                  llvm::function_ref<Error(CallInst *CI)> ReplaceCall) {
     for (User *U : make_early_inc_range(F.users())) {
       CallInst *CI = dyn_cast<CallInst>(U);
       if (!CI)
@@ -95,17 +97,18 @@ class OpLowerer {
         DiagnosticInfoUnsupported Diag(*CI->getFunction(), Message,
                                        CI->getDebugLoc());
         M.getContext().diagnose(Diag);
-        HasErrors = true;
-        continue;
+        return true;
       }
     }
     if (F.user_empty())
       F.eraseFromParent();
+    return false;
   }
 
-  void replaceFunctionWithOp(Function &F, dxil::OpCode DXILOp) {
+  [[nodiscard]]
+  bool replaceFunctionWithOp(Function &F, dxil::OpCode DXILOp) {
     bool IsVectorArgExpansion = isVectorArgExpansion(F);
-    replaceFunction(F, [&](CallInst *CI) -> Error {
+    return replaceFunction(F, [&](CallInst *CI) -> Error {
       SmallVector<Value *> Args;
       OpBuilder.getIRB().SetInsertPoint(CI);
       if (IsVectorArgExpansion) {
@@ -177,12 +180,12 @@ class OpLowerer {
     CleanupCasts.clear();
   }
 
-  void lowerToCreateHandle(Function &F) {
+  [[nodiscard]] bool lowerToCreateHandle(Function &F) {
     IRBuilder<> &IRB = OpBuilder.getIRB();
     Type *Int8Ty = IRB.getInt8Ty();
     Type *Int32Ty = IRB.getInt32Ty();
 
-    replaceFunction(F, [&](CallInst *CI) -> Error {
+    return replaceFunction(F, [&](CallInst *CI) -> Error {
       IRB.SetInsertPoint(CI);
 
       auto *It = DRM.find(CI);
@@ -207,10 +210,10 @@ class OpLowerer {
     });
   }
 
-  void lowerToBindAndAnnotateHandle(Function &F) {
+  [[nodiscard]] bool lowerToBindAndAnnotateHandle(Function &F) {
     IRBuilder<> &IRB = OpBuilder.getIRB();
 
-    replaceFunction(F, [&](CallInst *CI) -> Error {
+    return replaceFunction(F, [&](CallInst *CI) -> Error {
       IRB.SetInsertPoint(CI);
 
       auto *It = DRM.find(CI);
@@ -253,12 +256,11 @@ class OpLowerer {
 
   /// Lower `dx.handle.fromBinding` intrinsics depending on the shader model and
   /// taking into account binding information from DXILResourceAnalysis.
-  void lowerHandleFromBinding(Function &F) {
+  bool lowerHandleFromBinding(Function &F) {
     Triple TT(Triple(M.getTargetTriple()));
     if (TT.getDXILVersion() < VersionTuple(1, 6))
-      lowerToCreateHandle(F);
-    else
-      lowerToBindAndAnnotateHandle(F);
+      return lowerToCreateHandle(F);
+    return lowerToBindAndAnnotateHandle(F);
   }
 
   /// Replace uses of \c Intrin with the values in the `dx.ResRet` of \c Op.
@@ -344,11 +346,11 @@ class OpLowerer {
     return Error::success();
   }
 
-  void lowerTypedBufferLoad(Function &F) {
+  [[nodiscard]] bool lowerTypedBufferLoad(Function &F) {
     IRBuilder<> &IRB = OpBuilder.getIRB();
     Type *Int32Ty = IRB.getInt32Ty();
 
-    replaceFunction(F, [&](CallInst *CI) -> Error {
+    return replaceFunction(F, [&](CallInst *CI) -> Error {
       IRB.SetInsertPoint(CI);
 
       Value *Handle =
@@ -370,12 +372,12 @@ class OpLowerer {
     });
   }
 
-  void lowerTypedBufferStore(Function &F) {
+  [[nodiscard]] bool lowerTypedBufferStore(Function &F) {
     IRBuilder<> &IRB = OpBuilder.getIRB();
     Type *Int8Ty = IRB.getInt8Ty();
     Type *Int32Ty = IRB.getInt32Ty();
 
-    replaceFunction(F, [&](CallInst *CI) -> Error {
+    return replaceFunction(F, [&](CallInst *CI) -> Error {
       IRB.SetInsertPoint(CI);
 
       Value *Handle =
@@ -414,6 +416,7 @@ class OpLowerer {
 
   bool lowerIntrinsics() {
     bool Updated = false;
+    bool HasErrors = false;
 
     for (Function &F : make_early_inc_range(M.functions())) {
       if (!F.isDeclaration())
@@ -424,17 +427,17 @@ class OpLowerer {
         continue;
 #define DXIL_OP_INTRINSIC(OpCode, Intrin)                                      \
   case Intrin:                                                                 \
-    replaceFunctionWithOp(F, OpCode);                                          \
+    HasErrors |= replaceFunctionWithOp(F, OpCode);                             \
     break;
 #include "DXILOperation.inc"
       case Intrinsic::dx_handle_fromBinding:
-        lowerHandleFromBinding(F);
+        HasErrors |= lowerHandleFromBinding(F);
         break;
       case Intrinsic::dx_typedBufferLoad:
-        lowerTypedBufferLoad(F);
+        HasErrors |= lowerTypedBufferLoad(F);
         break;
       case Intrinsic::dx_typedBufferStore:
-        lowerTypedBufferStore(F);
+        HasErrors |= lowerTypedBufferStore(F);
         break;
       }
       Updated = true;