-
Notifications
You must be signed in to change notification settings - Fork 14
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
Update AZSLc (and O3DE) To Support HLSL 2021 Features #45
Comments
I'm thinking about this task, and I have to say, I still have to shape it in my brain enough so as to be sure about what can be done, at what cost, and also what should be done (is potentially orthogonal). So allow me to put the bits --necessary for brainstorming-- down:
This is tricky. So the problem resides in multiple facets:
I suppose point 3 can cause some head scratching, especially "why would AZSLc need to be fully semantically aware of the template program?".
That is probably not a can of worms; the problem is to preserve the quality of type tracking but I'm thinking that it's doable.
I'm afraid for all the packing/padding/alignment rules that we're going to have to update because of that.
That is actually already the case in AZSL, AZSL never tolerated type equivalence through layout. So on the contrary, this will give us more robustness for free since the DXC-input will not be ambiguous in places when we re-emitted naively.
seems like a llvm thing. no impact for us?
I'm appalled that it wasn't the case before. I'll need to look in detail what really changed.
This is properly insane. AZSL never behaved like that, so this is again a fix we're benefitting from that robustifies the output understanding by DXC. |
@siliconvoodoo thanks for your analysis. I believe the original motivation for this task was to incorporate support for template functions. The (incorrect) assumption was that doing so would be relatively easy because we could rely on DXC to do the heavy lifting. In addition to what you explained, I imagine AZSLc would have to do template expansion in order to properly transpile SRG references possibly do other dependency tracking, so there would be little benefit or even opportunity to rely on DXC's template expansion. Perhaps @jeremyong-az and fill in other background regarding the motivation behind this task. In my view, this task could be undertaken if it is low-hanging fruit (which apparently it is not), otherwise there are higher priorities. |
@siliconvoodoo Thanks for the comment replies to follow:
I would argue that it shouldn't, and that it simply needs to be lexically aware enough that AZSLc can extract binding/reflection data. Realistically, HLSL/DXC continue to make progress, and I think it's not sustainable to mirror their efforts, or attempt to provide our own validation/warning/error layer on top. The value AZSLc provides is an RHI-agnostic, consistent, scheme to map shader resource and binding schemes across platforms in a user-friendly way. As upstream syntax changes roll out, having AZSLc operate as a thinner layer on top of DXC is desirable in my mind, so as to work in a more complementary rather than competing fashion. I would argue that trying to exhaustively expand the test suite with comprehensive coverage over template handling is counterproductive. If we could simply pass-over template expressions to extracting reflection information, replace variables as needed and pass the rest of the shader over to DXC, that would be ideal.
We should certainly discuss with the community, but in my mind, this is a moot point until there is sufficient appetite to actually subsume this work. Templates, while not always a perfect solution by any means, solve certain problems immediately. For example: // Fresnel factor
// Specular reflectivity increases at grazing incident angles
// The f0 parameter represents specular reflectance at normal incidence and is
// achromatic for dieletrics but chromatic for metals.
// The f90 parameter represents the reflectance at grazing angles.
template <typename T, int N>
vector<T, N> f_schlick(float u, vector<T, N> f0, T f90)
{
vector<T, N> f90_t = f90;
return f0 + (f90_t - f0) * pow(1.0 - u, 5.0);
} Here's a simple function from my home renderer which I upgraded to a recent DXC early this year. This function works for an RGB vector, a greyscale component, for a single precision
I don't know that we'd need to do anything except track the total size of the bitfield entry and the offset. The packing/padding/alignment rules should be the same (legacy rules for cbuffer, tight packing for everything else). As an initial pass, I would suggest that we shouldn't need to reflect the bitfield members themselves since they have uses outside of shader bindings. |
O3DE already uses the latest release of DXC: 1.6.2112. This includes preview support for the new HLSL 2021 feature set (see https://devblogs.microsoft.com/directx/announcing-hlsl-2021/) but AZSLc does not support these new features. I haven't tested this, but I suspect that at least some of the new features will fail to be parsed by AZSLc and will need upgrades to be passed through to the HLSL output.
We should update AZSLc's grammar to support these new features, and cut a new release. To demonstrate the new features we should look for some ways to improve existing shader code in O3DE, maybe templatize some utility functions
Note that there is a command line option for DXC to enable these features: "-HV 2021". We will need to update O3DE's configuration to pass this command line option when running DXC. See Gems\Atom\Asset\Shader\Config\shader_build_options.json
The text was updated successfully, but these errors were encountered: