Skip to content

Conversation

bogner
Copy link
Contributor

@bogner bogner commented Oct 15, 2025

Move our handling of setting up a compatible triple for DXIL to the bitcode writer itself, and do the same for the datalayout. This avoids us needing to temporarily change the triple just to change it back, and it also allows us to make the datalayout truly compatible, including the i8:32 alignment that isn't accepted by modern LLVM's DataLayout.

Move our handling of setting up a compatible triple for DXIL to the
bitcode writer itself, and do the same for the datalayout. This avoids
us needing to temporarily change the triple just to change it back, and
it also allows us to make the datalayout truly compatible, including the
`i8:32` alignment that isn't accepted by modern LLVM's DataLayout.
@llvmbot
Copy link
Member

llvmbot commented Oct 15, 2025

@llvm/pr-subscribers-backend-directx

Author: Justin Bogner (bogner)

Changes

Move our handling of setting up a compatible triple for DXIL to the bitcode writer itself, and do the same for the datalayout. This avoids us needing to temporarily change the triple just to change it back, and it also allows us to make the datalayout truly compatible, including the i8:32 alignment that isn't accepted by modern LLVM's DataLayout.


Full diff: https://github.com/llvm/llvm-project/pull/163587.diff

2 Files Affected:

  • (modified) llvm/lib/Target/DirectX/DXILWriter/DXILBitcodeWriter.cpp (+9-6)
  • (modified) llvm/lib/Target/DirectX/DXILWriter/DXILWriterPass.cpp (-8)
diff --git a/llvm/lib/Target/DirectX/DXILWriter/DXILBitcodeWriter.cpp b/llvm/lib/Target/DirectX/DXILWriter/DXILBitcodeWriter.cpp
index 82c43ff8dc352..26a8728e1f37c 100644
--- a/llvm/lib/Target/DirectX/DXILWriter/DXILBitcodeWriter.cpp
+++ b/llvm/lib/Target/DirectX/DXILWriter/DXILBitcodeWriter.cpp
@@ -1165,12 +1165,15 @@ void DXILBitcodeWriter::writeValueSymbolTableForwardDecl() {}
 /// Returns the bit offset to backpatch with the location of the real VST.
 void DXILBitcodeWriter::writeModuleInfo() {
   // Emit various pieces of data attached to a module.
-  if (!M.getTargetTriple().empty())
-    writeStringRecord(Stream, bitc::MODULE_CODE_TRIPLE,
-                      M.getTargetTriple().str(), 0 /*TODO*/);
-  const std::string &DL = M.getDataLayoutStr();
-  if (!DL.empty())
-    writeStringRecord(Stream, bitc::MODULE_CODE_DATALAYOUT, DL, 0 /*TODO*/);
+
+  // We need to hardcode a triple and datalayout that's compatible with the
+  // historical DXIL triple and datalayout from DXC.
+  StringRef Triple = "dxil-ms-dx";
+  StringRef DL = "e-m:e-p:32:32-i1:8-i8:8-i16:32-i32:32-i64:64-"
+                 "f16:32-f32:32-f64:64-n8:16:32:64";
+  writeStringRecord(Stream, bitc::MODULE_CODE_TRIPLE, Triple, 0 /*TODO*/);
+  writeStringRecord(Stream, bitc::MODULE_CODE_DATALAYOUT, DL, 0 /*TODO*/);
+
   if (!M.getModuleInlineAsm().empty())
     writeStringRecord(Stream, bitc::MODULE_CODE_ASM, M.getModuleInlineAsm(),
                       0 /*TODO*/);
diff --git a/llvm/lib/Target/DirectX/DXILWriter/DXILWriterPass.cpp b/llvm/lib/Target/DirectX/DXILWriter/DXILWriterPass.cpp
index 1eb03bfc087e3..725f2b1e7c76c 100644
--- a/llvm/lib/Target/DirectX/DXILWriter/DXILWriterPass.cpp
+++ b/llvm/lib/Target/DirectX/DXILWriter/DXILWriterPass.cpp
@@ -149,11 +149,6 @@ class EmbedDXILPass : public llvm::ModulePass {
     std::string Data;
     llvm::raw_string_ostream OS(Data);
 
-    Triple OriginalTriple = M.getTargetTriple();
-    // Set to DXIL triple when write to bitcode.
-    // Only the output bitcode need to be DXIL triple.
-    M.setTargetTriple(Triple("dxil-ms-dx"));
-
     // Perform late legalization of lifetime intrinsics that would otherwise
     // fail the Module Verifier if performed in an earlier pass
     legalizeLifetimeIntrinsics(M);
@@ -165,9 +160,6 @@ class EmbedDXILPass : public llvm::ModulePass {
     // not-so-legal legalizations
     removeLifetimeIntrinsics(M);
 
-    // Recover triple.
-    M.setTargetTriple(OriginalTriple);
-
     Constant *ModuleConstant =
         ConstantDataArray::get(M.getContext(), arrayRefFromStringRef(Data));
     auto *GV = new llvm::GlobalVariable(M, ModuleConstant->getType(), true,


// We need to hardcode a triple and datalayout that's compatible with the
// historical DXIL triple and datalayout from DXC.
StringRef Triple = "dxil-ms-dx";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It may make sense to assert here that M.getTargetTriple() is DXIL before ignoring it. This is all deep in the DirectX backend though so the value of that check seems kind of low.

@bogner bogner merged commit c87e0e8 into llvm:main Oct 15, 2025
11 of 13 checks passed
@Icohedron
Copy link
Contributor

Icohedron commented Oct 16, 2025

99 DML shaders fail to validate after this PR was merged: before and after
The error is

error: Total Thread Group Shared Memory storage is 43688, exceeded 32768.
Validation failed.

I compiled one of the DML shaders with and without this PR and the difference in the dxil assembly is just the target datalayout

1c1
< 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 datalayout = "e-m:e-p:32:32-i1:8-i8:8-i16:32-i32:32-i64:64-f16:32-f32:32-f64:64-n8:16:32:64"
270c270
< !1 = !{!"clang version 22.0.0git (git@github.com:Icohedron/llvm-project.git 72c6e4b230ddb5ca85361e145e177245319b271e)"}
---
> !1 = !{!"clang version 22.0.0git (git@github.com:Icohedron/llvm-project.git c87e0e8fe0ea14dcd84e835c0f7b02c5b0edca70)"}

Compiling the same DML shader with DXC, DXC gives the shader a datalayout of

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"

which matches the data layout that Clang emitted before this PR.

Minimal reproducible test case

// compile args: -T cs_6_7 -E CSMain -enable-16bit-types -Fo output.dat
groupshared float16_t smem[10240];
[numthreads(1, 1, 1)] 
void CSMain() {
 smem[0] = 0;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants