[PATCH] D31183: [OpenCL] Added parsing for OpenCL vector types.
Anastasia accepted this revision. Anastasia added a comment. This revision is now accepted and ready to land. LGTM! https://reviews.llvm.org/D31183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31183: [OpenCL] Added parsing for OpenCL vector types.
echuraev updated this revision to Diff 93132.
echuraev marked an inline comment as done.
https://reviews.llvm.org/D31183
Files:
include/clang/Parse/Parser.h
lib/Parse/ParseExpr.cpp
test/Parser/vector-cast-define.cl
Index: test/Parser/vector-cast-define.cl
===
--- /dev/null
+++ test/Parser/vector-cast-define.cl
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
+// expected-no-diagnostics
+
+typedef int int3 __attribute__((ext_vector_type(3)));
+
+void test()
+{
+int index = (int3)(1, 2, 3).x * (int3)(3, 2, 1).y;
+}
+
Index: lib/Parse/ParseExpr.cpp
===
--- lib/Parse/ParseExpr.cpp
+++ lib/Parse/ParseExpr.cpp
@@ -473,12 +473,14 @@
///
ExprResult Parser::ParseCastExpression(bool isUnaryExpression,
bool isAddressOfOperand,
- TypeCastState isTypeCast) {
+ TypeCastState isTypeCast,
+ bool isVectorLiteral) {
bool NotCastExpr;
ExprResult Res = ParseCastExpression(isUnaryExpression,
isAddressOfOperand,
NotCastExpr,
- isTypeCast);
+ isTypeCast,
+ isVectorLiteral);
if (NotCastExpr)
Diag(Tok, diag::err_expected_expression);
return Res;
@@ -694,7 +696,8 @@
ExprResult Parser::ParseCastExpression(bool isUnaryExpression,
bool isAddressOfOperand,
bool &NotCastExpr,
- TypeCastState isTypeCast) {
+ TypeCastState isTypeCast,
+ bool isVectorLiteral) {
ExprResult Res;
tok::TokenKind SavedKind = Tok.getKind();
NotCastExpr = false;
@@ -722,6 +725,9 @@
Res = ParseParenExpression(ParenExprType, false/*stopIfCastExr*/,
isTypeCast == IsTypeCast, CastTy, RParenLoc);
+if (isVectorLiteral)
+return Res;
+
switch (ParenExprType) {
case SimpleExpr: break;// Nothing else to do.
case CompoundStmt: break; // Nothing else to do.
@@ -2350,6 +2356,48 @@
return ParseCompoundLiteralExpression(Ty.get(), OpenLoc, RParenLoc);
}
+ if (Tok.is(tok::l_paren)) {
+// This could be OpenCL vector Literals
+if (getLangOpts().OpenCL)
+{
+ TypeResult Ty;
+ {
+InMessageExpressionRAIIObject InMessage(*this, false);
+Ty = Actions.ActOnTypeName(getCurScope(), DeclaratorInfo);
+ }
+ if(Ty.isInvalid())
+ {
+ return ExprError();
+ }
+ QualType QT = Ty.get().get().getCanonicalType();
+ if (QT->isVectorType())
+ {
+// We parsed '(' vector-type-name ')' followed by '('
+
+// Parse the cast-expression that follows it next.
+// isVectorLiteral = true will make sure we don't parse any
+// Postfix expression yet
+Result = ParseCastExpression(/*isUnaryExpression=*/false,
+ /*isAddressOfOperand=*/false,
+ /*isTypeCast=*/IsTypeCast,
+ /*isVectorLiteral=*/true);
+
+if (!Result.isInvalid()) {
+ Result = Actions.ActOnCastExpr(getCurScope(), OpenLoc,
+ DeclaratorInfo, CastTy,
+ RParenLoc, Result.get());
+}
+
+// After we performed the cast we can check for postfix-expr pieces.
+if (!Result.isInvalid()) {
+ Result = ParsePostfixExpressionSuffix(Result);
+}
+
+return Result;
+ }
+}
+ }
+
if (ExprType == CastExpr) {
// We parsed '(' type-name ')' and the thing after it wasn't a '{'.
@@ -2379,10 +2427,13 @@
}
// Parse the cast-expression that follows it next.
+// isVectorLiteral = true will make sure we don't parse any
+// Postfix expression yet
// TODO: For cast expression with CastTy.
Result = ParseCastExpression(/*isUnaryExpression=*/false,
/*isAddressOfOperand=*/false,
- /*isTypeCast=*/IsTypeCast);
+ /*isTypeCast=*/IsTypeCast,
+ /*isVectorLiteral=*/true);
if (!Result.isInvalid()) {
Result = Actions.ActOnCastExpr(getCurScope(), OpenLoc,
DeclaratorInfo, CastTy,
Index: include/c
[PATCH] D31183: [OpenCL] Added parsing for OpenCL vector types.
Anastasia added a comment. I don't think that diagnostics can always be very clear. This is not the case neither for C nor C++. As I said I don't see any issue to continue with this patch. I would just like to see the test simplified a bit. https://reviews.llvm.org/D31183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31183: [OpenCL] Added parsing for OpenCL vector types.
echuraev added a comment. In https://reviews.llvm.org/D31183#710202, @Anastasia wrote: > In https://reviews.llvm.org/D31183#709566, @echuraev wrote: > > > In https://reviews.llvm.org/D31183#708833, @yaxunl wrote: > > > > > I think this is a good feature for the convenience of user. I've seen > > > usage like this. > > > > > > I agree. I don't see any reasons why this case doesn't have the right to > > exist. I don't think that using extra parenthesis is a good solution for > > solving this problem. > > > I am just saying that I don't see a big use case for this. I am guessing it > can largely come from the macro expansions, but those are generally good > style to parenthesize. Ok. But in current implementation if I forget to parenthesize the defined expression (as in the test) I will get the following message: "error: member reference base type 'int' is not a structure or union". I don't think that the message is clear to understand that you just forgot to add parenthesis. So, should we change the diagnostic message to do it more understandable or push this patch because it can be more convenience for users? https://reviews.llvm.org/D31183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31183: [OpenCL] Added parsing for OpenCL vector types.
Anastasia added a comment. In https://reviews.llvm.org/D31183#709566, @echuraev wrote: > In https://reviews.llvm.org/D31183#708833, @yaxunl wrote: > > > I think this is a good feature for the convenience of user. I've seen usage > > like this. > > > I agree. I don't see any reasons why this case doesn't have the right to > exist. I don't think that using extra parenthesis is a good solution for > solving this problem. I am just saying that I don't see a big use case for this. I am guessing it can largely come from the macro expansions, but those are generally good style to parenthesize. https://reviews.llvm.org/D31183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31183: [OpenCL] Added parsing for OpenCL vector types.
echuraev added a comment. In https://reviews.llvm.org/D31183#708833, @yaxunl wrote: > I think this is a good feature for the convenience of user. I've seen usage > like this. I agree. I don't see any reasons why this case doesn't have the right to exist. I don't think that using extra parenthesis is a good solution for solving this problem. https://reviews.llvm.org/D31183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31183: [OpenCL] Added parsing for OpenCL vector types.
yaxunl added a comment. I think this is a good feature for the convenience of user. I've seen usage like this. https://reviews.llvm.org/D31183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31183: [OpenCL] Added parsing for OpenCL vector types.
Anastasia added a comment.
Although nothing wrong with the implementation apart from it complicates a bit
the understanding of conventional C parse flow by adding extra corner case, I
don't see anything in the spec that states explicitly how the vector components
should be parsed. I guess with extra parenthesis the parsing problem can easily
be avoided:
((int3)(32, 32, 32)).x
Comment at: test/Parser/vector-cast-define.cl:11
+{
+int index = dgSize.x * dgSize.y;
+}
I would prefer to simply the test and avoid using macros... it will be easier
for us to maintain it in the future.
https://reviews.llvm.org/D31183
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31183: [OpenCL] Added parsing for OpenCL vector types.
echuraev created this revision.
Herald added a subscriber: yaxunl.
https://reviews.llvm.org/D31183
Files:
include/clang/Parse/Parser.h
lib/Parse/ParseExpr.cpp
test/Parser/vector-cast-define.cl
Index: test/Parser/vector-cast-define.cl
===
--- /dev/null
+++ test/Parser/vector-cast-define.cl
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
+// expected-no-diagnostics
+
+typedef int int3 __attribute__((ext_vector_type(3)));
+
+#define i3(x, y, z) (int3)(x, y, z)
+#define dgSize i3(32+2,32+2,32+2)
+
+void test()
+{
+int index = dgSize.x * dgSize.y;
+}
+
Index: lib/Parse/ParseExpr.cpp
===
--- lib/Parse/ParseExpr.cpp
+++ lib/Parse/ParseExpr.cpp
@@ -473,12 +473,14 @@
///
ExprResult Parser::ParseCastExpression(bool isUnaryExpression,
bool isAddressOfOperand,
- TypeCastState isTypeCast) {
+ TypeCastState isTypeCast,
+ bool isVectorLiteral) {
bool NotCastExpr;
ExprResult Res = ParseCastExpression(isUnaryExpression,
isAddressOfOperand,
NotCastExpr,
- isTypeCast);
+ isTypeCast,
+ isVectorLiteral);
if (NotCastExpr)
Diag(Tok, diag::err_expected_expression);
return Res;
@@ -694,7 +696,8 @@
ExprResult Parser::ParseCastExpression(bool isUnaryExpression,
bool isAddressOfOperand,
bool &NotCastExpr,
- TypeCastState isTypeCast) {
+ TypeCastState isTypeCast,
+ bool isVectorLiteral) {
ExprResult Res;
tok::TokenKind SavedKind = Tok.getKind();
NotCastExpr = false;
@@ -722,6 +725,9 @@
Res = ParseParenExpression(ParenExprType, false/*stopIfCastExr*/,
isTypeCast == IsTypeCast, CastTy, RParenLoc);
+if (isVectorLiteral)
+return Res;
+
switch (ParenExprType) {
case SimpleExpr: break;// Nothing else to do.
case CompoundStmt: break; // Nothing else to do.
@@ -2350,6 +2356,48 @@
return ParseCompoundLiteralExpression(Ty.get(), OpenLoc, RParenLoc);
}
+ if (Tok.is(tok::l_paren)) {
+// This could be OpenCL vector Literals
+if (getLangOpts().OpenCL)
+{
+ TypeResult Ty;
+ {
+InMessageExpressionRAIIObject InMessage(*this, false);
+Ty = Actions.ActOnTypeName(getCurScope(), DeclaratorInfo);
+ }
+ if(Ty.isInvalid())
+ {
+ return ExprError();
+ }
+ QualType QT = Ty.get().get().getCanonicalType();
+ if (QT->isVectorType())
+ {
+// We parsed '(' vector-type-name ')' followed by '('
+
+// Parse the cast-expression that follows it next.
+// isVectorLiteral = true will make sure we don't parse any
+// Postfix expression yet
+Result = ParseCastExpression(/*isUnaryExpression=*/false,
+ /*isAddressOfOperand=*/false,
+ /*isTypeCast=*/IsTypeCast,
+ /*isVectorLiteral=*/true);
+
+if (!Result.isInvalid()) {
+ Result = Actions.ActOnCastExpr(getCurScope(), OpenLoc,
+ DeclaratorInfo, CastTy,
+ RParenLoc, Result.get());
+}
+
+// After we performed the cast we can check for postfix-expr pieces.
+if (!Result.isInvalid()) {
+ Result = ParsePostfixExpressionSuffix(Result);
+}
+
+return Result;
+ }
+}
+ }
+
if (ExprType == CastExpr) {
// We parsed '(' type-name ')' and the thing after it wasn't a '{'.
@@ -2379,10 +2427,13 @@
}
// Parse the cast-expression that follows it next.
+// isVectorLiteral = true will make sure we don't parse any
+// Postfix expression yet
// TODO: For cast expression with CastTy.
Result = ParseCastExpression(/*isUnaryExpression=*/false,
/*isAddressOfOperand=*/false,
- /*isTypeCast=*/IsTypeCast);
+ /*isTypeCast=*/IsTypeCast,
+ /*isVectorLiteral=*/true);
if (!Result.isInvalid()) {
Result = Actions.ActOnCastExpr(getCurScope(), OpenLoc,
Declara
