Skip to content

Conversation

@FreddyLeaf
Copy link
Contributor

@FreddyLeaf FreddyLeaf commented Oct 31, 2024

This fixes correctness of https://gcc.godbolt.org/z/vexf5fW5r

@FreddyLeaf
Copy link
Contributor Author

First, for usages like in https://gcc.godbolt.org/z/vexf5fW5r, clang didn't report warning/errors. Furthermore, it calls the builtin with a 4 byte aligned ptr instead of 8, which brought correctness issues.

@FreddyLeaf FreddyLeaf marked this pull request as ready for review October 31, 2024 06:08
@FreddyLeaf FreddyLeaf requested a review from phoebewang October 31, 2024 06:08
@llvmbot llvmbot added clang Clang issues not falling into any other category backend:X86 clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:headers Headers provided by Clang, e.g. for intrinsics labels Oct 31, 2024
@FreddyLeaf FreddyLeaf requested a review from RKSimon October 31, 2024 06:08
@llvmbot
Copy link
Member

llvmbot commented Oct 31, 2024

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-backend-x86

Author: Freddy Ye (FreddyLeaf)

Changes

This fixes correctness of https://gcc.godbolt.org/z/vexf5fW5r


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

3 Files Affected:

  • (modified) clang/include/clang/Basic/BuiltinsX86_64.def (+1-1)
  • (modified) clang/lib/Headers/cmpccxaddintrin.h (+1-1)
  • (modified) clang/test/CodeGen/X86/cmpccxadd-builtins-error.c (+5-1)
diff --git a/clang/include/clang/Basic/BuiltinsX86_64.def b/clang/include/clang/Basic/BuiltinsX86_64.def
index e1e613560167ac..2c3e1a69610bcc 100644
--- a/clang/include/clang/Basic/BuiltinsX86_64.def
+++ b/clang/include/clang/Basic/BuiltinsX86_64.def
@@ -150,7 +150,7 @@ TARGET_BUILTIN(__builtin_ia32_tcmmrlfp16ps, "vIUcIUcIUc", "n", "amx-complex")
 
 TARGET_BUILTIN(__builtin_ia32_prefetchi, "vvC*Ui", "nc", "prefetchi")
 TARGET_BUILTIN(__builtin_ia32_cmpccxadd32, "Siv*SiSiIi", "n", "cmpccxadd")
-TARGET_BUILTIN(__builtin_ia32_cmpccxadd64, "SLLiv*SLLiSLLiIi", "n", "cmpccxadd")
+TARGET_BUILTIN(__builtin_ia32_cmpccxadd64, "SLLiSLLi*SLLiSLLiIi", "n", "cmpccxadd")
 
 // AMX_FP16 FP16
 TARGET_BUILTIN(__builtin_ia32_tdpfp16ps, "vIUcIUcIUc", "n", "amx-fp16")
diff --git a/clang/lib/Headers/cmpccxaddintrin.h b/clang/lib/Headers/cmpccxaddintrin.h
index 6957498996c89b..0076c402f5ffc9 100644
--- a/clang/lib/Headers/cmpccxaddintrin.h
+++ b/clang/lib/Headers/cmpccxaddintrin.h
@@ -63,7 +63,7 @@ typedef enum {
                                     (int)(__D))))
 
 #define _cmpccxadd_epi64(__A, __B, __C, __D)                                   \
-  ((long long)(__builtin_ia32_cmpccxadd64((void *)(__A), (long long)(__B),     \
+  ((long long)(__builtin_ia32_cmpccxadd64((__A), (long long)(__B),             \
                                           (long long)(__C), (int)(__D))))
 
 #endif // __x86_64__
diff --git a/clang/test/CodeGen/X86/cmpccxadd-builtins-error.c b/clang/test/CodeGen/X86/cmpccxadd-builtins-error.c
index 8d9ca671f30f8f..e30fc4f46aa506 100644
--- a/clang/test/CodeGen/X86/cmpccxadd-builtins-error.c
+++ b/clang/test/CodeGen/X86/cmpccxadd-builtins-error.c
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 %s -ffreestanding -triple=x86_64-unknown-unknown \
-// RUN: -target-feature +cmpccxadd  -fsyntax-only -verify
+// RUN: -target-feature +cmpccxadd  -fsyntax-only -Werror -verify
 
 #include <immintrin.h>
 
@@ -10,3 +10,7 @@ int test_cmpccxadd32(void *__A, int __B, int __C) {
 long long test_cmpccxadd64(void *__A, long long __B, long long __C) {
   return _cmpccxadd_epi64(__A, __B, __C, 16); // expected-error {{argument value 16 is outside the valid range [0, 15]}}
 }
+
+long long test_cmpccxadd64_2(int *__A, long long __B, long long __C) {
+  return _cmpccxadd_epi64(__A, __B, __C, 3); // expected-error {{incompatible pointer types passing 'int *' to parameter of type 'long long *'}}
+}

Copy link
Contributor

@phoebewang phoebewang 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!

@FreddyLeaf FreddyLeaf merged commit 8cb6b65 into llvm:main Nov 1, 2024
8 checks passed
smallp-o-p pushed a commit to smallp-o-p/llvm-project that referenced this pull request Nov 3, 2024
NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this pull request Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend:X86 clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:headers Headers provided by Clang, e.g. for intrinsics clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants