[Sema] Support Comma operator for fp16 vectors.

[arames]

Summary

The current half vector was enforcing an assert expecting
"(LHS is half vector) == (RHS is half vector)"
for comma.

[fhahn]

clang/lib/Sema/SemaExpr.cpp

13936–13939

I think you need braces around the expression before && "...", otherwise some compilers may complain.

clang/test/Sema/fp16vec-sema.c

29

nit: it might be slightly better to move it 2 lines further down, so all operators that assign the result are grouped together. Also, should we add 1, hv0 as well?

[arames]

clang/test/Sema/fp16vec-sema.c

29

Moved.
I added 1, hv0 for completeness. It would hit the same path as the reverse.

[arames]

Addressed review comments.

[ahatanak]

clang/test/Sema/fp16vec-sema.c

33

Since these diagnostics aren't relevant to the test, can you silence them by passing -Wno-unused-value or assigning the expressions to variables?

[arames]

Address review comments.

[arames]

I do not have commit rights, so it would be great if you can land it.

Thanks.

[fhahn]

Could you adjust the commit message to be a bit more descriptive, e.g something like [Sema] Support Comma operator for fp16 vectors.

[arames]

Update commit message.

[arames]

In D88265#2302653, @fhahn wrote:

Could you adjust the commit message to be a bit more descriptive, e.g something like [Sema] Support Comma operator for fp16 vectors.

Done!

[fhahn]

In D88265#2303841, @arames wrote:

In D88265#2302653, @fhahn wrote:

Could you adjust the commit message to be a bit more descriptive, e.g something like [Sema] Support Comma operator for fp16 vectors.

Done!

Oh I think you'd need to edit the revision on Phabricator (top right on this page, edit revision). But if you are fine with it I can commit the patch for you with the adjusted commit title.

[arames]

Oh I think you'd need to edit the revision on Phabricator (top right on this page, edit revision). But if you are fine with it I can commit the patch for you with the adjusted commit title.

Done !
Thanks for your help !

[fhahn]

LGTM, thanks. I'll commit this in a minute.

clang/lib/Sema/SemaExpr.cpp

ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc,
   }
   if (ResultTy.isNull() || LHS.isInvalid() || RHS.isInvalid())
     return ExprError();
 
   // Some of the binary operations require promoting operands of half vector to
   // float vectors and truncating the result back to half vector. For now, we do
   // this only when HalfArgsAndReturn is set (that is, when the target is arm or
   // arm64).
-  assert(isVector(RHS.get()->getType(), Context.HalfTy) ==
-         isVector(LHS.get()->getType(), Context.HalfTy) &&
-         "both sides are half vectors or neither sides are");
+  assert(
+      (Opc == BO_Comma || isVector(RHS.get()->getType(), Context.HalfTy) ==
+                              isVector(LHS.get()->getType(), Context.HalfTy)) &&
   ConvertHalfVec =
       needsConversionOfHalfVec(ConvertHalfVec, Context, LHS.get(), RHS.get());
 
   // Check for array bounds violations for both sides of the BinaryOperator
   CheckArrayAccess(LHS.get());
   CheckArrayAccess(RHS.get());
 
   if (const ObjCIsaExpr *OISA = dyn_cast<ObjCIsaExpr>(LHS.get()->IgnoreParenCasts())) {

clang/test/Sema/fp16vec-sema.c

-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-unused-value -verify %s
 
 typedef __fp16 half4 __attribute__ ((vector_size (8)));
 typedef float float4 __attribute__ ((vector_size (16)));
 typedef short short4 __attribute__ ((vector_size (8)));
 typedef int int4 __attribute__ ((vector_size (16)));
 
 half4 hv0, hv1;
 float4 fv0, fv1;
void testFP16Vec(int c) {
   hv0 *= hv1;
   hv0 /= hv1;
   sv0 = hv0 == hv1;
   sv0 = hv0 != hv1;
   sv0 = hv0 < hv1;
   sv0 = hv0 > hv1;
   sv0 = hv0 <= hv1;
   sv0 = hv0 >= hv1;
   sv0 = hv0 || hv1; // expected-error{{logical expression with vector types 'half4' (vector of 4 '__fp16' values) and 'half4' is only supported in C++}}
   sv0 = hv0 && hv1; // expected-error{{logical expression with vector types 'half4' (vector of 4 '__fp16' values) and 'half4' is only supported in C++}}
+  hv0, 1;
+  1, hv0;
 
   // Implicit conversion between half vectors and float vectors are not allowed.
   hv0 = fv0; // expected-error{{assigning to}}
   fv0 = hv0; // expected-error{{assigning to}}
   hv0 = (half4)fv0; // expected-error{{invalid conversion between}}
   fv0 = (float4)hv0; // expected-error{{invalid conversion between}}
   hv0 = fv0 + fv1; // expected-error{{assigning to}}
   fv0 = hv0 + hv1; // expected-error{{assigning to}}
   hv0 = hv0 + fv1; // expected-error{{cannot convert between vector}}