[OpenMP] Lower taskwait using OpenMP IR Builder

[rogfer01]

Summary

The code generation is exactly the same as it was.

But note that the special handling of untied tasks is still handled by emitUntiedSwitch in clang.

[jdoerfert]

Also for taskwait inside untied tasks we should be reusing the available global_tid instead of calling __kmpc_global_thread_num again.

FWIW, calling getThreadID(getIdent(SrcLocStr)) is the right thing to do here. We do use a cache in that method and at the end of the optimization pipeline, after inlining, etc. we can replace/merge omp_get_thread_id, __kmpc_global_thread_num, and global_tid arguments as a separate (more powerful) step.

llvm/lib/IR/OpenMPIRBuilder.cpp

230 ↗

(On Diff #227790)

I will modify my patch to allow an insertion point without block (in order to mimic Clang for now). Other than that in the emit vs. create thing, I think this is fine.

Nit: We both could actually cache getIdent(SrcLocStr) so we don't have to go through the map ;)

[rogfer01]

ChangeLog:

• Update patch against changes in D69785

[rogfer01]

ChangeLog:

• Enable the new code generation in codegen tests using taskwait

[jdoerfert]

I think this is fine, one comment though.

llvm/include/llvm/Frontend/OpenMPIRBuilder.h

81 ↗

(On Diff #231114)

If taskwait cannot cause control flow we don't need to return a new insert point, I think.

126 ↗

(On Diff #231114)

Copy & paste, also see my other comment.

[jdoerfert]

Also for taskwait inside untied tasks we should be reusing the available global_tid instead of calling __kmpc_global_thread_num again.

FWIW, We will just clean up in a later pass (under review). We have to do that anyway, adding some smarts here is barely worth it and I also removed similar logic from my patches.

[jdoerfert]

LGTM, assuming the comments are addressed (incl. the commit message)

[rogfer01]

In D69828#1761133, @jdoerfert wrote:

Also for taskwait inside untied tasks we should be reusing the available global_tid instead of calling __kmpc_global_thread_num again.

FWIW, We will just clean up in a later pass (under review). We have to do that anyway, adding some smarts here is barely worth it and I also removed similar logic from my patches.

Sure. I actually forgot to update the summary because you already mentioned this IIRC.

clang/lib/CodeGen/CGOpenMPRuntime.cpp

6347

This is to preserve exactly the same behaviour as before.

[rogfer01]

llvm/include/llvm/Frontend/OpenMPIRBuilder.h

81 ↗

(On Diff #231114)

Ah I see, we want a similar behaviour to that of IRBuilder in which the new IP is updated after something has been emitted.

Makes sense.

[rogfer01]

ChangeLog:

• Remove unnecessary return of the insertion point and update code accordingly.

[kiranchandramohan]

llvm/lib/Frontend/OpenMPIRBuilder.cpp

258 ↗

(On Diff #231207)

Is the emit function required here? We could have this in the create function itself right?

[rogfer01]

ChangeLog:

• Rebase

[rogfer01]

ChangeLog:

• Rebase

[rogfer01]

ChangeLog:

• Rebase

clang/lib/CodeGen/CGOpenMPRuntime.cpp

return Address(
           createRuntimeFunction(OMPRTL__kmpc_task_reduction_get_th_data), Args),
       SharedLVal.getAlignment());
 }
 
 void CGOpenMPRuntime::emitTaskwaitCall(CodeGenFunction &CGF,
                                        SourceLocation Loc) {
   if (!CGF.HaveInsertPoint())
     return;
+
+  llvm::OpenMPIRBuilder *OMPBuilder = CGF.CGM.getOpenMPIRBuilder();
+  if (OMPBuilder) {
+    OMPBuilder->CreateTaskwait(CGF.Builder);
+  } else {
     // Build call kmp_int32 __kmpc_omp_taskwait(ident_t *loc, kmp_int32
     // global_tid);
     llvm::Value *Args[] = {emitUpdateLocation(CGF, Loc), getThreadID(CGF, Loc)};
     // Ignore return result until untied tasks are supported.
     CGF.EmitRuntimeCall(createRuntimeFunction(OMPRTL__kmpc_omp_taskwait), Args);
+  }
+
   if (auto *Region = dyn_cast_or_null<CGOpenMPRegionInfo>(CGF.CapturedStmtInfo))
     Region->emitUntiedSwitch(CGF);
 }
 
 void CGOpenMPRuntime::emitInlinedDirective(CodeGenFunction &CGF,
                                            OpenMPDirectiveKind InnerKind,
                                            const RegionCodeGenTy &CodeGen,
                                            bool HasCancel) {

clang/test/OpenMP/task_codegen.cpp

 // RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -x c++ -emit-llvm %s -o - | FileCheck %s
 // RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-apple-darwin10 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-apple-darwin10 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+//
+// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -fopenmp-enable-irbuilder -x c++ -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-enable-irbuilder -x c++ -triple x86_64-apple-darwin10 -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-enable-irbuilder -x c++ -triple x86_64-apple-darwin10 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
 
 // RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp-simd -x c++ -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
 // RUN: %clang_cc1 -fopenmp-simd -x c++ -triple x86_64-apple-darwin10 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp-simd -x c++ -triple x86_64-apple-darwin10 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY0 %s
 // SIMD-ONLY0-NOT: {{__kmpc|__tgt}}
 // expected-no-diagnostics
 #ifndef HEADER
 #define HEADER

clang/test/OpenMP/taskwait_codegen.cpp

 // RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -x c++ -emit-llvm %s -o - | FileCheck %s
 // RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-apple-darwin10 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-apple-darwin10 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+//
+// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -fopenmp-enable-irbuilder -x c++ -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-enable-irbuilder -x c++ -triple x86_64-apple-darwin10 -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-enable-irbuilder -x c++ -triple x86_64-apple-darwin10 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
 
 // RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp-simd -x c++ -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
 // RUN: %clang_cc1 -fopenmp-simd -x c++ -triple x86_64-apple-darwin10 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp-simd -x c++ -triple x86_64-apple-darwin10 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY0 %s
 // SIMD-ONLY0-NOT: {{__kmpc|__tgt}}
 // expected-no-diagnostics
 #ifndef HEADER
 #define HEADER

llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h

CreateParallel(const LocationDescription &Loc, BodyGenCallbackTy BodyGenCB,
                  Value *IfCondition, Value *NumThreads,
                  omp::ProcBindKind ProcBind, bool IsCancellable);
 
   /// Generator for '#omp flush'
   ///
   /// \param Loc The location where the flush directive was encountered
   void CreateFlush(const LocationDescription &Loc);
 
+  /// Generator for '#omp taskwait'
+  ///
+  /// \param Loc The location where the taskwait directive was encountered.
+  void CreateTaskwait(const LocationDescription& Loc);
+
   ///}
 
 
 private:
   /// Update the internal location to \p Loc.
   bool updateToLocation(const LocationDescription &Loc) {
     Builder.restoreIP(Loc.IP);
     Builder.SetCurrentDebugLocation(Loc.DL);
private:
   /// Return true if the last entry in the finalization stack is of kind \p DK
   /// and cancellable.
   bool isLastFinalizationInfoCancellable(omp::Directive DK) {
     return !FinalizationStack.empty() &&
            FinalizationStack.back().IsCancellable &&
            FinalizationStack.back().DK == DK;
   }
 
+  /// Generate a taskwait runtime call.
+  ///
+  /// \param Loc The location at which the request originated and is fulfilled.
+  void emitTaskwaitImpl(const LocationDescription &Loc);
+
   /// Return the current thread ID.
   ///
   /// \param Ident The ident (ident_t*) describing the query origin.
   Value *getOrCreateThreadID(Value *Ident);
 
   /// The underlying LLVM-IR module
   Module &M;
 

llvm/include/llvm/Frontend/OpenMP/OMPKinds.def

#define __OMP_RTL(Name, IsVarArg, ReturnType, ...)                             \
   OMP_RTL(OMPRTL_##Name, #Name, IsVarArg, ReturnType, __VA_ARGS__)
 
 __OMP_RTL(__kmpc_barrier, false, Void, IdentPtr, Int32)
 __OMP_RTL(__kmpc_cancel, false, Int32, IdentPtr, Int32, Int32)
 __OMP_RTL(__kmpc_cancel_barrier, false, Int32, IdentPtr, Int32)
 __OMP_RTL(__kmpc_flush, false, Void, IdentPtr)
 __OMP_RTL(__kmpc_global_thread_num, false, Int32, IdentPtr)
 __OMP_RTL(__kmpc_fork_call, true, Void, IdentPtr, Int32, ParallelTaskPtr)
+__OMP_RTL(__kmpc_omp_taskwait, false, Int32, IdentPtr, Int32)
 __OMP_RTL(__kmpc_push_num_threads, false, Void, IdentPtr, Int32,
           /* Int */ Int32)
 __OMP_RTL(__kmpc_push_proc_bind, false, Void, IdentPtr, Int32, /* Int */ Int32)
 __OMP_RTL(__kmpc_serialized_parallel, false, Void, IdentPtr, Int32)
 __OMP_RTL(__kmpc_end_serialized_parallel, false, Void, IdentPtr, Int32)
 
 __OMP_RTL(omp_get_thread_num, false, Int32, )
 __OMP_RTL(omp_get_num_threads, false, Int32, )

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

 }
 
 void OpenMPIRBuilder::CreateFlush(const LocationDescription &Loc)
 {
   if (!updateToLocation(Loc))
    return;
   emitFlush(Loc);
 }
+
+void OpenMPIRBuilder::emitTaskwaitImpl(const LocationDescription &Loc) {
+  // Build call kmp_int32 __kmpc_omp_taskwait(ident_t *loc, kmp_int32
+  // global_tid);
+  Constant *SrcLocStr = getOrCreateSrcLocStr(Loc);
+  Value *Ident = getOrCreateIdent(SrcLocStr);
+  Value *Args[] = {Ident, getOrCreateThreadID(Ident)};
+
+  // Ignore return result until untied tasks are supported.
+  Builder.CreateCall(getOrCreateRuntimeFunction(OMPRTL___kmpc_omp_taskwait),
+                     Args);
+}
+
+void OpenMPIRBuilder::CreateTaskwait(const LocationDescription &Loc) {
+  if (!updateToLocation(Loc))
+    return;
+  emitTaskwaitImpl(Loc);
+}