Make DXC and DXV use new `DxcDllExtValidationLoader` to load external validator before validation. #7749

bob80905 · 2025-09-09T23:19:54Z

This PR focuses on allowing dxc to use the existing infrastructure to use a DxcDllExtValidationLoader object, and load an external dxil.dll with it, via the environment variable. The validate*.test was added to verify that the dll is indeed being loaded and used for external validation, which causes a failure, since the dll's validator version is older than the required minimum validator version that the metadata indicates.

github-actions · 2025-09-13T00:32:40Z

✅ With the latest revision this PR passed the C/C++ code formatter.

include/dxc/Support/dxcapi.extval.h

lib/DxcSupport/dxcapi.extval.cpp

V-FEXrt · 2025-09-18T20:21:47Z

lib/DxcSupport/dxcapi.extval.cpp

+  UINT32 newArgCount = ArgCount + 1;
+  std::vector<LPCWSTR> newArgs;
+  newArgs.reserve(newArgCount);
+
+  // Copy existing args
+  for (unsigned int i = 0; i < ArgCount; ++i) {
+    newArgs.push_back(pArguments[i]);
+  }


nit: I think rewriting this as below is equally performant and certainly cleaner

Suggested change

UINT32 newArgCount = ArgCount + 1;

std::vector<LPCWSTR> newArgs;

newArgs.reserve(newArgCount);

// Copy existing args

for (unsigned int i = 0; i < ArgCount; ++i) {

newArgs.push_back(pArguments[i]);

}

std::vector<LPCWSTR> newArgs = {pArguments, pArguments + ArgCount};

lib/DxcSupport/dxcapi.extval.cpp

V-FEXrt · 2025-09-18T21:31:59Z

lib/DxcSupport/dxcapi.extval.cpp

+  if (pResult == nullptr)
+    return E_POINTER;


nit:

Suggested change

if (pResult == nullptr)

return E_POINTER;

if (!pResult)

return E_POINTER;

lib/DxcSupport/dxcapi.extval.cpp

V-FEXrt · 2025-09-18T21:44:24Z

lib/DxcSupport/dxcapi.extval.cpp

+      hr = evh->QueryInterface(riid, reinterpret_cast<void **>(pResult));
+      return hr;
+
+    } else if (clsid == CLSID_DxcValidator) {


nit: you can delete the else here. Might even be worth moving the entire if to the top of this block since its quite short comparted to the other one

V-FEXrt · 2025-09-18T21:44:45Z

lib/DxcSupport/dxcapi.extval.cpp

                                                       IUnknown **pResult) {
-  if (DxilExtValSupport.IsEnabled() && clsid == CLSID_DxcValidator)
-    return DxilExtValSupport.CreateInstance2(pMalloc, clsid, riid, pResult);
+  if (pResult == nullptr)


nit:

Suggested change

if (pResult == nullptr)

if (!pResult)

lib/DxcSupport/dxcapi.extval.cpp

tools/clang/test/CMakeLists.txt

tools/clang/test/LitDXILValidation/validate_1_6_2112.hlsl

V-FEXrt · 2025-09-18T22:01:15Z

tools/clang/tools/dxclib/dxc.cpp

+            if (DXC_FAILED(ValHR)) {
+              if (SUCCEEDED(pCompileResult->QueryInterface(&pResult)))


nit: it looks like these can be a single if statement

lib/DxcSupport/dxcapi.extval.cpp

tools/clang/tools/dxclib/dxc.cpp

lib/DxcSupport/dxcapi.extval.cpp

tools/clang/tools/dxclib/dxc.cpp

lib/DxcSupport/dxcapi.extval.cpp

tools/clang/tools/dxclib/dxc.cpp

utils/hct/hcttest.cmd

lib/DxcSupport/dxcapi.extval.cpp

tools/clang/tools/dxclib/dxc.cpp

tex3d · 2025-09-22T16:43:12Z

lib/DxcSupport/dxcapi.extval.cpp

+// validation scenario, so there is no point in also running
+// the internal validator. This wrapper wraps both the
+// IDxcCompiler and IDxcCompiler3 interfaces.
+class ExternalValidationHelper : public IDxcCompiler, public IDxcCompiler3 {


For one, you'll need to wrap all the interfaces that the compiler object implements so you don't lose the object wrapping for any of them.

But also, IDxcCompiler and IDxcCompiler3 should be incompatible to derive from and implement in the same class due to the nature of COM not supporting separate overloaded methods of the same name. This requires a separate object to implement the other interfaces.

So to be clear, are you suggesting one parent class, ExternalValidationHelper, that implements 4 interfaces and has 4 wrapper member objects, that each implement one interface: IDxcCompiler, IDxcCompiler2, IDxcCompiler3, and IDxcValidator? Your comment above, "This code should all be inside the compiler object wrapper.", makes me think that this class should also implement the IDxcValidator interface, is that correct?

But also, IDxcCompiler and IDxcCompiler3 should be incompatible to derive from and implement in the same class due to the nature of COM not supporting separate overloaded methods of the same name.

It's lucky that IDxcCompiler and IDxcCompiler3 don't actually have any identical methods in them so that this isn't a problem.

IDxcCompiler::Compile takes 10 args, while IDxcCompiler3::Compile takes 7
Preprocess only exists on IDxcCompiler
IDxcCompiler::Disassemble takes 2 args while IDxcCompiler3::Disassemble take 3.

If you only try and implement one of these when you derive from both interfaces you'll get a warning about hiding the base class version.

Interesting. I thought it was not allowed, hence the hoops we jumped through multiple times for multiple implementations of the compiler object supporting both interfaces to avoid deriving from both on the same implementation class.

In any case the first point stands, that there are more interfaces to support than these two.

You'll want to derive from each of these:

IDxcCompiler2

IDxcCompiler3

IDxcLangExtensions3

IDxcContainerEvent

IDxcVersionInfo3

IDxcVersionInfo2 (or IDxcVersionInfo if SUPPORT_QUERY_GIT_COMMIT_INFO not defined)

And no need to derive from IDxcCompiler when deriving from IDxcCompiler2, since IDxcCompiler2 already derives from IDxcCompiler.

The implementation in this PR will cause failures if any of the tests attempt to use these interfaces. I don't think we need to wrap additional ones for the sake of it.

Like Damyan said, I think I'll worry about deriving from each of those in a future PR.

lib/DxcSupport/dxcapi.extval.cpp

tools/clang/tools/dxclib/dxc.cpp

lib/DxcSupport/dxcapi.extval.cpp

tex3d

There are quite a few issues, and as I tried to write something that could express the exact logic necessary for the arguments, I started rewriting the ExternalValidationHelper implementation to also fix many of the other issues. I put a branch up, so here's the link to the commit:

tex3d@08bacdb

See the commit for the general list of issues this rewrite fixes.

lib/DxcSupport/dxcapi.extval.cpp

include/dxc/Support/Global.h

lib/DxcSupport/dxcapi.extval.cpp

… fix bug

…ions

damyanp

I think this still looks good to me. It's very hard for me to review, because the rebase means that I can't easily tell what changed since my last review. Since we squash all our commits into main there's no real benefit to rebasing rather than merging with main.

V-FEXrt · 2025-10-02T22:13:21Z

lib/DxcSupport/dxcapi.extval.cpp

+      if (Result)
+        return Result->QueryInterface(Riid, ValResult);
+      else
+        return E_FAIL;


nit:

Suggested change

if (Result)

return Result->QueryInterface(Riid, ValResult);

else

return E_FAIL;

if (!Result)

return E_FAIL;

return Result->QueryInterface(Riid, ValResult);

tools/clang/test/LitDXILValidation/validate_1_6_2112.hlsl

alsepkow

Submitting a block of comments while I continue to review.

alsepkow · 2025-10-03T00:53:05Z

lib/DxcSupport/dxcapi.extval.cpp

+      if (Result)
+        return Result->QueryInterface(Riid, ValResult);
+      else
+        return E_FAIL;


I suggest returning E_POINTER as I think that's what this HRESULT is intended to reflect?
That is, we expect that the caller gives us a valid IDxcOperationResult*.

alsepkow · 2025-10-03T01:02:58Z

lib/DxcSupport/dxcapi.extval.cpp

+
+    // No validation needed; just set the result and return.
+    if (!NeedsValidation)
+      return UseResult(CompileResult);


The UseResult name seems misleading to me? It really looks like it's just a glorified QueryInterface call?

I think it would be cleaner to

Remove the UseResult lambda

Add a check to return E_INVALID_ARG if CompileResult is an invalid pointer at the start of doValidation.

Update line 59 to 'return CompileResult->QueryInterface(Riid, ValResuilt);

Same update to line 67.

Same update for line 83.

Same update on line 87.

alsepkow · 2025-10-03T01:23:46Z

lib/DxcSupport/dxcapi.extval.cpp

+      if (ValidatorVersionInfo->GetVersion(&ValidatorVersionMajor,
+                                           &ValidatorVersionMinor)) {


Suggested change

if (ValidatorVersionInfo->GetVersion(&ValidatorVersionMajor,

&ValidatorVersionMinor)) {

if (FAILED(ValidatorVersionInfo->GetVersion(&ValidatorVersionMajor,

&ValidatorVersionMinor))) {

lib/DxcSupport/dxcapi.extval.cpp

alsepkow · 2025-10-03T01:28:09Z

lib/DxcSupport/dxcapi.extval.cpp

+#endif
+                           !Opts.DumpDependencies && !Opts.VerifyDiagnostics &&
+                           Opts.Preprocess.empty();
+    bool ProduceFullContainer = ProduceDxModule && !Opts.CodeGenHighLevel;


Suggested change

bool ProduceFullContainer = ProduceDxModule && !Opts.CodeGenHighLevel;

const bool ProduceFullContainer = ProduceDxModule && !Opts.CodeGenHighLevel;

alsepkow · 2025-10-03T17:46:09Z

lib/DxcSupport/dxcapi.extval.cpp

+      return DoBasicQueryInterface<IDxcCompiler, IDxcCompiler2, IDxcCompiler3>(
+          NewWrapper.p, Iid, ResultObject);
+    } catch (...) {
+      return E_FAIL;


I think the exception expected here would be due to an allocation failure.
So would ERROR_OUTOFMEMORY be a more appropriate HRESULT?

alsepkow · 2025-10-03T17:47:54Z

lib/DxcSupport/dxcapi.extval.cpp

+      if (ValidatorVersionInfo->GetVersion(&ValidatorVersionMajor,
+                                           &ValidatorVersionMinor)) {
+        DXASSERT(false, "Failed to get validator version");
+        return E_FAIL;


nit: I like to try and avoid E_FAIL because its pretty generic.
There is an 'ERROR_VERSION_PARSE_ERROR' you could use here.

Gonna stick to E_FAIL since there is no definition for that Error code in non-windows.
Same applies to ERROR_OUTOFMEMORY

lib/DxcSupport/dxcapi.extval.cpp

alsepkow · 2025-10-03T18:10:27Z

lib/DxcSupport/dxcapi.extval.cpp

+// The ExtValidationArgHelper class helps manage arguments for external
+// validation, as well as perform the validation step if needed.
+class ExtValidationArgHelper {
+  std::vector<std::wstring> ArgStorage;


I think we generally do all publics first and then all privates second?
Should we move the member variables to the private section?

alsepkow · 2025-10-03T18:10:59Z

lib/DxcSupport/dxcapi.extval.cpp

+// IDxcValidator interface to validate a successful compilation result, when
+// validation would normally be performed.
+class ExternalValidationCompiler : public IDxcCompiler2, public IDxcCompiler3 {
+private:


nit: public first for consistency with the rest of this file

Actually the reason this broke that consistency is because there's a special rule in linux / macos builds on initialization order, and the Compiler object can't be initialized after the m_pAlloc field, which was originally taken care of in the previous private block. So I will leave this as is.

there's a special rule in linux / macos builds on initialization order

There's not a special rule, it's just that MSVC doesn't warn if you mismatch the order that things appear in the constructor's initialization list with the order that they are actually initialized (ie the order they appear in the class itself).

If you wanted to put the private block at the end then you'd just need to update the constructor to list m_pMalloc first (since it'd be listed first in the class as the field is added by the DXC_MICROCOM_TM_REF_FIELDS macro).

Or you could have your private block at the end look like this and not need to modify the constructor:

private: CComPtr<IDxcValidator> Validator; IID CompilerIID; CComPtr<IUnknown> Compiler; DXC_MICROCOM_TM_REF_FIELDS()

lib/DxcSupport/dxcapi.extval.cpp

…go in

alsepkow · 2025-10-06T22:14:05Z

lib/DxcSupport/dxcapi.extval.cpp

+    CComPtr<IDxcBlob> CompiledBlob;
+    IFR(CompileResult->GetResult(&CompiledBlob));
+
+    // If no compiled blob; just return the compile result.


nit: comment doesn't seem accurate. We're returning the HRESULT of QI. That doesn't seem like a compile result?

This language is a bit overloaded, I meant return through an out-parameter, by placing the result into ValResult.
I can rewrite this.

The main thing that threw me off is that you wouldn't generally expect a QI call to do anything other than get you a pointer to the requested interface. The fact that the private implementation of QI is doing more than that is a little weird.

alsepkow · 2025-10-06T22:24:02Z

lib/DxcSupport/dxcapi.extval.cpp

+    // Return the validation result if it failed.
+    HRESULT HR;
+    IFR(TempValidationResult->GetStatus(&HR));
+    if (FAILED(HR)) {


I see uses of DXC_FAILED and FAILED. Hadn't seen DXC_FAILED before but it looks like they do the same thing.
We should just use one.

alsepkow · 2025-10-06T22:42:21Z

lib/DxcSupport/dxcapi.extval.cpp

+                                 !Opts.VerifyDiagnostics &&
+                                 Opts.Preprocess.empty();
+    const bool ProduceFullContainer = ProduceDxModule && !Opts.CodeGenHighLevel;
+    NeedsValidation = ProduceFullContainer && !Opts.DisableValidation;


nit: I found it a little confusing to follow the logic for toggling NeedsValidation at first. Wondering if it might be a little cleaner and more readable if you add a helper function that toggles it for you.

I think I want to leave it as is. There are intermediate variables that are used to determine whether or not to modify the new args, and that dependency would reach beyond the scope of the new helper function if I make a new one.

ProduceDxModule could be turned into a const accessor method on DxcOpts, then dxcompilerobj.cpp could use that instead of keeping these duplicated blocks in sync across projects. The ProduceFullContainer is simpler based on that one and could be separate, or it could also be const accessors that use the ProduceDxModule one.

NeedsValidation could also have an accessor, and could encode the root signature logic below, using the new ShaderModel::ParseTargetProfile method merged from your recently merged PR. DxcOpts already has a IsRootSignatureProfile() for initializing the RootSignatureOnly flag, though I don't think that flag is used here, so it could be removed instead.

I agree with @alsepkow, it would be cleaner, plus locating it in DxcOpts makes more sense.

tex3d · 2025-10-10T19:13:19Z

lib/DxcSupport/dxcapi.extval.cpp

+  // correct for the method call.
+  // This will either be casting to the original interface retrieved by
+  // QueryInterface, or to one from which that interface derives.
+  template <typename T> T *castSafe() const {


I don't see how this, as it is, adds any value.

If we want safe casting, we could use a template method specialization approach for base interfaces to try casting to derived interfaces first.

Here's a safe casting pattern using the castSafe method:

// A recursive cast using specializations to handle derived interfaces. template <typename T> T *cast() const { return castSafe<T>(); } // Specialize cast to recurse into derived interfaces. template <> IDxcCompiler *cast<IDxcCompiler>() const { if (IDxcCompiler2 *Result = cast<IDxcCompiler2>()) return Result; return castSafe<IDxcCompiler>(); }

Now you can use cast<Interface>() instead of castUnsafe<Interface>() for all cases.

tex3d · 2025-10-10T21:01:46Z

tools/clang/test/LitDXILValidation/validate_1_6_2112.hlsl

@@ -0,0 +1,22 @@
+// REQUIRES: v1.6.2112


This string seems too generic for identifying an external validator version. Shouldn't it be more like "dxilval-v1.6.2112" or something like that?

tex3d · 2025-10-10T21:12:31Z

tools/clang/test/LitDXILValidation/validate_1_6_2112.hlsl

+// CHECK: error: RWStructuredBuffers may increment or decrement their counters, but not both.
+// CHECK: note: at '%3 = call i32 @dx.op.bufferUpdateCounter(i32 70, %dx.types.Handle %1, i8 1)' in block '#0' of function 'main'.
+// CHECK: Validation failed.
+
+RWStructuredBuffer<uint> UAV         : register(u0);
+
+[numthreads(64, 1, 1)]
+void main(uint tid : SV_GroupIndex) {
+  UAV[UAV.IncrementCounter()] = tid;
+  UAV[UAV.DecrementCounter()] = tid * 2;
+}


I thought we discussed the approach before and you were going to use a different approach, where you have a simple valid shader, with the explicit validator version that's too high by 1. That would match the comments, but this shader still wouldn't pass a newer validator due to the increment and decrement, so it doesn't behave the way the comments above describe.

It would be simpler and clearer if you changed to this, I think:

Suggested change

// CHECK: error: RWStructuredBuffers may increment or decrement their counters, but not both.

// CHECK: note: at '%3 = call i32 @dx.op.bufferUpdateCounter(i32 70, %dx.types.Handle %1, i8 1)' in block '#0' of function 'main'.

// CHECK: Validation failed.

RWStructuredBuffer<uint> UAV : register(u0);

[numthreads(64, 1, 1)]

void main(uint tid : SV_GroupIndex) {

UAV[UAV.IncrementCounter()] = tid;

UAV[UAV.DecrementCounter()] = tid * 2;

}

[numthreads(1, 1, 1)]

void main() {}

Then it would be clear that the point of the test is to see the matching validator version error, which proves we are using the expected validator.

tex3d · 2025-10-10T21:15:05Z

tools/clang/test/lit.cfg

    config.environment.update({'DXC_DXIL_DLL_PATH' : dxc_dll_path})
    config.available_features.add("dxc_dxil_dll_path")
+    dxc_version = lit_config.params.get('DXC_VERSION', None)
+    config.available_features.add(dxc_version)


What exactly is DXC_VERSION? Why would we add the version value without a clear prefix identifying what it applies to?
If DXC_VERSION is the lit variable for the external validator version, I don't think the name is clear at all. It makes me think it's the version of the dxc driver or compiler instead.

tex3d · 2025-10-10T21:20:03Z

tools/clang/tools/dxclib/dxc.cpp

+  int MaybeRunExternalValidatorAndPrintValidationOutput(
+      const DxcOpts &opts, CComPtr<IDxcOperationResult> pCompileResult);


Is this unused now? Should remove it if so.

tex3d · 2025-10-10T22:55:38Z

tools/clang/tools/dxclib/dxc.cpp

+        // function is deferred to whatever object was chosen to be pCompiler,
+        // which must implement the IDxcCompiler interface. External validation
+        // will only take place if the DXC_DXIL_DLL_PATH env var is set
+        // correctly.


Why is this comment on this one compile call? Why not put a comment where you initialize the DxcDllExtValidationLoader in dxc::main instead?

tex3d · 2025-10-10T23:10:48Z

tools/clang/tools/dxclib/dxc.cpp

-      if (dllResult)
+      std::string dllLogString;
+      llvm::raw_string_ostream dllErrorStream(dllLogString);
+      HRESULT dllResult = dxcSupport.initialize(dllErrorStream);


Personal preference: I really dislike that initialize takes an error stream, instead of communicating some status enum for the caller to translate into an output message if desired.
Something like:

HRESULT dllResult = dxcSupport.initialize(); if (DXC_FAILED(dllResult)) { switch(dxcSupport.getFailureReason()) { // print different messages for different failure modes, // using getDxilDllPath() as necessary. } return dllResult; } if (dxcSupport.isUsingExternalValidator()) { // print message about using getDxilDllPath(). }

tex3d · 2025-10-10T23:13:42Z

tools/clang/tools/dxclib/dxc.cpp

 private:
  DxcOpts &m_Opts;
-  SpecificDllLoader &m_dxcSupport;
+  DxcDllExtValidationLoader &m_dxcSupport;


Shouldn't we use DllLoader instead? Isn't that the point of DllLoader as a base class?

Suggested change

DxcDllExtValidationLoader &m_dxcSupport;

DllLoader &m_dxcSupport;

tex3d · 2025-10-10T23:18:36Z

tools/clang/tools/dxv/dxv.cpp

 class DxvContext {
 private:
-  SpecificDllLoader &m_dxcSupport;
+  DxcDllExtValidationLoader &m_dxcSupport;


Same comment as for dxc.cpp:
Shouldn't we use DllLoader instead? Isn't that the point of DllLoader as a base class?

Suggested change

DxcDllExtValidationLoader &m_dxcSupport;

DllLoader &m_dxcSupport;

tex3d · 2025-10-10T23:37:11Z

lib/DxcSupport/dxcapi.extval.cpp

+                                 !Opts.VerifyDiagnostics &&
+                                 Opts.Preprocess.empty();
+    const bool ProduceFullContainer = ProduceDxModule && !Opts.CodeGenHighLevel;
+    NeedsValidation = ProduceFullContainer && !Opts.DisableValidation;


ProduceDxModule could be turned into a const accessor method on DxcOpts, then dxcompilerobj.cpp could use that instead of keeping these duplicated blocks in sync across projects. The ProduceFullContainer is simpler based on that one and could be separate, or it could also be const accessors that use the ProduceDxModule one.

NeedsValidation could also have an accessor, and could encode the root signature logic below, using the new ShaderModel::ParseTargetProfile method merged from your recently merged PR. DxcOpts already has a IsRootSignatureProfile() for initializing the RootSignatureOnly flag, though I don't think that flag is used here, so it could be removed instead.

I agree with @alsepkow, it would be cleaner, plus locating it in DxcOpts makes more sense.

github-project-automation bot added this to HLSL Roadmap Sep 9, 2025

github-project-automation bot moved this to New in HLSL Roadmap Sep 9, 2025

bob80905 changed the title ~~Make DXC use new DxcDllExtValidationLoader to load external validator before validation.~~ Make DXC and DXV use new DxcDllExtValidationLoader to load external validator before validation. Sep 16, 2025

bob80905 force-pushed the add_extvalsupport_class_to_val_infra branch from 6c69325 to 78cace7 Compare September 16, 2025 23:24

bob80905 marked this pull request as ready for review September 17, 2025 18:21

damyanp reviewed Sep 17, 2025

View reviewed changes

damyanp reviewed Sep 18, 2025

View reviewed changes

lib/DxcSupport/dxcapi.extval.cpp Outdated Show resolved Hide resolved

damyanp reviewed Sep 18, 2025

View reviewed changes

lib/DxcSupport/dxcapi.extval.cpp Outdated Show resolved Hide resolved

damyanp reviewed Sep 18, 2025

View reviewed changes

lib/DxcSupport/dxcapi.extval.cpp Outdated Show resolved Hide resolved

damyanp reviewed Sep 18, 2025

View reviewed changes

lib/DxcSupport/dxcapi.extval.cpp Outdated Show resolved Hide resolved

damyanp reviewed Sep 18, 2025

View reviewed changes

lib/DxcSupport/dxcapi.extval.cpp Outdated Show resolved Hide resolved

V-FEXrt reviewed Sep 18, 2025

View reviewed changes

damyanp requested changes Sep 19, 2025

View reviewed changes

github-project-automation bot moved this from New to In progress in HLSL Roadmap Sep 19, 2025

damyanp requested changes Sep 20, 2025

View reviewed changes

damyanp reviewed Sep 20, 2025

View reviewed changes

lib/DxcSupport/dxcapi.extval.cpp Outdated Show resolved Hide resolved

damyanp reviewed Sep 20, 2025

View reviewed changes

tools/clang/tools/dxclib/dxc.cpp Outdated Show resolved Hide resolved

damyanp reviewed Sep 20, 2025

View reviewed changes

tools/clang/tools/dxclib/dxc.cpp Outdated Show resolved Hide resolved

damyanp reviewed Sep 20, 2025

View reviewed changes

tools/clang/tools/dxclib/dxc.cpp Outdated Show resolved Hide resolved

damyanp reviewed Sep 20, 2025

View reviewed changes

tools/clang/tools/dxclib/dxc.cpp Outdated Show resolved Hide resolved

damyanp reviewed Sep 20, 2025

View reviewed changes

tools/clang/tools/dxclib/dxc.cpp Outdated Show resolved Hide resolved

tex3d reviewed Sep 22, 2025

View reviewed changes

damyanp requested changes Sep 23, 2025

View reviewed changes

tex3d requested changes Sep 24, 2025

View reviewed changes

bob80905 force-pushed the add_extvalsupport_class_to_val_infra branch from c6ac790 to eb69e26 Compare September 24, 2025 05:36

tex3d reviewed Sep 24, 2025

View reviewed changes

lib/DxcSupport/dxcapi.extval.cpp Outdated Show resolved Hide resolved

damyanp reviewed Sep 24, 2025

View reviewed changes

damyanp reviewed Sep 25, 2025

View reviewed changes

lib/DxcSupport/dxcapi.extval.cpp Outdated Show resolved Hide resolved

damyanp requested changes Sep 26, 2025

View reviewed changes

bob80905 added 13 commits October 1, 2025 17:35

address Damyan except for deduplication of createinstance*

be42093

Incorporate Tex's commit / add some comments for self clarification /…

55ccb16

… fix bug

make a use out of m_compilerIID, and fix initializer list order

d5106a5

fix var names in extval.cpp

cc2338a

address tex, fix up safe cast

7680af7

address Damyan, add new macro definition for clear exception descript…

65928a6

…ions

address Damyan, fix function names, remove IFTs in favor of asserts

a4fce30

address Damyan

9a6a9d5

update comment

bac50d2

don't validate if compilation failed

475f8fa

clang format

609ee5a

make sure lit uses specified env to detect validation version

8c857fd

rebase and update test that was missed due to stale dxc checkout

1d8e2ef

bob80905 force-pushed the add_extvalsupport_class_to_val_infra branch from fc32e76 to 1d8e2ef Compare October 2, 2025 00:39

appropriately pass errorcode to exit code

4829520

damyanp approved these changes Oct 2, 2025

View reviewed changes

V-FEXrt approved these changes Oct 2, 2025

View reviewed changes

alsepkow reviewed Oct 3, 2025

View reviewed changes

lib/DxcSupport/dxcapi.extval.cpp Show resolved Hide resolved

bob80905 added 6 commits October 3, 2025 13:03

address Alex

cedc965

revert some non-windows incompatible stuff

b5babf1

Merge branch 'main' into add_extvalsupport_class_to_val_infra

c708f6a

add requires to new test that was added while waiting for this PR to …

17b35b5

…go in

try rearranging public private order

0dabdd1

clang format

f12af78

alsepkow reviewed Oct 6, 2025

View reviewed changes

address Alex

03c1d54

tex3d reviewed Oct 10, 2025

View reviewed changes

		if (DXC_FAILED(ValHR)) {
		if (SUCCEEDED(pCompileResult->QueryInterface(&pResult)))

		if (ValidatorVersionInfo->GetVersion(&ValidatorVersionMajor,
		&ValidatorVersionMinor)) {

	bool ProduceFullContainer = ProduceDxModule && !Opts.CodeGenHighLevel;
	const bool ProduceFullContainer = ProduceDxModule && !Opts.CodeGenHighLevel;

		int MaybeRunExternalValidatorAndPrintValidationOutput(
		const DxcOpts &opts, CComPtr<IDxcOperationResult> pCompileResult);

	DxcDllExtValidationLoader &m_dxcSupport;
	DllLoader &m_dxcSupport;

Make DXC and DXV use new DxcDllExtValidationLoader to load external validator before validation. #7749

Are you sure you want to change the base?

Make DXC and DXV use new DxcDllExtValidationLoader to load external validator before validation. #7749

Uh oh!

Conversation

bob80905 commented Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Sep 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Make DXC and DXV use new `DxcDllExtValidationLoader` to load external validator before validation. #7749

Make DXC and DXV use new `DxcDllExtValidationLoader` to load external validator before validation. #7749

bob80905 commented Sep 9, 2025 •

edited

Loading

github-actions bot commented Sep 13, 2025 •

edited

Loading

tex3d left a comment •

edited

Loading