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

[Xiang Li via Phabricator via cfe-commits]

python3kgae marked 3 inline comments as done.
python3kgae added inline comments.


================
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;
----------------
aaron.ballman wrote:
> Shouldn't this be looking for HLSL 2018? Or shader model 6.2?
half keyword is always available.
Without enable_16bit_types, half will be like using half=float.
With enable_16bit_types, half will be real half.

The check for HLSL 2018 and shader model 6.2 will be in another PR, still WIP. I'll add FIXME about it.


================
Comment at: clang/lib/Basic/Targets/DirectX.h:61
   }
-
+  bool useFP16ConversionIntrinsics() const override { return false; }
   void getTargetDefines(const LangOptions &Opts,
----------------
aaron.ballman wrote:
> Should this be tied to the `Half` language option?
We don't want to conversion FP16, with or without enable_16bit_types.
With enable_16bit_types, it is half, don't need conversion.
Without enable_16bit_types, it will be a float, don't need conversion either.


================
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;
----------------
aaron.ballman wrote:
> 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?
Half keyword is always available for hlsl.
When enable_16bit_types, NativeHalfType will be true, half will be a real half.
When not enable_16bit_types, NativeHalfType will be false, half will be float.



================
Comment at: clang/test/CodeGenHLSL/half.hlsl:15
+  return a+b;
+}
----------------
aaron.ballman wrote:
> 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.
I think the issue is this test require build DirectX backend target.
I'll change it to work without DirectX backend target.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124790