Page MenuHomePhabricator

[OPENMP] Parsing and Sema support for 'omp declare target' directive
ClosedPublic

Authored by DmitryPolukhin on Mar 29 2016, 1:51 AM.

Details

Summary

Add parsing, sema analysis for 'declare target' construct for OpenMP 4.0
(4.5 support will be added in separate patch).

The declare target directive specifies that variables, functions (C, C++
and Fortran), and subroutines (Fortran) are mapped to a device. The declare
target directive is a declarative directive. In Clang declare target is
implemented as implicit attribute for the declaration.

The syntax of the declare target directive is as follows:

#pragma omp declare target
declarations-definition-seq
#pragma omp end declare target

Based on patch from Michael Wong http://reviews.llvm.org/D15321

Diff Detail

Event Timeline

DmitryPolukhin retitled this revision from to [OPENMP] Parsing and Sema support for 'omp declare target' directive.
DmitryPolukhin updated this object.
  • Added test for templates in declare target region

Aaron,

Could you please take a look to OMPDeclareTargetDecl attribute implementation and printPrettyEnd approach in general?
For post print mechanism alternative approach is to use ad hoc solution in DeclPrinter.

Thanks,
Dmitry

Software Engineer
Intel Compiler Team

aaron.ballman edited edge metadata.Mar 31 2016, 9:11 AM

Could you please take a look to OMPDeclareTargetDecl attribute implementation and printPrettyEnd approach in general?
For post print mechanism alternative approach is to use ad hoc solution in DeclPrinter.

Attributes are meant to be wholly-contained entities syntactically, so I'm not entirely comfortable with the notion of an attribute having an "ending" because that breaks the model. However, pragmas and statement-level attributes are examples of attribute constructs that sometimes want to be paired together, so I see utility in the overall idea. I sort of feel like what we need is the notion of grouping related attributes together (we already need this for mutual exclusion of attributes, but I see no reason why it couldn't also cover mutual inclusion). Then pretty printing has a natural place for "end" logic within this declarative container.

If you don't feel like doing all that design work (which I'm not attempting to sign you up for!), I think the ad hoc solution may be preferable to the proposed approach. Do you intend to add several more pragmas that need this same sort of behavior, or is this a one-off?

include/clang/Basic/Attr.td
2263

It's a bit odd to say it has no spellings and then give it a spelling list. ;-)

DmitryPolukhin edited edge metadata.
DmitryPolukhin marked an inline comment as done.
  • implemented ad hoc solution for printing
  • added documentation for the attrbute
  • reabse

If you don't feel like doing all that design work (which I'm not attempting to sign you up for!), I think the ad hoc solution may be preferable to the proposed approach. Do you intend to add several more pragmas that need this same sort of behavior, or is this a one-off?

I don't expect more pragmas like declare target and grouping mechanism design might be overkill for my case. So I implemented ad hoc solution that is much shorter. PTAL!

Alexey, please review OpenMP specific things.

Aaron, are you OK with the attribute implementation and printing approach?

ABataev accepted this revision.Apr 5 2016, 3:31 AM
ABataev edited edge metadata.

LG

This revision is now accepted and ready to land.Apr 5 2016, 3:31 AM
aaron.ballman accepted this revision.Apr 5 2016, 7:12 AM
aaron.ballman edited edge metadata.

Just some very minor nits, otherwise LGTM!

include/clang/Basic/DiagnosticSemaKinds.td
7937

s/into/within

include/clang/Sema/Sema.h
7925

This method should be marked const.

lib/Frontend/MultiplexConsumer.cpp
225

Use std::for_each (or at least a range-based for loop) over Listeners.

lib/Sema/SemaOpenMP.cpp
10405

Please do not use auto since the type is not spelled anywhere else in the expression.

10420

Please do not use auto since the type is not spelled anywhere else in the expression.

10450

Please do not use auto since the type is not spelled anywhere else in the expression.

10486

Please do not use auto since the type is not spelled anywhere else in the expression.

10497

Please do not use auto since the type is not spelled anywhere else in the expression.

DmitryPolukhin edited edge metadata.
DmitryPolukhin marked 8 inline comments as done.
  • fixed all comments
  • rebase

Aaron and Alexey, thank you for the review!