[HLSL] Enable -fconvergent-functions by default

[aniplcc]

Fixes #86506

[llvmbot]

@llvm/pr-subscribers-hlsl

@llvm/pr-subscribers-clang

Author: aniplcc (aniplcc)

Changes

Fixes #86506

---

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

1 Files Affected:

• (modified) clang/lib/Frontend/CompilerInvocation.cpp (+3-2)

diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index 7bd91d4791ecf0..30638a9999ca1d 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -3516,7 +3516,8 @@ void CompilerInvocationBase::GenerateLangArgs(const LangOptions &Opts,
     GenerateArg(Consumer, OPT_fblocks);
 
   if (Opts.ConvergentFunctions &&
-      !(Opts.OpenCL || (Opts.CUDA && Opts.CUDAIsDevice) || Opts.SYCLIsDevice))
+      !(Opts.OpenCL || (Opts.CUDA && Opts.CUDAIsDevice) || Opts.SYCLIsDevice ||
+        Opts.HLSL))
     GenerateArg(Consumer, OPT_fconvergent_functions);
 
   if (Opts.NoBuiltin && !Opts.Freestanding)
@@ -3914,7 +3915,7 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
 
   Opts.ConvergentFunctions = Args.hasArg(OPT_fconvergent_functions) ||
                              Opts.OpenCL || (Opts.CUDA && Opts.CUDAIsDevice) ||
-                             Opts.SYCLIsDevice;
+                             Opts.SYCLIsDevice || Opts.HLSL;
 
   Opts.NoBuiltin = Args.hasArg(OPT_fno_builtin) || Opts.Freestanding;
   if (!Opts.NoBuiltin)

[github-actions]

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

[github-actions]

✅ With the latest revision this PR passed the Python code formatter.

[aniplcc]

Requesting review, @llvm-beanz

[llvm-beanz]

Hi @aniplcc, thank you for the PR.

We do require test cases for bug fixes and new features (see the Developer Policy.

Because this general feature is already tested for OpenCL, we can make a really simple test for this. Something like this should be fine:

// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.4-library -emit-llvm -disable-llvm-passes -o - %s | FileCheck %s

void fn() {
};

// CHECK: define void @"?fn@@YAXXZ"() local_unnamed_addr #[[Attr:[0-9]+]]
// CHECK: attributes #[[Attr]] = { {{[^}]*}}convergent{{[^}]*}} }

Put that into a file clang/test/CodeGenHLSL/convergent-functions.hlsl and build the check-clang target to verify that it works.

[aniplcc]

I was getting match errors with the RUN script. I went ahead and updated it with the checks in:
clang/test/CodeGen/convergent-functions.cpp
Hope that's correct/OK?

// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.4-library -emit-llvm -disable-llvm-passes -o - %s | FileCheck -check-prefixes=CHECK,CONVFUNC %s 

// CHECK: attributes
// NOCONVFUNC-NOT: convergent
// CONVFUNC-SAME: convergent
// CHECK-SAME: }
void fn() {
};

Now it just checks the presence of convergent in attributes, as it does in convergent-functions.cpp.
Note: I also left out the #0 in the attributes to generalize it better. Is that okay?

Also for reference, the emitted llvmir from my forked clang build

; ModuleID = 'CodeGenHLSL/convergent-funtions.hlsl'
source_filename = "CodeGenHLSL/convergent-funtions.hlsl"
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-pc-shadermodel6.4-library"

; Function Attrs: convergent noinline nounwind optnone
define void @"?fn@@YAXXZ"() #0 {
entry:
  ret void
}

attributes #0 = { convergent noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" }
...*some more lines*

[llvm-beanz]

I was getting match errors with the RUN script. I went ahead and updated it with the checks in: clang/test/CodeGen/convergent-functions.cpp Hope that's correct/OK?

// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.4-library -emit-llvm -disable-llvm-passes -o - %s | FileCheck -check-prefixes=CHECK,CONVFUNC %s 

// CHECK: attributes
// NOCONVFUNC-NOT: convergent
// CONVFUNC-SAME: convergent
// CHECK-SAME: }
void fn() {
};

With this the NOCONVFUNC-NOT line does nothing. Having multiple check prefixes here unnecessarily complicates the test. Since we're only running FileCheck once we really should only need the CHECK prefix.

Now it just checks the presence of convergent in attributes, as it does in convergent-functions.cpp. Note: I also left out the #0 in the attributes to generalize it better. Is that okay?

I'd prefer a more specific match where we actually verify that the convergent attribute is applied to the function not just that the string convergent appears on the same line as the string attributes.

Something like this should work and insulates from the name mangling differences. I also added a run line to verify the SPIR-V targeting path which should be the same.

// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.4-library -emit-llvm -disable-llvm-passes -o - %s | FileCheck %s
// RUN: %clang_cc1 -triple spirv-linux-vulkan-library -emit-llvm -disable-llvm-passes -o - %s | FileCheck %s

void fn() {
};

// CHECK: define void {{.*}}fn{{.*}}()
// CHECK-SAME: #[[Attr:[0-9]+]]
// CHECK: attributes #[[Attr]] = { {{[^}]*}}convergent{{[^}]*}} }

[aniplcc]

Updated

[aniplcc]

check fails with

/var/lib/buildkite-agent/builds/linux-56-59b8f5d88-6mhbj-1/llvm-project/github-pull-requests/clang/test/CodeGenHLSL/convergent-functions.hlsl:7:11: error: CHECK: expected string not found in input
// CHECK: define void {{.*}}fn{{.*}}()
          ^
<stdin>:1:1: note: scanning from here
; ModuleID = '/var/lib/buildkite-agent/builds/linux-56-59b8f5d88-6mhbj-1/llvm-project/github-pull-requests/clang/test/CodeGenHLSL/convergent-functions.hlsl'
^
<stdin>:7:10: note: possible intended match here
define spir_func void @_Z2fnv() #0 {
         ^
Input file: <stdin>
Check file: /var/lib/buildkite-agent/builds/linux-56-59b8f5d88-6mhbj-1/llvm-project/github-pull-requests/clang/test/CodeGenHLSL/convergent-functions.hlsl
-dump-input=help explains the following input dump.
Input was:
<<<<<<
           1: ; ModuleID = '/var/lib/buildkite-agent/builds/linux-56-59b8f5d88-6mhbj-1/llvm-project/github-pull-requests/clang/test/CodeGenHLSL/convergent-functions.hlsl'
check:7'0     X~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: no match found
           2: source_filename = "/var/lib/buildkite-agent/builds/linux-56-59b8f5d88-6mhbj-1/llvm-project/github-pull-requests/clang/test/CodeGenHLSL/convergent-functions.hlsl"
check:7'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           3: target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024"
check:7'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           4: target triple = "spirv-linux-vulkan-library"
check:7'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           5:
check:7'0     ~
           6: ; Function Attrs: convergent noinline nounwind optnone
check:7'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           7: define spir_func void @_Z2fnv() #0 {
check:7'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
check:7'1              ?                            possible intended match
           8: entry:
check:7'0     ~~~~~~~
           9:  ret void
check:7'0     ~~~~~~~~~~
          10: }
check:7'0     ~~
          11:
check:7'0     ~
          12: attributes #0 = { convergent noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" }
check:7'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           .
           .
           .
>>>>>>
--

[aniplcc]

Ah it's the spir-v check, apologies, forgot to check that

[dyung]

@llvm-beanz and @aniplcc, this change seems to be causing a test failure of CodeGenHLSL/builtins/wave_get_lane_index_subcall.hlsl on several bots including:

• https://lab.llvm.org/buildbot/#/builders/139/builds/62602
• https://lab.llvm.org/buildbot/#/builders/216/builds/36867
• https://lab.llvm.org/buildbot/#/builders/247/builds/16189
• https://lab.llvm.org/buildbot/#/builders/280/builds/1206

Can you take a look and revert if you need time to investigate?

[aniplcc]

Looking at it now.

[llvm-beanz]

@aniplcc, I just pushed a fix in f119a4f