Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

float16 HLSL support #6

Open
oscarbg opened this issue Mar 12, 2018 · 7 comments
Open

float16 HLSL support #6

oscarbg opened this issue Mar 12, 2018 · 7 comments

Comments

@oscarbg
Copy link

oscarbg commented Mar 12, 2018

Hi,
I asked SPIRV-Cross devs to add FP16 HLSL support output (via min16float which is supported without using SM6.0) and they delivered:
KhronosGroup/SPIRV-Cross@47d94ff
the problem I see is your spirv-cross branch has some commits not upstreamed in SPIRV-Cross so asking if you plan to upstream all your changes and point to a recent enough SPIRV-Cross..
in case not, can you update your branch to a commit newer than the one I referenced?
I have modified vulkan test (https://github.com/Erkaman/vulkan_minimal_compute) shader to use fp16:
it's as easy as adding after #version 450 to existing shader.comp file:

#extension GL_AMD_gpu_shader_half_float: enable
#define vec2 f16vec2

and regenerate SPIR-V file..
the using up to date SPIRV-Cross for rostkatze you should get output HLSL min16float in shader..
so in theory rostkatze should gain fp16 support "automagically"..

also you can test double support by adding only:
#define vec2 dvec2
seems SPIRV-Cross supports double in HLSL output for a while but still don't know but this sample doesn't work ok on this case (mandelbrot.png it generates is corrupt)
altough all seems right (d3dcompiler api returns no error)

For min16float testing in theory you need AMD recent enough (Vega and perhaps Polaris) for doubles more..
check support with dxcapsviewer in Windows SDK and tab "Direct3D 11" shows as "double precision shaders" "yes" and "Direct3D 11.1" or higher search "Pixel shader precision" "other stage precision" should show at least "32bits/16bits" to be supported..

@oscarbg oscarbg changed the title can you update spirv-cross submodule to latest (includes FP16 HLSL support).. can you update your spirv-cross submodule to latest (includes FP16 HLSL support).. Mar 12, 2018
@oscarbg
Copy link
Author

oscarbg commented Mar 12, 2018

Forgot I also requested min16int shader support for SPIRV-Cross (KhronosGroup/SPIRV-Cross#491)..
in the meanwhile I experimented and a simple patch to enable is there, basically:

in CompilerHLSL::type_to_glsl I modifify to (similar to msl backend):

case SPIRType::Int:
return (type.width == 16 ? "min16int" : "int");

perhaps you could also add that to your custom branch..
that way roskatze should could expose Vulkan shaderInt16 cap and shaderFloat64 if you and also AMD VK_AMD_gpu_shader_half_float (not complete perhaps for ok for basic ops that HLSL supports (+-*/)..
all left is shaderInt64 but that will require using new DXCompiler and dropping d3dcompiler..

@msiglreith
Copy link
Owner

If I understand it correctly, min16float might have a different precision and alignment than the float16 exposed via the AMD extension?
Is there a way to enforce float16/half to be emitted instead of min16float and corresponding checks on the API side? (I don't mind adding optional suport for the new dxil compiler)

What I don't want is to expose VK_AMD_gpu_shader_half_float but internally translate it to min16float as this can result in some ugly bugs (for example 32bit alignment).

@oscarbg
Copy link
Author

oscarbg commented Mar 14, 2018

Ok sorry for lag,
first precision: I don't know but it ultimately maps to hardware,right? GPU FPU16 units must be following fp16 IEEE format that is well defined after all..
regarding float16/half vs min16float if if understand correctly using min16float is more portable than half and may map "automatically" to float on unsuported hardware..
I asked spirv-cross dev to use min16float but if you prefer "half" or float16_t you may change all ocurrences of "min16float" to "float16_t" from this commit:
KhronosGroup/SPIRV-Cross@47d94ff
the most important ones are all the three:
case SPIRType::Half:
return "min16float";
for case scalar,vector and float..

I already have support for new DXIL working.. I wanted to clean up but anyway I'm lazy too..
I opened a separate issue with the current diff ISSUE-> (#8) comments there..

@msiglreith
Copy link
Owner

Updated the spirv-cross dependency to the latest master.

Concerning fp16 and the AMD extension:
I don't want to expose the AMD extension without real 16bit support aka float16_t due to the aforementioned alignment size issues (ie f16 in the shader resource interface).

@msiglreith msiglreith changed the title can you update your spirv-cross submodule to latest (includes FP16 HLSL support).. float16 HLSL support Mar 24, 2018
@oscarbg
Copy link
Author

oscarbg commented Apr 5, 2018

Sorry for delay in testing..
spirv-cross dependency is broken.. can you fix it?
No problem with fp16 I might hack it to play with it in the meanwhile..
Thanks..

@oscarbg
Copy link
Author

oscarbg commented Apr 7, 2018

Ping..
@msiglreith Please can take a look at why https://github.com/msiglreith/SPIRV-Cross/tree/71bac83f5200661fb204b61b0fcf2db55e69668a is broken?
Thanks..

@msiglreith
Copy link
Owner

Sry, updated again. Should be working now (tried with a fresh clone of the repository).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants