This is an archive of the discontinued LLVM Phabricator instance.

[HLSL] Enable half type for hlsl.
ClosedPublic

Authored by python3kgae on May 2 2022, 11:16 AM.

Details

Summary

HLSL supports half type.
When enable-16bit-types is not set, half will be treated as float.
When enable-16bit-types is set, half will be treated like real 16bit float type and map to llvm half type.
Also change CXXABI to Microsoft to match dxc behavior.
The mangle name for half is "$f16@" when half is treat as native half type and "$halff@" when treat as float.

In AST, half is still half.
The special thing is done at clang codeGen, when NativeHalfType is false, half will translated into float.

Diff Detail

Event Timeline

python3kgae created this revision.May 2 2022, 11:16 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 2 2022, 11:16 AM
Herald added a subscriber: dexonsmith. · View Herald Transcript
python3kgae requested review of this revision.May 2 2022, 11:16 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 2 2022, 11:16 AM
aaron.ballman added inline comments.May 3 2022, 5:11 AM
clang/include/clang/Driver/Options.td
6762–6763
clang/lib/Basic/LangOptions.cpp
196–197

Shouldn't this be looking for HLSL 2018? Or shader model 6.2?

clang/lib/Basic/Targets/DirectX.h
61

Should this be tied to the Half language option?

clang/lib/Sema/SemaType.cpp
1513

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?

clang/test/CodeGenHLSL/half.hlsl
16

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.

python3kgae marked 3 inline comments as done.May 3 2022, 10:05 AM
python3kgae added inline comments.
clang/lib/Basic/LangOptions.cpp
196–197

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.

clang/lib/Basic/Targets/DirectX.h
61

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.

clang/lib/Sema/SemaType.cpp
1513

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.

clang/test/CodeGenHLSL/half.hlsl
16

I think the issue is this test require build DirectX backend target.
I'll change it to work without DirectX backend target.

aaron.ballman added inline comments.May 3 2022, 11:51 AM
clang/lib/Basic/LangOptions.cpp
196–197

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

python3kgae marked 2 inline comments as done.May 3 2022, 12:27 PM
python3kgae added inline comments.
clang/lib/Basic/LangOptions.cpp
196–197

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.

Add option -fcgl which output clang codeGen result to avoid test dependent on build DirectX backend.

python3kgae added inline comments.May 3 2022, 3:54 PM
clang/lib/Basic/LangOptions.cpp
196–197

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.

Add option -fcgl which output clang codeGen result to avoid test dependent on build DirectX backend.

Thanks -- I think this should actually be a separate patch though, because it's not really related to the half datatype. (You can always wait to land this patch until after the -fcgl one has landed.)

Aside from the -fcgl flag, this looks ready to go though.

clang/lib/Basic/LangOptions.cpp
196–197

Thank you for the extra information, it sounds like we're stuck supporting this.

beanz added inline comments.May 4 2022, 8:42 AM
clang/lib/Basic/LangOptions.cpp
196–197

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.

clang/lib/Driver/ToolChains/HLSL.cpp
177

-fcgl also should imply -disable-llvm-options

Rebase for fcgl change.

This change should likely also have some Sema tests demonstrating what happens during constant expression evaluation, or narrowing conversions, etc given that the type may have different behavior.

clang/lib/Basic/LangOptions.cpp
196–197

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.

Add Sema test.

python3kgae added inline comments.Jun 16 2022, 2:46 PM
clang/lib/Basic/LangOptions.cpp
196–197

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.

Change CXXABI to Microsoft to match dxc behavior.
The mangle name for half is "$f16@" when half is treat as native half type and "$halff@" when treat as float.

And now in AST, half is still half. Only in clang codeGen half will translated into float when enable-16bit-types is flase.

python3kgae edited the summary of this revision. (Show Details)Jun 19 2022, 3:42 PM
aaron.ballman accepted this revision.Jun 23 2022, 8:05 AM

LGTM aside from some minor changes.

clang/lib/AST/ASTContext.cpp
1712 ↗(On Diff #438220)
1713–1719 ↗(On Diff #438220)
clang/lib/Basic/Targets/DirectX.h
60

Though I'd also be fine removing the comment as it doesn't really do much except restate the next line.

This revision is now accepted and ready to land.Jun 23 2022, 8:05 AM
python3kgae marked 3 inline comments as done.

Cleanup comments.

Thanks for the review.
Updated the comments.

This revision was automatically updated to reflect the committed changes.