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

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

Reply via email to