[PATCH] D40112: [CodeGen][X86] Fix handling of __fp16 vectors

2017-12-08 Thread Akira Hatanaka via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL320215: [CodeGen][X86] Fix handling of __fp16 vectors. 
(authored by ahatanak).

Changed prior to commit:
  https://reviews.llvm.org/D40112?vs=123100=126229#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D40112

Files:
  cfe/trunk/include/clang/Basic/TargetInfo.h
  cfe/trunk/lib/Basic/Targets/AArch64.h
  cfe/trunk/lib/Basic/Targets/ARM.h
  cfe/trunk/lib/Basic/Targets/X86.h
  cfe/trunk/lib/CodeGen/CGExprConstant.cpp
  cfe/trunk/lib/CodeGen/CGExprScalar.cpp
  cfe/trunk/lib/CodeGen/CodeGenTypes.cpp
  cfe/trunk/lib/Sema/SemaExpr.cpp
  cfe/trunk/test/CodeGen/fp16-ops.c
  cfe/trunk/test/CodeGen/fp16vec-ops.c
  cfe/trunk/test/CodeGenCXX/float16-declarations.cpp
  cfe/trunk/test/CodeGenCXX/fp16-mangle.cpp

Index: cfe/trunk/include/clang/Basic/TargetInfo.h
===
--- cfe/trunk/include/clang/Basic/TargetInfo.h
+++ cfe/trunk/include/clang/Basic/TargetInfo.h
@@ -559,6 +559,14 @@
 return ComplexLongDoubleUsesFP2Ret;
   }
 
+  /// Check whether llvm intrinsics such as llvm.convert.to.fp16 should be used
+  /// to convert to and from __fp16.
+  /// FIXME: This function should be removed once all targets stop using the
+  /// conversion intrinsics.
+  virtual bool useFP16ConversionIntrinsics() const {
+return true;
+  }
+
   /// \brief Specify if mangling based on address space map should be used or
   /// not for language specific address spaces
   bool useAddressSpaceMapMangling() const {
Index: cfe/trunk/test/CodeGen/fp16-ops.c
===
--- cfe/trunk/test/CodeGen/fp16-ops.c
+++ cfe/trunk/test/CodeGen/fp16-ops.c
@@ -1,8 +1,9 @@
 // REQUIRES: arm-registered-target
-// RUN: %clang_cc1 -emit-llvm -o - -triple arm-none-linux-gnueabi %s | FileCheck %s --check-prefix=NOHALF --check-prefix=CHECK
-// RUN: %clang_cc1 -emit-llvm -o - -triple aarch64-none-linux-gnueabi %s | FileCheck %s --check-prefix=NOHALF --check-prefix=CHECK
-// RUN: %clang_cc1 -emit-llvm -o - -triple arm-none-linux-gnueabi -fallow-half-arguments-and-returns %s | FileCheck %s --check-prefix=HALF --check-prefix=CHECK
-// RUN: %clang_cc1 -emit-llvm -o - -triple aarch64-none-linux-gnueabi -fallow-half-arguments-and-returns %s | FileCheck %s --check-prefix=HALF --check-prefix=CHECK
+// RUN: %clang_cc1 -emit-llvm -o - -triple arm-none-linux-gnueabi %s | FileCheck %s --check-prefix=NOTNATIVE --check-prefix=CHECK
+// RUN: %clang_cc1 -emit-llvm -o - -triple aarch64-none-linux-gnueabi %s | FileCheck %s --check-prefix=NOTNATIVE --check-prefix=CHECK
+// RUN: %clang_cc1 -emit-llvm -o - -triple x86_64-linux-gnu %s | FileCheck %s --check-prefix=NOTNATIVE --check-prefix=CHECK
+// RUN: %clang_cc1 -emit-llvm -o - -triple arm-none-linux-gnueabi -fallow-half-arguments-and-returns %s | FileCheck %s --check-prefix=NOTNATIVE --check-prefix=CHECK
+// RUN: %clang_cc1 -emit-llvm -o - -triple aarch64-none-linux-gnueabi -fallow-half-arguments-and-returns %s | FileCheck %s --check-prefix=NOTNATIVE --check-prefix=CHECK
 // RUN: %clang_cc1 -emit-llvm -o - -triple arm-none-linux-gnueabi -fnative-half-type %s \
 // RUN:   | FileCheck %s --check-prefix=NATIVE-HALF
 // RUN: %clang_cc1 -emit-llvm -o - -triple aarch64-none-linux-gnueabi -fnative-half-type %s \
@@ -16,30 +17,28 @@
 volatile __fp16 h0 = 0.0, h1 = 1.0, h2;
 volatile float f0, f1, f2;
 volatile double d0;
+short s0;
 
 void foo(void) {
   // CHECK-LABEL: define void @foo()
 
   // Check unary ops
 
-  // NOHALF: [[F16TOF32:call float @llvm.convert.from.fp16.f32]]
-  // HALF: [[F16TOF32:fpext half]]
+  // NOTNATIVE: [[F16TOF32:fpext half]]
   // CHECK: fptoui float
   // NATIVE-HALF: fptoui half
   test = (h0);
   // CHECK: uitofp i32
-  // NOHALF: [[F32TOF16:call i16 @llvm.convert.to.fp16.f32]]
-  // HALF: [[F32TOF16:fptrunc float]]
+  // NOTNATIVE: [[F32TOF16:fptrunc float]]
   // NATIVE-HALF: uitofp i32 {{.*}} to half
   h0 = (test);
   // CHECK: [[F16TOF32]]
   // CHECK: fcmp une float
   // NATIVE-HALF: fcmp une half
   test = (!h1);
   // CHECK: [[F16TOF32]]
   // CHECK: fsub float
-  // NOHALF: [[F32TOF16]]
-  // HALF: [[F32TOF16]]
+  // NOTNATIVE: [[F32TOF16]]
   // NATIVE-HALF: fsub half
   h1 = -h1;
   // CHECK: [[F16TOF32]]
@@ -76,8 +75,6 @@
   // NATIVE-HALF: fmul half
   h1 = h0 * h2;
   // CHECK: [[F16TOF32]]
-  // NOHALF: [[F32TOF16]]
-  // NOHALF: [[F16TOF32]]
   // CHECK: fmul float
   // CHECK: [[F32TOF16]]
   // NATIVE-HALF: fmul half
@@ -107,7 +104,6 @@
   // NATIVE-HALF: fdiv half
   h1 = (h0 / h2);
   // CHECK: [[F16TOF32]]
-  // NOHALF: [[F16TOF32]]
   // CHECK: fdiv float
   // CHECK: [[F32TOF16]]
   // NATIVE-HALF: fdiv half
@@ -137,7 +133,6 @@
   // NATIVE-HALF: fadd half
   h1 = (h2 + h0);
   // CHECK: [[F16TOF32]]
-  // NOHALF: [[F16TOF32]]
   // CHECK: fadd float
   // CHECK: [[F32TOF16]]
   // NATIVE-HALF: fadd half
@@ -167,7 +162,6 @@
   // NATIVE-HALF: fsub half
   h1 = (h2 - 

[PATCH] D40112: [CodeGen][X86] Fix handling of __fp16 vectors

2017-12-08 Thread Akira Hatanaka via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC320215: [CodeGen][X86] Fix handling of __fp16 vectors. 
(authored by ahatanak).

Changed prior to commit:
  https://reviews.llvm.org/D40112?vs=123100=126228#toc

Repository:
  rC Clang

https://reviews.llvm.org/D40112

Files:
  include/clang/Basic/TargetInfo.h
  lib/Basic/Targets/AArch64.h
  lib/Basic/Targets/ARM.h
  lib/Basic/Targets/X86.h
  lib/CodeGen/CGExprConstant.cpp
  lib/CodeGen/CGExprScalar.cpp
  lib/CodeGen/CodeGenTypes.cpp
  lib/Sema/SemaExpr.cpp
  test/CodeGen/fp16-ops.c
  test/CodeGen/fp16vec-ops.c
  test/CodeGenCXX/float16-declarations.cpp
  test/CodeGenCXX/fp16-mangle.cpp

Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -11565,7 +11565,8 @@
 static bool needsConversionOfHalfVec(bool OpRequiresConversion, ASTContext ,
  QualType SrcType) {
   return OpRequiresConversion && !Ctx.getLangOpts().NativeHalfType &&
- Ctx.getLangOpts().HalfArgsAndReturns && isVector(SrcType, Ctx.HalfTy);
+ !Ctx.getTargetInfo().useFP16ConversionIntrinsics() &&
+ isVector(SrcType, Ctx.HalfTy);
 }
 
 /// CreateBuiltinBinOp - Creates a new built-in binary operation with
Index: lib/Basic/Targets/AArch64.h
===
--- lib/Basic/Targets/AArch64.h
+++ lib/Basic/Targets/AArch64.h
@@ -48,6 +48,10 @@
   bool isValidCPUName(StringRef Name) const override;
   bool setCPU(const std::string ) override;
 
+  bool useFP16ConversionIntrinsics() const override {
+return false;
+  }
+
   void getTargetDefinesARMV81A(const LangOptions ,
MacroBuilder ) const;
   void getTargetDefinesARMV82A(const LangOptions ,
Index: lib/Basic/Targets/ARM.h
===
--- lib/Basic/Targets/ARM.h
+++ lib/Basic/Targets/ARM.h
@@ -126,6 +126,10 @@
 
   bool setFPMath(StringRef Name) override;
 
+  bool useFP16ConversionIntrinsics() const override {
+return false;
+  }
+
   void getTargetDefinesARMV81A(const LangOptions ,
MacroBuilder ) const;
 
Index: lib/Basic/Targets/X86.h
===
--- lib/Basic/Targets/X86.h
+++ lib/Basic/Targets/X86.h
@@ -193,6 +193,10 @@
 return "";
   }
 
+  bool useFP16ConversionIntrinsics() const override {
+return false;
+  }
+
   void getTargetDefines(const LangOptions ,
 MacroBuilder ) const override;
   
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -1825,7 +1825,7 @@
 const llvm::APFloat  = Value.getFloat();
 if (() == ::APFloat::IEEEhalf() &&
 !CGM.getContext().getLangOpts().NativeHalfType &&
-!CGM.getContext().getLangOpts().HalfArgsAndReturns)
+CGM.getContext().getTargetInfo().useFP16ConversionIntrinsics())
   return llvm::ConstantInt::get(CGM.getLLVMContext(),
 Init.bitcastToAPInt());
 else
Index: lib/CodeGen/CGExprScalar.cpp
===
--- lib/CodeGen/CGExprScalar.cpp
+++ lib/CodeGen/CGExprScalar.cpp
@@ -951,15 +951,15 @@
   if (SrcType->isHalfType() && !CGF.getContext().getLangOpts().NativeHalfType) {
 // Cast to FP using the intrinsic if the half type itself isn't supported.
 if (DstTy->isFloatingPointTy()) {
-  if (!CGF.getContext().getLangOpts().HalfArgsAndReturns)
+  if (CGF.getContext().getTargetInfo().useFP16ConversionIntrinsics())
 return Builder.CreateCall(
 CGF.CGM.getIntrinsic(llvm::Intrinsic::convert_from_fp16, DstTy),
 Src);
 } else {
   // Cast to other types through float, using either the intrinsic or FPExt,
   // depending on whether the half type itself is supported
   // (as opposed to operations on half, available with NativeHalfType).
-  if (!CGF.getContext().getLangOpts().HalfArgsAndReturns) {
+  if (CGF.getContext().getTargetInfo().useFP16ConversionIntrinsics()) {
 Src = Builder.CreateCall(
 CGF.CGM.getIntrinsic(llvm::Intrinsic::convert_from_fp16,
  CGF.CGM.FloatTy),
@@ -1068,7 +1068,7 @@
 if (SrcTy->isFloatingPointTy()) {
   // Use the intrinsic if the half type itself isn't supported
   // (as opposed to operations on half, available with NativeHalfType).
-  if (!CGF.getContext().getLangOpts().HalfArgsAndReturns)
+  if (CGF.getContext().getTargetInfo().useFP16ConversionIntrinsics())
 return Builder.CreateCall(
 CGF.CGM.getIntrinsic(llvm::Intrinsic::convert_to_fp16, SrcTy), Src);
   // If the half type is supported, just use an fptrunc.
@@ -1104,7 

[PATCH] D40112: [CodeGen][X86] Fix handling of __fp16 vectors

2017-12-04 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno accepted this revision.
bruno added a comment.
This revision is now accepted and ready to land.

LGTM with the minor comment below.




Comment at: include/clang/Basic/TargetInfo.h:563
+  /// Check whether llvm intrinsics such as llvm.convert.to.fp16 should be used
+  /// to convert to and from __fp16. This function should be removed once all
+  /// targets stop using the conversion intrinsics.

Add a "FIXME"


https://reviews.llvm.org/D40112



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D40112: [CodeGen][X86] Fix handling of __fp16 vectors

2017-12-04 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment.

Any other comments from anyone?


https://reviews.llvm.org/D40112



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D40112: [CodeGen][X86] Fix handling of __fp16 vectors

2017-11-27 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno added inline comments.



Comment at: lib/CodeGen/CGExprScalar.cpp:954
 if (DstTy->isFloatingPointTy()) {
-  if (!CGF.getContext().getLangOpts().HalfArgsAndReturns)
+  if (CGF.getContext().getTargetInfo().useFP16ConversionIntrinsics())
 return Builder.CreateCall(

This (and in the other places in the patch) means that regardless of 
`HalfArgsAndReturns` state we want to generate an intrinsic call if 
`useFP16ConversionIntrinsics()` is true, is that always the intended behavior?  


https://reviews.llvm.org/D40112



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D40112: [CodeGen][X86] Fix handling of __fp16 vectors

2017-11-15 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak created this revision.
Herald added a subscriber: javed.absar.

IRGen for __fp16 vectors on X86 is currently completely broken. For example 
when the following code is compiled:

  half4 hv0, hv1, hv2; // these are vectors of __fp16.
  
  void foo221() {
hv0 = hv1 + hv2;
  }

clang generates the following IR, in which two i16 values are added:

  @hv1 = common global <4 x i16> zeroinitializer, align 8
  @hv2 = common global <4 x i16> zeroinitializer, align 8
  @hv0 = common global <4 x i16> zeroinitializer, align 8
  
  define void @foo221() {
  entry:
%0 = load <4 x i16>, <4 x i16>* @hv1, align 8
%1 = load <4 x i16>, <4 x i16>* @hv2, align 8
%add = add <4 x i16> %0, %1
store <4 x i16> %add, <4 x i16>* @hv0, align 8
ret void
  }

To fix IRGen for __fp16 vectors, this patch uses the code committed in r314056, 
which modified clang to promote and truncate __fp16 vectors to and from float 
vectors in the AST. Also, as the first step toward doing away with the __fp16 
conversion intrinsics such as @llvm.convert.to.fp16 (see 
http://lists.llvm.org/pipermail/llvm-dev/2014-July/074689.html),  I made 
changes to IRGen for __fp16 scalars so that fpext/fptrunc instructions are 
emitted instead of the __fp16 conversion intrinsics IRGen currently emits. This 
fixes another IRGen bug where a short value is assigned to an __fp16 variable 
without any integer-to-floating-point conversion, as shown in the following 
example:

C code
--

  __fp16 a;
  short b;
  
  void foo1() {
a = b;
  }



generated IR


  @b = common global i16 0, align 2
  @a = common global i16 0, align 2
  
  define void @foo1() #0 {
  entry:
%0 = load i16, i16* @b, align 2
store i16 %0, i16* @a, align 2
ret void
  }

I haven't spent too much time inspecting the code the X86 backend emits, but 
the code I've seen so far seems at least functionally correct (although it 
doesn't look very efficient since the backend scalarizes __fp16 vectors).


https://reviews.llvm.org/D40112

Files:
  include/clang/Basic/TargetInfo.h
  lib/Basic/Targets/AArch64.h
  lib/Basic/Targets/ARM.h
  lib/Basic/Targets/X86.h
  lib/CodeGen/CGExprConstant.cpp
  lib/CodeGen/CGExprScalar.cpp
  lib/CodeGen/CodeGenTypes.cpp
  lib/Sema/SemaExpr.cpp
  test/CodeGen/fp16-ops.c
  test/CodeGen/fp16vec-ops.c
  test/CodeGenCXX/float16-declarations.cpp
  test/CodeGenCXX/fp16-mangle.cpp

Index: test/CodeGenCXX/fp16-mangle.cpp
===
--- test/CodeGenCXX/fp16-mangle.cpp
+++ test/CodeGenCXX/fp16-mangle.cpp
@@ -4,9 +4,9 @@
 template  struct S { static int i; };
 template <> int S<__fp16, __fp16>::i = 3;
 
-// CHECK-LABEL: define void @_Z1fPDh(i16* %x)
+// CHECK-LABEL: define void @_Z1fPDh(half* %x)
 void f (__fp16 *x) { }
 
-// CHECK-LABEL: define void @_Z1gPDhS_(i16* %x, i16* %y)
+// CHECK-LABEL: define void @_Z1gPDhS_(half* %x, half* %y)
 void g (__fp16 *x, __fp16 *y) { }
 
Index: test/CodeGenCXX/float16-declarations.cpp
===
--- test/CodeGenCXX/float16-declarations.cpp
+++ test/CodeGenCXX/float16-declarations.cpp
@@ -11,16 +11,14 @@
 // CHECK-DAG: @_ZN12_GLOBAL__N_13f1nE = internal global half 0xH, align 2
 
   _Float16 f2n = 33.f16;
-// CHECK-AARCH64-DAG: @_ZN12_GLOBAL__N_13f2nE = internal global half 0xH5020, align 2
-// CHECK-X86-DAG: @_ZN12_GLOBAL__N_13f2nE = internal global i16 20512, align 2
+// CHECK-DAG: @_ZN12_GLOBAL__N_13f2nE = internal global half 0xH5020, align 2
 
   _Float16 arr1n[10];
 // CHECK-AARCH64-DAG: @_ZN12_GLOBAL__N_15arr1nE = internal global [10 x half] zeroinitializer, align 2
 // CHECK-X86-DAG: @_ZN12_GLOBAL__N_15arr1nE = internal global [10 x half] zeroinitializer, align 16
 
   _Float16 arr2n[] = { 1.2, 3.0, 3.e4 };
-// CHECK-AARCH64-DAG: @_ZN12_GLOBAL__N_15arr2nE = internal global [3 x half] [half 0xH3CCD, half 0xH4200, half 0xH7753], align 2
-// CHECK-X86-DAG: @_ZN12_GLOBAL__N_15arr2nE = internal global [3 x i16] [i16 15565, i16 16896, i16 30547], align 2
+// CHECK-DAG: @_ZN12_GLOBAL__N_15arr2nE = internal global [3 x half] [half 0xH3CCD, half 0xH4200, half 0xH7753], align 2
 
   const volatile _Float16 func1n(const _Float16 ) {
 return arg + f2n + arr1n[4] - arr2n[1];
@@ -35,16 +33,14 @@
 // CHECK-X86-DAG: @f1f = global half 0xH, align 2
 
 _Float16 f2f = 32.4;
-// CHECK-AARCH64-DAG: @f2f = global half 0xH500D, align 2
-// CHECK-X86-DAG: @f2f = global i16 20493, align 2
+// CHECK-DAG: @f2f = global half 0xH500D, align 2
 
 _Float16 arr1f[10];
 // CHECK-AARCH64-DAG: @arr1f = global [10 x half] zeroinitializer, align 2
 // CHECK-X86-DAG: @arr1f = global [10 x half] zeroinitializer, align 16
 
 _Float16 arr2f[] = { -1.2, -3.0, -3.e4 };
-// CHECK-AARCH64-DAG: @arr2f = global [3 x half] [half 0xHBCCD, half 0xHC200, half 0xHF753], align 2
-// CHECK-X86-DAG: @arr2f = global [3 x i16] [i16 -17203, i16 -15872, i16 -2221], align 2
+// CHECK-DAG: @arr2f = global