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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits