Update DirectX-Headers to latest by amaiorano · Pull Request #8431 · microsoft/DirectXShaderCompiler

amaiorano · 2026-05-08T00:10:33Z

The version being used was from four years ago. Updating to upstream's main broke a lot of code because DirectX-Headers now has the equivalent of WinAdapter.h under include/wsl for non-Windows machines, and these collided with DXC's own definitions. I fixed this by including the relevant DirectX-Headers in WinAdapter.h, removed duplicate macros, and adapted some of the existing ones.

Fixes #5079

The version being used was from four years ago. Updating to upstream's main broke a lot of code because DirectX-Headers now has the equivalent of WinAdapter.h under `include/wsl` for non-Windows machines, and these collided with DXC's own definitions. I fixed this by including the relevant DirectX-Headers in WinAdapter.h, removed duplicate macros, and adapted some of the existing ones. Fixes microsoft#5079

llvm-beanz · 2026-05-08T00:29:52Z

/azp run

azure-pipelines · 2026-05-08T00:30:01Z

Azure Pipelines successfully started running 1 pipeline(s).

amaiorano · 2026-05-08T12:56:37Z

All tests pass, but I will share some questions/concerns:

The types that I removed from WinAdapter.h are because they are already defined in external/DirectX-Headers/include/wsl/stubs/basetsd.h. Most of them are effectively the same, but there are some differences:

The BOOL type going from bool to uint32_t might be a concern, especially if used in the public facing API, but I'm not sure.
BOOLEAN was char and is now BYTE which is unsigned char
The rest went from non-sized types like int, long, char, to sized types like int32_t, int64_t, int8_t, which should be mostly okay on current 32 and 64 bit platforms, except maybe long, which went from long to int32_t. I know that long is usually 32-bits on Windows, and 64-bits on Linux (and Mac?).

The next concern to bring up: I don't know if this was the right move, but I modified the root CMakeLists.txt to move the include_directories of DirectX-Headers paths higher up so that it would be visible to all targets. This allowed me to include one DirectX-Headers file, wsl/winadapter.h, which ultimately includes other files like wsl/stubs/basetsd.h, directly at the top of WinAdapter.h. The concern here is that I noticed that WinAdapter.h is one of the install files in CMake (see DXC_PLATFORM_PUBLIC_HEADERS in include/dxc/CMakeLists.txt). So this file was meant to be a standalone includable file, so I'm not sure if this will work for users of an installed DXC (I don't think there are any tests for this scenario). Users of DXC would also need DirectX-Includes and add DirectX-Headers/include to their include directories. Is this okay? If not, what should be done?

amaiorano · 2026-05-08T15:13:26Z

I did a bit more digging to understand the root of what changed in DirectX-Headers. Basically:

In old DirectX-Headers:

wsl/stubs really contains only stubs. For example, external/DirectX-Headers/include/wsl/stubs/rpcndr.h was basically empty.

external/DirectX-Headers/include/wsl/winadapter.h did contain Windows types for non-Windows, but this file was not included in DXC.

In latest DirectX-Headers:

wsl/stubs are no longer really stubs. Now external/DirectX-Headers/include/wsl/stubs/rpcndr.h defines __CRT_UUID_DECL and includes external/DirectX-Headers/include/wsl/stubs/basetsd.h (also in stubs) which is where most of what was in external/DirectX-Headers/include/wsl/winadapter.h has moved to.

A concrete example of a build error we get when updating DirectX-Headers:

Building lib/DxilContainer/CMakeFiles/LLVMDxilContainer.dir/D3DReflectionStrings.cpp.o fails because of the inclusion chain:

lib/DxilContainer/D3DReflectionStrings.cpp
  include/dxc/Test/D3DReflectionStrings.h
    include/dxc/Support/D3DReflection.h
      external/DirectX-Headers/include/directx/d3d12shader.h
        external/DirectX-Headers/include/directx/d3dcommon.h
          external/DirectX-Headers/include/wsl/stubs/rpcndr.h
            external/DirectX-Headers/include/wsl/stubs/basetsd.h

If we comment out the include of d3d12shader.h in D3DReflection.h, then we get compile errors because of unknown enums:

/home/amaiorano/src/external/DirectXShaderCompiler/include/dxc/Test/D3DReflectionStrings.h:24:17: error: unknown type name 'D3D_CBUFFER_TYPE'
   24 | LPCSTR ToString(D3D_CBUFFER_TYPE CBType);
      |                 ^
/home/amaiorano/src/external/DirectXShaderCompiler/include/dxc/Test/D3DReflectionStrings.h:25:17: error: unknown type name 'D3D_SHADER_INPUT_TYPE'
   25 | LPCSTR ToString(D3D_SHADER_INPUT_TYPE Type);
      |                 ^

I thought of maybe "disabling" the ultimate inclusion of "basetsd.h" by defining the header guard, but unfortunately this file doesn't use a classic header guard, but uses #pragma once. When I delete the contents of the file locally just to test, I am able to compile D3DReflectionStrings.cpp.

In fact, if I delete the contents of "basetsd.h", and "rpcndr.h", but make sure to #define __RPCNDR_H_VERSION__ before including d3d headers, all of dxc builds.

So I'm not really seeing a way out of this that's much different from the change I've put up here. As I wrote in my previous comment, users of DXC on Linux/Mac would need DirectX-Includes and add DirectX-Headers/include/wsl/stubs to their include directories.

Add root of DirectX-Headers/include to CMake include_directories, and only include 'wsl/winadapter.h' at the top of DXC's WinAdapter.h. This should make it easier for users of an installed DXC to add DirectX-Headers as a dep.

damyanp · 2026-05-08T18:48:47Z

/azp run

azure-pipelines · 2026-05-08T18:48:58Z

Azure Pipelines successfully started running 1 pipeline(s).

damyanp

I've left a few questions. I think that the main thing we need to figure out is how do we convince ourselves that this change doesn't break anything? Is it enough that the build pipelines pass? Do we need to do some manual live testing on non-Windows systems?

@tex3d, @jenatali, do you have any thoughts here?

damyanp · 2026-05-08T19:04:25Z


-#define SUCCEEDED(hr) (((HRESULT)(hr)) >= 0)
-#define FAILED(hr) (((HRESULT)(hr)) < 0)
 #define DXC_FAILED(hr) (((HRESULT)(hr)) < 0)


Does DXC_FAILED still need to exist?

It's used in a few places. It could be removed and replaced with FAILED, but that should be done in a follow-up change, I think.

damyanp · 2026-05-08T19:05:29Z


-#define E_ABORT (HRESULT)0x80004004
-#define E_ACCESSDENIED (HRESULT)0x80070005
 #define E_BOUNDS (HRESULT)0x8000000B


Presumably the defines removed here conflicted with ones in DirectX headers? Are the remaining ones still required?

Yes and yes. Both E_BOUNDS and E_NOT_VALID_STATE are used in the DXC codebase, but are not defined in wsl/stubs/basetsd.h (DirectX-Includes).

jenatali · 2026-05-08T19:24:49Z

This looks right to me. The DirectX-Headers definitions mean that datatypes and behaviors will be the same across Windows and Linux so I think standardizing on that is the right call even if it's different than how they were defined before. I'm not sure how thorough the tests are for the Linux builds, but as long as we can compile some basic shaders I suspect that this change would be fine.

So I'm not really seeing a way out of this that's much different from the change I've put up here. As I wrote in my previous comment, users of DXC on Linux/Mac would need DirectX-Includes and add DirectX-Headers/include/wsl/stubs to their include directories.

I don't have a great answer here but at least for now this seems okay?

amaiorano · 2026-05-08T19:56:30Z

I've left a few questions. I think that the main thing we need to figure out is how do we convince ourselves that this change doesn't break anything? Is it enough that the build pipelines pass? Do we need to do some manual live testing on non-Windows systems?

I suspect it may break projects that depend on an installed DXC on non-Windows as they would now need to also depend on DirectX-Headers (and add it to their include path). I think projects that use CMake add_subdirectory should work out of the box. It would be nice to test these cases, I'm just not sure where to look.

I can at least make sure this new dep on DirectX-Headers is clearly stated in the commit comment, which may help downstream users of DXC.

damyanp · 2026-05-08T20:21:35Z

So by "use dxc" here, I think we mean people who are compiling and linking against the DXC libraries? This doesn't impact anyone who just installs a binary release of dxc, right?

I suspect it may break projects that depend on an installed DXC on non-Windows as they would now need to also depend on DirectX-Headers (and add it to their include path).

Would also break people including it as a submodule?

I think projects that use CMake add_subdirectory should work out of the box. It would be nice to test these cases, I'm just not sure where to look.

Will this mean that the vcpkg maintainers will need to react to this change as well?

damyanp · 2026-05-08T20:23:12Z

/azp run

azure-pipelines · 2026-05-08T20:23:23Z

Azure Pipelines successfully started running 1 pipeline(s).

amaiorano · 2026-05-08T20:42:57Z

So by "use dxc" here, I think we mean people who are compiling and linking against the DXC libraries? This doesn't impact anyone who just installs a binary release of dxc, right?

Yes.

Would also break people including it as a submodule?

I think by this you mean using add_subdirectory in CMake, which I think will work.

Will this mean that the vcpkg maintainers will need to react to this change as well?

Good question.

What I'm going to do is test different scenarios on Linux:

Simple CMake project that uses an "install" of DXC (adds dxc/include to path).
CMake project that depends on DXC using add_subdirectory (we actually do this in Dawn, but it's not easy.
Project that consumes DXC via vcpkg.

As a side note, I tried building with gcc and got a bunch of warnings similar to those I got with Clang, so I'll add some pragmas to ignore them like I did for Clang.

tex3d · 2026-05-08T21:32:40Z

In the past, we have intended to include all the (non-std) headers required to use the API in our GitHub release package. With this change, as mentioned, you must separately get and include at least basestd.h from DirectX-Headers.

Dependencies on DirectX-Headers were originally meant only for the DXC execution tests. DirectX headers weren't meant to be required by users of the DXC API, and developers who acquire the API through the GitHub release package might not be happy with a new project dependency just for a new 12k header that supplies a few definitions that used to be independently declared.

Maybe we could add this header to our GitHub release package to avoid any additional project dependencies when using the DXC API from that package. We could put it under another folder matching DirectX-Headers sub-path structure so it can easily be used as an include path instead of the full DirectX_headers, when minimal dependencies are desired. Otherwise, it seems excessive to add the whole DirectX-Headers project dependency just to include a 12k header.

Alternatively, we could move all our DXC API headers to the DirectX-Headers project, and no longer package them in the GitHub release packages. Then force everyone to get all the DXC API headers form DirectX-Headers for future releases.

github-project-automation Bot added this to HLSL Roadmap May 8, 2026

github-project-automation Bot moved this to New in HLSL Roadmap May 8, 2026

amaiorano assigned llvm-beanz May 8, 2026

Simplify inclusion of DirectX-Headers winadapter.h

feef8fe

Add root of DirectX-Headers/include to CMake include_directories, and only include 'wsl/winadapter.h' at the top of DXC's WinAdapter.h. This should make it easier for users of an installed DXC to add DirectX-Headers as a dep.

amaiorano requested a review from damyanp May 8, 2026 17:51

amaiorano assigned amaiorano and unassigned llvm-beanz May 8, 2026

amaiorano requested a review from llvm-beanz May 8, 2026 17:52

damyanp reviewed May 8, 2026

View reviewed changes

Remove comment

066598d

Conversation

amaiorano commented May 8, 2026

Uh oh!

llvm-beanz commented May 8, 2026

Uh oh!

azure-pipelines Bot commented May 8, 2026

Uh oh!

amaiorano commented May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

amaiorano commented May 8, 2026

Uh oh!

damyanp commented May 8, 2026

Uh oh!

azure-pipelines Bot commented May 8, 2026

Uh oh!

damyanp left a comment

Choose a reason for hiding this comment

Uh oh!

damyanp May 8, 2026

Choose a reason for hiding this comment

Uh oh!

amaiorano May 8, 2026

Choose a reason for hiding this comment

Uh oh!

damyanp May 8, 2026

Choose a reason for hiding this comment

Uh oh!

amaiorano May 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jenatali commented May 8, 2026

Uh oh!

amaiorano commented May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

damyanp commented May 8, 2026

Uh oh!

damyanp commented May 8, 2026

Uh oh!

azure-pipelines Bot commented May 8, 2026

Uh oh!

amaiorano commented May 8, 2026

Uh oh!

tex3d commented May 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

amaiorano commented May 8, 2026 •

edited

Loading

amaiorano commented May 8, 2026 •

edited

Loading