Hi,
The attached patch moves the checking for vector operations after the usual
arithmetic conversions are performed. This matters because:
* The OpenCL specification requires that we perform the usual arithmetic
conversions on vectors (although it requires changes to the conversions
themselves, which we do not yet implement).
* The lvalue-to-rvalue conversion on vector operands is otherwise missing in
many cases (leading to problems with analysis passes, and soon with C++11
constant evaluation).
The patch adds both some tests and a new assert that ImpCastExprToType doesn't
accidentally cast an lvalue to an rvalue; this assert fires on the new tests
without the rest of the patch.
I'm not very familiar with the vector handling code, so I'd appreciate someone
looking over the patch and the approach to make sure that it's reasonable.
One unit test changed behavior (for an ext_vector_type vector):
ish8 += 5; // expected-error {{...}}
ish8 += (short)5;
We now reject the second line just like the first, since the short is promoted
back to int before being assigned. However, according to the OpenCL
specification (which I've been informed that ext_vector_type is supposed to
support), both of these lines should be legal. The right fix for this would
seem to be to implement the OpenCL rules for the usual arithmetic conversions.
Thanks,
RichardIndex: test/Sema/ext_vector_comparisons.c
===================================================================
--- test/Sema/ext_vector_comparisons.c (revision 143150)
+++ test/Sema/ext_vector_comparisons.c (working copy)
@@ -6,12 +6,12 @@
int4 vec, rv;
// comparisons to self...
- return vec == vec; // expected-warning{{self-comparison always evaluates to a constant}}
- return vec != vec; // expected-warning{{self-comparison always evaluates to a constant}}
- return vec < vec; // expected-warning{{self-comparison always evaluates to a constant}}
- return vec <= vec; // expected-warning{{self-comparison always evaluates to a constant}}
- return vec > vec; // expected-warning{{self-comparison always evaluates to a constant}}
- return vec >= vec; // expected-warning{{self-comparison always evaluates to a constant}}
+ return vec == vec; // expected-warning{{self-comparison always evaluates to true}}
+ return vec != vec; // expected-warning{{self-comparison always evaluates to false}}
+ return vec < vec; // expected-warning{{self-comparison always evaluates to false}}
+ return vec <= vec; // expected-warning{{self-comparison always evaluates to true}}
+ return vec > vec; // expected-warning{{self-comparison always evaluates to false}}
+ return vec >= vec; // expected-warning{{self-comparison always evaluates to true}}
}
Index: test/Sema/ext_vector_casts.c
===================================================================
--- test/Sema/ext_vector_casts.c (revision 143150)
+++ test/Sema/ext_vector_casts.c (working copy)
@@ -32,7 +32,7 @@
vec4 = (float4)vec2; // expected-error {{invalid conversion between ext-vector type 'float4' and 'float2'}}
ish8 += 5; // expected-error {{can't convert between vector values of different size ('short8' and 'int')}}
- ish8 += (short)5;
+ ish8 += (short8)5;
ivec4 *= 5;
vec4 /= 5.2f;
vec4 %= 4; // expected-error {{invalid operands to binary expression ('float4' and 'int')}}
Index: test/Parser/cxx-altivec.cpp
===================================================================
--- test/Parser/cxx-altivec.cpp (revision 143150)
+++ test/Parser/cxx-altivec.cpp (working copy)
@@ -130,12 +130,12 @@
// bug 7553 - Problem with '==' and vectors
void func() {
bool res_b;
- res_b = (vv_sc == vv_sc);
- res_b = (vv_uc != vv_uc);
- res_b = (vv_s > vv_s);
- res_b = (vv_us >= vv_us);
- res_b = (vv_i < vv_i);
- res_b = (vv_ui <= vv_ui);
+ res_b = (vv_sc == vv_sc); // expected-warning {{always evaluates to true}}
+ res_b = (vv_uc != vv_uc); // expected-warning {{always evaluates to false}}
+ res_b = (vv_s > vv_s); // expected-warning {{always evaluates to false}}
+ res_b = (vv_us >= vv_us); // expected-warning {{always evaluates to true}}
+ res_b = (vv_i < vv_i); // expected-warning {{always evaluates to false}}
+ res_b = (vv_ui <= vv_ui); // expected-warning {{always evaluates to true}}
res_b = (vv_f <= vv_f);
}
Index: test/Parser/altivec.c
===================================================================
--- test/Parser/altivec.c (revision 143150)
+++ test/Parser/altivec.c (working copy)
@@ -103,12 +103,12 @@
int res_i;
// bug 7553 - Problem with '==' and vectors
- res_i = (vv_sc == vv_sc);
- res_i = (vv_uc != vv_uc);
- res_i = (vv_s > vv_s);
- res_i = (vv_us >= vv_us);
- res_i = (vv_i < vv_i);
- res_i = (vv_ui <= vv_ui);
+ res_i = (vv_sc == vv_sc); // expected-warning {{self-comparison always evaluates to true}}
+ res_i = (vv_uc != vv_uc); // expected-warning {{self-comparison always evaluates to false}}
+ res_i = (vv_s > vv_s); // expected-warning {{self-comparison always evaluates to false}}
+ res_i = (vv_us >= vv_us); // expected-warning {{self-comparison always evaluates to true}}
+ res_i = (vv_i < vv_i); // expected-warning {{self-comparison always evaluates to false}}
+ res_i = (vv_ui <= vv_ui); // expected-warning {{self-comparison always evaluates to true}}
res_i = (vv_f <= vv_f);
}
Index: test/SemaCXX/vector.cpp
===================================================================
--- test/SemaCXX/vector.cpp (revision 143150)
+++ test/SemaCXX/vector.cpp (working copy)
@@ -218,3 +218,55 @@
// Scalar-to-vector conversions.
accept_fltx2(1.0); // expected-error{{no matching function for call to 'accept_fltx2'}}
}
+
+typedef int intx4 __attribute__((__vector_size__(16)));
+typedef int inte4 __attribute__((__ext_vector_type__(4)));
+typedef int flte4 __attribute__((__ext_vector_type__(4)));
+
+void test_mixed_vector_types(fltx4 f, intx4 n, flte4 g, flte4 m) {
+ (void)(f == g);
+ (void)(g != f);
+ (void)(f <= g);
+ (void)(g >= f);
+ (void)(f < g);
+ (void)(g > f);
+
+ (void)(+g);
+ (void)(-g);
+
+ (void)(f + g);
+ (void)(f - g);
+ (void)(f * g);
+ (void)(f / g);
+ (void)(f = g);
+ (void)(f += g);
+ (void)(f -= g);
+ (void)(f *= g);
+ (void)(f /= g);
+
+
+ (void)(n == m);
+ (void)(m != n);
+ (void)(n <= m);
+ (void)(m >= n);
+ (void)(n < m);
+ (void)(m > n);
+
+ (void)(+m);
+ (void)(-m);
+ (void)(~m);
+
+ (void)(n + m);
+ (void)(n - m);
+ (void)(n * m);
+ (void)(n / m);
+ (void)(n % m);
+ (void)(n = m);
+ (void)(n += m);
+ (void)(n -= m);
+ (void)(n *= m);
+ (void)(n /= m);
+}
+
+struct S { intx4 f(); } s;
+intx4 k = s.f + s.f;
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp (revision 143166)
+++ lib/Sema/SemaExpr.cpp (working copy)
@@ -5843,14 +5843,14 @@
bool IsCompAssign, bool IsDiv) {
checkArithmeticNull(*this, LHS, RHS, Loc, /*isCompare=*/false);
- if (LHS.get()->getType()->isVectorType() ||
- RHS.get()->getType()->isVectorType())
- return CheckVectorOperands(LHS, RHS, Loc, IsCompAssign);
-
QualType compType = UsualArithmeticConversions(LHS, RHS, IsCompAssign);
if (LHS.isInvalid() || RHS.isInvalid())
return QualType();
+ if (LHS.get()->getType()->isVectorType() ||
+ RHS.get()->getType()->isVectorType())
+ return CheckVectorOperands(LHS, RHS, Loc, IsCompAssign);
+
if (!LHS.get()->getType()->isArithmeticType() ||
!RHS.get()->getType()->isArithmeticType())
return InvalidOperands(Loc, LHS, RHS);
@@ -5869,6 +5869,10 @@
ExprResult &LHS, ExprResult &RHS, SourceLocation Loc, bool IsCompAssign) {
checkArithmeticNull(*this, LHS, RHS, Loc, /*isCompare=*/false);
+ QualType compType = UsualArithmeticConversions(LHS, RHS, IsCompAssign);
+ if (LHS.isInvalid() || RHS.isInvalid())
+ return QualType();
+
if (LHS.get()->getType()->isVectorType() ||
RHS.get()->getType()->isVectorType()) {
if (LHS.get()->getType()->hasIntegerRepresentation() &&
@@ -5877,10 +5881,6 @@
return InvalidOperands(Loc, LHS, RHS);
}
- QualType compType = UsualArithmeticConversions(LHS, RHS, IsCompAssign);
- if (LHS.isInvalid() || RHS.isInvalid())
- return QualType();
-
if (!LHS.get()->getType()->isIntegerType() ||
!RHS.get()->getType()->isIntegerType())
return InvalidOperands(Loc, LHS, RHS);
@@ -6061,13 +6061,6 @@
ExprResult &LHS, ExprResult &RHS, SourceLocation Loc, QualType* CompLHSTy) {
checkArithmeticNull(*this, LHS, RHS, Loc, /*isCompare=*/false);
- if (LHS.get()->getType()->isVectorType() ||
- RHS.get()->getType()->isVectorType()) {
- QualType compType = CheckVectorOperands(LHS, RHS, Loc, CompLHSTy);
- if (CompLHSTy) *CompLHSTy = compType;
- return compType;
- }
-
QualType compType = UsualArithmeticConversions(LHS, RHS, CompLHSTy);
if (LHS.isInvalid() || RHS.isInvalid())
return QualType();
@@ -6079,6 +6072,13 @@
return compType;
}
+ if (LHS.get()->getType()->isVectorType() ||
+ RHS.get()->getType()->isVectorType()) {
+ QualType compType = CheckVectorOperands(LHS, RHS, Loc, CompLHSTy);
+ if (CompLHSTy) *CompLHSTy = compType;
+ return compType;
+ }
+
// Put any potential pointer into PExp
Expr* PExp = LHS.get(), *IExp = RHS.get();
if (IExp->getType()->isAnyPointerType())
@@ -6119,13 +6119,6 @@
QualType* CompLHSTy) {
checkArithmeticNull(*this, LHS, RHS, Loc, /*isCompare=*/false);
- if (LHS.get()->getType()->isVectorType() ||
- RHS.get()->getType()->isVectorType()) {
- QualType compType = CheckVectorOperands(LHS, RHS, Loc, CompLHSTy);
- if (CompLHSTy) *CompLHSTy = compType;
- return compType;
- }
-
QualType compType = UsualArithmeticConversions(LHS, RHS, CompLHSTy);
if (LHS.isInvalid() || RHS.isInvalid())
return QualType();
@@ -6139,6 +6132,13 @@
return compType;
}
+ if (LHS.get()->getType()->isVectorType() ||
+ RHS.get()->getType()->isVectorType()) {
+ QualType compType = CheckVectorOperands(LHS, RHS, Loc, CompLHSTy);
+ if (CompLHSTy) *CompLHSTy = compType;
+ return compType;
+ }
+
// Either ptr - int or ptr - ptr.
if (LHS.get()->getType()->isAnyPointerType()) {
QualType lpointee = LHS.get()->getType()->getPointeeType();
@@ -6282,11 +6282,6 @@
return InvalidOperands(Loc, LHS, RHS);
}
- // Vector shifts promote their scalar inputs to vector type.
- if (LHS.get()->getType()->isVectorType() ||
- RHS.get()->getType()->isVectorType())
- return CheckVectorOperands(LHS, RHS, Loc, IsCompAssign);
-
// Shifts don't perform usual arithmetic conversions, they just do integer
// promotions on each operand. C99 6.5.7p3
@@ -6304,6 +6299,11 @@
if (RHS.isInvalid())
return QualType();
+ // Vector shifts promote their scalar inputs to vector type.
+ if (LHS.get()->getType()->isVectorType() ||
+ RHS.get()->getType()->isVectorType())
+ return CheckVectorOperands(LHS, RHS, Loc, IsCompAssign);
+
// Sanity-check shift operands
DiagnoseBadShiftValues(*this, LHS, RHS, Loc, Opc, LHSType);
@@ -6423,11 +6423,6 @@
BinaryOperatorKind Opc = (BinaryOperatorKind) OpaqueOpc;
- // Handle vector comparisons separately.
- if (LHS.get()->getType()->isVectorType() ||
- RHS.get()->getType()->isVectorType())
- return CheckVectorCompareOperands(LHS, RHS, Loc, IsRelational);
-
QualType LHSType = LHS.get()->getType();
QualType RHSType = RHS.get()->getType();
@@ -6524,6 +6519,14 @@
}
}
+ LHS = UsualUnaryConversions(LHS.take());
+ if (LHS.isInvalid())
+ return QualType();
+
+ RHS = UsualUnaryConversions(RHS.take());
+ if (RHS.isInvalid())
+ return QualType();
+
// C99 6.5.8p3 / C99 6.5.9p4
if (LHS.get()->getType()->isArithmeticType() &&
RHS.get()->getType()->isArithmeticType()) {
@@ -6531,15 +6534,11 @@
if (LHS.isInvalid() || RHS.isInvalid())
return QualType();
}
- else {
- LHS = UsualUnaryConversions(LHS.take());
- if (LHS.isInvalid())
- return QualType();
- RHS = UsualUnaryConversions(RHS.take());
- if (RHS.isInvalid())
- return QualType();
- }
+ // Handle vector comparisons separately.
+ if (LHS.get()->getType()->isVectorType() ||
+ RHS.get()->getType()->isVectorType())
+ return CheckVectorCompareOperands(LHS, RHS, Loc, IsRelational);
LHSType = LHS.get()->getType();
RHSType = RHS.get()->getType();
@@ -6815,20 +6814,6 @@
if (vType->getAs<VectorType>()->getVectorKind() == VectorType::AltiVecVector)
return Context.getLogicalOperationType();
- // For non-floating point types, check for self-comparisons of the form
- // x == x, x != x, x < x, etc. These always evaluate to a constant, and
- // often indicate logic errors in the program.
- if (!LHSType->hasFloatingRepresentation()) {
- if (DeclRefExpr* DRL = dyn_cast<DeclRefExpr>(LHS.get()->IgnoreParens()))
- if (DeclRefExpr* DRR = dyn_cast<DeclRefExpr>(RHS.get()->IgnoreParens()))
- if (DRL->getDecl() == DRR->getDecl())
- DiagRuntimeBehavior(Loc, 0,
- PDiag(diag::warn_comparison_always)
- << 0 // self-
- << 2 // "a constant"
- );
- }
-
// Check for comparisons of floating point operands using != and ==.
if (!IsRelational && LHSType->hasFloatingRepresentation()) {
assert (RHSType->hasFloatingRepresentation());
@@ -6857,6 +6842,14 @@
ExprResult &LHS, ExprResult &RHS, SourceLocation Loc, bool IsCompAssign) {
checkArithmeticNull(*this, LHS, RHS, Loc, /*isCompare=*/false);
+ ExprResult LHSResult = Owned(LHS), RHSResult = Owned(RHS);
+ QualType compType = UsualArithmeticConversions(LHSResult, RHSResult,
+ IsCompAssign);
+ if (LHSResult.isInvalid() || RHSResult.isInvalid())
+ return QualType();
+ LHS = LHSResult.take();
+ RHS = RHSResult.take();
+
if (LHS.get()->getType()->isVectorType() ||
RHS.get()->getType()->isVectorType()) {
if (LHS.get()->getType()->hasIntegerRepresentation() &&
@@ -6866,14 +6859,6 @@
return InvalidOperands(Loc, LHS, RHS);
}
- ExprResult LHSResult = Owned(LHS), RHSResult = Owned(RHS);
- QualType compType = UsualArithmeticConversions(LHSResult, RHSResult,
- IsCompAssign);
- if (LHSResult.isInvalid() || RHSResult.isInvalid())
- return QualType();
- LHS = LHSResult.take();
- RHS = RHSResult.take();
-
if (LHS.get()->getType()->isIntegralOrUnscopedEnumerationType() &&
RHS.get()->getType()->isIntegralOrUnscopedEnumerationType())
return compType;
Index: lib/Sema/Sema.cpp
===================================================================
--- lib/Sema/Sema.cpp (revision 143150)
+++ lib/Sema/Sema.cpp (working copy)
@@ -240,6 +240,20 @@
CastKind Kind, ExprValueKind VK,
const CXXCastPath *BasePath,
CheckedConversionKind CCK) {
+#ifndef NDEBUG
+ if (VK == VK_RValue && !E->isRValue()) {
+ switch (Kind) {
+ default:
+ assert(0 && "can't implicitly cast lvalue to rvalue with this cast kind");
+ case CK_LValueToRValue:
+ case CK_ArrayToPointerDecay:
+ case CK_FunctionToPointerDecay:
+ case CK_ToVoid:
+ break;
+ }
+ }
+#endif
+
QualType ExprTy = Context.getCanonicalType(E->getType());
QualType TypeTy = Context.getCanonicalType(Ty);
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
