Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
d7d1936
implement stage 2
bob80905 Jun 6, 2025
c527502
add crucial headers
bob80905 Jun 6, 2025
7f7dc30
chore: autopublish 2025-06-06T00:27:21Z
github-actions[bot] Jun 6, 2025
e7ec854
address Tex non-header changes
bob80905 Jun 6, 2025
39f913c
handle includes
bob80905 Jun 6, 2025
3700bfd
swap include order
bob80905 Jun 6, 2025
33c88f9
address Tex, and make a flawed attempt at writing a unit test
bob80905 Jun 18, 2025
544a9c7
chore: autopublish 2025-06-18T04:25:45Z
github-actions[bot] Jun 18, 2025
eeb1516
validated that local test passes
bob80905 Jun 18, 2025
9800c91
address tex, update unit test
bob80905 Jun 25, 2025
a4c9629
ifdef for windows vs unix env var addition
bob80905 Jun 25, 2025
a9282fd
add wstring conversion stuff and env var resets
bob80905 Jun 25, 2025
e781627
address tex
bob80905 Jun 25, 2025
2915a8d
special casing for unix systems
bob80905 Jun 25, 2025
57dd853
address tex, make wstrings more universal too
bob80905 Jun 30, 2025
98b25af
add includes for _wgetenv
bob80905 Jun 30, 2025
c274e5b
remove wstring for simplicity
bob80905 Jun 30, 2025
b46def4
fix undeclared variables in non windows case, and pointer lifetime bug
bob80905 Jun 30, 2025
b76fab2
adjust for build errors
bob80905 Jul 1, 2025
89dcc4a
address final forgotten comments
bob80905 Jul 1, 2025
acbf68a
try removing unicode from cmakelists
bob80905 Jul 2, 2025
f2890f5
update comment Justin pointed out was outdated
bob80905 Jul 3, 2025
eb1e96f
remove Linux definitions from windows only testing, address ambiguity…
bob80905 Jul 7, 2025
36c33a2
do not inherit from DxcDllSupport
bob80905 Jul 7, 2025
6577046
add win32 preprocessor gate to allow non windows envs to build
bob80905 Jul 7, 2025
80b50a6
remove test from non-windows since it depends on wide strings
bob80905 Jul 7, 2025
bfc508f
remove friend, remove protected:
bob80905 Jul 8, 2025
a144ca1
address Damyan final concerns
bob80905 Jul 11, 2025
bd01fa5
try another way to emit an hresult
bob80905 Jul 12, 2025
647c946
try E_INVALIDARG
bob80905 Jul 14, 2025
a29f6ee
use an interface
bob80905 Jul 18, 2025
fffb407
fix nix error
bob80905 Jul 18, 2025
5411894
remove comment on header that is likely to become stale
bob80905 Jul 24, 2025
45f74fa
perform rename
bob80905 Jul 28, 2025
64f61a8
resolve merge conflicts
bob80905 Jul 28, 2025
ed23ada
update old class name
bob80905 Jul 28, 2025
2383541
rename to IDllLoader, rename to OverrideDll, remove defaulted functio…
bob80905 Aug 6, 2025
209582f
address all of Justin + Damyan 2c
bob80905 Aug 7, 2025
daf6f0a
add override
bob80905 Aug 7, 2025
c8d867e
add more overrides
bob80905 Aug 7, 2025
0aa3c8d
add virtual to destructor for HlslIntellisense
bob80905 Aug 7, 2025
15fceac
address Damyan: specify constructors in interface, remove cleanup fro…
bob80905 Aug 7, 2025
09be489
address Damyan
bob80905 Aug 8, 2025
be99192
Merge branch 'main' into create_DxcDllExtValidationSupport
bob80905 Aug 8, 2025
cba829b
we keep trying, this should be it
bob80905 Aug 8, 2025
17f26c6
clang format
bob80905 Aug 8, 2025
c0ee094
remove move ctor and copy ctor
bob80905 Aug 8, 2025
cdf321e
address all but justins last comment
bob80905 Aug 11, 2025
c8b704a
do a function rename
bob80905 Aug 11, 2025
965f78c
try undoing spirv change
bob80905 Aug 12, 2025
48fb6fa
remove unneeded forward decl
bob80905 Aug 12, 2025
b100acd
add DxCompilerDllLoader class, and use it
bob80905 Aug 13, 2025
f66408c
address Justin
bob80905 Aug 13, 2025
c8bebee
fix build err
bob80905 Aug 13, 2025
4633dcc
remove specificdllloader from ensureenabled, use new LibraryDllLoader…
bob80905 Aug 14, 2025
005ea83
rename to DXCLibraryDllSupport, address Justin
bob80905 Aug 14, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion include/dxc/Support/HLSLOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ int ReadDxcOpts(const llvm::opt::OptTable *optionTable, unsigned flagsToInclude,
llvm::raw_ostream &errors);

/// Sets up the specified DllLoader instance as per the given options.
int SetupDllLoader(const DxcOpts &opts, dxc::DllLoader &dxcSupport,
int SetupDllLoader(const DxcOpts &opts, dxc::SpecificDllLoader &dxcSupport,
llvm::raw_ostream &errors);

void CopyArgsToWStrings(const llvm::opt::InputArgList &inArgs,
Expand Down
11 changes: 3 additions & 8 deletions include/dxc/Support/dxcapi.extval.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,24 +30,19 @@ class DxcDllExtValidationLoader : public DllLoader {
to satisfy the IDllLoader interface. The parameter is ignored
because the relevant dlls are specific and known: dxcompiler.dll
and dxil.dll. This class is not designed to handle any other dlls */
HRESULT OverrideDll(LPCSTR dll, LPCSTR entryPoint) override {
HRESULT OverrideDll(LPCSTR dll, LPCSTR entryPoint) {
return InitializeInternal(entryPoint);
}

bool HasCreateWithMalloc() const override {
bool HasCreateWithMalloc() const {
assert(DxCompilerSupport.HasCreateWithMalloc() &&
DxilExtValSupport.HasCreateWithMalloc());
return true;
}

bool IsEnabled() const override { return DxCompilerSupport.IsEnabled(); }

void Cleanup() {
DxilExtValSupport.Cleanup();
DxCompilerSupport.Cleanup();
}

HMODULE Detach() override {
HMODULE Detach() {
// Can't Detach and return a handle for DxilSupport. Cleanup() instead.
DxilExtValSupport.Cleanup();
return DxCompilerSupport.Detach();
Expand Down
24 changes: 10 additions & 14 deletions include/dxc/Support/dxcapi.use.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,19 @@ extern const char *kDxilLib;

// Interface for common dll operations
class DllLoader {
public:
virtual HRESULT OverrideDll(LPCSTR dll, LPCSTR entryPoint) = 0;
DllLoader() = default;
DllLoader(const DllLoader &) = delete;
DllLoader(DllLoader &&) = delete;

protected:
virtual HRESULT CreateInstanceImpl(REFCLSID clsid, REFIID riid,
IUnknown **pResult) = 0;
virtual HRESULT CreateInstance2Impl(IMalloc *pMalloc, REFCLSID clsid,
REFIID riid, IUnknown **pResult) = 0;
virtual ~DllLoader() {}

public:
DllLoader() = default;
DllLoader(const DllLoader &) = delete;
DllLoader(DllLoader &&) = delete;

template <typename TInterface>
HRESULT CreateInstance(REFCLSID clsid, TInterface **pResult) {
return CreateInstanceImpl(clsid, __uuidof(TInterface),
Expand All @@ -54,11 +54,7 @@ class DllLoader {
return CreateInstance2Impl(pMalloc, clsid, riid, (IUnknown **)pResult);
}

virtual bool HasCreateWithMalloc() const = 0;

virtual bool IsEnabled() const = 0;

virtual HMODULE Detach() = 0;
};

// Helper class to dynamically load the dxcompiler or a compatible libraries.
Expand Down Expand Up @@ -128,13 +124,13 @@ class SpecificDllLoader : public DllLoader {
other.m_createFn2 = nullptr;
}

virtual ~SpecificDllLoader() { Cleanup(); }
~SpecificDllLoader() override { Cleanup(); }

HRESULT Initialize() {
return InitializeInternal(kDxCompilerLib, "DxcCreateInstance");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having this particular implementation of Initialize() in SpecificDllLoader is kind of irksome. This means that SpecificDllLoader really does want to load dxcompiler.dll in particular, even in cases where we're never using it for that dll. It would be nice for DxcompilerLoader to be a subclass akin to HlslIntellisenseSupport that specifically bakes in the defaults.

I don't think this is really possible without either removing Initialize() from the interface or explicitly implementing SpecificDllLoader::Initialize() to throw a runtime error though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it would raise an eyebrow on first glance, but the initialize function needs a dll to work with, because it has no args. We need some dll to default to, and dxcompilerlib is a fine one to default to. If users wanted something else, they could initialize with OverrideDll and specify the dll.
I've removed Initialize from the Interface abstract class, but I still think this function is fine as is, as a default constructor of sorts.

Copy link
Collaborator

@bogner bogner Aug 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I'm saying is is "do we even want an Initialize() function with no parameters on SpecificDllLoader? What if SpecificDllLoader only had InitializeForDll() and to use it you had to specify which dll you wanted to load?

For convenience, we could subclass this with DxcompilerDllLoader that specifically loads dxcompiler.dll, much like we do with HlslIntellisenseSupport. That object could reasonably have a Initialize() that knows what Dll to load. This would match the Initialize() on the external validator class that knows which two Dlls it's going to load.

}

HRESULT OverrideDll(LPCSTR dll, LPCSTR entryPoint) override {
HRESULT OverrideDll(LPCSTR dll, LPCSTR entryPoint) {
return InitializeInternal(dll, entryPoint);
}

Expand Down Expand Up @@ -166,7 +162,7 @@ class SpecificDllLoader : public DllLoader {
return hr;
}

bool HasCreateWithMalloc() const override { return m_createFn2 != nullptr; }
bool HasCreateWithMalloc() const { return m_createFn2 != nullptr; }

bool IsEnabled() const override { return m_dll != nullptr; }

Expand All @@ -192,7 +188,7 @@ class SpecificDllLoader : public DllLoader {
}
}

HMODULE Detach() override {
HMODULE Detach() {
HMODULE hModule = m_dll;
m_dll = nullptr;
return hModule;
Expand All @@ -209,7 +205,7 @@ inline DxcDefine GetDefine(LPCWSTR name, LPCWSTR value) {
// Checks an HRESULT and formats an error message with the appended data.
void IFT_Data(HRESULT hr, LPCWSTR data);

void EnsureEnabled(DllLoader &dxcSupport);
void EnsureEnabled(SpecificDllLoader &dxcSupport);
void ReadFileIntoBlob(DllLoader &dxcSupport, LPCWSTR pFileName,
IDxcBlobEncoding **ppBlobEncoding);
void WriteBlobToConsole(IDxcBlob *pBlob, DWORD streamType = STD_OUTPUT_HANDLE);
Expand Down
3 changes: 2 additions & 1 deletion include/dxc/Test/DxcTestUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,8 @@ bool CheckNotMsgs(const LPCSTR pText, size_t TextCount,
const LPCSTR *pErrorMsgs, size_t errorMsgCount, bool bRegex);
void GetDxilPart(dxc::DllLoader &dllSupport, IDxcBlob *pProgram,
IDxcBlob **pDxilPart);
std::string DisassembleProgram(dxc::DllLoader &dllSupport, IDxcBlob *pProgram);
std::string DisassembleProgram(dxc::SpecificDllLoader &dllSupport,
IDxcBlob *pProgram);
void SplitPassList(LPWSTR pPassesBuffer, std::vector<LPCWSTR> &passes);
void MultiByteStringToBlob(dxc::DllLoader &dllSupport, const std::string &val,
UINT32 codePoint, IDxcBlob **ppBlob);
Expand Down
2 changes: 1 addition & 1 deletion lib/DxcSupport/HLSLOptions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1384,7 +1384,7 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude,
}

/// Sets up the specified DllLoader instance as per the given options.
int SetupDllLoader(const DxcOpts &opts, dxc::DllLoader &dxcSupport,
int SetupDllLoader(const DxcOpts &opts, dxc::SpecificDllLoader &dxcSupport,
llvm::raw_ostream &errors) {
if (!opts.ExternalLib.empty()) {
DXASSERT(!opts.ExternalFn.empty(), "else ReadDxcOpts should have failed");
Expand Down
2 changes: 1 addition & 1 deletion lib/DxcSupport/dxcapi.use.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ void IFT_Data(HRESULT hr, LPCWSTR data) {
throw ::hlsl::Exception(hr, errMsg);
}

void EnsureEnabled(DllLoader &dxcSupport) {
void EnsureEnabled(SpecificDllLoader &dxcSupport) {
if (!dxcSupport.IsEnabled()) {
IFT(dxcSupport.OverrideDll(kDxCompilerLib, "DxcCreateInstance"));
}
Expand Down
2 changes: 1 addition & 1 deletion tools/clang/unittests/HLSLExec/HlslExecTestUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ static bool createDevice(ID3D12Device **D3DDevice,
}

inline void readHlslDataIntoNewStream(LPCWSTR RelativePath, IStream **Stream,
dxc::DllLoader &Support) {
dxc::SpecificDllLoader &Support) {
VERIFY_SUCCEEDED(
Support.OverrideDll(dxc::kDxCompilerLib, "DxcCreateInstance"));
CComPtr<IDxcLibrary> Library;
Expand Down
3 changes: 2 additions & 1 deletion tools/clang/unittests/HLSLTestLib/DxcTestUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,8 @@ bool CheckOperationResultMsgs(IDxcOperationResult *pResult,
maySucceedAnyway, bRegex);
}

std::string DisassembleProgram(dxc::DllLoader &dllSupport, IDxcBlob *pProgram) {
std::string DisassembleProgram(dxc::SpecificDllLoader &dllSupport,
IDxcBlob *pProgram) {
CComPtr<IDxcCompiler> pCompiler;
CComPtr<IDxcBlobEncoding> pDisassembly;

Expand Down