[clang] 45ccfd9 - Treat opencl_unroll_hint subject errors as semantic rather than parse errors
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 5 04:20:59 PST 2021
Author: Aaron Ballman
Date: 2021-02-05T07:20:41-05:00
New Revision: 45ccfd9c9d0311371a8217c15c2ef3ba969a0aff
URL: https://github.com/llvm/llvm-project/commit/45ccfd9c9d0311371a8217c15c2ef3ba969a0aff
DIFF: https://github.com/llvm/llvm-project/commit/45ccfd9c9d0311371a8217c15c2ef3ba969a0aff.diff
LOG: Treat opencl_unroll_hint subject errors as semantic rather than parse errors
The attribute definition claimed the attribute was inheritable (which
only applies to declaration attributes) and not a statement attribute.
Further, it treats subject appertainment errors as being parse errors
rather than semantic errors, which leads to us accepting invalid code.
For instance, we currently fail to reject:
void foo() {
int i = 1000;
__attribute__((nomerge, opencl_unroll_hint(8)))
if (i) { foo(); }
}
This addresses the issues by clarifying that opencl_unroll_hint is a
statement attribute and handles its appertainment checks in the
semantic layer instead of the parsing layer. This changes the output of
the diagnostic text to be more consistent with other appertainment
errors.
Added:
Modified:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/DiagnosticParseKinds.td
clang/include/clang/Parse/Parser.h
clang/lib/Parse/ParseStmt.cpp
clang/lib/Sema/SemaStmtAttr.cpp
clang/test/Parser/opencl-unroll-hint.cl
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index bfd50f6a6779..5eb8db516305 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -1178,8 +1178,10 @@ def OpenCLKernel : InheritableAttr {
let SimpleHandler = 1;
}
-def OpenCLUnrollHint : InheritableAttr {
+def OpenCLUnrollHint : StmtAttr {
let Spellings = [GNU<"opencl_unroll_hint">];
+// let Subjects = SubjectList<[ForStmt, CXXForRangeStmt, WhileStmt, DoStmt],
+// ErrorDiag, "'for', 'while', and 'do' statements">;
let Args = [UnsignedArgument<"UnrollHint">];
let Documentation = [OpenCLUnrollHintDocs];
}
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 0ed80a481e78..da228cc4be7a 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1220,9 +1220,6 @@ def note_pragma_attribute_namespace_on_attribute : Note<
"omit the namespace to add attributes to the most-recently"
" pushed attribute group">;
-def err_opencl_unroll_hint_on_non_loop : Error<
- "OpenCL only supports 'opencl_unroll_hint' attribute on for, while, and do statements">;
-
// OpenCL EXTENSION pragma (OpenCL 1.1 [9.1])
def warn_pragma_expected_colon : Warning<
"missing ':' after %0 - ignoring">, InGroup<IgnoredPragmas>;
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index f8b746446e7e..7c2f14cd83b9 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -2821,17 +2821,6 @@ class Parser : public CodeCompletionHandler {
void ParseBorlandTypeAttributes(ParsedAttributes &attrs);
void ParseOpenCLKernelAttributes(ParsedAttributes &attrs);
void ParseOpenCLQualifiers(ParsedAttributes &Attrs);
- /// Parses opencl_unroll_hint attribute if language is OpenCL v2.0
- /// or higher.
- /// \return false if error happens.
- bool MaybeParseOpenCLUnrollHintAttribute(ParsedAttributes &Attrs) {
- if (getLangOpts().OpenCL)
- return ParseOpenCLUnrollHintAttribute(Attrs);
- return true;
- }
- /// Parses opencl_unroll_hint attribute.
- /// \return false if error happens.
- bool ParseOpenCLUnrollHintAttribute(ParsedAttributes &Attrs);
void ParseNullabilityTypeSpecifiers(ParsedAttributes &attrs);
VersionTuple ParseVersionTuple(SourceRange &Range);
diff --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp
index 26a02575010c..71344ff10155 100644
--- a/clang/lib/Parse/ParseStmt.cpp
+++ b/clang/lib/Parse/ParseStmt.cpp
@@ -98,10 +98,15 @@ Parser::ParseStatementOrDeclaration(StmtVector &Stmts,
ParenBraceBracketBalancer BalancerRAIIObj(*this);
+ // Because we're parsing either a statement or a declaration, the order of
+ // attribute parsing is important. [[]] attributes at the start of a
+ // statement are
diff erent from [[]] attributes that follow an __attribute__
+ // at the start of the statement. Thus, we're not using MaybeParseAttributes
+ // here because we don't want to allow arbitrary orderings.
ParsedAttributesWithRange Attrs(AttrFactory);
MaybeParseCXX11Attributes(Attrs, nullptr, /*MightBeObjCMessageSend*/ true);
- if (!MaybeParseOpenCLUnrollHintAttribute(Attrs))
- return StmtError();
+ if (getLangOpts().OpenCL)
+ MaybeParseGNUAttributes(Attrs);
StmtResult Res = ParseStatementOrDeclarationAfterAttributes(
Stmts, StmtCtx, TrailingElseLoc, Attrs);
@@ -2548,19 +2553,3 @@ void Parser::ParseMicrosoftIfExistsStatement(StmtVector &Stmts) {
}
Braces.consumeClose();
}
-
-bool Parser::ParseOpenCLUnrollHintAttribute(ParsedAttributes &Attrs) {
- MaybeParseGNUAttributes(Attrs);
-
- if (Attrs.empty())
- return true;
-
- if (Attrs.begin()->getKind() != ParsedAttr::AT_OpenCLUnrollHint)
- return true;
-
- if (!(Tok.is(tok::kw_for) || Tok.is(tok::kw_while) || Tok.is(tok::kw_do))) {
- Diag(Tok, diag::err_opencl_unroll_hint_on_non_loop);
- return false;
- }
- return true;
-}
diff --git a/clang/lib/Sema/SemaStmtAttr.cpp b/clang/lib/Sema/SemaStmtAttr.cpp
index 8031aa6b0ece..86a09c42863f 100644
--- a/clang/lib/Sema/SemaStmtAttr.cpp
+++ b/clang/lib/Sema/SemaStmtAttr.cpp
@@ -378,6 +378,12 @@ static Attr *handleOpenCLUnrollHint(Sema &S, Stmt *St, const ParsedAttr &A,
// determines unrolling factor) or 1 argument (the unroll factor provided
// by the user).
+ if (!isa<ForStmt, CXXForRangeStmt, DoStmt, WhileStmt>(St)) {
+ S.Diag(A.getLoc(), diag::err_attribute_wrong_decl_type_str)
+ << A << "'for', 'while', and 'do' statements";
+ return nullptr;
+ }
+
unsigned NumArgs = A.getNumArgs();
if (NumArgs > 1) {
diff --git a/clang/test/Parser/opencl-unroll-hint.cl b/clang/test/Parser/opencl-unroll-hint.cl
index 5742dcde2195..adfe18d99672 100644
--- a/clang/test/Parser/opencl-unroll-hint.cl
+++ b/clang/test/Parser/opencl-unroll-hint.cl
@@ -1,8 +1,21 @@
-//RUN: %clang_cc1 -O0 -cl-std=CL2.0 -fsyntax-only -verify %s
+//RUN: %clang_cc1 -cl-std=CL2.0 -fsyntax-only -verify %s
kernel void B (global int *x) {
- __attribute__((opencl_unroll_hint(42)))
- if (x[0]) // expected-error {{OpenCL only supports 'opencl_unroll_hint' attribute on for, while, and do statements}}
+ __attribute__((opencl_unroll_hint(42))) // expected-error {{'opencl_unroll_hint' attribute only applies to 'for', 'while', and 'do' statements}}
+ if (x[0])
x[0] = 15;
}
+void parse_order_error() {
+ // Ensure we properly diagnose OpenCL loop attributes on the incorrect
+ // subject in the presence of other attributes.
+ int i = 1000;
+ __attribute__((nomerge, opencl_unroll_hint(8))) // expected-error {{'opencl_unroll_hint' attribute only applies to 'for', 'while', and 'do' statements}}
+ if (i) { parse_order_error(); } // Recursive call silences unrelated diagnostic about nomerge.
+
+ __attribute__((opencl_unroll_hint(8), nomerge)) // expected-error {{'opencl_unroll_hint' attribute only applies to 'for', 'while', and 'do' statements}}
+ if (i) { parse_order_error(); } // Recursive call silences unrelated diagnostic about nomerge.
+
+ __attribute__((nomerge, opencl_unroll_hint(8))) // OK
+ while (1) { parse_order_error(); } // Recursive call silences unrelated diagnostic about nomerge.
+}
More information about the cfe-commits
mailing list