Skip to content
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

[clang][AMDGPU] precommit test for ballot on Windows #73920

Merged
merged 1 commit into from
Dec 4, 2023

Conversation

ssahasra
Copy link
Collaborator

The Clang declaration of the wave-64 builtin uses "UL" as the return type, which is interpreted as a 32-bit unsigned integer on Windows. This emits an incorrect LLVM declaration with i32 return type instead of i64. The clang declaration needs to be fixed to use "WU" instead.

@llvmbot llvmbot added the clang Clang issues not falling into any other category label Nov 30, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 30, 2023

@llvm/pr-subscribers-clang

Author: Sameer Sahasrabuddhe (ssahasra)

Changes

The Clang declaration of the wave-64 builtin uses "UL" as the return type, which is interpreted as a 32-bit unsigned integer on Windows. This emits an incorrect LLVM declaration with i32 return type instead of i64. The clang declaration needs to be fixed to use "WU" instead.


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

1 Files Affected:

  • (added) clang/test/CodeGenHIP/ballot.cpp (+21)
diff --git a/clang/test/CodeGenHIP/ballot.cpp b/clang/test/CodeGenHIP/ballot.cpp
new file mode 100644
index 000000000000000..823b1583a42fc02
--- /dev/null
+++ b/clang/test/CodeGenHIP/ballot.cpp
@@ -0,0 +1,21 @@
+// REQUIRES: amdgpu-registered-target
+// XFAIL: system-windows
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -target-cpu gfx900 -x hip -emit-llvm -fcuda-is-device -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -target-cpu gfx900 -x hip -S -fcuda-is-device -o - %s | FileCheck %s --check-prefix=GFX9
+
+// FIXME: The Clang declaration of the wave-64 builtin uses "UL" as the return
+// type, which is interpreted as a 32-bit unsigned integer on Windows. This
+// emits an incorrect LLVM declaration with i32 return type instead of i64. The
+// clang declaration needs to be fixed to use "WU" instead.
+
+// CHECK-LABEL: @_Z3fooi
+// CHECK: call i64 @llvm.amdgcn.ballot.i64
+
+// GFX9-LABEL: _Z3fooi:
+// GFX9: v_cmp_ne_u32_e64
+
+#define __device__ __attribute__((device))
+
+__device__ unsigned long long foo(int p) {
+  return __builtin_amdgcn_ballot_w64(p);
+}

The Clang declaration of the wave-64 builtin uses "UL" as the return type, which
is interpreted as a 32-bit unsigned integer on Windows. This emits an incorrect
LLVM declaration with i32 return type instead of i64. The clang declaration
needs to be fixed to use "WU" instead.
// CHECK: call i64 @llvm.amdgcn.ballot.i64

// GFX9-LABEL: _Z3fooi:
// GFX9: v_cmp_ne_u32_e64
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it really needed to test codegen here?

Nice find, BTW.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, we should check codegen because ultimately that was the symptom ... llc crashed when running hipcc on Windows.

Copy link
Contributor

@vpykhtin vpykhtin left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@ssahasra ssahasra merged commit 0f8681b into llvm:main Dec 4, 2023
3 checks passed
@ssahasra ssahasra deleted the ssahasra/ballot-precheckin branch January 2, 2024 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants