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