This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP] Codegen for use_device_ptr clause.
ClosedPublic

Authored by sfantao on Jul 22 2016, 11:08 AM.

Details

Summary

This patch adds support for the use_device_ptr clause. It includes changes in SEMA that could not be tested without codegen, namely, the use of the first private logic and mappable expressions support.

Diff Detail

Event Timeline

sfantao updated this revision to Diff 65105.Jul 22 2016, 11:08 AM
sfantao retitled this revision from to [OpenMP] Codegen for use_device_ptr clause..
sfantao updated this object.
sfantao added subscribers: cfe-commits, caomhin.
sfantao updated this revision to Diff 65123.Jul 22 2016, 12:39 PM
  • Use the correct enumerator for return pointer, as specified in the design document.
ABataev requested changes to this revision.Jul 24 2016, 9:30 PM
ABataev edited edge metadata.
ABataev added inline comments.
include/clang/AST/OpenMPClause.h
4252

No \brief

4272

Remove \brief

4289–4309

No \brief

4318

Do not add \brief

4336

Do not add \brief

lib/CodeGen/CGStmtOpenMP.cpp
3410–3411

EmitOMPUseDevicePtrClause() should emit the code for single clause. The outer loop must be used in directive emission function.

3471–3489

PvtCodeGen and NoPvtCodeGen are pretty similar. Could you merge them somehow into the one? Maybe some custom PrePostAction could be used here?

lib/Sema/SemaOpenMP.cpp
11866

Are the restrictions for firstprivates applied also? What if inner directive has some clauses that cannot be used with variables previously marked as firstprivates, but can be used with variables, used in use_device_ptr?

This revision now requires changes to proceed.Jul 24 2016, 9:30 PM
sfantao updated this revision to Diff 65810.Jul 27 2016, 2:53 PM
sfantao edited edge metadata.
sfantao marked 8 inline comments as done.
  • Remove unnecessary brief directives and refactor target data privatization code genenration.

Hi Alexey,

Thanks for the review!

lib/CodeGen/CGStmtOpenMP.cpp
3410–3411

Ok ,makes sense, Doing as you say.

3471–3489

I replaced this code by a single region codegen controlling the privatization with a flag and a pre-action. Let me know if this what you had in mind.

lib/Sema/SemaOpenMP.cpp
11866

I was revisiting the spec and I couldn't find any conflict with the firstprivate restrictions. There are conflicts for variables used in a firstprivate clause that are also used in clauses of enclosing worksharing constructs but not the other way around. Note that we are using the firstprivate attribute but do not do all the firstprivate checks, so the conflicts would only arise from other enclosed constructs, not other enclosing constructs. Let me know if you have any specific case in mind or if you'd like me to do this differently.

ABataev accepted this revision.Jul 27 2016, 7:52 PM
ABataev edited edge metadata.

LG

This revision is now accepted and ready to land.Jul 27 2016, 7:52 PM
sfantao closed this revision.Jul 28 2016, 7:31 AM