This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP] Support for implicit "declare target" functions - Sema patch
AbandonedPublic

Authored by grokos on Oct 11 2017, 7:13 AM.

Details

Summary

This patch completes the support for the "declare target" directive in Sema. With this patch Sema handles implicitly used functions (i.e. functions which are used inside a target region without having been "declared target") including lambdas, templated functions, functions called from within target functions and ctors/dtors.

By default, use of implicit declare target functions is enabled. An upcoming driver patch will change that.

Diff Detail

Repository
rL LLVM

Event Timeline

grokos created this revision.Oct 11 2017, 7:13 AM
ABataev edited edge metadata.Oct 26 2017, 8:22 AM

Tests?

include/clang/Basic/LangOptions.def
193

Where is it assigned a value 1? Currently it is disabled by default (initial value is 0).

include/clang/Sema/SemaInternal.h
65

Function should start from small letter

70

This function should be called only we're using CUDA NVPTX as an offloading device. Also, this function requires that LangOpts.CUDA was true. Do we set LangOpts.CUDA to trues when trying to offload to CUDE devices at all? If no, this function is useless.

lib/Parse/ParseOpenMP.cpp
785–789

Why do you need it?

810–813

Again, why do you need it?

lib/Sema/SemaDecl.cpp
12661–12665

I think this should be done only if the function has attached OMPDeclareTargetDeclAttr and if !getLangOpts().IsOpenMPDevice. Also, I don't like the idea of recursive scanning. It is better to add such markings on the fly (when trying to build the expression)

lib/Sema/SemaOpenMP.cpp
1170

\param

1177–1178

Formatting

1178

There is no such option yet.

1191

auto *FD

1193

const auto *RI

1194–1201

Could you reuse the same attribute attached to one of the redeclarations?

1212–1214

Reuse the existing attribute

1233–1235

Remove braces

1245–1247

const auto *RD = SemaRef.Context.getBaseElementType(Ty)->getAsCXXRecordDecl();

mikerice added inline comments.
lib/Sema/SemaOpenMP.cpp
1197

When D already has an OMPDeclareTargetAttr this seems to add another.

kkwli0 added a subscriber: kkwli0.Feb 7 2018, 9:54 AM
grokos abandoned this revision.Mar 21 2018, 8:15 AM

@ABataev came up with a much simpler solution to the implementation of declare target: https://reviews.llvm.org/rL327636

I am abandoning this obsolete revision.