This is an archive of the discontinued LLVM Phabricator instance.

[OPENMP] Codegen for 'private' clause in 'task' directive.
ClosedPublic

Authored by ABataev on Apr 28 2015, 5:25 AM.

Details

Summary

For tasks codegen for private/firstprivate variables are different rather than for other directives.

  1. Build an internal structure of privates for each private variable:
struct .kmp_privates_t. {
  Ty1 var1;
  ...
  Tyn varn;
};
  1. Add a new field to kmp_task_t type with list of privates.
struct kmp_task_t {
void *              shareds;
kmp_routine_entry_t routine;
kmp_int32           part_id;
kmp_routine_entry_t destructors;
.kmp_privates_t.    privates;
};
  1. Create a function with destructors calls for all privates after end of task region.
kmp_int32 .omp_task_destructor.(kmp_int32 gtid, kmp_task_t *tt) {
  ~Destructor(&tt->privates.var1);
  ...
  ~Destructor(&tt->privates.varn);
  return 0;
}
  1. Perform default initialization of all private fields (no initialization for POD data, default constructor calls for classes) + provide address of a destructor function after kmpc_omp_task_alloc() and before kmpc_omp_task() calls.
kmp_task_t *new_task = __kmpc_omp_task_alloc(ident_t *, kmp_int32 gtid, kmp_int32 flags, size_t sizeof_kmp_task_t, size_t sizeof_shareds, kmp_routine_entry_t *task_entry);

DefaultConstructor(new_task->privates.var1);
new_task->shareds.var1_ref = &new_task->privates.var1;
...
DefaultConstructor(new_task->privates.varn);
new_task->shareds.varn_ref = &new_task->privates.varn;

new_task->destructors = .omp_task_destructor.;
kmp_int32 __kmpc_omp_task(ident_t *, kmp_int32 gtid, kmp_task_t *new_task)

Diff Detail

Repository
rL LLVM

Event Timeline

ABataev updated this revision to Diff 24542.Apr 28 2015, 5:25 AM
ABataev retitled this revision from to [OPENMP] Codegen for 'private' clause in 'task' directive..
ABataev updated this object.
ABataev edited the test plan for this revision. (Show Details)
ABataev added reviewers: rjmccall, hfinkel.
ABataev added subscribers: fraggamuffin, ejstotzer, Unknown Object (MLST).
ABataev updated this revision to Diff 24596.Apr 28 2015, 9:35 PM
ABataev updated this object.

Moved propagation of private values to shared from proxy function to initialization section.

rjmccall edited edge metadata.Apr 29 2015, 10:14 AM

A few minor suggestions, but overall LGTM.

lib/CodeGen/CGOpenMPRuntime.cpp
1865 ↗(On Diff #24596)

"Copy *addresses* of privates", I think.

lib/CodeGen/CGStmtOpenMP.cpp
1391 ↗(On Diff #24596)

You might want to consider sorting this array by alignment, which will give you an optimal packing in the structure you end up building. On the other hand, it might be better to do that implicitly when building the structure later, then just keep a map from the index in this array to the index there.

Is there a good reason to pass around Privates as an array of Expr* instead of an array of VarDecl*? It feels like you just end up having to drill down to the declaration constantly.

Also, why not make a single SmallVector of a struct containing both the VarDecl* and the copy expr?

John, thanks for the review!

lib/CodeGen/CGStmtOpenMP.cpp
1391 ↗(On Diff #24596)

I will sort it when building the structure, I don't think that we need to expose some specific details to general codegen procedure.
I can provide a list of VarDecls*, but all other similar methods accept ArrayRef<const Expr *>, not ArrayRef<const VarDecl *>. I just don't want to break some "uniformness" of CGOpenMPRuntime methods.
Also I thought about joining two lists in one, but again it will break some already used format of interface. But I think I will join them internally in CGOpenMPRuntime.

This revision was automatically updated to reflect the committed changes.