Skip to content

Commit 1dd66e6

Browse files
committed
[OpenMP] Delay more diagnostics of potentially non-emitted code
Even code in target and declare target regions might not be emitted. With this patch we delay more diagnostics and use laziness and linkage to determine if a function is emitted (for the device). Note that we still eagerly emit diagnostics for target regions, unfortunately, see the TODO for the reason. This hopefully fixes PR48933. Reviewed By: JonChesterfield Differential Revision: https://reviews.llvm.org/D95928
1 parent f9286b4 commit 1dd66e6

File tree

5 files changed

+59
-74
lines changed

5 files changed

+59
-74
lines changed

clang/lib/Sema/SemaDecl.cpp

Lines changed: 42 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -18333,42 +18333,51 @@ Sema::FunctionEmissionStatus Sema::getEmissionStatus(FunctionDecl *FD,
1833318333
if (FD->isDependentContext())
1833418334
return FunctionEmissionStatus::TemplateDiscarded;
1833518335

18336-
FunctionEmissionStatus OMPES = FunctionEmissionStatus::Unknown;
18336+
// Check whether this function is an externally visible definition.
18337+
auto IsEmittedForExternalSymbol = [this, FD]() {
18338+
// We have to check the GVA linkage of the function's *definition* -- if we
18339+
// only have a declaration, we don't know whether or not the function will
18340+
// be emitted, because (say) the definition could include "inline".
18341+
FunctionDecl *Def = FD->getDefinition();
18342+
18343+
return Def && !isDiscardableGVALinkage(
18344+
getASTContext().GetGVALinkageForFunction(Def));
18345+
};
18346+
1833718347
if (LangOpts.OpenMPIsDevice) {
18348+
// In OpenMP device mode we will not emit host only functions, or functions
18349+
// we don't need due to their linkage.
1833818350
Optional<OMPDeclareTargetDeclAttr::DevTypeTy> DevTy =
1833918351
OMPDeclareTargetDeclAttr::getDeviceType(FD->getCanonicalDecl());
18340-
if (DevTy.hasValue()) {
18352+
// DevTy may be changed later by
18353+
// #pragma omp declare target to(*) device_type(*).
18354+
// Therefore DevTyhaving no value does not imply host. The emission status
18355+
// will be checked again at the end of compilation unit with Final = true.
18356+
if (DevTy.hasValue())
1834118357
if (*DevTy == OMPDeclareTargetDeclAttr::DT_Host)
18342-
OMPES = FunctionEmissionStatus::OMPDiscarded;
18343-
else if (*DevTy == OMPDeclareTargetDeclAttr::DT_NoHost ||
18344-
*DevTy == OMPDeclareTargetDeclAttr::DT_Any) {
18345-
OMPES = FunctionEmissionStatus::Emitted;
18346-
}
18347-
}
18348-
} else if (LangOpts.OpenMP) {
18349-
// In OpenMP 4.5 all the functions are host functions.
18350-
if (LangOpts.OpenMP <= 45) {
18351-
OMPES = FunctionEmissionStatus::Emitted;
18352-
} else {
18353-
Optional<OMPDeclareTargetDeclAttr::DevTypeTy> DevTy =
18354-
OMPDeclareTargetDeclAttr::getDeviceType(FD->getCanonicalDecl());
18355-
// In OpenMP 5.0 or above, DevTy may be changed later by
18356-
// #pragma omp declare target to(*) device_type(*). Therefore DevTy
18357-
// having no value does not imply host. The emission status will be
18358-
// checked again at the end of compilation unit.
18359-
if (DevTy.hasValue()) {
18360-
if (*DevTy == OMPDeclareTargetDeclAttr::DT_NoHost) {
18361-
OMPES = FunctionEmissionStatus::OMPDiscarded;
18362-
} else if (*DevTy == OMPDeclareTargetDeclAttr::DT_Host ||
18363-
*DevTy == OMPDeclareTargetDeclAttr::DT_Any)
18364-
OMPES = FunctionEmissionStatus::Emitted;
18365-
} else if (Final)
18366-
OMPES = FunctionEmissionStatus::Emitted;
18367-
}
18368-
}
18369-
if (OMPES == FunctionEmissionStatus::OMPDiscarded ||
18370-
(OMPES == FunctionEmissionStatus::Emitted && !LangOpts.CUDA))
18371-
return OMPES;
18358+
return FunctionEmissionStatus::OMPDiscarded;
18359+
// If we have an explicit value for the device type, or we are in a target
18360+
// declare context, we need to emit all extern and used symbols.
18361+
if (isInOpenMPDeclareTargetContext() || DevTy.hasValue())
18362+
if (IsEmittedForExternalSymbol())
18363+
return FunctionEmissionStatus::Emitted;
18364+
// Device mode only emits what it must, if it wasn't tagged yet and needed,
18365+
// we'll omit it.
18366+
if (Final)
18367+
return FunctionEmissionStatus::OMPDiscarded;
18368+
} else if (LangOpts.OpenMP > 45) {
18369+
// In OpenMP host compilation prior to 5.0 everything was an emitted host
18370+
// function. In 5.0, no_host was introduced which might cause a function to
18371+
// be ommitted.
18372+
Optional<OMPDeclareTargetDeclAttr::DevTypeTy> DevTy =
18373+
OMPDeclareTargetDeclAttr::getDeviceType(FD->getCanonicalDecl());
18374+
if (DevTy.hasValue())
18375+
if (*DevTy == OMPDeclareTargetDeclAttr::DT_NoHost)
18376+
return FunctionEmissionStatus::OMPDiscarded;
18377+
}
18378+
18379+
if (Final && LangOpts.OpenMP && !LangOpts.CUDA)
18380+
return FunctionEmissionStatus::Emitted;
1837218381

1837318382
if (LangOpts.CUDA) {
1837418383
// When compiling for device, host functions are never emitted. Similarly,
@@ -18382,17 +18391,7 @@ Sema::FunctionEmissionStatus Sema::getEmissionStatus(FunctionDecl *FD,
1838218391
(T == Sema::CFT_Device || T == Sema::CFT_Global))
1838318392
return FunctionEmissionStatus::CUDADiscarded;
1838418393

18385-
// Check whether this function is externally visible -- if so, it's
18386-
// known-emitted.
18387-
//
18388-
// We have to check the GVA linkage of the function's *definition* -- if we
18389-
// only have a declaration, we don't know whether or not the function will
18390-
// be emitted, because (say) the definition could include "inline".
18391-
FunctionDecl *Def = FD->getDefinition();
18392-
18393-
if (Def &&
18394-
!isDiscardableGVALinkage(getASTContext().GetGVALinkageForFunction(Def))
18395-
&& (!LangOpts.OpenMP || OMPES == FunctionEmissionStatus::Emitted))
18394+
if (IsEmittedForExternalSymbol())
1839618395
return FunctionEmissionStatus::Emitted;
1839718396
}
1839818397

clang/lib/Sema/SemaOpenMP.cpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1884,8 +1884,7 @@ void Sema::popOpenMPFunctionRegion(const FunctionScopeInfo *OldFSI) {
18841884
static bool isOpenMPDeviceDelayedContext(Sema &S) {
18851885
assert(S.LangOpts.OpenMP && S.LangOpts.OpenMPIsDevice &&
18861886
"Expected OpenMP device compilation.");
1887-
return !S.isInOpenMPTargetExecutionDirective() &&
1888-
!S.isInOpenMPDeclareTargetContext();
1887+
return !S.isInOpenMPTargetExecutionDirective();
18891888
}
18901889

18911890
namespace {
@@ -1911,6 +1910,13 @@ Sema::SemaDiagnosticBuilder Sema::diagIfOpenMPDeviceCode(SourceLocation Loc,
19111910
Kind = SemaDiagnosticBuilder::K_Immediate;
19121911
break;
19131912
case FunctionEmissionStatus::Unknown:
1913+
// TODO: We should always delay diagnostics here in case a target
1914+
// region is in a function we do not emit. However, as the
1915+
// current diagnostics are associated with the function containing
1916+
// the target region and we do not emit that one, we would miss out
1917+
// on diagnostics for the target region itself. We need to anchor
1918+
// the diagnostics with the new generated function *or* ensure we
1919+
// emit diagnostics associated with the surrounding function.
19141920
Kind = isOpenMPDeviceDelayedContext(*this)
19151921
? SemaDiagnosticBuilder::K_Deferred
19161922
: SemaDiagnosticBuilder::K_Immediate;

clang/test/OpenMP/nvptx_allocate_messages.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,7 @@ int main () {
8181
#endif // DEVICE && !REQUIRES
8282
#pragma omp allocate(b)
8383
#if defined(DEVICE) && !defined(REQUIRES)
84-
// expected-note@+3 {{in instantiation of function template specialization 'foo<int>' requested here}}
85-
// expected-note@+2 {{called by 'main'}}
84+
// expected-note@+2 2{{called by 'main'}}
8685
#endif // DEVICE && !REQUIRES
8786
return (foo<int>() + bar());
8887
}

clang/test/OpenMP/nvptx_target_exceptions_messages.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ int maini1() {
5252
#pragma omp target map(tofrom \
5353
: a, b)
5454
{
55+
// expected-note@+1 {{called by 'maini1'}}
5556
S s(a);
5657
static long aaa = 23;
5758
a = foo() + bar() + b + c + d + aa + aaa + FA<int>(); // expected-note{{called by 'maini1'}}

clang/test/OpenMP/nvptx_unsupported_type_messages.cpp

Lines changed: 7 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -81,18 +81,12 @@ void baz1() {
8181
T1 t = bar1();
8282
}
8383

84-
// TODO: We should not emit an error for dead functions we do not emit.
8584
inline void dead_inline_declare_target() {
86-
// expected-note@+1 {{'b' defined here}}
8785
long double *a, b = 0;
88-
// expected-error@+1 {{'b' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
8986
a = &b;
9087
}
91-
// TODO: We should not emit an error for dead functions we do not emit.
9288
static void dead_static_declare_target() {
93-
// expected-note@+1 {{'b' defined here}}
9489
long double *a, b = 0;
95-
// expected-error@+1 {{'b' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
9690
a = &b;
9791
}
9892
template<bool>
@@ -108,7 +102,6 @@ long double ld_return1a() { return 0; }
108102
// expected-error@+1 {{'ld_arg1a' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
109103
void ld_arg1a(long double ld) {}
110104

111-
// TODO: We should diagnose the return type and argument type here.
112105
typedef long double ld_ty;
113106
// expected-note@+2 {{'ld_return1b' defined here}}
114107
// expected-error@+1 {{'ld_return1b' requires 128 bit size 'ld_ty' (aka 'long double') type support, but device 'nvptx64-unknown-unknown' does not support it}}
@@ -117,48 +110,28 @@ ld_ty ld_return1b() { return 0; }
117110
// expected-error@+1 {{'ld_arg1b' requires 128 bit size 'ld_ty' (aka 'long double') type support, but device 'nvptx64-unknown-unknown' does not support it}}
118111
void ld_arg1b(ld_ty ld) {}
119112

120-
// TODO: These errors should not be emitted.
121-
// expected-note@+2 {{'ld_return1c' defined here}}
122-
// expected-error@+1 {{'ld_return1c' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
123113
static long double ld_return1c() { return 0; }
124-
// expected-note@+2 {{'ld_arg1c' defined here}}
125-
// expected-error@+1 {{'ld_arg1c' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
126114
static void ld_arg1c(long double ld) {}
127115

128-
// TODO: These errors should not be emitted.
129-
// expected-note@+2 {{'ld_return1d' defined here}}
130-
// expected-error@+1 {{'ld_return1d' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
131116
inline long double ld_return1d() { return 0; }
132-
// expected-note@+2 {{'ld_arg1d' defined here}}
133-
// expected-error@+1 {{'ld_arg1d' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
134117
inline void ld_arg1d(long double ld) {}
135118

136-
// expected-error@+2 {{'ld_return1e' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
137119
// expected-note@+1 {{'ld_return1e' defined here}}
138120
static long double ld_return1e() { return 0; }
139-
// expected-error@+2 {{'ld_arg1e' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
140121
// expected-note@+1 {{'ld_arg1e' defined here}}
141122
static void ld_arg1e(long double ld) {}
142123

143-
// expected-error@+2 {{'ld_return1f' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
144124
// expected-note@+1 {{'ld_return1f' defined here}}
145125
inline long double ld_return1f() { return 0; }
146-
// expected-error@+2 {{'ld_arg1f' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
147126
// expected-note@+1 {{'ld_arg1f' defined here}}
148127
inline void ld_arg1f(long double ld) {}
149128

150129
inline void ld_use1() {
151-
// expected-note@+1 {{'ld' defined here}}
152130
long double ld = 0;
153-
// TODO: We should not diagnose this as the function is dead.
154-
// expected-error@+1 {{'ld' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
155131
ld += 1;
156132
}
157133
static void ld_use2() {
158-
// expected-note@+1 {{'ld' defined here}}
159134
long double ld = 0;
160-
// TODO: We should not diagnose this as the function is dead.
161-
// expected-error@+1 {{'ld' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
162135
ld += 1;
163136
}
164137

@@ -176,11 +149,18 @@ static void ld_use4() {
176149
}
177150

178151
void external() {
152+
// expected-error@+1 {{'ld_return1e' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
179153
void *p1 = reinterpret_cast<void*>(&ld_return1e);
154+
// expected-error@+1 {{'ld_arg1e' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
180155
void *p2 = reinterpret_cast<void*>(&ld_arg1e);
156+
// expected-error@+1 {{'ld_return1f' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
181157
void *p3 = reinterpret_cast<void*>(&ld_return1f);
158+
// expected-error@+1 {{'ld_arg1f' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
182159
void *p4 = reinterpret_cast<void*>(&ld_arg1f);
160+
// TODO: The error message "called by" is not great.
161+
// expected-note@+1 {{called by 'external'}}
183162
void *p5 = reinterpret_cast<void*>(&ld_use3);
163+
// expected-note@+1 {{called by 'external'}}
184164
void *p6 = reinterpret_cast<void*>(&ld_use4);
185165
}
186166

0 commit comments

Comments
 (0)