Skip to content

[Clang][SPIR-V] Fix convergence tokens for dtor #133469

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.

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

Merged
merged 1 commit into from
Apr 1, 2025

Conversation

Keenuts
Copy link
Contributor

@Keenuts Keenuts commented Mar 28, 2025

Destructor calls were emitted without convergence
intrinsics when building for SPIR-V, which means invalid IR since we mixed controlled and non-controlled convergence.

Destructor calls were emitted without convergence
intrinsics when building for SPIR-V, which means invalid
IR since we mixed controlled and non-controlled convergence.
@Keenuts Keenuts requested a review from s-perron March 28, 2025 16:53
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. HLSL HLSL Language Support labels Mar 28, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 28, 2025

@llvm/pr-subscribers-hlsl

@llvm/pr-subscribers-clang

Author: Nathan Gauër (Keenuts)

Changes

Destructor calls were emitted without convergence
intrinsics when building for SPIR-V, which means invalid IR since we mixed controlled and non-controlled convergence.


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

2 Files Affected:

  • (modified) clang/lib/CodeGen/CGDeclCXX.cpp (+4-1)
  • (modified) clang/test/CodeGenHLSL/GlobalDestructors.hlsl (+28-12)
diff --git a/clang/lib/CodeGen/CGDeclCXX.cpp b/clang/lib/CodeGen/CGDeclCXX.cpp
index 1ad34ae61f96a..b142aaf3fe0ee 100644
--- a/clang/lib/CodeGen/CGDeclCXX.cpp
+++ b/clang/lib/CodeGen/CGDeclCXX.cpp
@@ -1152,7 +1152,7 @@ void CodeGenFunction::GenerateCXXGlobalCleanUpFunc(
       llvm::Constant *Arg;
       std::tie(CalleeTy, Callee, Arg) = DtorsOrStermFinalizers[e - i - 1];
 
-      llvm::CallInst *CI = nullptr;
+      llvm::CallBase *CI = nullptr;
       if (Arg == nullptr) {
         assert(
             CGM.getCXXABI().useSinitAndSterm() &&
@@ -1164,6 +1164,9 @@ void CodeGenFunction::GenerateCXXGlobalCleanUpFunc(
       // Make sure the call and the callee agree on calling convention.
       if (llvm::Function *F = dyn_cast<llvm::Function>(Callee))
         CI->setCallingConv(F->getCallingConv());
+
+      if (CGM.shouldEmitConvergenceTokens() && CI->isConvergent())
+        CI = addConvergenceControlToken(CI);
     }
   }
 
diff --git a/clang/test/CodeGenHLSL/GlobalDestructors.hlsl b/clang/test/CodeGenHLSL/GlobalDestructors.hlsl
index f98318601134b..9f90971bafd05 100644
--- a/clang/test/CodeGenHLSL/GlobalDestructors.hlsl
+++ b/clang/test/CodeGenHLSL/GlobalDestructors.hlsl
@@ -1,5 +1,6 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -emit-llvm -disable-llvm-passes %s -o - | FileCheck %s --check-prefixes=CS,NOINLINE,CHECK
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -emit-llvm -disable-llvm-passes %s -o - | FileCheck %s --check-prefixes=LIB,NOINLINE,CHECK
+// RUN: %clang_cc1 -triple spirv-unknown-vulkan1.3-compute -emit-llvm -disable-llvm-passes %s -o - | FileCheck %s --check-prefixes=CS,NOINLINE-SPIRV,CHECK
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -emit-llvm -disable-llvm-passes %s -o - | FileCheck %s --check-prefixes=CS,NOINLINE-DXIL,CHECK
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -emit-llvm -disable-llvm-passes %s -o - | FileCheck %s --check-prefixes=LIB,NOINLINE-DXIL,CHECK
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -emit-llvm -O0 %s -o - | FileCheck %s --check-prefixes=INLINE,CHECK
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -emit-llvm -O0 %s -o - | FileCheck %s --check-prefixes=INLINE,CHECK
 
@@ -57,11 +58,19 @@ void main(unsigned GI : SV_GroupIndex) {
 // CHECK:      define void @main()
 // CHECK-NEXT: entry:
 // Verify destructor is emitted
-// NOINLINE-NEXT:   call void @_GLOBAL__sub_I_GlobalDestructors.hlsl()
-// NOINLINE-NEXT:   %0 = call i32 @llvm.dx.flattened.thread.id.in.group()
-// NOINLINE-NEXT:   call void @_Z4mainj(i32 %0)
-// NOINLINE-NEXT:   call void @_GLOBAL__D_a()
-// NOINLINE-NEXT:   ret void
+// NOINLINE-DXIL-NEXT:   call void @_GLOBAL__sub_I_GlobalDestructors.hlsl()
+// NOINLINE-DXIL-NEXT:   %0 = call i32 @llvm.dx.flattened.thread.id.in.group()
+// NOINLINE-DXIL-NEXT:   call void @_Z4mainj(i32 %0)
+// NOINLINE-DXIL-NEXT:   call void @_GLOBAL__D_a()
+// NOINLINE-DXIL-NEXT:   ret void
+
+// NOINLINE-SPIRV-NEXT:   %0 = call token @llvm.experimental.convergence.entry()
+// NOINLINE-SPIRV-NEXT:   call spir_func void @_GLOBAL__sub_I_GlobalDestructors.hlsl() [ "convergencectrl"(token %0) ]
+// NOINLINE-SPIRV-NEXT:   %1 = call i32 @llvm.spv.flattened.thread.id.in.group()
+// NOINLINE-SPIRV-NEXT:   call spir_func void @_Z4mainj(i32 %1) [ "convergencectrl"(token %0) ]
+// NOINLINE-SPIRV-NEXT:   call spir_func void @_GLOBAL__D_a() [ "convergencectrl"(token %0) ]
+// NOINLINE-SPIRV-NEXT:   ret void
+
 // Verify inlining leaves only calls to "llvm." intrinsics
 // INLINE-NOT:   call {{[^@]*}} @{{[^l][^l][^v][^m][^\.]}}
 // INLINE:   ret void
@@ -69,10 +78,17 @@ void main(unsigned GI : SV_GroupIndex) {
 // This is really just a sanity check I needed for myself to verify that
 // function scope static variables also get destroyed properly.
 
-// NOINLINE: define internal void @_GLOBAL__D_a() [[IntAttr:\#[0-9]+]]
-// NOINLINE-NEXT: entry:
-// NOINLINE-NEXT:   call void @_ZN4TailD1Ev(ptr @_ZZ3WagvE1T)
-// NOINLINE-NEXT:   call void @_ZN6PupperD1Ev(ptr @GlobalPup)
-// NOINLINE-NEXT:   ret void
+// NOINLINE-DXIL:       define internal void @_GLOBAL__D_a() [[IntAttr:\#[0-9]+]]
+// NOINLINE-DXIL-NEXT:  entry:
+// NOINLINE-DXIL-NEXT:    call void @_ZN4TailD1Ev(ptr @_ZZ3WagvE1T)
+// NOINLINE-DXIL-NEXT:    call void @_ZN6PupperD1Ev(ptr @GlobalPup)
+// NOINLINE-DXIL-NEXT:    ret void
+
+// NOINLINE-SPIRV:      define internal spir_func void @_GLOBAL__D_a() [[IntAttr:\#[0-9]+]]
+// NOINLINE-SPIRV-NEXT: entry:
+// NOINLINE-SPIRV-NEXT:   %0 = call token @llvm.experimental.convergence.entry()
+// NOINLINE-SPIRV-NEXT:   call spir_func void @_ZN4TailD1Ev(ptr @_ZZ3WagvE1T) [ "convergencectrl"(token %0) ]
+// NOINLINE-SPIRV-NEXT:   call spir_func void @_ZN6PupperD1Ev(ptr @GlobalPup) [ "convergencectrl"(token %0) ]
+// NOINLINE-SPIRV-NEXT:   ret void
 
 // NOINLINE: attributes [[IntAttr]] = {{.*}} alwaysinline

@llvmbot
Copy link
Member

llvmbot commented Mar 28, 2025

@llvm/pr-subscribers-clang-codegen

Author: Nathan Gauër (Keenuts)

Changes

Destructor calls were emitted without convergence
intrinsics when building for SPIR-V, which means invalid IR since we mixed controlled and non-controlled convergence.


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

2 Files Affected:

  • (modified) clang/lib/CodeGen/CGDeclCXX.cpp (+4-1)
  • (modified) clang/test/CodeGenHLSL/GlobalDestructors.hlsl (+28-12)
diff --git a/clang/lib/CodeGen/CGDeclCXX.cpp b/clang/lib/CodeGen/CGDeclCXX.cpp
index 1ad34ae61f96a..b142aaf3fe0ee 100644
--- a/clang/lib/CodeGen/CGDeclCXX.cpp
+++ b/clang/lib/CodeGen/CGDeclCXX.cpp
@@ -1152,7 +1152,7 @@ void CodeGenFunction::GenerateCXXGlobalCleanUpFunc(
       llvm::Constant *Arg;
       std::tie(CalleeTy, Callee, Arg) = DtorsOrStermFinalizers[e - i - 1];
 
-      llvm::CallInst *CI = nullptr;
+      llvm::CallBase *CI = nullptr;
       if (Arg == nullptr) {
         assert(
             CGM.getCXXABI().useSinitAndSterm() &&
@@ -1164,6 +1164,9 @@ void CodeGenFunction::GenerateCXXGlobalCleanUpFunc(
       // Make sure the call and the callee agree on calling convention.
       if (llvm::Function *F = dyn_cast<llvm::Function>(Callee))
         CI->setCallingConv(F->getCallingConv());
+
+      if (CGM.shouldEmitConvergenceTokens() && CI->isConvergent())
+        CI = addConvergenceControlToken(CI);
     }
   }
 
diff --git a/clang/test/CodeGenHLSL/GlobalDestructors.hlsl b/clang/test/CodeGenHLSL/GlobalDestructors.hlsl
index f98318601134b..9f90971bafd05 100644
--- a/clang/test/CodeGenHLSL/GlobalDestructors.hlsl
+++ b/clang/test/CodeGenHLSL/GlobalDestructors.hlsl
@@ -1,5 +1,6 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -emit-llvm -disable-llvm-passes %s -o - | FileCheck %s --check-prefixes=CS,NOINLINE,CHECK
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -emit-llvm -disable-llvm-passes %s -o - | FileCheck %s --check-prefixes=LIB,NOINLINE,CHECK
+// RUN: %clang_cc1 -triple spirv-unknown-vulkan1.3-compute -emit-llvm -disable-llvm-passes %s -o - | FileCheck %s --check-prefixes=CS,NOINLINE-SPIRV,CHECK
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -emit-llvm -disable-llvm-passes %s -o - | FileCheck %s --check-prefixes=CS,NOINLINE-DXIL,CHECK
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -emit-llvm -disable-llvm-passes %s -o - | FileCheck %s --check-prefixes=LIB,NOINLINE-DXIL,CHECK
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -emit-llvm -O0 %s -o - | FileCheck %s --check-prefixes=INLINE,CHECK
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -emit-llvm -O0 %s -o - | FileCheck %s --check-prefixes=INLINE,CHECK
 
@@ -57,11 +58,19 @@ void main(unsigned GI : SV_GroupIndex) {
 // CHECK:      define void @main()
 // CHECK-NEXT: entry:
 // Verify destructor is emitted
-// NOINLINE-NEXT:   call void @_GLOBAL__sub_I_GlobalDestructors.hlsl()
-// NOINLINE-NEXT:   %0 = call i32 @llvm.dx.flattened.thread.id.in.group()
-// NOINLINE-NEXT:   call void @_Z4mainj(i32 %0)
-// NOINLINE-NEXT:   call void @_GLOBAL__D_a()
-// NOINLINE-NEXT:   ret void
+// NOINLINE-DXIL-NEXT:   call void @_GLOBAL__sub_I_GlobalDestructors.hlsl()
+// NOINLINE-DXIL-NEXT:   %0 = call i32 @llvm.dx.flattened.thread.id.in.group()
+// NOINLINE-DXIL-NEXT:   call void @_Z4mainj(i32 %0)
+// NOINLINE-DXIL-NEXT:   call void @_GLOBAL__D_a()
+// NOINLINE-DXIL-NEXT:   ret void
+
+// NOINLINE-SPIRV-NEXT:   %0 = call token @llvm.experimental.convergence.entry()
+// NOINLINE-SPIRV-NEXT:   call spir_func void @_GLOBAL__sub_I_GlobalDestructors.hlsl() [ "convergencectrl"(token %0) ]
+// NOINLINE-SPIRV-NEXT:   %1 = call i32 @llvm.spv.flattened.thread.id.in.group()
+// NOINLINE-SPIRV-NEXT:   call spir_func void @_Z4mainj(i32 %1) [ "convergencectrl"(token %0) ]
+// NOINLINE-SPIRV-NEXT:   call spir_func void @_GLOBAL__D_a() [ "convergencectrl"(token %0) ]
+// NOINLINE-SPIRV-NEXT:   ret void
+
 // Verify inlining leaves only calls to "llvm." intrinsics
 // INLINE-NOT:   call {{[^@]*}} @{{[^l][^l][^v][^m][^\.]}}
 // INLINE:   ret void
@@ -69,10 +78,17 @@ void main(unsigned GI : SV_GroupIndex) {
 // This is really just a sanity check I needed for myself to verify that
 // function scope static variables also get destroyed properly.
 
-// NOINLINE: define internal void @_GLOBAL__D_a() [[IntAttr:\#[0-9]+]]
-// NOINLINE-NEXT: entry:
-// NOINLINE-NEXT:   call void @_ZN4TailD1Ev(ptr @_ZZ3WagvE1T)
-// NOINLINE-NEXT:   call void @_ZN6PupperD1Ev(ptr @GlobalPup)
-// NOINLINE-NEXT:   ret void
+// NOINLINE-DXIL:       define internal void @_GLOBAL__D_a() [[IntAttr:\#[0-9]+]]
+// NOINLINE-DXIL-NEXT:  entry:
+// NOINLINE-DXIL-NEXT:    call void @_ZN4TailD1Ev(ptr @_ZZ3WagvE1T)
+// NOINLINE-DXIL-NEXT:    call void @_ZN6PupperD1Ev(ptr @GlobalPup)
+// NOINLINE-DXIL-NEXT:    ret void
+
+// NOINLINE-SPIRV:      define internal spir_func void @_GLOBAL__D_a() [[IntAttr:\#[0-9]+]]
+// NOINLINE-SPIRV-NEXT: entry:
+// NOINLINE-SPIRV-NEXT:   %0 = call token @llvm.experimental.convergence.entry()
+// NOINLINE-SPIRV-NEXT:   call spir_func void @_ZN4TailD1Ev(ptr @_ZZ3WagvE1T) [ "convergencectrl"(token %0) ]
+// NOINLINE-SPIRV-NEXT:   call spir_func void @_ZN6PupperD1Ev(ptr @GlobalPup) [ "convergencectrl"(token %0) ]
+// NOINLINE-SPIRV-NEXT:   ret void
 
 // NOINLINE: attributes [[IntAttr]] = {{.*}} alwaysinline

Copy link
Contributor

@s-perron s-perron left a comment

Choose a reason for hiding this comment

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

The changes look correct, but I want to double check that they are complete.

@Keenuts Keenuts merged commit da5fb42 into llvm:main Apr 1, 2025
15 checks passed
@Keenuts Keenuts deleted the fix-dtor-convergence branch April 1, 2025 09:03
Ankur-0429 pushed a commit to Ankur-0429/llvm-project that referenced this pull request Apr 2, 2025
Destructor calls were emitted without convergence intrinsics when building for SPIR-V, which means invalid IR since we
mixed controlled and non-controlled convergence.
@damyanp damyanp moved this to Closed in HLSL Support Apr 25, 2025
@damyanp damyanp removed this from HLSL Support Jun 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen IR generation bugs: mangling, exceptions, etc. clang Clang issues not falling into any other category HLSL HLSL Language Support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants