Skip to content

Reapply "[Win/X86] Make _m_prefetch[w] builtins to avoid winnt.h conflicts (#115099)" #138360

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

rnk
Copy link
Collaborator

@rnk rnk commented May 2, 2025

This reverts commit 83ff9d4.

Don't change the builtin signature of _mm_prefetch this time.

…licts (llvm#115099)"

This reverts commit 83ff9d4.

Don't change the builtin signature of _mm_prefetch this time.
@rnk rnk requested review from zmodem and sarnex May 2, 2025 23:20
@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 clang:codegen IR generation bugs: mangling, exceptions, etc. labels May 2, 2025
@llvmbot
Copy link
Member

llvmbot commented May 2, 2025

@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: Reid Kleckner (rnk)

Changes

This reverts commit 83ff9d4.

Don't change the builtin signature of _mm_prefetch this time.


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

4 Files Affected:

  • (modified) clang/include/clang/Basic/BuiltinsX86.td (+8-2)
  • (modified) clang/lib/CodeGen/TargetBuiltins/X86.cpp (+11)
  • (modified) clang/lib/Headers/prfchwintrin.h (+10-13)
  • (modified) clang/lib/Headers/xmmintrin.h (+5-4)
diff --git a/clang/include/clang/Basic/BuiltinsX86.td b/clang/include/clang/Basic/BuiltinsX86.td
index 67cbbfdec7aaf..dbf3cca19546e 100644
--- a/clang/include/clang/Basic/BuiltinsX86.td
+++ b/clang/include/clang/Basic/BuiltinsX86.td
@@ -138,6 +138,12 @@ let Attributes = [Const, NoThrow, RequiredVectorWidth<256>], Features = "avx" in
   }
 }
 
+// PRFCHW
+let Features = "prfchw", Header = "intrin.h", Attributes = [NoThrow, Const] in {
+  def _m_prefetch : X86LibBuiltin<"void(void *)">;
+  def _m_prefetchw : X86LibBuiltin<"void(void volatile const *)">;
+}
+
 
 // Mechanically ported builtins from the original `.def` file.
 //
@@ -146,8 +152,8 @@ let Attributes = [Const, NoThrow, RequiredVectorWidth<256>], Features = "avx" in
 // current formulation is based on what was easiest to recognize from the
 // pre-TableGen version.
 
-let Features = "mmx", Attributes = [NoThrow, Const] in {
-  def _mm_prefetch : X86NoPrefixBuiltin<"void(char const *, int)">;
+let Features = "sse", Header = "xmmintrin.h", Attributes = [NoThrow, Const] in {
+  def _mm_prefetch : X86LibBuiltin<"void(char const *, int)">;
 }
 
 let Features = "sse", Attributes = [NoThrow] in {
diff --git a/clang/lib/CodeGen/TargetBuiltins/X86.cpp b/clang/lib/CodeGen/TargetBuiltins/X86.cpp
index 3c2a77ab3fe4e..e23d19d2f6b6b 100644
--- a/clang/lib/CodeGen/TargetBuiltins/X86.cpp
+++ b/clang/lib/CodeGen/TargetBuiltins/X86.cpp
@@ -804,6 +804,17 @@ Value *CodeGenFunction::EmitX86BuiltinExpr(unsigned BuiltinID,
     Function *F = CGM.getIntrinsic(Intrinsic::prefetch, Address->getType());
     return Builder.CreateCall(F, {Address, RW, Locality, Data});
   }
+  case X86::BI_m_prefetch:
+  case X86::BI_m_prefetchw: {
+    Value *Address = Ops[0];
+    // The 'w' suffix implies write.
+    Value *RW =
+        ConstantInt::get(Int32Ty, BuiltinID == X86::BI_m_prefetchw ? 1 : 0);
+    Value *Locality = ConstantInt::get(Int32Ty, 0x3);
+    Value *Data = ConstantInt::get(Int32Ty, 1);
+    Function *F = CGM.getIntrinsic(Intrinsic::prefetch, Address->getType());
+    return Builder.CreateCall(F, {Address, RW, Locality, Data});
+  }
   case X86::BI_mm_clflush: {
     return Builder.CreateCall(CGM.getIntrinsic(Intrinsic::x86_sse2_clflush),
                               Ops[0]);
diff --git a/clang/lib/Headers/prfchwintrin.h b/clang/lib/Headers/prfchwintrin.h
index eaea5f3cf8feb..8ec55d7073716 100644
--- a/clang/lib/Headers/prfchwintrin.h
+++ b/clang/lib/Headers/prfchwintrin.h
@@ -14,6 +14,10 @@
 #ifndef __PRFCHWINTRIN_H
 #define __PRFCHWINTRIN_H
 
+#if defined(__cplusplus)
+extern "C" {
+#endif
+
 /// Loads a memory sequence containing the specified memory address into
 ///    all data cache levels.
 ///
@@ -26,11 +30,7 @@
 ///
 /// \param __P
 ///    A pointer specifying the memory address to be prefetched.
-static __inline__ void __attribute__((__always_inline__, __nodebug__))
-_m_prefetch(void *__P)
-{
-  __builtin_prefetch (__P, 0, 3 /* _MM_HINT_T0 */);
-}
+void _m_prefetch(void *__P);
 
 /// Loads a memory sequence containing the specified memory address into
 ///    the L1 data cache and sets the cache-coherency state to modified.
@@ -48,13 +48,10 @@ _m_prefetch(void *__P)
 ///
 /// \param __P
 ///    A pointer specifying the memory address to be prefetched.
-static __inline__ void __attribute__((__always_inline__, __nodebug__))
-_m_prefetchw(volatile const void *__P)
-{
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wcast-qual"
-  __builtin_prefetch ((const void*)__P, 1, 3 /* _MM_HINT_T0 */);
-#pragma clang diagnostic pop
-}
+void _m_prefetchw(volatile const void *__P);
+
+#if defined(__cplusplus)
+} // extern "C"
+#endif
 
 #endif /* __PRFCHWINTRIN_H */
diff --git a/clang/lib/Headers/xmmintrin.h b/clang/lib/Headers/xmmintrin.h
index 20e66d190113a..1fb070bca827e 100644
--- a/clang/lib/Headers/xmmintrin.h
+++ b/clang/lib/Headers/xmmintrin.h
@@ -2197,10 +2197,7 @@ _mm_storer_ps(float *__p, __m128 __a)
 #define _MM_HINT_T2  1
 #define _MM_HINT_NTA 0
 
-#ifndef _MSC_VER
-/* FIXME: We have to #define this because "sel" must be a constant integer, and
-   Sema doesn't do any form of constant propagation yet. */
-
+#if 0
 /// Loads one cache line of data from the specified address to a location
 ///    closer to the processor.
 ///
@@ -2225,6 +2222,10 @@ _mm_storer_ps(float *__p, __m128 __a)
 ///    be generated. \n
 ///    _MM_HINT_T2: Move data using the T2 hint. The PREFETCHT2 instruction will
 ///    be generated.
+///
+/// _mm_prefetch is implemented as a "library builtin" directly in Clang,
+/// similar to how it is done in MSVC. Clang will warn if the user doesn't
+/// include xmmintrin.h or immintrin.h.
 #define _mm_prefetch(a, sel) (__builtin_prefetch((const void *)(a), \
                                                  ((sel) >> 2) & 1, (sel) & 0x3))
 #endif

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.

The pre-checkin failures seem related.

@@ -138,6 +138,12 @@ let Attributes = [Const, NoThrow, RequiredVectorWidth<256>], Features = "avx" in
}
}

// PRFCHW
let Features = "prfchw", Header = "intrin.h", Attributes = [NoThrow, Const] in {
Copy link
Contributor

Choose a reason for hiding this comment

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

Although they are in the same file, both don't have "prfchw" feature in prior defination. I'm not sure if it's due to it's used by cl mode only, but I'm afraid the change here may result in compatiablilty issues.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looks like I actually had a pending change to go back to the mmx target feature, so my past self shared your concern. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:X86 clang:codegen IR generation bugs: mangling, exceptions, etc. 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