Skip to content

Commit 3bd34ec

Browse files
committed
[OpenACC] Fix checking of sub-expressions in cache
Running an external test suite (UDel) showed that our expression comparison for the 'cache' rule checking was overly strict in the presence of irrelevant parens/casts/etc. This patch ensures we skip them when checking. This also changes the diagnostic to say 'sub-expression' instead of variable, which is more correct.
1 parent 3ebe5d6 commit 3bd34ec

File tree

3 files changed

+51
-27
lines changed

3 files changed

+51
-27
lines changed

clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13438,13 +13438,13 @@ def note_acc_atomic_mismatch_operand
1343813438
: Note<"left hand side of assignment operation(%0) must match one side "
1343913439
"of the sub-operation on the right hand side(%1 and %2)">;
1344013440
def note_acc_atomic_mismatch_compound_operand
13441-
: Note<"variable %select{|in unary expression|on right hand side of "
13441+
: Note<"sub-expression %select{|in unary expression|on right hand side of "
1344213442
"assignment|on left hand side of assignment|on left hand side of "
1344313443
"compound assignment|on left hand side of assignment}2(%3) must "
13444-
"match variable used %select{|in unary expression|on right hand "
13445-
"side of assignment|<not possible>|on left hand side of compound "
13446-
"assignment|on left hand side of assignment}0(%1) from the first "
13447-
"statement">;
13444+
"match sub-expression used %select{|in unary expression|on right "
13445+
"hand side of assignment|<not possible>|on left hand side of "
13446+
"compound assignment|on left hand side of assignment}0(%1) from the "
13447+
"first statement">;
1344813448
def err_acc_declare_required_clauses
1344913449
: Error<"no valid clauses specified in OpenACC 'declare' directive">;
1345013450
def err_acc_declare_clause_at_global

clang/lib/Sema/SemaOpenACCAtomic.cpp

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -576,6 +576,11 @@ class AtomicOperandChecker {
576576
return AssocStmt;
577577
}
578578

579+
const Expr *IgnoreBeforeCompare(const Expr *E) {
580+
return E->IgnoreParenImpCasts()->IgnoreParenNoopCasts(
581+
SemaRef.getASTContext());
582+
}
583+
579584
bool CheckVarRefsSame(IDACInfo::ExprKindTy FirstKind, const Expr *FirstX,
580585
IDACInfo::ExprKindTy SecondKind, const Expr *SecondX) {
581586
llvm::FoldingSetNodeID First_ID, Second_ID;
@@ -648,8 +653,10 @@ class AtomicOperandChecker {
648653
if (CheckOperandVariable(AssignRes->RHS, PD))
649654
return getRecoveryExpr();
650655

651-
if (CheckVarRefsSame(FirstExprResults.ExprKind, FirstExprResults.X_Var,
652-
IDACInfo::SimpleAssign, AssignRes->RHS))
656+
if (CheckVarRefsSame(FirstExprResults.ExprKind,
657+
IgnoreBeforeCompare(FirstExprResults.X_Var),
658+
IDACInfo::SimpleAssign,
659+
IgnoreBeforeCompare(AssignRes->RHS)))
653660
return getRecoveryExpr();
654661
break;
655662
}
@@ -660,9 +667,10 @@ class AtomicOperandChecker {
660667
if (SecondExprResults.Failed)
661668
return getRecoveryExpr();
662669

663-
if (CheckVarRefsSame(FirstExprResults.ExprKind, FirstExprResults.X_Var,
670+
if (CheckVarRefsSame(FirstExprResults.ExprKind,
671+
IgnoreBeforeCompare(FirstExprResults.X_Var),
664672
SecondExprResults.ExprKind,
665-
SecondExprResults.X_Var))
673+
IgnoreBeforeCompare(SecondExprResults.X_Var)))
666674
return getRecoveryExpr();
667675
break;
668676
}

clang/test/SemaOpenACC/atomic-construct.cpp

Lines changed: 34 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1098,7 +1098,7 @@ void AtomicCaptureTemplateCompound(T LHS, T RHS) {
10981098
{
10991099
LHS--;
11001100
// expected-error@-3{{statement associated with OpenACC 'atomic capture' directive is invalid}}
1101-
// expected-note@+1{{variable on right hand side of assignment('RHS') must match variable used in unary expression('LHS') from the first statement}}
1101+
// expected-note@+1{{sub-expression on right hand side of assignment('RHS') must match sub-expression used in unary expression('LHS') from the first statement}}
11021102
LHS = RHS;
11031103
}
11041104

@@ -1128,7 +1128,7 @@ void AtomicCaptureTemplateCompound(T LHS, T RHS) {
11281128
{
11291129
LHS *= 1;
11301130
// expected-error@-3{{statement associated with OpenACC 'atomic capture' directive is invalid}}
1131-
// expected-note@+1{{variable on right hand side of assignment('RHS') must match variable used on left hand side of compound assignment('LHS') from the first statement}}
1131+
// expected-note@+1{{sub-expression on right hand side of assignment('RHS') must match sub-expression used on left hand side of compound assignment('LHS') from the first statement}}
11321132
LHS = RHS;
11331133
}
11341134
#pragma acc atomic capture
@@ -1157,7 +1157,7 @@ void AtomicCaptureTemplateCompound(T LHS, T RHS) {
11571157
{
11581158
LHS = LHS * 1;
11591159
// expected-error@-3{{statement associated with OpenACC 'atomic capture' directive is invalid}}
1160-
// expected-note@+1{{variable on right hand side of assignment('RHS') must match variable used on left hand side of assignment('LHS') from the first statement}}
1160+
// expected-note@+1{{sub-expression on right hand side of assignment('RHS') must match sub-expression used on left hand side of assignment('LHS') from the first statement}}
11611161
RHS = RHS;
11621162
}
11631163
#pragma acc atomic capture
@@ -1186,7 +1186,7 @@ void AtomicCaptureTemplateCompound(T LHS, T RHS) {
11861186
{
11871187
LHS = LHS | 1;
11881188
// expected-error@-3{{statement associated with OpenACC 'atomic capture' directive is invalid}}
1189-
// expected-note@+1{{variable on right hand side of assignment('RHS') must match variable used on left hand side of assignment('LHS') from the first statement}}
1189+
// expected-note@+1{{sub-expression on right hand side of assignment('RHS') must match sub-expression used on left hand side of assignment('LHS') from the first statement}}
11901190
RHS = RHS;
11911191
}
11921192
#pragma acc atomic capture
@@ -1255,7 +1255,7 @@ void AtomicCaptureTemplateCompound(T LHS, T RHS) {
12551255
{
12561256
LHS = RHS;
12571257
// expected-error@-3{{statement associated with OpenACC 'atomic capture' directive is invalid}}
1258-
// expected-note@+1{{variable on left hand side of assignment('LHS') must match variable used on right hand side of assignment('RHS') from the first statement}}
1258+
// expected-note@+1{{sub-expression on left hand side of assignment('LHS') must match sub-expression used on right hand side of assignment('RHS') from the first statement}}
12591259
LHS = 1;
12601260
}
12611261

@@ -1294,7 +1294,7 @@ void AtomicCaptureTemplateCompound(T LHS, T RHS) {
12941294
{
12951295
LHS = RHS;
12961296
// expected-error@-3{{statement associated with OpenACC 'atomic capture' directive is invalid}}
1297-
// expected-note@+1{{variable in unary expression('LHS') must match variable used on right hand side of assignment('RHS') from the first statement}}
1297+
// expected-note@+1{{sub-expression in unary expression('LHS') must match sub-expression used on right hand side of assignment('RHS') from the first statement}}
12981298
LHS++;
12991299
}
13001300
}
@@ -1352,7 +1352,7 @@ void AtomicCaptureTemplateCompound2(T LHS, T RHS) {
13521352
{
13531353
LHS--;
13541354
// expected-error@-3{{statement associated with OpenACC 'atomic capture' directive is invalid}}
1355-
// expected-note@+1{{variable on right hand side of assignment('RHS') must match variable used in unary expression('LHS') from the first statement}}
1355+
// expected-note@+1{{sub-expression on right hand side of assignment('RHS') must match sub-expression used in unary expression('LHS') from the first statement}}
13561356
LHS = RHS;
13571357
}
13581358

@@ -1384,7 +1384,7 @@ void AtomicCaptureTemplateCompound2(T LHS, T RHS) {
13841384
{
13851385
LHS *= 1;
13861386
// expected-error@-3{{statement associated with OpenACC 'atomic capture' directive is invalid}}
1387-
// expected-note@+1{{variable on right hand side of assignment('RHS') must match variable used on left hand side of compound assignment('LHS') from the first statement}}
1387+
// expected-note@+1{{sub-expression on right hand side of assignment('RHS') must match sub-expression used on left hand side of compound assignment('LHS') from the first statement}}
13881388
LHS = RHS;
13891389
}
13901390
#pragma acc atomic capture
@@ -1415,7 +1415,7 @@ void AtomicCaptureTemplateCompound2(T LHS, T RHS) {
14151415
{
14161416
LHS = LHS * 1;
14171417
// expected-error@-3{{statement associated with OpenACC 'atomic capture' directive is invalid}}
1418-
// expected-note@+1{{variable on right hand side of assignment('RHS') must match variable used on left hand side of assignment('LHS') from the first statement}}
1418+
// expected-note@+1{{sub-expression on right hand side of assignment('RHS') must match sub-expression used on left hand side of assignment('LHS') from the first statement}}
14191419
RHS = RHS;
14201420
}
14211421
#pragma acc atomic capture
@@ -1446,7 +1446,7 @@ void AtomicCaptureTemplateCompound2(T LHS, T RHS) {
14461446
{
14471447
LHS = LHS | 1;
14481448
// expected-error@-3{{statement associated with OpenACC 'atomic capture' directive is invalid}}
1449-
// expected-note@+1{{variable on right hand side of assignment('RHS') must match variable used on left hand side of assignment('LHS') from the first statement}}
1449+
// expected-note@+1{{sub-expression on right hand side of assignment('RHS') must match sub-expression used on left hand side of assignment('LHS') from the first statement}}
14501450
RHS = RHS;
14511451
}
14521452
#pragma acc atomic capture
@@ -1523,7 +1523,7 @@ void AtomicCaptureTemplateCompound2(T LHS, T RHS) {
15231523
{
15241524
LHS = RHS;
15251525
// expected-error@-3{{statement associated with OpenACC 'atomic capture' directive is invalid}}
1526-
// expected-note@+1{{variable on left hand side of assignment('LHS') must match variable used on right hand side of assignment('RHS') from the first statement}}
1526+
// expected-note@+1{{sub-expression on left hand side of assignment('LHS') must match sub-expression used on right hand side of assignment('RHS') from the first statement}}
15271527
LHS = 1;
15281528
}
15291529

@@ -1570,7 +1570,7 @@ void AtomicCaptureTemplateCompound2(T LHS, T RHS) {
15701570
{
15711571
LHS = RHS;
15721572
// expected-error@-3{{statement associated with OpenACC 'atomic capture' directive is invalid}}
1573-
// expected-note@+1{{variable in unary expression('LHS') must match variable used on right hand side of assignment('RHS') from the first statement}}
1573+
// expected-note@+1{{sub-expression in unary expression('LHS') must match sub-expression used on right hand side of assignment('RHS') from the first statement}}
15741574
LHS++;
15751575
}
15761576
}
@@ -1629,7 +1629,7 @@ void AtomicCaptureCompound(int LHS, int RHS) {
16291629
{
16301630
LHS--;
16311631
// expected-error@-3{{statement associated with OpenACC 'atomic capture' directive is invalid}}
1632-
// expected-note@+1{{variable on right hand side of assignment('RHS') must match variable used in unary expression('LHS') from the first statement}}
1632+
// expected-note@+1{{sub-expression on right hand side of assignment('RHS') must match sub-expression used in unary expression('LHS') from the first statement}}
16331633
LHS = RHS;
16341634
}
16351635

@@ -1666,7 +1666,7 @@ void AtomicCaptureCompound(int LHS, int RHS) {
16661666
{
16671667
LHS *= 1;
16681668
// expected-error@-3{{statement associated with OpenACC 'atomic capture' directive is invalid}}
1669-
// expected-note@+1{{variable on right hand side of assignment('RHS') must match variable used on left hand side of compound assignment('LHS') from the first statement}}
1669+
// expected-note@+1{{sub-expression on right hand side of assignment('RHS') must match sub-expression used on left hand side of compound assignment('LHS') from the first statement}}
16701670
LHS = RHS;
16711671
}
16721672
#pragma acc atomic capture
@@ -1702,7 +1702,7 @@ void AtomicCaptureCompound(int LHS, int RHS) {
17021702
{
17031703
LHS = LHS * 1;
17041704
// expected-error@-3{{statement associated with OpenACC 'atomic capture' directive is invalid}}
1705-
// expected-note@+1{{variable on right hand side of assignment('RHS') must match variable used on left hand side of assignment('LHS') from the first statement}}
1705+
// expected-note@+1{{sub-expression on right hand side of assignment('RHS') must match sub-expression used on left hand side of assignment('LHS') from the first statement}}
17061706
RHS = RHS;
17071707
}
17081708
#pragma acc atomic capture
@@ -1738,7 +1738,7 @@ void AtomicCaptureCompound(int LHS, int RHS) {
17381738
{
17391739
LHS = LHS | 1;
17401740
// expected-error@-3{{statement associated with OpenACC 'atomic capture' directive is invalid}}
1741-
// expected-note@+1{{variable on right hand side of assignment('RHS') must match variable used on left hand side of assignment('LHS') from the first statement}}
1741+
// expected-note@+1{{sub-expression on right hand side of assignment('RHS') must match sub-expression used on left hand side of assignment('LHS') from the first statement}}
17421742
RHS = RHS;
17431743
}
17441744
#pragma acc atomic capture
@@ -1815,7 +1815,7 @@ void AtomicCaptureCompound(int LHS, int RHS) {
18151815
{
18161816
LHS = RHS;
18171817
// expected-error@-3{{statement associated with OpenACC 'atomic capture' directive is invalid}}
1818-
// expected-note@+1{{variable on left hand side of assignment('LHS') must match variable used on right hand side of assignment('RHS') from the first statement}}
1818+
// expected-note@+1{{sub-expression on left hand side of assignment('LHS') must match sub-expression used on right hand side of assignment('RHS') from the first statement}}
18191819
LHS = 1;
18201820
}
18211821

@@ -1861,7 +1861,23 @@ void AtomicCaptureCompound(int LHS, int RHS) {
18611861
{
18621862
LHS = RHS;
18631863
// expected-error@-3{{statement associated with OpenACC 'atomic capture' directive is invalid}}
1864-
// expected-note@+1{{variable in unary expression('LHS') must match variable used on right hand side of assignment('RHS') from the first statement}}
1864+
// expected-note@+1{{sub-expression in unary expression('LHS') must match sub-expression used on right hand side of assignment('RHS') from the first statement}}
18651865
LHS++;
18661866
}
1867+
1868+
// Example from UDel test suite, which wasn't working because of irrelevant
1869+
// parens, make sure we work with these. This should not diagnose.
1870+
typedef double real_t;
1871+
int * distribution;
1872+
real_t *a;
1873+
real_t *b;
1874+
int *c;
1875+
for (int x = 0; x < 5; ++x) {
1876+
#pragma acc atomic capture
1877+
{
1878+
c[x] = distribution[(int) (a[x]*b[x]/10)];
1879+
(distribution[(int)(a[x]*b[x]/10)])--;
1880+
}
1881+
}
1882+
18671883
}

0 commit comments

Comments
 (0)