[clang] 86bba6c - [Sema] Use the canonical type in function isVector

[Akira Hatanaka via cfe-commits]

Author: Akira Hatanaka
Date: 2020-03-13T13:08:48-07:00
New Revision: 86bba6c6410c31e669b10f182c6d1a03f704555a

URL: https://github.com/llvm/llvm-project/commit/86bba6c6410c31e669b10f182c6d1a03f704555a
DIFF: https://github.com/llvm/llvm-project/commit/86bba6c6410c31e669b10f182c6d1a03f704555a.diff

LOG: [Sema] Use the canonical type in function isVector

This reapplies the following patch, which was reverted because it caused
neon CodeGen tests to fail:

https://reviews.llvm.org/rGa6150b48cea00ab31e9335cc73770327acc4cb3a

I've added checks to detect half precision neon vectors and avoid
promiting them to vectors of floats.

See the discussion here: https://reviews.llvm.org/rG825235c140e7

Original commit message:

This fixes an assertion in Sema::CreateBuiltinBinOp that fails when one
of the vector operand's element type is a typedef of __fp16.

rdar://problem/55983556

Added: 
    

Modified: 
    clang/lib/Sema/SemaExpr.cpp
    clang/test/CodeGen/fp16-ops.c

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 9ab6a16154d5..b9fe53f18891 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -8272,7 +8272,7 @@ Sema::CheckAssignmentConstraints(SourceLocation Loc,
 /// type ElementType.
 static bool isVector(QualType QT, QualType ElementType) {
   if (const VectorType *VT = QT->getAs<VectorType>())
-    return VT->getElementType() == ElementType;
+    return VT->getElementType().getCanonicalType() == ElementType;
   return false;
 }
 
@@ -12920,10 +12920,27 @@ CorrectDelayedTyposInBinOp(Sema &S, BinaryOperatorKind Opc, Expr *LHSExpr,
 /// Returns true if conversion between vectors of halfs and vectors of floats
 /// is needed.
 static bool needsConversionOfHalfVec(bool OpRequiresConversion, ASTContext &Ctx,
-                                     QualType SrcType) {
-  return OpRequiresConversion && !Ctx.getLangOpts().NativeHalfType &&
-         !Ctx.getTargetInfo().useFP16ConversionIntrinsics() &&
-         isVector(SrcType, Ctx.HalfTy);
+                                     Expr *E0, Expr *E1 = nullptr) {
+  if (!OpRequiresConversion || Ctx.getLangOpts().NativeHalfType ||
+      Ctx.getTargetInfo().useFP16ConversionIntrinsics())
+    return false;
+
+  auto HasVectorOfHalfType = [&Ctx](Expr *E) {
+    QualType Ty = E->IgnoreImplicit()->getType();
+
+    // Don't promote half precision neon vectors like float16x4_t in arm_neon.h
+    // to vectors of floats. Although the element type of the vectors is __fp16,
+    // the vectors shouldn't be treated as storage-only types. See the
+    // discussion here: https://reviews.llvm.org/rG825235c140e7
+    if (const VectorType *VT = Ty->getAs<VectorType>()) {
+      if (VT->getVectorKind() == VectorType::NeonVector)
+        return false;
+      return VT->getElementType().getCanonicalType() == Ctx.HalfTy;
+    }
+    return false;
+  };
+
+  return HasVectorOfHalfType(E0) && (!E1 || HasVectorOfHalfType(E1));
 }
 
 /// CreateBuiltinBinOp - Creates a new built-in binary operation with
@@ -13158,8 +13175,8 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc,
   assert(isVector(RHS.get()->getType(), Context.HalfTy) ==
          isVector(LHS.get()->getType(), Context.HalfTy) &&
          "both sides are half vectors or neither sides are");
-  ConvertHalfVec = needsConversionOfHalfVec(ConvertHalfVec, Context,
-                                            LHS.get()->getType());
+  ConvertHalfVec =
+      needsConversionOfHalfVec(ConvertHalfVec, Context, LHS.get(), RHS.get());
 
   // Check for array bounds violations for both sides of the BinaryOperator
   CheckArrayAccess(LHS.get());
@@ -13651,8 +13668,7 @@ ExprResult Sema::CreateBuiltinUnaryOp(SourceLocation OpLoc,
     // float vector and truncating the result back to a half vector. For now, we
     // do this only when HalfArgsAndReturns is set (that is, when the target is
     // arm or arm64).
-    ConvertHalfVec =
-        needsConversionOfHalfVec(true, Context, Input.get()->getType());
+    ConvertHalfVec = needsConversionOfHalfVec(true, Context, Input.get());
 
     // If the operand is a half vector, promote it to a float vector.
     if (ConvertHalfVec)

diff  --git a/clang/test/CodeGen/fp16-ops.c b/clang/test/CodeGen/fp16-ops.c
index f74552b85921..6401dd125d2a 100644
--- a/clang/test/CodeGen/fp16-ops.c
+++ b/clang/test/CodeGen/fp16-ops.c
@@ -11,6 +11,7 @@
 // RUN: %clang_cc1 -emit-llvm -o - -x renderscript %s \
 // RUN:   | FileCheck %s --check-prefix=NATIVE-HALF
 typedef unsigned cond_t;
+typedef __fp16 float16_t;
 
 volatile cond_t test;
 volatile int i0;
@@ -541,3 +542,15 @@ void foo(void) {
   // NOTNATIVE: store volatile half [[TRUNC]], half* @h0
   h0 = s0;
 }
+
+// CHECK-LABEL: define void @testTypeDef(
+// CHECK: %[[CONV:.*]] = fpext <4 x half> %{{.*}} to <4 x float>
+// CHECK: %[[CONV1:.*]] = fpext <4 x half> %{{.*}} to <4 x float>
+// CHECK: %[[ADD:.*]] = fadd <4 x float> %[[CONV]], %[[CONV1]]
+// CHECK: fptrunc <4 x float> %[[ADD]] to <4 x half>
+
+void testTypeDef() {
+  __fp16 t0 __attribute__((vector_size(8)));
+  float16_t t1 __attribute__((vector_size(8)));
+  t1 = t0 + t1;
+}