[Mlir-commits] [clang] [llvm] [mlir] [OMPIRBuilder] - Handle dependencies in `createTarget` (PR #93977)

[Pranav Bhandarkar]

================
@@ -5272,36 +5682,53 @@ static void emitTargetCall(OpenMPIRBuilder &OMPBuilder, IRBuilderBase &Builder,
   Value *DynCGGroupMem = Builder.getInt32(0);
 
   bool HasNoWait = false;
+  bool HasDependencies = Dependencies.size() > 0;
+  bool RequiresOuterTargetTask = HasNoWait || HasDependencies;
 
   OpenMPIRBuilder::TargetKernelArgs KArgs(NumTargetItems, RTArgs, NumIterations,
                                           NumTeamsVal, NumThreadsVal,
                                           DynCGGroupMem, HasNoWait);
 
-  Builder.restoreIP(OMPBuilder.emitKernelLaunch(
-      Builder, OutlinedFn, OutlinedFnID, EmitTargetCallFallbackCB, KArgs,
-      DeviceID, RTLoc, AllocaIP));
+  // The presence of certain clauses on the target directive require the
+  // explicit generation of the target task.
+  if (RequiresOuterTargetTask) {
+    OMPBuilder.emitTargetTask(OutlinedFn, OutlinedFnID,
----------------
bhandarkar-pranav wrote:

Thanks for point this out. I want to use this opportunity to understand this idiom I see all around in `OMPIRBuilder.cpp` which is ``Builder.restoreIP(FunctThatReturnsInsertPointTy())``. Now, if the only return from `FuncThatReturnsInsertPointTy` is `Builder.saveIP()`, then do I need a call to `Builder.restoreIP()` on return ? Because `emitTargetTask()` returns `Builder.saveIP()`, which, in other words, means it is returning the state of the Builder's insert point. We then restore the same `Builder` to that insert point which sounds redundant. However, I see this in a number of places in `OMPIRBuilder.cpp`

https://github.com/llvm/llvm-project/pull/93977