This is an archive of the discontinued LLVM Phabricator instance.

[OPENMP] Basic teams directive implementation
ClosedPublic

Authored by carlo.bertolli on Feb 11 2016, 9:10 AM.

Details

Summary

This patch implements code generation for the teams directive. It only provides support for num_teams and thread_limit clauses, while all other clauses will come with later patches.

Diff Detail

Repository
rL LLVM

Event Timeline

carlo.bertolli retitled this revision from to [OPENMP] Basic teams directive implementation.
carlo.bertolli updated this object.
carlo.bertolli set the repository for this revision to rL LLVM.
carlo.bertolli updated this object.
carlo.bertolli removed rL LLVM as the repository for this revision.

Remove handling of reductions - not supported by this patch.

ABataev added inline comments.Feb 11 2016, 9:17 PM
lib/CodeGen/CGOpenMPRuntime.cpp
4616–4636

I don't like the idea of processing num_teams and thread_limit clauses in runtime lib. This must be done in emitCommonOMPTeamsDirective(), not in runtime

4651–4662

This is very similar to emitParallelOutlinedFunction(). Maybe it is a good idea to join them into a single one?

carlo.bertolli marked 2 inline comments as done.Feb 12 2016, 7:58 PM

I applied your suggestions and generated a new diff file.
Thanks!

Apply changes to reflect review: have a single emit outlined function call for both parallel and teams; call push_num_teams outside of runtime class.

ABataev added inline comments.Feb 15 2016, 12:52 AM
lib/CodeGen/CGOpenMPRuntime.h
799–805

This must be split in 2 functions: emitNumTeamsClause() and emitThreadLimitClause().

lib/CodeGen/CGStmtOpenMP.cpp
2710

Add comment with the name of argument for 'true' parameter.

carlo.bertolli marked an inline comment as done.Feb 15 2016, 9:05 AM

I updated the patch to reflect the comments. I will wait for an answer to the comment asking to split the emit function for push_num_teams. Thanks!

lib/CodeGen/CGOpenMPRuntime.h
799–805

I do not understand this: thread_limit and num_teams can only be input to the library function kmpc_push_num_teams, which accepts values corresponding to those two parameters.
The thread_limit clause does not provoke calling of kmpc_push_num_threads.
If I have two emit functions, I would have to either have two push_num_teams calls (both with a 0 parameter) or save somewhere either ThreadLimit or NumTeams.

carlo.bertolli set the repository for this revision to rL LLVM.

Added comment for boolean parameter.

Update against dependence D17019.

ABataev added inline comments.Feb 18 2016, 5:28 AM
lib/CodeGen/CGOpenMPRuntime.cpp
4627–4632

What is this? Copy-paste?

4633–4634

This must be checked at the start of the function

lib/CodeGen/CGOpenMPRuntime.h
799–805

Ok, just missed it.

lib/CodeGen/CGStmtOpenMP.cpp
2704–2705

These vars must be declared inside 'if' stmt for clauses.

2730

I don't think we need this comment now.

carlo.bertolli marked 4 inline comments as done.Feb 18 2016, 11:05 AM
carlo.bertolli added inline comments.
lib/CodeGen/CGOpenMPRuntime.cpp
4627–4632

Yes, it is - sorry!

Addressed latest comments.

Update against trunk.

ABataev accepted this revision.Feb 19 2016, 8:06 PM
ABataev edited edge metadata.

LG

This revision is now accepted and ready to land.Feb 19 2016, 8:06 PM

Thanks - waiting for dependence D17019 before commit.

Committed revision 262652.