[PATCH] D124790: [HLSL] Enable half type for hlsl.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/include/clang/Driver/Options.td:6766-6767
+def enable_16bit_types : DXCFlag<"enable-16bit-types">, Alias<fnative_half_type>,
+  HelpText<"Enable 16bit types and disable min precision types."
+           "Available in HLSL 2018 and shader model 6.2">;
----------------



================
Comment at: clang/lib/Basic/LangOptions.cpp:197
 
-  // OpenCL has half keyword
-  Opts.Half = Opts.OpenCL;
+  // OpenCL and HLSL have half keyword
+  Opts.Half = Opts.OpenCL || Opts.HLSL;
----------------
Shouldn't this be looking for HLSL 2018? Or shader model 6.2?


================
Comment at: clang/lib/Basic/Targets/DirectX.h:61
   }
-
+  bool useFP16ConversionIntrinsics() const override { return false; }
   void getTargetDefines(const LangOptions &Opts,
----------------
Should this be tied to the `Half` language option?


================
Comment at: clang/lib/Sema/SemaType.cpp:1514
+    // For HLSL, when not enable native half type, half will be treat as float.
+    if (S.getLangOpts().HLSL && !S.getLangOpts().NativeHalfType)
+      Result = Context.FloatTy;
----------------
This change seems wrong to me -- if the half type isn't supported, how does the user spell the type such that we can even get here?


================
Comment at: clang/test/CodeGenHLSL/half.hlsl:15
+  return a+b;
+}
----------------
FWIW, this test seems to be failing precommit CI.

We should also have tests for the new driver flag and Sema tests showing that you can't spell `half` in unsupported HLSL modes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124790/new/

https://reviews.llvm.org/D124790