[PATCH] Add flag to enable native half type

[Ahmed Bougacha]

LGTM, see inline nits.  I'll trust you know what you're doing in making this a cc1 option.

-Ahmed


================
Comment at: include/clang/Driver/CC1Options.td:550-553
@@ -549,4 +549,6 @@
   HelpText<"Control emission of RTTI data">;
+def fnative_half_type: Flag<["-"], "fnative-half-type">,
+  HelpText<"Allow half types to appear natively in IR">;
 def fallow_half_arguments_and_returns : Flag<["-"], "fallow-half-arguments-and-returns">,
   HelpText<"Allow function arguments and returns of type half">;
 
----------------
I don't like "half": it should really be about "__fp16".  But that's also a problem for -fallow-half-*, so I'm fine with it.

We can have a more explicit description though.  How about "Use the native half type for __fp16 instead of promoting to float" ?

================
Comment at: lib/Frontend/CompilerInvocation.cpp:1588-1589
@@ -1587,3 +1587,4 @@
   Opts.ModuleFeatures = Args.getAllArgValues(OPT_fmodule_feature);
-  Opts.NativeHalfType = Opts.NativeHalfType;
+  if (!Opts.NativeHalfType && Args.hasArg(OPT_fnative_half_type))
+    Opts.NativeHalfType = Args.hasArg(OPT_fnative_half_type);
   Opts.HalfArgsAndReturns = Args.hasArg(OPT_fallow_half_arguments_and_returns);
----------------
How about:

    Ops.NativeHalfType |= Arg.hasArg(Opt_fnative_half_type);

================
Comment at: lib/Frontend/CompilerInvocation.cpp:1593
@@ -1591,2 +1592,3 @@
 
+
   if (!Opts.CurrentModule.empty() && !Opts.ImplementationOfModule.empty() &&
----------------
Spurious whitespace?

================
Comment at: test/CodeGen/fp16-ops.c:2
@@ -1,5 +1,3 @@
 // 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 \
+// RUN:   | FileCheck %s --check-prefix=NOHALF --check-prefix=CHECK
----------------
You can split these lines in a separate commit, helps readability.

================
Comment at: test/CodeGen/fp16-ops.c:176
@@ -133,2 +175,3 @@
   // CHECK: [[F16TOF32]]
   // CHECK: fcmp olt
+  // NATIVE-HALF: fcmp olt half
----------------
Now that it makes a difference, can you add "float" in the ones where it's missing?
You can do that separately as well.

================
Comment at: test/CodeGen/fp16-ops.c:228-229
@@ -171,2 +227,4 @@
   // CHECK: fcmp ole
+  // NATIVE-HALF: fpext half
+  // NATIVE-HALF: fcmp ole float
   test = (h2 <= f0);
----------------
One thing you could do is separate all the clang-promoted comparisons like this and group them together.  You can then share CHECK- and F16TOF32 for these with HALF, if you add a NATIVE-HALF: [[F16TOF32:...]] and yet another, *really* shared CHECK prefix.  Not sure it helps though.

http://reviews.llvm.org/D9781

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/