[DirectX] Write DXIL with debug info to ILDB part by kuilpd · Pull Request #158 · access-softek/llvm-project

kuilpd · 2026-06-02T17:36:56Z

Internal review before creating this PR in the upstream.

hvdijk · 2026-06-02T17:43:12Z

+          for (unsigned I : seq(FlagEntries.size())) {
+            llvm::Module::ModuleFlagEntry &Entry = FlagEntries[I];


I may be missing something but why isn't this simply

Suggested change

for (unsigned I : seq(FlagEntries.size())) {

llvm::Module::ModuleFlagEntry &Entry = FlagEntries[I];

for (llvm::Module::ModuleFlagEntry &Entry : FlagEntries) {

?

hvdijk · 2026-06-02T17:45:18Z

+              continue;
+            }
+            M.addModuleFlag(Entry.Behavior, Entry.Key->getString(),
+                            cast<ConstantAsMetadata>(Entry.Val)->getValue());


Do we know that Entry.Val is always a ConstantAsMetadata?

I've copied this loop from DXC's code, not sure how else to do this.

addModuleFlag has an overload that takes a Metadata* which seems like it should work.

hvdijk · 2026-06-02T17:49:00Z

+        replaceNamedMetadataArray(M, "dx.source.args", {});
+      } else {
+        // If we have an ILDB part, strip DXIL from all debug info.
+        StripDebugInfo(M);


This function returns a bool indicating whether any debug info was stripped, we can use this to avoid checking through debug_compile_units().empty() and get a more reliable answer.

I think we specifically need to strip debug info only if we already know if there is debug info. Is there any other way to know this?

This function returns a bool indicating whether any debug info was stripped, we can use this to avoid checking through debug_compile_units().empty() and get a more reliable answer.

AFAIK if !llvm.dbg.cu is empty, LLVM assumes no debug info is present in the file (see #150 (comment)). Are you aware of cases when we have correct and full LLVM IR with debug info but without compile units?

There is no harm in stripping debug info if there is no debug info, the function will just tell you there was nothing to strip, and it will have done a better search than the current code. So something like std::unique_ptr<Module> Clone = llvm::CloneModule(M); bool HaveDebugInfo = llvm::StripDebugInfo(*Clone);, then write the DXIL section from *Clone (which assuredly has no debug info, regardless of whether there was originally debug info), then if HaveDebugInfo is true, create your second clone and write the ILDB section from that.

There is no harm in stripping debug info if there is no debug info

But it performs an extra pass over the whole module which can be avoided when shader is compiled without debug info.

(Anyway, my opinion on that matter should not block this PR.)

But it performs an extra pass over the whole module which can be avoided when shader is compiled without debug info.

But we don't know that. It may be that it's impossible to get into that situation when compiling HLSL (I haven't checked), but we do not require IR to have come from HLSL, our own tests don't do that, and our own tests already test scenarios that we care about that are impossible to come up from compiling HLSL. It's easy to write IR that calls llvm.dbg.value without defining !llvm.dbg.cu, and it's easy to imagine that some third-party LLVM IR producer actually ends up doing this.

and it's easy to imagine that some third-party LLVM IR producer actually ends up doing this.

Not sure about that when it comes to HLSL, but it definitely shouldn't be like that when the goal is to emit DWARF or CodeView (since their emission will just not run).

The thing is, that if we have no compile units, but still have debug info nodes, it may be an issue somewhere in LLVM IR producers, or in an LLVM IR pass which runs before DXILWriterPass. By calling strip on such IRs, DXILWriterPass will just hide such an issue.

It's easy to write IR that calls llvm.dbg.value without defining !llvm.dbg.cu

It's not that easy to produce such LLVM IR and get a reasonable result, because

If a function has no #dbg !DISubprogram attachment, it will cause problems with many passes. Particularly, LiveDebugVariables will drop debug intrinsics from MIR generated for this function. (Yes, MIR is not part of DX pipeline, but it is for most of other targets).

Definition DISubprograms must have compile unit field, which is enforced by Verifier.

The thing is, that if we have no compile units, but still have debug info nodes, it may be an issue somewhere in LLVM IR producers, or in an LLVM IR pass which runs before DXILWriterPass.

It may be, sure, but...

By calling strip on such IRs, DXILWriterPass will just hide such an issue.

...the issue won't be hidden: it will accurately write a DXIL section without debug info, and an ILDB section with the weird debug info.

I'd be fine with turning this into an assert instead somewhere, but silently leaving debug info in the section that specifically is meant to not have debug info seems wrong, especially if we don't know if there may be legitimate reasons for it that we're just not thinking of now.

Definition DISubprograms must have compile unit field, which is enforced by Verifier.

Note that I didn't say no DICompileUnit, just no !llvm.dbg.cu named metadata. There are loads of LLVM tests that have a DICompileUnit that is not linked through named metadata, a random example is llvm/test/Transforms/Coroutines/declare-value.ll.

Definition DISubprograms must have compile unit field, which is enforced by Verifier.

Note that I didn't say no DICompileUnit, just no !llvm.dbg.cu named metadata. There are loads of LLVM tests that have a DICompileUnit that is not linked through named metadata

Such IRs are definitely incorrect from the Verifier's point of view - see Verifier::verifyCompileUnits() - it discards debug info containing compile units that are not listed in !llvm.dbg.cu,
For the test you referenced:

DICompileUnit not listed in llvm.dbg.cu !4 = distinct !DICompileUnit(language: DW_LANG_Swift, file: !5, isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug) llvm-dis.exe: warning: ignoring invalid debug info in tmp.bc

Tests from llvm\test\Transforms sometimes do not care about that - they run a single pass, not full llc pipeline (and opt silences invalid debug info warning). So it may have happen that there they've been unverified.

By calling strip on such IRs, DXILWriterPass will just hide such an issue.

...the issue won't be hidden: it will accurately write a DXIL section without debug info, and an ILDB section with the weird debug info.

It may be easier to spot a problem when debug info section requested by the user is not created than when it is created but later debug info is discarded by IR consumers. Usually, IR loader does not raise an error if debug info is incorrect - it silently strips it (or it raises a warning if user explicitly invokes llvm-dis, but not an error).

I'd be fine with turning this into an assert instead somewhere, but silently leaving debug info in the section that specifically is meant to not have debug info seems wrong, especially if we don't know if there may be legitimate reasons for it that we're just not thinking of now.

That's definitely worth doing if we leave unconditional Strip call 👍
We could also do it the other way around: detect debug info presence by checking compile units, and add an assert(StripDebugInfo(M) == HasDebugInfo && "This module must not contain anything to be stripped")

dzhidzhoev

llvm/test/CodeGen/DirectX/embed-ildb.ll from dx-debug has lines for testing dx.source presence in output sections:

ILDB-DIS: !dx.source.contents
ILDB-DIS: !dx.source.defines
ILDB-DIS: !dx.source.mainFileName
ILDB-DIS: !dx.source.args
...
+; DXIL-DIS-NOT: !dx.source

Could you please import that lines into this PR?
So that there will be test coverage for changes made to !dx.source processing in this PR.

dzhidzhoev · 2026-06-02T17:38:37Z

    // The DXIL part also writes a program header, so we need to include its
    // size when computing the offset for a part after the DXIL part.
-    if (Sec.getName() == "DXIL")
+    if (Sec.getName() == "DXIL" || Sec.getName() == "ILDB")


Could we use dxbc::isProgramPart()? If it's not merged into mainline, let's do it as well.

dzhidzhoev · 2026-06-02T17:39:28Z

    uint64_t PartSize = SectionSize;

-    if (Sec.getName() == "DXIL")
+    if (Sec.getName() == "DXIL" || Sec.getName() == "ILDB")


dzhidzhoev · 2026-06-02T17:39:35Z

    PartSize = alignTo(PartSize, Align(4));
    W.write<uint32_t>(static_cast<uint32_t>(PartSize));
-    if (Sec.getName() == "DXIL") {
+    if (Sec.getName() == "DXIL" || Sec.getName() == "ILDB") {


dzhidzhoev · 2026-06-02T17:44:23Z

+}
+
 class EmbedDXILPass : public llvm::ModulePass {
+  std::string writeModule(Module &M, bool HasDebugInfo, bool IsDebug) {


Let's rename these variables: HasDebugInfo -> ShouldStripDebug , IsDebug -> WriteDebugPart, or something like that. So that their purpose will be less confusing for readers.

dzhidzhoev · 2026-06-02T17:46:24Z

+      }
+    }
+
+    const auto DIMap = DXILDebugInfoPass::run(M);


Let's move this under under if (IsDebug) above, following the email discussion we had (Andrew's email).

The call to WriteDXILToFile requires a DXILDebugInfoMap argument though, I don't think we can not call it. Doesn't seem to do any harm.

@hvdijk mentioned that we can pass an empty map.

Like this?

const DXILDebugInfoMap DIMap; WriteDXILToFile(M, OS, DIMap);

I tried, it crashes everything.

It should be like

const DXILDebugInfoMap DIMap; ... if (IsDebug) { ... DIMap = DebugInfoPass::run(M); } else { ... // No DebugInfoPass::run(M); } ... WriteDXILToFile(M, OS, DIMap); ... }

Does it crash in such case as well?

This is for test/tools/dxil-dis/di-label.ll:
Before stripping:

target datalayout = "e-m:e-p:32:32-i1:32-i8:8-i16:16-i32:32-i64:64-f16:16-f32:32-f64:64-n8:16:32:64" target triple = "dxil-ms-dx" define void @foo(i32 %i) { entry: tail call void @llvm.dbg.value(metadata i32 %i, i64 0, metadata !12, metadata !14), !dbg !15 br label %label label: ; preds = %entry ret void } ; Function Attrs: nounwind readnone declare void @llvm.dbg.value(metadata, i64, metadata, metadata) #0 ; Function Attrs: nounwind readnone declare void @llvm.dbg.label(metadata) #0 attributes #0 = { nounwind readnone } !llvm.dbg.cu = !{!0} !dx.shaderModel = !{!7} !dx.version = !{!8} !dx.entryPoints = !{!9} !llvm.module.flags = !{!10, !11} !0 = distinct !DICompileUnit(language: DW_LANG_C11, file: !1, isOptimized: false, runtimeVersion: 0, emissionKind: 1, subprograms: !2) !1 = !DIFile(filename: "label.c", directory: "") !2 = !{!3} !3 = !DISubprogram(name: "foo", scope: !1, file: !1, line: 1, type: !4, isLocal: false, isDefinition: true, scopeLine: 1, isOptimized: false, function: void (i32)* @foo, variables: !6) !4 = !DISubroutineType(types: !5) !5 = !{null} !6 = !{} !7 = !{!"lib", i32 6, i32 3} !8 = !{i32 1, i32 3} !9 = !{null, !"", null, null, null} !10 = !{i32 2, !"Dwarf Version", i32 5} !11 = !{i32 2, !"Debug Info Version", i32 3} !12 = !DILocalVariable(tag: DW_TAG_arg_variable, name: "i", arg: 1, scope: !3, file: !1, line: 4, type: !13) !13 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed) !14 = !DIExpression() !15 = !DILocation(line: 1, column: 1, scope: !3)

After only StripDebugInfo(M) (without manually removing "Debug Info Version"):

target datalayout = "e-m:e-p:32:32-i1:32-i8:8-i16:16-i32:32-i64:64-f16:16-f32:32-f64:64-n8:16:32:64" target triple = "dxil-ms-dx" define void @foo(i32 %i) { entry: tail call void @llvm.dbg.value(metadata i32 %i, i64 0, metadata !5, metadata !12) br label %label label: ; preds = %entry ret void } ; Function Attrs: nounwind readnone declare void @llvm.dbg.value(metadata, i64, metadata, metadata) #0 ; Function Attrs: nounwind readnone declare void @llvm.dbg.label(metadata) #0 attributes #0 = { nounwind readnone } !dx.shaderModel = !{!0} !dx.version = !{!1} !dx.entryPoints = !{!2} !llvm.module.flags = !{!3, !4} !0 = !{!"lib", i32 6, i32 3} !1 = !{i32 1, i32 3} !2 = !{null, !"", null, null, null} !3 = !{i32 2, !"Dwarf Version", i32 5} !4 = !{i32 2, !"Debug Info Version", i32 3} !5 = !DILocalVariable(tag: DW_TAG_arg_variable, name: "i", arg: 1, scope: !6, file: !7, line: 4, type: !11) !6 = !DISubprogram(name: "foo", scope: !7, file: !7, line: 1, type: !8, isLocal: false, isDefinition: true, scopeLine: 1, isOptimized: false, variables: !10) !7 = !DIFile(filename: "label.c", directory: "") !8 = !DISubroutineType(types: !9) !9 = !{null} !10 = !{} !11 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed) !12 = !DIExpression()

Both @llvm.dbg.label and @llvm.dbg.value are still there.

Can you try on top of hvdijk/llvm-project@directx-delay-converting-debug-info (which in turn is on top of llvm/llvm-project#198318) ?

It worked for StripDebugInfo, now the same file above looks like this:

target datalayout = "e-m:e-p:32:32-i1:32-i8:8-i16:16-i32:32-i64:64-f16:16-f32:32-f64:64-n8:16:32:64" target triple = "dxil-ms-dx" define void @foo(i32 %i) { entry: br label %label label: ; preds = %entry ret void } !dx.shaderModel = !{!0} !dx.version = !{!1} !dx.entryPoints = !{!2} !llvm.module.flags = !{!3, !4} !0 = !{!"lib", i32 6, i32 3} !1 = !{i32 1, i32 3} !2 = !{null, !"", null, null, null} !3 = !{i32 2, !"Dwarf Version", i32 5} !4 = !{i32 2, !"Debug Info Version", i32 3}

And I was able to not call the DebugInfoPass after stripping the module.

But now in the unstripped module @llvm.dbg.value are printed as #dbg_value, failing the test test/CodeGen/DirectX/debug-info.ll. Is this better this way maybe? The tests for dxil-dis are written using #dbg_ format as well.

But now in the unstripped module @llvm.dbg.value are printed as #dbg_value, failing the test test/CodeGen/DirectX/debug-info.ll

That suggests that you cherry-picked only the individual commit that I linked to, rather than checking that out and applying your changes on top of it, is that right? The commit is on top of #198318 because it depends on that, it will do the wrong thing otherwise, it will fail in exactly the way that you are seeing.

Ah I see, I rebased and this works now. So how do we approach merging this to upstream? Do I wait for your PRs to be merged first, or make this PR as is now?

Write DXIL with debug info to ILDB part

b495ef3

kuilpd requested review from dzhidzhoev and hvdijk June 2, 2026 17:36

hvdijk reviewed Jun 2, 2026

View reviewed changes

dzhidzhoev reviewed Jun 2, 2026

View reviewed changes

kuilpd and others added 4 commits June 2, 2026 23:15

Redo the loop that removes version metadata

28108be

Add dx.source checks to embed-ildb.ll test

b13ae6d

Introduce isProgramPart

5560932

Rename IsDebug to WriteDebug

0281db9

kuilpd requested a review from asavonic June 2, 2026 22:24

		for (unsigned I : seq(FlagEntries.size())) {
		llvm::Module::ModuleFlagEntry &Entry = FlagEntries[I];

	for (unsigned I : seq(FlagEntries.size())) {
	llvm::Module::ModuleFlagEntry &Entry = FlagEntries[I];
	for (llvm::Module::ModuleFlagEntry &Entry : FlagEntries) {

Conversation

kuilpd commented Jun 2, 2026

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

Uh oh!

dzhidzhoev Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dzhidzhoev Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dzhidzhoev Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dzhidzhoev Jun 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dzhidzhoev left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

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

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

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dzhidzhoev Jun 2, 2026 •

edited

Loading

dzhidzhoev Jun 2, 2026 •

edited

Loading

dzhidzhoev Jun 2, 2026 •

edited

Loading

dzhidzhoev Jun 3, 2026 •

edited

Loading

dzhidzhoev left a comment •

edited

Loading