Skip to content

Commit 1881d04

Browse files
mikaoPmikaoP
committed
Reword task outline dependencies. See more.
We were too strict in Sema. This commit reuse task inline deps mechanics adding an additional check for task outline dependencies over non lvalue references, which are not allowed Closes llvm#148
1 parent b848646 commit 1881d04

File tree

3 files changed

+114
-127
lines changed

3 files changed

+114
-127
lines changed

‎clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11341,8 +11341,8 @@ def err_oss_call_expr_support : Error<
1134111341
"call expressions are not supported">;
1134211342
def err_oss_expected_addressable_lvalue_or_array_item : Error<
1134311343
"expected addressable lvalue expression, array element, array shape or array section">;
11344-
def err_oss_expected_dereference_or_array_item : Error<
11345-
"expected dereference, array element, array shape or array section">;
11344+
def err_oss_expected_lvalue_reference_dereference_or_array_item : Error<
11345+
"expected lvalue reference, dereference, array element, array shape or array section">;
1134611346
def err_oss_expected_var_name_member_expr_or_array_shaping : Error<
1134711347
"expected variable name%select{|, data member of current class}0 or array shaping">;
1134811348
def err_oss_expected_var_name_member_expr : Error<

‎clang/lib/Sema/SemaOmpSs.cpp

Lines changed: 108 additions & 123 deletions
Original file line numberDiff line numberDiff line change
@@ -2341,36 +2341,109 @@ StmtResult Sema::ActOnOmpSsTaskLoopForDirective(
23412341
return OSSTaskLoopForDirective::Create(Context, StartLoc, EndLoc, Clauses, AStmt, B);
23422342
}
23432343

2344-
static void checkOutlineDependency(Sema &S, Expr *RefExpr, bool OSSSyntax=false) {
2344+
static bool checkDependency(Sema &S, Expr *RefExpr, bool OSSSyntax, bool Outline) {
23452345
SourceLocation ELoc = RefExpr->getExprLoc();
23462346
Expr *SimpleExpr = RefExpr->IgnoreParenCasts();
2347-
if (RefExpr->isTypeDependent() || RefExpr->isValueDependent() ||
2348-
RefExpr->containsUnexpandedParameterPack()) {
2347+
if (RefExpr->containsUnexpandedParameterPack()) {
2348+
S.Diag(RefExpr->getExprLoc(), diag::err_oss_variadic_templates_not_clause_allowed);
2349+
return false;
2350+
} else if (RefExpr->isTypeDependent() || RefExpr->isValueDependent()) {
23492351
// It will be analyzed later.
2350-
return;
2352+
return true;
23512353
}
2354+
2355+
if (S.RequireCompleteExprType(RefExpr, diag::err_oss_incomplete_type))
2356+
return false;
2357+
23522358
auto *ASE = dyn_cast<ArraySubscriptExpr>(SimpleExpr);
2353-
if (!RefExpr->IgnoreParenImpCasts()->isLValue() ||
2359+
// Allow only LValues, forbid ArraySubscripts over things
2360+
// that are not an array like:
2361+
// typedef float V __attribute__((vector_size(16)));
2362+
// V a;
2363+
// #pragma oss task in(a[3])
2364+
// and functions:
2365+
// void foo() { #pragma oss task in(foo) {} }
2366+
if (RefExpr->IgnoreParenImpCasts()->getType()->isFunctionType() ||
2367+
!RefExpr->IgnoreParenImpCasts()->isLValue() ||
23542368
(ASE &&
23552369
!ASE->getBase()->getType().getNonReferenceType()->isPointerType() &&
23562370
!ASE->getBase()->getType().getNonReferenceType()->isArrayType())) {
2357-
S.Diag(ELoc, diag::err_oss_expected_dereference_or_array_item)
2358-
<< RefExpr->getSourceRange();
2359-
return;
2371+
if (!Outline)
2372+
S.Diag(ELoc, diag::err_oss_expected_addressable_lvalue_or_array_item)
2373+
<< RefExpr->getSourceRange();
2374+
else
2375+
S.Diag(ELoc, diag::err_oss_expected_lvalue_reference_dereference_or_array_item)
2376+
<< RefExpr->getSourceRange();
2377+
return false;
23602378
}
2361-
if (isa<DeclRefExpr>(SimpleExpr) || isa<MemberExpr>(SimpleExpr)) {
2362-
S.Diag(ELoc, diag::err_oss_expected_dereference_or_array_item)
2379+
2380+
if (Outline) {
2381+
if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(RefExpr->IgnoreParenImpCasts())) {
2382+
if (const VarDecl *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
2383+
if (!VD->getType()->isReferenceType())
2384+
S.Diag(ELoc, diag::err_oss_expected_lvalue_reference_dereference_or_array_item)
2385+
<< RefExpr->getSourceRange();
2386+
}
2387+
}
2388+
}
2389+
2390+
class CheckCallExpr
2391+
: public ConstStmtVisitor<CheckCallExpr, bool> {
2392+
// This Visitor checks the base of the
2393+
// dependency is over a CallExpr, which is error.
2394+
// int *get();
2395+
// auto l = []() -> int * {...};
2396+
// #pragma oss task in(get()[1], l()[3])
2397+
public:
2398+
bool VisitOSSMultiDepExpr(const OSSMultiDepExpr *E) {
2399+
return Visit(E->getDepExpr());
2400+
}
2401+
2402+
bool VisitOSSArrayShapingExpr(const OSSArrayShapingExpr *E) {
2403+
return Visit(E->getBase());
2404+
}
2405+
2406+
bool VisitOSSArraySectionExpr(const OSSArraySectionExpr *E) {
2407+
return Visit(E->getBase());
2408+
}
2409+
2410+
bool VisitArraySubscriptExpr(const ArraySubscriptExpr *E) {
2411+
return Visit(E->getBase());
2412+
}
2413+
2414+
bool VisitUnaryOperator(const UnaryOperator *E) {
2415+
return Visit(E->getSubExpr());
2416+
}
2417+
2418+
bool VisitMemberExpr(const MemberExpr *E) {
2419+
return Visit(E->getBase());
2420+
}
2421+
2422+
bool VisitCallExpr(const CallExpr *E) {
2423+
return true;
2424+
}
2425+
};
2426+
CheckCallExpr CCE;
2427+
if (CCE.Visit(RefExpr)) {
2428+
S.Diag(ELoc, diag::err_oss_call_expr_support)
23632429
<< RefExpr->getSourceRange();
2364-
return;
2430+
return false;
23652431
}
2432+
2433+
bool InvalidArraySection = false;
23662434
while (auto *OASE = dyn_cast<OSSArraySectionExpr>(SimpleExpr)) {
23672435
if (!OASE->isColonForm() && !OSSSyntax) {
23682436
S.Diag(OASE->getColonLoc(), diag::err_oss_section_invalid_form)
23692437
<< RefExpr->getSourceRange();
2370-
return;
2438+
// Only diagnose the first error
2439+
InvalidArraySection = true;
2440+
break;
23712441
}
2372-
SimpleExpr = OASE->getBase()->IgnoreParenCasts();
2442+
SimpleExpr = OASE->getBase()->IgnoreParenImpCasts();
23732443
}
2444+
if (InvalidArraySection)
2445+
return false;
2446+
return true;
23742447
}
23752448

23762449
Sema::DeclGroupPtrTy Sema::ActOnOmpSsDeclareTaskDirective(
@@ -2460,64 +2533,64 @@ Sema::DeclGroupPtrTy Sema::ActOnOmpSsDeclareTaskDirective(
24602533
OnreadyRes = Onready;
24612534
}
24622535
for (Expr *RefExpr : Ins) {
2463-
checkOutlineDependency(*this, RefExpr, /*OSSSyntax=*/true);
2536+
checkDependency(*this, RefExpr, /*OSSSyntax=*/true, /*Outline=*/true);
24642537
}
24652538
for (Expr *RefExpr : Outs) {
2466-
checkOutlineDependency(*this, RefExpr, /*OSSSyntax=*/true);
2539+
checkDependency(*this, RefExpr, /*OSSSyntax=*/true, /*Outline=*/true);
24672540
}
24682541
for (Expr *RefExpr : Inouts) {
2469-
checkOutlineDependency(*this, RefExpr, /*OSSSyntax=*/true);
2542+
checkDependency(*this, RefExpr, /*OSSSyntax=*/true, /*Outline=*/true);
24702543
}
24712544
for (Expr *RefExpr : Concurrents) {
2472-
checkOutlineDependency(*this, RefExpr, /*OSSSyntax=*/true);
2545+
checkDependency(*this, RefExpr, /*OSSSyntax=*/true, /*Outline=*/true);
24732546
}
24742547
for (Expr *RefExpr : Commutatives) {
2475-
checkOutlineDependency(*this, RefExpr, /*OSSSyntax=*/true);
2548+
checkDependency(*this, RefExpr, /*OSSSyntax=*/true, /*Outline=*/true);
24762549
}
24772550
for (Expr *RefExpr : WeakIns) {
2478-
checkOutlineDependency(*this, RefExpr, /*OSSSyntax=*/true);
2551+
checkDependency(*this, RefExpr, /*OSSSyntax=*/true, /*Outline=*/true);
24792552
}
24802553
for (Expr *RefExpr : WeakOuts) {
2481-
checkOutlineDependency(*this, RefExpr, /*OSSSyntax=*/true);
2554+
checkDependency(*this, RefExpr, /*OSSSyntax=*/true, /*Outline=*/true);
24822555
}
24832556
for (Expr *RefExpr : WeakInouts) {
2484-
checkOutlineDependency(*this, RefExpr, /*OSSSyntax=*/true);
2557+
checkDependency(*this, RefExpr, /*OSSSyntax=*/true, /*Outline=*/true);
24852558
}
24862559
for (Expr *RefExpr : WeakConcurrents) {
2487-
checkOutlineDependency(*this, RefExpr, /*OSSSyntax=*/true);
2560+
checkDependency(*this, RefExpr, /*OSSSyntax=*/true, /*Outline=*/true);
24882561
}
24892562
for (Expr *RefExpr : WeakCommutatives) {
2490-
checkOutlineDependency(*this, RefExpr, /*OSSSyntax=*/true);
2563+
checkDependency(*this, RefExpr, /*OSSSyntax=*/true, /*Outline=*/true);
24912564
}
24922565
for (Expr *RefExpr : DepIns) {
2493-
checkOutlineDependency(*this, RefExpr);
2566+
checkDependency(*this, RefExpr, /*OSSSyntax=*/false, /*Outline=*/true);
24942567
}
24952568
for (Expr *RefExpr : DepOuts) {
2496-
checkOutlineDependency(*this, RefExpr);
2569+
checkDependency(*this, RefExpr, /*OSSSyntax=*/false, /*Outline=*/true);
24972570
}
24982571
for (Expr *RefExpr : DepInouts) {
2499-
checkOutlineDependency(*this, RefExpr);
2572+
checkDependency(*this, RefExpr, /*OSSSyntax=*/false, /*Outline=*/true);
25002573
}
25012574
for (Expr *RefExpr : DepConcurrents) {
2502-
checkOutlineDependency(*this, RefExpr);
2575+
checkDependency(*this, RefExpr, /*OSSSyntax=*/false, /*Outline=*/true);
25032576
}
25042577
for (Expr *RefExpr : DepCommutatives) {
2505-
checkOutlineDependency(*this, RefExpr);
2578+
checkDependency(*this, RefExpr, /*OSSSyntax=*/false, /*Outline=*/true);
25062579
}
25072580
for (Expr *RefExpr : DepWeakIns) {
2508-
checkOutlineDependency(*this, RefExpr);
2581+
checkDependency(*this, RefExpr, /*OSSSyntax=*/false, /*Outline=*/true);
25092582
}
25102583
for (Expr *RefExpr : DepWeakOuts) {
2511-
checkOutlineDependency(*this, RefExpr);
2584+
checkDependency(*this, RefExpr, /*OSSSyntax=*/false, /*Outline=*/true);
25122585
}
25132586
for (Expr *RefExpr : DepWeakInouts) {
2514-
checkOutlineDependency(*this, RefExpr);
2587+
checkDependency(*this, RefExpr, /*OSSSyntax=*/false, /*Outline=*/true);
25152588
}
25162589
for (Expr *RefExpr : DepWeakConcurrents) {
2517-
checkOutlineDependency(*this, RefExpr);
2590+
checkDependency(*this, RefExpr, /*OSSSyntax=*/false, /*Outline=*/true);
25182591
}
25192592
for (Expr *RefExpr : DepWeakCommutatives) {
2520-
checkOutlineDependency(*this, RefExpr);
2593+
checkDependency(*this, RefExpr, /*OSSSyntax=*/false, /*Outline=*/true);
25212594
}
25222595

25232596
auto *NewAttr = OSSTaskDeclAttr::CreateImplicit(
@@ -3499,96 +3572,8 @@ Sema::ActOnOmpSsDependClause(ArrayRef<OmpSsDependClauseKind> DepKinds, SourceLoc
34993572
return nullptr;
35003573

35013574
for (Expr *RefExpr : VarList) {
3502-
SourceLocation ELoc = RefExpr->getExprLoc();
3503-
Expr *SimpleExpr = RefExpr->IgnoreParenCasts();
3504-
if (RefExpr->containsUnexpandedParameterPack()) {
3505-
Diag(RefExpr->getExprLoc(), diag::err_oss_variadic_templates_not_clause_allowed);
3506-
continue;
3507-
} else if (RefExpr->isTypeDependent() || RefExpr->isValueDependent()) {
3508-
// It will be analyzed later.
3509-
ClauseVars.push_back(RefExpr);
3510-
continue;
3511-
}
3512-
3513-
if (RequireCompleteExprType(RefExpr, diag::err_oss_incomplete_type))
3514-
continue;
3515-
3516-
// TODO: check with OSSArraySectionExpr
3517-
auto *ASE = dyn_cast<ArraySubscriptExpr>(SimpleExpr);
3518-
// Allow only LValues, forbid ArraySubscripts over things
3519-
// that are not an array like:
3520-
// typedef float V __attribute__((vector_size(16)));
3521-
// V a;
3522-
// #pragma oss task in(a[3])
3523-
// and functions:
3524-
// void foo() { #pragma oss task in(foo) {} }
3525-
if (RefExpr->IgnoreParenImpCasts()->getType()->isFunctionType() ||
3526-
!RefExpr->IgnoreParenImpCasts()->isLValue() ||
3527-
(ASE &&
3528-
!ASE->getBase()->getType().getNonReferenceType()->isPointerType() &&
3529-
!ASE->getBase()->getType().getNonReferenceType()->isArrayType())) {
3530-
Diag(ELoc, diag::err_oss_expected_addressable_lvalue_or_array_item)
3531-
<< RefExpr->getSourceRange();
3532-
continue;
3533-
}
3534-
3535-
class CheckCallExpr
3536-
: public ConstStmtVisitor<CheckCallExpr, bool> {
3537-
// This Visitor checks the base of the
3538-
// dependency is over a CallExpr, which is error.
3539-
// int *get();
3540-
// auto l = []() -> int * {...};
3541-
// #pragma oss task in(get()[1], l()[3])
3542-
public:
3543-
bool VisitOSSMultiDepExpr(const OSSMultiDepExpr *E) {
3544-
return Visit(E->getDepExpr());
3545-
}
3546-
3547-
bool VisitOSSArrayShapingExpr(const OSSArrayShapingExpr *E) {
3548-
return Visit(E->getBase());
3549-
}
3550-
3551-
bool VisitOSSArraySectionExpr(const OSSArraySectionExpr *E) {
3552-
return Visit(E->getBase());
3553-
}
3554-
3555-
bool VisitArraySubscriptExpr(const ArraySubscriptExpr *E) {
3556-
return Visit(E->getBase());
3557-
}
3558-
3559-
bool VisitUnaryOperator(const UnaryOperator *E) {
3560-
return Visit(E->getSubExpr());
3561-
}
3562-
3563-
bool VisitMemberExpr(const MemberExpr *E) {
3564-
return Visit(E->getBase());
3565-
}
3566-
3567-
bool VisitCallExpr(const CallExpr *E) {
3568-
return true;
3569-
}
3570-
};
3571-
CheckCallExpr CCE;
3572-
if (CCE.Visit(RefExpr)) {
3573-
Diag(ELoc, diag::err_oss_call_expr_support)
3574-
<< RefExpr->getSourceRange();
3575-
continue;
3576-
}
3577-
3578-
bool InvalidArraySection = false;
3579-
while (auto *OASE = dyn_cast<OSSArraySectionExpr>(SimpleExpr)) {
3580-
if (!OASE->isColonForm() && !OSSSyntax) {
3581-
Diag(OASE->getColonLoc(), diag::err_oss_section_invalid_form)
3582-
<< RefExpr->getSourceRange();
3583-
// Only diagnose the first error
3584-
InvalidArraySection = true;
3585-
break;
3586-
}
3587-
SimpleExpr = OASE->getBase()->IgnoreParenImpCasts();
3588-
}
3589-
if (InvalidArraySection)
3590-
continue;
3591-
ClauseVars.push_back(RefExpr->IgnoreParenImpCasts());
3575+
if (checkDependency(*this, RefExpr, OSSSyntax, /*Outline=*/false))
3576+
ClauseVars.push_back(RefExpr->IgnoreParenImpCasts());
35923577
}
35933578
return OSSDependClause::Create(Context, StartLoc, LParenLoc, EndLoc,
35943579
DepKinds, DepKindsOrdered,

‎clang/test/OmpSs/Sema/task_function1.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ struct Q {
3636
int *x;
3737
};
3838

39-
// expected-error@+1 5 {{expected dereference, array element, array shape or array section}}
40-
#pragma oss task in(*(s->x), &*p, s->x, a, a++, p, *p, p[0 : 4], [10]p)
39+
// expected-error@+1 4 {{expected lvalue reference, dereference, array element, array shape or array section}}
40+
#pragma oss task in((*s).x, *(s->x), &*p, s->x, a, a++, p, *p, p[0 : 4], [10]p)
4141
void foo(Q *s, int a, int *p){}
42+
#pragma oss task in(a)
43+
void foo1(int &a){}

0 commit comments

Comments
 (0)