[llvm] [OMPIRBuilder] Simplify error handling while emitting target calls, NFC (PR #122477)

[Sergio Afonso via llvm-commits]

https://github.com/skatrak created https://github.com/llvm/llvm-project/pull/122477

The OMPIRBuilder uses `llvm::Error`s to allow callbacks passed to it to signal errors and prevent OMPIRBuilder functions to continue after one has been triggered. This means that OMPIRBuilder functions taking callbacks needs to be able to forward these errors, which must always be checked.

However, in cases where these functions are called from within the OMPIRBuilder with callbacks also defined inside of it, it can be known in advance that no errors will be produced. This is the case of those defined in `emitTargetCall`.

This patch introduces calls to the `cantFail` function instead of the previous superfluous checks that still assumed calls wouldn't fail, making these assumptions more obvious and simplifying their implementation.

>From cef12697c79bf26e86aaa6388ec1a26b65345f10 Mon Sep 17 00:00:00 2001
From: Sergio Afonso <safonsof at amd.com>
Date: Fri, 10 Jan 2025 13:56:07 +0000
Subject: [PATCH] [OMPIRBuilder] Simplify error handling while emitting target
 calls, NFC

The OMPIRBuilder uses `llvm::Error`s to allow callbacks passed to it to signal
errors and prevent OMPIRBuilder functions to continue after one has been
triggered. This means that OMPIRBuilder functions taking callbacks needs to be
able to forward these errors, which must always be checked.

However, in cases where these functions are called from within the OMPIRBuilder
with callbacks also defined inside of it, it can be known in advance that no
errors will be produced. This is the case of those defined in `emitTargetCall`.

This patch introduces calls to the `cantFail` function instead of the previous
superfluous checks that still assumed calls wouldn't fail, making these
assumptions more obvious and simplifying their implementation.
---
 llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp | 38 ++++++++++-------------
 1 file changed, 17 insertions(+), 21 deletions(-)

diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index 1edf47dff8c4a8..db77c6a5869764 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -7331,7 +7331,9 @@ emitTargetCall(OpenMPIRBuilder &OMPBuilder, IRBuilderBase &Builder,
   auto TaskBodyCB =
       [&](Value *DeviceID, Value *RTLoc,
           IRBuilderBase::InsertPoint TargetTaskAllocaIP) -> Error {
-    llvm::OpenMPIRBuilder::InsertPointOrErrorTy AfterIP = [&]() {
+    // Assume no error was returned because EmitTargetCallFallbackCB doesn't
+    // produce any.
+    llvm::OpenMPIRBuilder::InsertPointTy AfterIP = cantFail([&]() {
       // emitKernelLaunch makes the necessary runtime call to offload the
       // kernel. We then outline all that code into a separate function
       // ('kernel_launch_function' in the pseudo code above). This function is
@@ -7346,19 +7348,18 @@ emitTargetCall(OpenMPIRBuilder &OMPBuilder, IRBuilderBase &Builder,
       // When OutlinedFnID is set to nullptr, then it's not an offloading call.
       // In this case, we execute the host implementation directly.
       return EmitTargetCallFallbackCB(OMPBuilder.Builder.saveIP());
-    }();
-
-    if (!AfterIP)
-      return AfterIP.takeError();
+    }());
 
-    OMPBuilder.Builder.restoreIP(*AfterIP);
+    OMPBuilder.Builder.restoreIP(AfterIP);
     return Error::success();
   };
 
   // If we don't have an ID for the target region, it means an offload entry
   // wasn't created. In this case we just run the host fallback directly.
   if (!OutlinedFnID) {
-    OpenMPIRBuilder::InsertPointOrErrorTy AfterIP = [&]() {
+    // Assume no error was returned because EmitTargetCallFallbackCB doesn't
+    // produce any.
+    OpenMPIRBuilder::InsertPointTy AfterIP = cantFail([&]() {
       if (RequiresOuterTargetTask) {
         // Arguments that are intended to be directly forwarded to an
         // emitKernelLaunch call are pased as nullptr, since
@@ -7368,12 +7369,9 @@ emitTargetCall(OpenMPIRBuilder &OMPBuilder, IRBuilderBase &Builder,
                                          Dependencies, HasNoWait);
       }
       return EmitTargetCallFallbackCB(Builder.saveIP());
-    }();
+    }());
 
-    // Assume no error was returned because EmitTargetCallFallbackCB doesn't
-    // produce any. The 'if' check enables accessing the returned value.
-    if (AfterIP)
-      Builder.restoreIP(*AfterIP);
+    Builder.restoreIP(AfterIP);
     return;
   }
 
@@ -7411,9 +7409,11 @@ emitTargetCall(OpenMPIRBuilder &OMPBuilder, IRBuilderBase &Builder,
       NumTargetItems, RTArgs, NumIterations, NumTeamsC, NumThreadsC,
       DynCGGroupMem, HasNoWait);
 
-  // The presence of certain clauses on the target directive require the
-  // explicit generation of the target task.
-  OpenMPIRBuilder::InsertPointOrErrorTy AfterIP = [&]() {
+  // Assume no error was returned because TaskBodyCB and
+  // EmitTargetCallFallbackCB don't produce any.
+  OpenMPIRBuilder::InsertPointTy AfterIP = cantFail([&]() {
+    // The presence of certain clauses on the target directive require the
+    // explicit generation of the target task.
     if (RequiresOuterTargetTask)
       return OMPBuilder.emitTargetTask(TaskBodyCB, DeviceID, RTLoc, AllocaIP,
                                        Dependencies, HasNoWait);
@@ -7421,13 +7421,9 @@ emitTargetCall(OpenMPIRBuilder &OMPBuilder, IRBuilderBase &Builder,
     return OMPBuilder.emitKernelLaunch(Builder, OutlinedFnID,
                                        EmitTargetCallFallbackCB, KArgs,
                                        DeviceID, RTLoc, AllocaIP);
-  }();
+  }());
 
-  // Assume no error was returned because TaskBodyCB and
-  // EmitTargetCallFallbackCB don't produce any. The 'if' check enables
-  // accessing the returned value.
-  if (AfterIP)
-    Builder.restoreIP(*AfterIP);
+  Builder.restoreIP(AfterIP);
 }
 
 OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::createTarget(