Skip to content

Enable prefetch on MSVC #70

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.

Sign up for GitHub

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
sschoener wants to merge 6 commits into
base: unity-master
Choose a base branch
from
Open

Enable prefetch on MSVC #70

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c704831
Enable prefetch on MSVC
Dec 10, 2022
f3c778a
Respect NO_PREFETCH and add missing prefetch in gc_inline.h
Dec 15, 2022
81823f1
Update gc_inline.h
sschoener Feb 1, 2023
a84e95a
Update gcconfig.h
sschoener Feb 1, 2023
729106f
Update gc_inline.h
sschoener Feb 1, 2023
d7369af
Update gcconfig.h
sschoener Feb 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions include/gc_inline.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@
#include "gc.h"
#include "gc_tiny_fl.h"

# ifdef _MSC_VER
Copy link
Copy Markdown

@ivmai ivmai Mar 26, 2023

Choose a reason for hiding this comment

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

In case of public headers (like gc_inline.h), it would be better to include additional headers only for the condition which require it, e.g. in this case it could be: !defined(GC_PREFETCH_FOR_WRITE) && !defined(GC_NO_PREFETCH_FOR_WRITE) && defined(_MSC_VER) && (defined(_M_X64) || ...)

# include <intrin.h> /* for prefetch intrinsics */
# endif

#if GC_GNUC_PREREQ(3, 0)
# define GC_EXPECT(expr, outcome) __builtin_expect(expr,outcome)
/* Equivalent to (expr), but predict that usually (expr)==outcome. */
Expand All @@ -54,6 +58,8 @@
#ifndef GC_PREFETCH_FOR_WRITE
# if GC_GNUC_PREREQ(3, 0) && !defined(GC_NO_PREFETCH_FOR_WRITE)
# define GC_PREFETCH_FOR_WRITE(x) __builtin_prefetch((x), 1)
# elif defined(_MSC_VER) && !defined(GC_NO_PREFETCH_FOR_WRITE) && !defined(_M_ARM) && !defined(_M_ARM64)
# define GC_PREFETCH_FOR_WRITE(x) _mm_prefetch((x), _MM_HINT_T0)
Copy link
Copy Markdown
Member

@joncham joncham Jan 10, 2023

Choose a reason for hiding this comment

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

Work to support WIndows ARM64 recently landed by @jem-patel . Assuming the Mono branch for these prefetch changes has that work, just make sure that platform/arch still builds on CI with these changes. I don't think this intrinsic exists on ARM64.

Copy link
Copy Markdown

@TautvydasZilys TautvydasZilys Jan 10, 2023

Choose a reason for hiding this comment

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

Does this file get included in IL2CPP? If so, we need both ARM32 & ARM64 paths as we support those for UWP. If it's only used by Mono, then we need ARM64 path for Windows Standalone.

From what I can tell looking at the headers, these intrinsics are available there:

__MACHINEARM_ARM64(void __cdecl __prefetch(const void *))
__MACHINEARM64(void __cdecl __prefetch2(const void *, uint8_t prfop))
__MACHINEARM(void __cdecl __prefetchw(const void *))

I don't know which one(s) we want to use.

Copy link
Copy Markdown
Author

@sschoener sschoener Feb 1, 2023

Choose a reason for hiding this comment

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

My proposal is this

  • we have shipped without any of this for the longest time,
  • I have no idea which of the ARM intrinsics to use and I have no machine available for testing as of writing,
  • hence we should exclude ARM, i.e. wrap in !defined(_M_ARM) && !defined(_M_ARM64)

Thoughts?

# else
# define GC_PREFETCH_FOR_WRITE(x) (void)0
# endif
Expand Down
8 changes: 8 additions & 0 deletions include/private/gcconfig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@
# include <stddef.h> /* For size_t etc. */
# endif

# ifdef _MSC_VER
# include <intrin.h> /* for prefetch intrinsics */
# endif

/* Note: Only wrap our own declarations, and not the included headers. */
/* In this case, wrap our entire file, but temporarily unwrap/rewrap */
/* around #includes. Types and macros do not need such wrapping, only */
Expand Down Expand Up @@ -3195,6 +3199,8 @@ EXTERN_C_BEGIN
#ifndef PREFETCH
# if GC_GNUC_PREREQ(3, 0) && !defined(NO_PREFETCH)
# define PREFETCH(x) __builtin_prefetch((x), 0, 0)
# elif defined(_MSC_VER) && !defined(NO_PREFETCH) && !defined(_M_ARM) && !defined(_M_ARM64)
# define PREFETCH(x) _mm_prefetch((x), _MM_HINT_T0)
# else
# define PREFETCH(x) (void)0
# endif
Expand All @@ -3203,6 +3209,8 @@ EXTERN_C_BEGIN
#ifndef GC_PREFETCH_FOR_WRITE
# if GC_GNUC_PREREQ(3, 0) && !defined(GC_NO_PREFETCH_FOR_WRITE)
# define GC_PREFETCH_FOR_WRITE(x) __builtin_prefetch((x), 1)
# elif defined(_MSC_VER) && !defined(GC_NO_PREFETCH_FOR_WRITE) && !defined(_M_ARM) && !defined(_M_ARM64)
# define GC_PREFETCH_FOR_WRITE(x) _mm_prefetch((x), _MM_HINT_T0)
# else
# define GC_PREFETCH_FOR_WRITE(x) (void)0
# endif
Expand Down