sdardis added a comment.
Thanks for the review.
Repository:
rL LLVM
https://reviews.llvm.org/D25866
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL302935: [Sema] Support implicit scalar to vector conversions
(authored by sdardis).
Changed prior to commit:
https://reviews.llvm.org/D25866?vs=98783=98815#toc
Repository:
rL LLVM
sdardis updated this revision to Diff 98783.
sdardis marked an inline comment as done.
sdardis edited the summary of this revision.
sdardis added a comment.
Updated error message for logical operations where one operand is a vector and
the other isn't.
Updated summary.
I'll commit this
bruno accepted this revision.
bruno added a comment.
This revision is now accepted and ready to land.
Thanks for your patience! LGTM with a minor comment below.
Comment at: test/Sema/vector-gcc-compat.c:101
+
+ v2i64_r = v2i64_a && 1; // expected-error {{invalid vector
sdardis updated this revision to Diff 96533.
sdardis added a comment.
Rebase + ping.
https://reviews.llvm.org/D25866
Files:
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/Sema.h
lib/Sema/SemaExpr.cpp
lib/Sema/SemaExprCXX.cpp
test/Sema/vector-cast.c
sdardis added a comment.
Realised I've some comments to submit.
Comment at: lib/Sema/SemaExpr.cpp:8032
+
+ return InvalidOperands(Loc, LHS, RHS);
+}
bruno wrote:
> Double checking here: are there tests for the `InvalidOperands` case above?
Yes, this case is
sdardis updated this revision to Diff 95109.
sdardis marked 2 inline comments as done.
sdardis added a comment.
Addressed review comments. I've highlighted the relevant places with FIXME:s
where we don't support GCC vector operations.
https://reviews.llvm.org/D25866
Files:
bruno added inline comments.
Comment at: lib/Sema/SemaExpr.cpp:10024
+ // FIXME: The check for C++ here is for GCC compatibility. GCC rejects the
+ //usage of logical operators with vectors in C. This check could be
+ //notionally dropped.
sdardis updated this revision to Diff 94434.
sdardis marked 7 inline comments as done.
sdardis added a comment.
Addressed review comments.
I've changed InvalidVectorOperands() to not use InvalidOperands() after
updating some tests. InvalidOperands() was receiving expressions with implicit
ahatanak added inline comments.
Comment at: lib/Sema/SemaExpr.cpp:8024
+ bool RHSNatVec = RHS.get()->IgnoreImpCasts()->getType()->isVectorType();
+
+ if (LHSNatVec ^ RHSNatVec) {
I think "!=" is easier to understand than "^" here.
Comment
bruno added inline comments.
Comment at: lib/Sema/SemaExpr.cpp:8188
+ //type and then perform the rest of the checks here. GCC as of
+ //pre-release 7.0 does not accept this though.
+ if (VectorEltTy->isIntegralType(S.Context) &&
sdardis
sdardis marked an inline comment as done.
sdardis added a comment.
Thanks for sticking with this.
I've held off updating the diff until https://reviews.llvm.org/D31667 (not
https://reviews.llvm.org/D31337 as previously posted) is finished. Responses
inlined.
Comment at:
bruno added inline comments.
Comment at: lib/Sema/SemaExpr.cpp:8032
+
+ return InvalidOperands(Loc, LHS, RHS);
+}
Double checking here: are there tests for the `InvalidOperands` case above?
Comment at: lib/Sema/SemaExpr.cpp:8163
+/// type
sdardis updated this revision to Diff 94070.
sdardis added a comment.
Factored out the changes from https://reviews.llvm.org/D31337.
https://reviews.llvm.org/D25866
Files:
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/Sema.h
lib/Sema/SemaExpr.cpp
test/Sema/vector-cast.c
bruno added inline comments.
Comment at: test/Sema/vector-gcc-compat.c:61
+ //match.
+ v2i64_r = v2i64_a == 1; // expected-warning {{incompatible vector types
assigning to 'v2i64' (vector of 2 'long long' values) from 'long
__attribute__((ext_vector_type(2)))'
sdardis updated this revision to Diff 91716.
sdardis added a comment.
Addressed review comments, add C++ specific test.
https://reviews.llvm.org/D25866
Files:
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/Sema.h
lib/Sema/SemaExpr.cpp
test/Sema/vector-cast.c
sdardis marked 3 inline comments as done.
sdardis added a comment.
Thanks for getting back to this. I've traced the appearance of the ext_vector
type to a piece of code that only produces ext-vector types for comparisons.
I'm presuming that's wrong when clang is producing vectors implicitly in
bruno added a comment.
Hey, really sorry for the delay here.
Comment at: lib/Sema/SemaExpr.cpp:8007
+static bool canConvertIntToOtherIntTy(Sema , ExprResult *Int,
+ QualType OtherIntTy) {
+ QualType IntTy =
sdardis added a comment.
Ping.
https://reviews.llvm.org/D25866
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
sdardis added a comment.
Ping.
https://reviews.llvm.org/D25866
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
sdardis added inline comments.
Comment at: lib/Sema/SemaExpr.cpp:8051
+ if (!LHSVecType) {
+assert(RHSVecType && "RHSVecType is not a vector!");
if (!tryVectorConvertAndSplat(*this, (IsCompAssign ? nullptr : ),
bruno wrote:
> `tryVectorConvertAndSplat`
sdardis updated this revision to Diff 81755.
sdardis marked an inline comment as done.
sdardis added a comment.
Addressed review comments, fixed assertion issue with expressions like scalar
-= vector.
https://reviews.llvm.org/D25866
Files:
include/clang/Basic/DiagnosticSemaKinds.td
bruno added inline comments.
Comment at: lib/Sema/SemaExpr.cpp:8064
+ ScalarCast = CK_FloatingCast;
+} else if (ScalarTy->isIntegralType(S.Context)) {
+ // Determine if the integer constant can be expressed as a floating point
sdardis wrote:
>
sdardis updated this revision to Diff 78531.
sdardis marked 4 inline comments as done.
sdardis added a comment.
Addressed review comments.
https://reviews.llvm.org/D25866
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaExpr.cpp
test/Sema/vector-cast.c
sdardis added inline comments.
Comment at: lib/Sema/SemaExpr.cpp:8064
+ ScalarCast = CK_FloatingCast;
+} else if (ScalarTy->isIntegralType(S.Context)) {
+ // Determine if the integer constant can be expressed as a floating point
bruno wrote:
> I
bruno added inline comments.
Comment at: lib/Sema/SemaExpr.cpp:7978
+/// without causing truncation of Scalar.
+
+static bool tryGCCVectorConvertAndSpalt(Sema , ExprResult *Scalar,
Remove this empty line.
Comment at: lib/Sema/SemaExpr.cpp:7991
sdardis added a comment.
Ping.
https://reviews.llvm.org/D25866
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
sdardis updated the summary for this revision.
sdardis updated this revision to Diff 76385.
sdardis added a comment.
Split out a variant of tryVectorConvertAndSplat called
tryGCCVectorConvertAndSplat. This variant checks the types more strictly than
tryVectorConvertAndSplat and handles implicit
bruno added a comment.
Hi,
Nice, thanks for working on this!
Comment at: lib/Sema/SemaExpr.cpp:8051
+ if (!LHSVecType) {
+assert(RHSVecType && "RHSVecType is not a vector!");
if (!tryVectorConvertAndSplat(*this, (IsCompAssign ? nullptr : ),
sdardis updated this revision to Diff 75708.
sdardis marked an inline comment as done.
sdardis added a comment.
Extra testing for cases where the operand on the left of an operation is a
vector.
Removed two spurious checks for vector types.
https://reviews.llvm.org/D25866
Files:
ahatanak added inline comments.
Comment at: lib/Sema/SemaExpr.cpp:8044
// the vector element type and splat.
- // FIXME: this should also work for regular vector types as supported in GCC.
- if (!RHSVecType && isa(LHSVecType)) {
+ if (!RHSVecType && isa(LHSVecType)) {
ahatanak added inline comments.
Comment at: lib/Sema/SemaExpr.cpp:8044
// the vector element type and splat.
- // FIXME: this should also work for regular vector types as supported in GCC.
- if (!RHSVecType && isa(LHSVecType)) {
+ if (!RHSVecType && isa(LHSVecType)) {
sdardis created this revision.
sdardis added subscribers: rnk, bruno, ahatanak, cfe-commits.
Herald added a reviewer: vkalintiris.
This patch teaches clang to perform implicit scalar to vector conversions
when one of the operands to a binary vector expression is a scalar like GCC.
The scalar is
33 matches
Mail list logo
