Skip to content

Create new class to handle external validation, and rename existing dll loading class. #7514

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.

Sign up for GitHub

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

Open
wants to merge 54 commits into
base: main
Choose a base branch
from
Open

Create new class to handle external validation, and rename existing dll loading class. #7514

Show file tree
Hide file tree
Changes from 36 commits
Commits
Show all changes
54 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
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
9 changes: 5 additions & 4 deletions include/dxc/Support/HLSLOptions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class raw_ostream;
} // namespace llvm

namespace dxc {
class DxcDllSupport;
class SpecificDllLoader;
}

namespace hlsl {
Expand Down Expand Up @@ -304,9 +304,10 @@ int ReadDxcOpts(const llvm::opt::OptTable *optionTable, unsigned flagsToInclude,
const MainArgs &argStrings, DxcOpts &opts,
llvm::raw_ostream &errors);

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

void CopyArgsToWStrings(const llvm::opt::InputArgList &inArgs,
unsigned flagsToInclude,
Expand Down
51 changes: 51 additions & 0 deletions include/dxc/Support/dxcapi.extval.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
#include "dxc/Support/dxcapi.use.h"
#include <string>

namespace dxc {
class DxcDllExtValidationLoader : public DllLoader {
// DxCompilerSupport manages the
// lifetime of dxcompiler.dll, while DxilExtValSupport
// manages the lifetime of dxil.dll
dxc::SpecificDllLoader DxCompilerSupport;
dxc::SpecificDllLoader DxilExtValSupport;

DxcCreateInstanceProc m_createFn;
DxcCreateInstance2Proc m_createFn2;
Copy link
Collaborator

Choose a reason for hiding this comment

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

These are never initialized to anything. Should they really be here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point, I don't think so. I imagine the relevant creation functions will only be stored in the specificdllloader member objects.


std::string DxilDllPath;
HRESULT InitializeInternal(LPCSTR fnName);

public:
std::string GetDxilDllPath() { return DxilDllPath; }
bool DxilDllFailedToLoad() {
return !DxilDllPath.empty() && !DxilExtValSupport.IsEnabled();
}

HRESULT CreateInstance(REFCLSID clsid, REFIID riid, IUnknown **pResult);
HRESULT CreateInstance2(IMalloc *pMalloc, REFCLSID clsid, REFIID riid,
IUnknown **pResult);

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

Choose a reason for hiding this comment

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

Why does InitializeInternal take a fnName argument if it's only ever called with a fixed string? Do we need InitializeInternal at all, or should we just update the implementation to be an implementation of Initialize()?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point, looks like we dont need it at all. I'll remove InitializeInternal and move its implementation to Initialize().

HRESULT InitializeForDll(LPCSTR dll, LPCSTR entryPoint) {
return InitializeInternal(dll);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This doesn't look right. InitializeInternal takes a fnName argument, not a dll name. If the InitializeForDll interface makes sense at all here, then I think you need to pass both of these parameters in.

I mention elsewhere in the review that I think a better name for this interface would be "overrideDll" or something to that effect. The users of this API are saying "use this other Dll I specified on the command line" or something to that effect. I guess we should pass on both parameters here when we call DxCompilerSupport's initializeForDll, and we should maybe forward the entryPointon to our call toDxilExtValSupport's initializeForDll` - though that will only really work if the dll we find in the environment is compatible with the one we're overriding with.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think the right fix here is simply to change what's passed from dll to entryPoint, like you said. But, I don't think we should pass both args to InitializeInternal.
DxcDllExtValidationLoader is a specific instance of the IDllLoader interface that shouldn't accept arbitrary dlls AFAIK. We hard-code the two dlls that it cares about, and we should ignore any other dlls passed as a param to InitializeForDll.
I think we make an implementation of InitializeForDll just to satisfy the interface, but under the hood it ignores whatever's passed and uses the two dll's that it's specifically designed for.
Note the name of the class, it isn't called TwoSpecificDllLoaders, it's called DxcDllExtValidationLoader, so we already know which two dll's we should care about. I can add a comment to the function warning devs that the parameter is ignored, and the two dlls that are loaded are hard-coded.

TLDR: I'll replace dll with entry point, that is correct. But InitialzieInternal shouldn't care about user dlls, the dlls that are loaded should be hard-coded into the internal function itself.


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

bool IsEnabled() const { return DxCompilerSupport.IsEnabled(); }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need to check both dlls here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think so. It is legal for this object to be initialized with no env var, and thus no dxil.dll loaded. So the 2nd dll won't be enabled.


bool GetCreateInstanceProcs(DxcCreateInstanceProc *pCreateFn,
DxcCreateInstance2Proc *pCreateFn2) const;
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is this for? Is it used for anything?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It was originally intended just to be defined for the SpecificDllLoader so that the DxcDllExtValidationLoader can access the creation functions. So I'll remove it from DxcDllExtValidationLoader and the interface.


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

HMODULE Detach() {
// Can't Detach and return a handle for DxilSupport. Cleanup() instead.
DxilExtValSupport.Cleanup();
return DxCompilerSupport.Detach();
}
};
} // namespace dxc
85 changes: 66 additions & 19 deletions include/dxc/Support/dxcapi.use.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,52 @@ namespace dxc {
extern const char *kDxCompilerLib;
extern const char *kDxilLib;

// Interface for common dll operations
class DllLoader {
public:
DllLoader() = default;

DllLoader(DllLoader &&other) = default;

virtual ~DllLoader() = default;

virtual HRESULT Initialize() = 0;

virtual HRESULT InitializeForDll(LPCSTR dll, LPCSTR entryPoint) = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be nice if we could remove Initialize() from the interface here and force the creator of the object to initialize it themselves. There are a couple of places that might be a bit awkward to update to do that, but I think it'd be cleaner if users of this class couldn't just arbitrarily reinitialize it in a way that could very much be confusing.

InitializeForDll would be harder to get rid of, but if we were to rename it to overrideDll or something like that I think it would make it clearer that it's there as an explicit hook to override which Dll you're using.


template <typename TInterface>
HRESULT CreateInstance(REFCLSID clsid, TInterface **pResult) {
return CreateInstance(clsid, __uuidof(TInterface), (IUnknown **)pResult);
}

virtual HRESULT CreateInstance(REFCLSID clsid, REFIID riid,
IUnknown **pResult) = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would simplify things to implement this whole overload set and then add a protected function for the implementation, ie:

protected:
  virtual HRESULT CreateInstanceImpl(REFCLSID clsid, REFIID riid,
                                     IUnknown **pResult) = 0;
public:
  template <typename TInterface>
  HRESULT CreateInstance(REFCLSID clsid, TInterface **pResult) {
    return CreateInstanceImpl(clsid, __uuidof(TInterface),
                              (IUnknown **)pResult);
  }
  HRESULT CreateInstance(REFCLSID clsid, REFIID riid,
                         IUnknown **pResult) {
    return CreateInstanceImpl(clsid, riid, (IUnknown **)pResult);
  }

This also applies to CreateInstance2


template <typename TInterface>
HRESULT CreateInstance2(IMalloc *pMalloc, REFCLSID clsid,
TInterface **pResult) {
return CreateInstance2(pMalloc, clsid, __uuidof(TInterface),
(IUnknown **)pResult);
}

virtual HRESULT CreateInstance2(IMalloc *pMalloc, REFCLSID clsid, REFIID riid,
IUnknown **pResult) = 0;

virtual bool HasCreateWithMalloc() const = 0;

virtual bool IsEnabled() const = 0;

virtual bool
GetCreateInstanceProcs(DxcCreateInstanceProc *pCreateFn,
DxcCreateInstance2Proc *pCreateFn2) const = 0;

virtual void Cleanup() = 0;
virtual HMODULE Detach() = 0;
};

// Helper class to dynamically load the dxcompiler or a compatible libraries.
class DxcDllSupport {
protected:
class SpecificDllLoader : public DllLoader {

HMODULE m_dll;
DxcCreateInstanceProc m_createFn;
DxcCreateInstance2Proc m_createFn2;
Expand Down Expand Up @@ -74,9 +117,10 @@ class DxcDllSupport {
}

public:
DxcDllSupport() : m_dll(nullptr), m_createFn(nullptr), m_createFn2(nullptr) {}
SpecificDllLoader()
: m_dll(nullptr), m_createFn(nullptr), m_createFn2(nullptr) {}

DxcDllSupport(DxcDllSupport &&other) {
SpecificDllLoader(SpecificDllLoader &&other) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this move constructor actually used? I would expect that we pass these around exclusively as pointers (and usually pointers to the interface class)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There is an instance here:
include/dxc/Test/CompilationResult.h

Copy link
Member

Choose a reason for hiding this comment

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

In HlslIntellisenseSupport? That instance isn't used either - so both the HlslIntellisenseSupport and SpecificDllLoader move constructors should be removed.

(I'm a bit surprised that this compiles as is, since we've deleted the base class's copy and move constructors)

Copy link
Member

Choose a reason for hiding this comment

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

This move constructor should be removed.

m_dll = other.m_dll;
other.m_dll = nullptr;
m_createFn = other.m_createFn;
Expand All @@ -85,7 +129,7 @@ class DxcDllSupport {
other.m_createFn2 = nullptr;
}

~DxcDllSupport() { Cleanup(); }
~SpecificDllLoader() { 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 •
edited
Loading

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.

Expand All @@ -95,11 +139,9 @@ class DxcDllSupport {
return InitializeInternal(dll, entryPoint);
}

template <typename TInterface>
HRESULT CreateInstance(REFCLSID clsid, TInterface **pResult) {
return CreateInstance(clsid, __uuidof(TInterface), (IUnknown **)pResult);
}

// Also bring visibility into the interface definition of this function
// which takes 2 args
using DllLoader::CreateInstance;
HRESULT CreateInstance(REFCLSID clsid, REFIID riid, IUnknown **pResult) {
if (pResult == nullptr)
return E_POINTER;
Expand All @@ -109,13 +151,9 @@ class DxcDllSupport {
return hr;
}

template <typename TInterface>
HRESULT CreateInstance2(IMalloc *pMalloc, REFCLSID clsid,
TInterface **pResult) {
return CreateInstance2(pMalloc, clsid, __uuidof(TInterface),
(IUnknown **)pResult);
}

// Also bring visibility into the interface definition of this function
// which takes 3 args
using DllLoader::CreateInstance2;
HRESULT CreateInstance2(IMalloc *pMalloc, REFCLSID clsid, REFIID riid,
IUnknown **pResult) {
if (pResult == nullptr)
Expand All @@ -132,6 +170,15 @@ class DxcDllSupport {

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

bool GetCreateInstanceProcs(DxcCreateInstanceProc *pCreateFn,
DxcCreateInstance2Proc *pCreateFn2) const {
if (pCreateFn == nullptr || pCreateFn2 == nullptr || m_createFn == nullptr)
return false;
*pCreateFn = m_createFn;
*pCreateFn2 = m_createFn2;
return true;
}

void Cleanup() {
if (m_dll != nullptr) {
m_createFn = nullptr;
Expand Down Expand Up @@ -162,8 +209,8 @@ 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(DxcDllSupport &dxcSupport);
void ReadFileIntoBlob(DxcDllSupport &dxcSupport, LPCWSTR pFileName,
void EnsureEnabled(SpecificDllLoader &dxcSupport);
void ReadFileIntoBlob(SpecificDllLoader &dxcSupport, LPCWSTR pFileName,
Copy link
Collaborator

Choose a reason for hiding this comment

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

These should be IDllLoader

IDxcBlobEncoding **ppBlobEncoding);
void WriteBlobToConsole(IDxcBlob *pBlob, DWORD streamType = STD_OUTPUT_HANDLE);
void WriteBlobToFile(IDxcBlob *pBlob, LPCWSTR pFileName, UINT32 textCodePage);
Expand Down
4 changes: 2 additions & 2 deletions include/dxc/Test/CompilationResult.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,11 @@ class TrivialDxcUnsavedFile : public IDxcUnsavedFile {
}
};

class HlslIntellisenseSupport : public dxc::DxcDllSupport {
class HlslIntellisenseSupport : public dxc::SpecificDllLoader {
public:
HlslIntellisenseSupport() {}
HlslIntellisenseSupport(HlslIntellisenseSupport &&other)
: dxc::DxcDllSupport(std::move(other)) {}
: dxc::SpecificDllLoader(std::move(other)) {}

HRESULT CreateIntellisense(IDxcIntelliSense **pResult) {
return CreateInstance(CLSID_DxcIntelliSense, pResult);
Expand Down
54 changes: 27 additions & 27 deletions include/dxc/Test/DxcTestUtils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,11 @@ class FileRunCommandPart {
FileRunCommandPart(const FileRunCommandPart &) = default;
FileRunCommandPart(FileRunCommandPart &&) = default;

FileRunCommandResult Run(dxc::DxcDllSupport &DllSupport,
FileRunCommandResult Run(dxc::SpecificDllLoader &DllSupport,
const FileRunCommandResult *Prior,
PluginToolsPaths *pPluginToolsPaths = nullptr,
LPCWSTR dumpName = nullptr);
FileRunCommandResult RunHashTests(dxc::DxcDllSupport &DllSupport);
FileRunCommandResult RunHashTests(dxc::SpecificDllLoader &DllSupport);

FileRunCommandResult ReadOptsForDxc(hlsl::options::MainArgs &argStrings,
hlsl::options::DxcOpts &Opts,
Expand All @@ -127,30 +127,30 @@ class FileRunCommandPart {
private:
FileRunCommandResult RunFileChecker(const FileRunCommandResult *Prior,
LPCWSTR dumpName = nullptr);
FileRunCommandResult RunDxc(dxc::DxcDllSupport &DllSupport,
FileRunCommandResult RunDxc(dxc::SpecificDllLoader &DllSupport,
const FileRunCommandResult *Prior);
FileRunCommandResult RunDxv(dxc::DxcDllSupport &DllSupport,
FileRunCommandResult RunDxv(dxc::SpecificDllLoader &DllSupport,
const FileRunCommandResult *Prior);
FileRunCommandResult RunOpt(dxc::DxcDllSupport &DllSupport,
FileRunCommandResult RunOpt(dxc::SpecificDllLoader &DllSupport,
const FileRunCommandResult *Prior);
FileRunCommandResult RunListParts(dxc::DxcDllSupport &DllSupport,
FileRunCommandResult RunListParts(dxc::SpecificDllLoader &DllSupport,
const FileRunCommandResult *Prior);
FileRunCommandResult RunD3DReflect(dxc::DxcDllSupport &DllSupport,
FileRunCommandResult RunD3DReflect(dxc::SpecificDllLoader &DllSupport,
const FileRunCommandResult *Prior);
FileRunCommandResult RunDxr(dxc::DxcDllSupport &DllSupport,
FileRunCommandResult RunDxr(dxc::SpecificDllLoader &DllSupport,
const FileRunCommandResult *Prior);
FileRunCommandResult RunLink(dxc::DxcDllSupport &DllSupport,
FileRunCommandResult RunLink(dxc::SpecificDllLoader &DllSupport,
const FileRunCommandResult *Prior);
FileRunCommandResult RunTee(const FileRunCommandResult *Prior);
FileRunCommandResult RunXFail(const FileRunCommandResult *Prior);
FileRunCommandResult RunDxilVer(dxc::DxcDllSupport &DllSupport,
FileRunCommandResult RunDxilVer(dxc::SpecificDllLoader &DllSupport,
const FileRunCommandResult *Prior);
FileRunCommandResult RunDxcHashTest(dxc::DxcDllSupport &DllSupport);
FileRunCommandResult RunDxcHashTest(dxc::SpecificDllLoader &DllSupport);
FileRunCommandResult RunFromPath(const std::string &path,
const FileRunCommandResult *Prior);
FileRunCommandResult RunFileCompareText(const FileRunCommandResult *Prior);
#ifdef _WIN32
FileRunCommandResult RunFxc(dxc::DxcDllSupport &DllSupport,
FileRunCommandResult RunFxc(dxc::SpecificDllLoader &DllSupport,
const FileRunCommandResult *Prior);
#endif

Expand All @@ -175,12 +175,12 @@ class FileRunTestResult {
PluginToolsPaths *pPluginToolsPaths = nullptr,
LPCWSTR dumpName = nullptr);
static FileRunTestResult
RunFromFileCommands(LPCWSTR fileName, dxc::DxcDllSupport &dllSupport,
RunFromFileCommands(LPCWSTR fileName, dxc::SpecificDllLoader &dllSupport,
PluginToolsPaths *pPluginToolsPaths = nullptr,
LPCWSTR dumpName = nullptr);
};

void AssembleToContainer(dxc::DxcDllSupport &dllSupport, IDxcBlob *pModule,
void AssembleToContainer(dxc::SpecificDllLoader &dllSupport, IDxcBlob *pModule,
IDxcBlob **pContainer);
std::string BlobToUtf8(IDxcBlob *pBlob);
std::wstring BlobToWide(IDxcBlob *pBlob);
Expand All @@ -195,35 +195,35 @@ bool CheckMsgs(const LPCSTR pText, size_t TextCount, const LPCSTR *pErrorMsgs,
size_t errorMsgCount, bool bRegex);
bool CheckNotMsgs(const LPCSTR pText, size_t TextCount,
const LPCSTR *pErrorMsgs, size_t errorMsgCount, bool bRegex);
void GetDxilPart(dxc::DxcDllSupport &dllSupport, IDxcBlob *pProgram,
void GetDxilPart(dxc::SpecificDllLoader &dllSupport, IDxcBlob *pProgram,
IDxcBlob **pDxilPart);
std::string DisassembleProgram(dxc::DxcDllSupport &dllSupport,
std::string DisassembleProgram(dxc::SpecificDllLoader &dllSupport,
IDxcBlob *pProgram);
void SplitPassList(LPWSTR pPassesBuffer, std::vector<LPCWSTR> &passes);
void MultiByteStringToBlob(dxc::DxcDllSupport &dllSupport,
void MultiByteStringToBlob(dxc::SpecificDllLoader &dllSupport,
const std::string &val, UINT32 codePoint,
IDxcBlob **ppBlob);
void MultiByteStringToBlob(dxc::DxcDllSupport &dllSupport,
void MultiByteStringToBlob(dxc::SpecificDllLoader &dllSupport,
const std::string &val, UINT32 codePoint,
IDxcBlobEncoding **ppBlob);
void Utf8ToBlob(dxc::DxcDllSupport &dllSupport, const std::string &val,
void Utf8ToBlob(dxc::SpecificDllLoader &dllSupport, const std::string &val,
IDxcBlob **ppBlob);
void Utf8ToBlob(dxc::DxcDllSupport &dllSupport, const std::string &val,
void Utf8ToBlob(dxc::SpecificDllLoader &dllSupport, const std::string &val,
IDxcBlobEncoding **ppBlob);
void Utf8ToBlob(dxc::DxcDllSupport &dllSupport, const char *pVal,
void Utf8ToBlob(dxc::SpecificDllLoader &dllSupport, const char *pVal,
IDxcBlobEncoding **ppBlob);
void WideToBlob(dxc::DxcDllSupport &dllSupport, const std::wstring &val,
void WideToBlob(dxc::SpecificDllLoader &dllSupport, const std::wstring &val,
IDxcBlob **ppBlob);
void WideToBlob(dxc::DxcDllSupport &dllSupport, const std::wstring &val,
void WideToBlob(dxc::SpecificDllLoader &dllSupport, const std::wstring &val,
IDxcBlobEncoding **ppBlob);
void VerifyCompileOK(dxc::DxcDllSupport &dllSupport, LPCSTR pText,
void VerifyCompileOK(dxc::SpecificDllLoader &dllSupport, LPCSTR pText,
LPCWSTR pTargetProfile, LPCWSTR pArgs,
IDxcBlob **ppResult);
void VerifyCompileOK(dxc::DxcDllSupport &dllSupport, LPCSTR pText,
void VerifyCompileOK(dxc::SpecificDllLoader &dllSupport, LPCSTR pText,
LPCWSTR pTargetProfile, std::vector<LPCWSTR> &args,
IDxcBlob **ppResult);

HRESULT GetVersion(dxc::DxcDllSupport &DllSupport, REFCLSID clsid,
HRESULT GetVersion(dxc::SpecificDllLoader &DllSupport, REFCLSID clsid,
unsigned &Major, unsigned &Minor);
bool ParseTargetProfile(llvm::StringRef targetProfile,
llvm::StringRef &outStage, unsigned &outMajor,
Expand All @@ -240,7 +240,7 @@ class VersionSupportInfo {

VersionSupportInfo();
// Initialize version info structure. TODO: add device shader model support
void Initialize(dxc::DxcDllSupport &dllSupport);
void Initialize(dxc::SpecificDllLoader &dllSupport);
// Return true if IR sensitive test should be skipped, and log comment
bool SkipIRSensitiveTest();
// Return true if test requiring DXIL of given version should be skipped, and
Expand Down
1 change: 1 addition & 0 deletions lib/DxcSupport/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ add_llvm_library(LLVMDxcSupport
WinAdapter.cpp
WinIncludes.cpp
WinFunctions.cpp
dxcapi.extval.cpp
)

#generate header with platform-specific library name
Expand Down
7 changes: 4 additions & 3 deletions lib/DxcSupport/HLSLOptions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1383,9 +1383,10 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude,
return 0;
}

/// Sets up the specified DxcDllSupport instance as per the given options.
int SetupDxcDllSupport(const DxcOpts &opts, dxc::DxcDllSupport &dxcSupport,
llvm::raw_ostream &errors) {
/// Sets up the specified SpecificDllLoader instance as per the given options.
int SetupSpecificDllLoader(const DxcOpts &opts,
dxc::SpecificDllLoader &dxcSupport,
llvm::raw_ostream &errors) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this rename is kind of confusing - the type of the object isn't what's interesting here, we're actually modifying how our dxcompiler.dll is being loaded in particular. In any case, this should probably use an IDxcLoader as I've suggested overall.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, in some cases this should be more abstracted. However, in cases like ShaderOpTest.cpp, where an assignment is being made directly to a member variable that has been defined to be a SpecificDllLoader, I think it makes sense to have the set up function be accurate, and SetSpecificDllLoader

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't understand this comment - this function is never called in ShaderOpTest.cpp - it's only used in dxc.cpp, dxr.cpp, and the test in dxc_batch.cpp. All of these are dealing with dxcompiler.dll in particular and not just some arbitrary "specific dll"

if (!opts.ExternalLib.empty()) {
DXASSERT(!opts.ExternalFn.empty(), "else ReadDxcOpts should have failed");
HRESULT hrLoad = dxcSupport.InitializeForDll(opts.ExternalLib.data(),
Expand Down
Loading