[Openmp-commits] [PATCH] D71989: [OpenMP][IRBuilder][WIP] Prototype `omp task` support

[Johannes Doerfert via Phabricator via Openmp-commits]

jdoerfert added subscribers: AMDChirag, anchu-rajendran, kiranktp, SouraVX, Meinersbur.
jdoerfert added a comment.
Herald added subscribers: sstefan1, yaxunl.

@fghanim @kiranchandramohan @kiranktp @AMDChirag @anchu-rajendran @SouraVX @Meinersbur, Anyone interested in taking this over? The required changes in addition to the diff have been discussed with @AndreyChurbanov in the comments above.



================
Comment at: openmp/runtime/src/kmp_tasking.cpp:1764
+    char *dest = static_cast<char *>(task->shareds);
+    memcpy(dest, shared_and_private_vars, sizeof_shared_and_private_vars);
+
----------------
rogfer01 wrote:
> Just double-checking here I understand what would happen here for C++ firstprivatized objects (sorry if I'm asking the obvious):
> 
> We would capture the copy onto an `alloc`ed memory and then we would copy that memory to the task struct?
> 
> So given something like
> 
> ```lang=cpp
> struct A {
>   A(int x = 3);
>   void bar();
> };
> 
> void foo() {
>   A a;
>   #pragma omp task
>   {
>     a.bar();
>   }
> }
> ```
> 
> we'd do (in pseudo-C)
> 
> ```lang=c
> void foo() {
>    struct task_env_0 { 
>       char A_storage[sizeof(A)]; // properly aligned and all that
>    } tenv_0;
> 
>    A::A(&tenv_0.A_storage, 3); // capture
> 
>    // This would happen (dynamically) inside __kmpc_task
>    kmp_task_t* new_task = __kmp_omp_task_alloc( , sizeof(tenv_0), ... );
>    memcpy(new_task->shareds, &tenv_0, sizeof(tenv_0));
>    kmpc_omp_task(..., new_task).
>    //
> }
> ```
> 
> So we go from
> 
>  - create task
>  - capture environment in the task context
>  - queue task / run immediately for if0
> 
> to
> 
>   - capture environment in a local storage
>   - allocate task + copy local environment to task environment if needed (for tasks that are not if0) + queue task / run immediately for if0
> 
> Did I get that right?
> 
> Thanks!
>  Did I get that right?

Yes, that was the idea.

(Apologies for the delayed response)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71989/new/

https://reviews.llvm.org/D71989