Skip to content

Conversation

@bogner
Copy link
Contributor

@bogner bogner commented Nov 28, 2024

This moves DXILFinalizeLinkage before the DXIL op lowering passes so that it doesn't end up internalizing any of the dx.op.* functions. This also exposed a bug when the pass is run on a module with intrinsics in them - marking the intrinsics as internal will fail the validator.

Fixes #117761

This moves DXILFinalizeLinkage before the DXIL op lowering passes so
that it doesn't end up internalizing any of the `dx.op.*` functions.
This also exposed a bug when the pass is run on a module with intrinsics
in them - marking the intrinsics as internal will fail the validator.

Fixes llvm#117761
@llvmbot
Copy link
Member

llvmbot commented Nov 28, 2024

@llvm/pr-subscribers-backend-directx

Author: Justin Bogner (bogner)

Changes

This moves DXILFinalizeLinkage before the DXIL op lowering passes so that it doesn't end up internalizing any of the dx.op.* functions. This also exposed a bug when the pass is run on a module with intrinsics in them - marking the intrinsics as internal will fail the validator.

Fixes #117761


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

5 Files Affected:

  • (modified) llvm/lib/Target/DirectX/DXILFinalizeLinkage.cpp (+2)
  • (modified) llvm/lib/Target/DirectX/DirectXTargetMachine.cpp (+1-1)
  • (modified) llvm/test/CodeGen/DirectX/BufferStore-errors.ll (+2-2)
  • (added) llvm/test/CodeGen/DirectX/finalize-linkage-intrinsics.ll (+8)
  • (modified) llvm/test/CodeGen/DirectX/llc-pipeline.ll (+1-1)
diff --git a/llvm/lib/Target/DirectX/DXILFinalizeLinkage.cpp b/llvm/lib/Target/DirectX/DXILFinalizeLinkage.cpp
index aa1f55c572df29..79ebbe0925e5c3 100644
--- a/llvm/lib/Target/DirectX/DXILFinalizeLinkage.cpp
+++ b/llvm/lib/Target/DirectX/DXILFinalizeLinkage.cpp
@@ -23,6 +23,8 @@ static bool finalizeLinkage(Module &M) {
 
   // Collect non-entry and non-exported functions to set to internal linkage.
   for (Function &EF : M.functions()) {
+    if (EF.isIntrinsic())
+      continue;
     if (EF.hasFnAttribute("hlsl.shader") || EF.hasFnAttribute("hlsl.export"))
       continue;
     Funcs.insert(&EF);
diff --git a/llvm/lib/Target/DirectX/DirectXTargetMachine.cpp b/llvm/lib/Target/DirectX/DirectXTargetMachine.cpp
index 655427a3e80209..de14c8d9f13e8d 100644
--- a/llvm/lib/Target/DirectX/DirectXTargetMachine.cpp
+++ b/llvm/lib/Target/DirectX/DirectXTargetMachine.cpp
@@ -89,6 +89,7 @@ class DirectXPassConfig : public TargetPassConfig {
 
   FunctionPass *createTargetRegisterAllocator(bool) override { return nullptr; }
   void addCodeGenPrepare() override {
+    addPass(createDXILFinalizeLinkageLegacyPass());
     addPass(createDXILIntrinsicExpansionLegacyPass());
     addPass(createDXILDataScalarizationLegacyPass());
     ScalarizerPassOptions DxilScalarOptions;
@@ -96,7 +97,6 @@ class DirectXPassConfig : public TargetPassConfig {
     addPass(createDXILFlattenArraysLegacyPass());
     addPass(createScalarizerPass(DxilScalarOptions));
     addPass(createDXILOpLoweringLegacyPass());
-    addPass(createDXILFinalizeLinkageLegacyPass());
     addPass(createDXILTranslateMetadataLegacyPass());
     addPass(createDXILPrepareModulePass());
   }
diff --git a/llvm/test/CodeGen/DirectX/BufferStore-errors.ll b/llvm/test/CodeGen/DirectX/BufferStore-errors.ll
index b00fc38c901f92..ffdcb037b8968e 100644
--- a/llvm/test/CodeGen/DirectX/BufferStore-errors.ll
+++ b/llvm/test/CodeGen/DirectX/BufferStore-errors.ll
@@ -6,7 +6,7 @@ target triple = "dxil-pc-shadermodel6.6-compute"
 ; CHECK: error:
 ; CHECK-SAME: in function storetoomany
 ; CHECK-SAME: typedBufferStore data must be a vector of 4 elements
-define void @storetoomany(<5 x float> %data, i32 %index) {
+define void @storetoomany(<5 x float> %data, i32 %index) "hlsl.export" {
   %buffer = call target("dx.TypedBuffer", <4 x float>, 1, 0, 0)
       @llvm.dx.handle.fromBinding.tdx.TypedBuffer_v4f32_1_0_0(
           i32 0, i32 0, i32 1, i32 0, i1 false)
@@ -21,7 +21,7 @@ define void @storetoomany(<5 x float> %data, i32 %index) {
 ; CHECK: error:
 ; CHECK-SAME: in function storetoofew
 ; CHECK-SAME: typedBufferStore data must be a vector of 4 elements
-define void @storetoofew(<3 x i32> %data, i32 %index) {
+define void @storetoofew(<3 x i32> %data, i32 %index) "hlsl.export" {
   %buffer = call target("dx.TypedBuffer", <4 x i32>, 1, 0, 0)
       @llvm.dx.handle.fromBinding.tdx.TypedBuffer_v4i32_1_0_0(
           i32 0, i32 0, i32 1, i32 0, i1 false)
diff --git a/llvm/test/CodeGen/DirectX/finalize-linkage-intrinsics.ll b/llvm/test/CodeGen/DirectX/finalize-linkage-intrinsics.ll
new file mode 100644
index 00000000000000..e2ac6094f0eb54
--- /dev/null
+++ b/llvm/test/CodeGen/DirectX/finalize-linkage-intrinsics.ll
@@ -0,0 +1,8 @@
+; RUN: opt -S -dxil-finalize-linkage -verify -mtriple=dxil-unknown-shadermodel6.5-library %s
+
+define float @f(float %f) "hlsl.export" {
+  %x = call float @llvm.atan.f32(float %f)
+  ret float %x
+}
+
+declare float @llvm.atan.f32(float)
diff --git a/llvm/test/CodeGen/DirectX/llc-pipeline.ll b/llvm/test/CodeGen/DirectX/llc-pipeline.ll
index f0950df08eff5b..40fa30778a1532 100644
--- a/llvm/test/CodeGen/DirectX/llc-pipeline.ll
+++ b/llvm/test/CodeGen/DirectX/llc-pipeline.ll
@@ -7,6 +7,7 @@
 ; CHECK-NEXT: Target Library Information
 ; CHECK-NEXT: Target Transform Information
 ; CHECK-NEXT: ModulePass Manager
+; CHECK-NEXT:   DXIL Finalize Linkage
 ; CHECK-NEXT:   DXIL Intrinsic Expansion
 ; CHECK-NEXT:   DXIL Data Scalarization
 ; CHECK-NEXT:   DXIL Array Flattener
@@ -15,7 +16,6 @@
 ; CHECK-NEXT:     Scalarize vector operations
 ; CHECK-NEXT:   DXIL Resource analysis
 ; CHECK-NEXT:   DXIL Op Lowering
-; CHECK-NEXT:   DXIL Finalize Linkage
 ; CHECK-NEXT:   DXIL resource Information
 ; CHECK-NEXT:   DXIL Shader Flag Analysis
 ; CHECK-NEXT:   DXIL Module Metadata analysis

@bogner bogner merged commit 93d2a2c into llvm:main Dec 2, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

[DirectX] Avoid marking DXIL ops as internal in DXILFinalizeLinkage

4 participants