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

[Xiang Li via Phabricator via cfe-commits]

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:
> beanz wrote:
> > aaron.ballman wrote:
> > > python3kgae wrote:
> > > > python3kgae wrote:
> > > > > aaron.ballman wrote:
> > > > > > python3kgae wrote:
> > > > > > > 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.
> > > > > > > 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.
> > > > > > 
> > > > > > Is there room for change here, or is this strictly required by HLSL? This strikes me as just begging to confuse users into creating ODR violations. CC @beanz 
> > > > > Here's the doc about half for dxc.
> > > > > https://github.com/microsoft/DirectXShaderCompiler/wiki/16-Bit-Scalar-Types
> > > > > 
> > > > > Old doc for fxc (the old shader compiler for shader model <= 5.1) is here
> > > > > https://docs.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl-scalar
> > > > > 
> > > > > Change the behavior might affect a lot of existing shaders.
> > > > More detail about half from https://github.com/tex3d.
> > > > 
> > > > half originally mapped to a fuzzy "partial precision" float, where some operations were designated as _pp, meaning the implementation was free to use lower-precision math for those operations (like 24-bit, but not specified for the language). All storage in host-visible memory would still be float. "partial precision" went away with DX9, eventually to be replaced with min-precision with a specific minimum precision an implementation was allowed to use. When "partial precision" went away, it simply mapped to float for DX10+.
> > > > People could have tried to use it liberally when they thought 32-bit precision wasn't necessary, without explicitly targeting/testing API/hardware that actually supported lower precision.
> > > Thank you for the extra information, it sounds like we're stuck supporting this.
> > Because HLSL’s library linking mode is pretty constrained, in practice this hasn’t hurt us yet. That said, I think we probably should consider some retroactive changes to how we handle float-16, especially in the presence of library shaders…
> > 
> > I’ll try and float this topic in some team meetings this week and see if we can come up with a set of tweaks that may have limited impact on existing code. We might be able to change the default behavior in clang with a switch to toggle back to the old mode for legacy code.
> > I’ll try and float this topic in some team meetings this week and see if we can come up with a set of tweaks that may have limited impact on existing code. We might be able to change the default behavior in clang with a switch to toggle back to the old mode for legacy code.
> 
> Thanks, both for the awesome pun ("float this topic", lol) and for checking on this. Let's hold off on this patch until we hear more on whether we want to change the default behavior in Clang here or not.
Finally got this discussed today.
The result is we should do the same thing as dxc for back-compat.
And that will require half has a special name when mangling even when half is 16bit type.
Will update the PR shortly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124790