Skip to content

[Mac] Fix metal device build#12

Merged
hughperkins merged 7 commits intomainfrom
hp/mac-fix-metal-device
Jun 10, 2025
Merged

[Mac] Fix metal device build#12
hughperkins merged 7 commits intomainfrom
hp/mac-fix-metal-device

Conversation

@hughperkins
Copy link
Copy Markdown
Collaborator

@hughperkins hughperkins commented Jun 4, 2025

Issue: #

Brief Summary

Upgrade metal device from building on Mac OS ~11 to building on Mac OS 15.

See https://genesis-ai-company.slack.com/archives/C08UKDR6KGR/p1749077734526669?thread_ts=1749076147.261559&cid=C08UKDR6KGR for context

Pre-requisite for merging #5

Fix metal device build, that is currently failing on https://github.com/genesis-company/taichi/actions/runs/15453980092/job/43502395019?pr=5

copilot:summary

Walkthrough

copilot:walkthrough

Comment thread taichi/rhi/metal/metal_device.mm Outdated
#include "taichi/rhi/device.h"
#include "taichi/rhi/device_capability.h"
#include "taichi/rhi/impl_support.h"
#include "taichi/rhi/metal/metal_device.h"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What does this include do? Is it required for passing the build?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

without it I get:

Screenshot 2025-06-06 at 12 32 12 PM

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This looks super clean.

Comment thread taichi/rhi/metal/metal_device.mm Outdated
@@ -1,8 +1,12 @@
#include "taichi/rhi/metal/metal_device.h"
#include <math.h>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nit: not really necessary since sincospi.h has math.h already.

Comment thread taichi/rhi/metal/sincospi.cpp Outdated
#include <math.h>

extern "C" {
void __sincospif(float x, float *sinp, float *cosp) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'm wondering whether this function was just moved to another library file. I'm just worried that redefining it my cause symbol conflict in future.

@hughperkins
Copy link
Copy Markdown
Collaborator Author

For the sincos, this is what copilot says:

Screenshot 2025-06-06 at 11 19 31 AM

@yuhongyi
Copy link
Copy Markdown

yuhongyi commented Jun 6, 2025

For the sincos, this is what copilot says:

Screenshot 2025-06-06 at 11 19 31 AM

In this case, I think we can keep our own implementation for now. I'll approve the PR.

@hughperkins
Copy link
Copy Markdown
Collaborator Author

Digging a little, math.h contains:

#if (defined __MAC_OS_X_VERSION_MIN_REQUIRED && __MAC_OS_X_VERSION_MIN_REQUIRED < 1090) || \
    (defined __IPHONE_OS_VERSION_MIN_REQUIRED && __IPHONE_OS_VERSION_MIN_REQUIRED < 70000)
/*  __sincos and __sincosf were introduced in OSX 10.9 and iOS 7.0.  When
    targeting an older system, we simply split them up into discrete calls
    to sin( ) and cos( ).                                                     */
__header_always_inline void __sincosf(float __x, float *__sinp, float *__cosp) {
  *__sinp = sinf(__x);
  *__cosp = cosf(__x);
}

__header_always_inline void __sincos(double __x, double *__sinp, double *__cosp) {
  *__sinp = sin(__x);
  *__cosp = cos(__x);
}
#else
/*  __sincospi(x,sinp,cosp) computes the sine and cosine of pi times x with a
    single function call, storing the sine in the memory pointed to by sinp,
    and the cosine in the memory pointed to by cosp.  Edge cases match those
    of separate calls to __sinpi( ) and __cospi( ), and are documented in the
    man pages.
 
    These functions were introduced in OSX 10.9 and iOS 7.0.  Because they are
    implemented as header inlines, weak-linking does not function as normal,
    and they are simply hidden when targeting earlier OS versions.            */
__header_always_inline void __sincospif(float __x, float *__sinp, float *__cosp);
__header_always_inline void __sincospi(double __x, double *__sinp, double *__cosp);

/*  Implementation details of __sincos and __sincospi allowing them to return
    two results while allowing the compiler to optimize away unnecessary load-
    store traffic.  Although these interfaces are exposed in the math.h header
    to allow compilers to generate better code, users should call __sincos[f]
    and __sincospi[f] instead and allow the compiler to emit these calls.     */
struct __float2 { float __sinval; float __cosval; };
struct __double2 { double __sinval; double __cosval; };

extern struct __float2 __sincosf_stret(float);
extern struct __double2 __sincos_stret(double);
extern struct __float2 __sincospif_stret(float);
extern struct __double2 __sincospi_stret(double);

__header_always_inline void __sincosf(float __x, float *__sinp, float *__cosp) {
    const struct __float2 __stret = __sincosf_stret(__x);
    *__sinp = __stret.__sinval; *__cosp = __stret.__cosval;
}

__header_always_inline void __sincos(double __x, double *__sinp, double *__cosp) {
    const struct __double2 __stret = __sincos_stret(__x);
    *__sinp = __stret.__sinval; *__cosp = __stret.__cosval;
}

__header_always_inline void __sincospif(float __x, float *__sinp, float *__cosp) {
    const struct __float2 __stret = __sincospif_stret(__x);
    *__sinp = __stret.__sinval; *__cosp = __stret.__cosval;
}

__header_always_inline void __sincospi(double __x, double *__sinp, double *__cosp) {
    const struct __double2 __stret = __sincospi_stret(__x);
    *__sinp = __stret.__sinval; *__cosp = __stret.__cosval;
}
#endif

So it looks like math.h should already be providing __sincospi(float, float*, float*) and __sincospi(double, double*, double*). Kind of mysterious 🤔

@yuhongyi
Copy link
Copy Markdown

yuhongyi commented Jun 6, 2025

Digging a little, math.h contains:

#if (defined __MAC_OS_X_VERSION_MIN_REQUIRED && __MAC_OS_X_VERSION_MIN_REQUIRED < 1090) || \
    (defined __IPHONE_OS_VERSION_MIN_REQUIRED && __IPHONE_OS_VERSION_MIN_REQUIRED < 70000)
/*  __sincos and __sincosf were introduced in OSX 10.9 and iOS 7.0.  When
    targeting an older system, we simply split them up into discrete calls
    to sin( ) and cos( ).                                                     */
__header_always_inline void __sincosf(float __x, float *__sinp, float *__cosp) {
  *__sinp = sinf(__x);
  *__cosp = cosf(__x);
}

__header_always_inline void __sincos(double __x, double *__sinp, double *__cosp) {
  *__sinp = sin(__x);
  *__cosp = cos(__x);
}
#else
/*  __sincospi(x,sinp,cosp) computes the sine and cosine of pi times x with a
    single function call, storing the sine in the memory pointed to by sinp,
    and the cosine in the memory pointed to by cosp.  Edge cases match those
    of separate calls to __sinpi( ) and __cospi( ), and are documented in the
    man pages.
 
    These functions were introduced in OSX 10.9 and iOS 7.0.  Because they are
    implemented as header inlines, weak-linking does not function as normal,
    and they are simply hidden when targeting earlier OS versions.            */
__header_always_inline void __sincospif(float __x, float *__sinp, float *__cosp);
__header_always_inline void __sincospi(double __x, double *__sinp, double *__cosp);

/*  Implementation details of __sincos and __sincospi allowing them to return
    two results while allowing the compiler to optimize away unnecessary load-
    store traffic.  Although these interfaces are exposed in the math.h header
    to allow compilers to generate better code, users should call __sincos[f]
    and __sincospi[f] instead and allow the compiler to emit these calls.     */
struct __float2 { float __sinval; float __cosval; };
struct __double2 { double __sinval; double __cosval; };

extern struct __float2 __sincosf_stret(float);
extern struct __double2 __sincos_stret(double);
extern struct __float2 __sincospif_stret(float);
extern struct __double2 __sincospi_stret(double);

__header_always_inline void __sincosf(float __x, float *__sinp, float *__cosp) {
    const struct __float2 __stret = __sincosf_stret(__x);
    *__sinp = __stret.__sinval; *__cosp = __stret.__cosval;
}

__header_always_inline void __sincos(double __x, double *__sinp, double *__cosp) {
    const struct __double2 __stret = __sincos_stret(__x);
    *__sinp = __stret.__sinval; *__cosp = __stret.__cosval;
}

__header_always_inline void __sincospif(float __x, float *__sinp, float *__cosp) {
    const struct __float2 __stret = __sincospif_stret(__x);
    *__sinp = __stret.__sinval; *__cosp = __stret.__cosval;
}

__header_always_inline void __sincospi(double __x, double *__sinp, double *__cosp) {
    const struct __double2 __stret = __sincospi_stret(__x);
    *__sinp = __stret.__sinval; *__cosp = __stret.__cosval;
}
#endif

So it looks like math.h should already be providing __sincospi(float, float*, float*) and __sincospi(double, double*, double*). Kind of mysterious 🤔

Cool. We could use it's implementation directly now.

@hughperkins
Copy link
Copy Markdown
Collaborator Author

Cool. We could use it's implementation directly now.

Yeah, but mysteriously we are following the other branch of the #if. Digging.

@hughperkins
Copy link
Copy Markdown
Collaborator Author

Removed a bunch of things. Much simpler now 🙌

@hughperkins
Copy link
Copy Markdown
Collaborator Author

Here's what I found by the way:

  • __sincospi is defined in math.h, but ONLY for later mac versions, > 10.8 or so
  • when I upgraded to build on Mac OS 15, I stopped using the build-in clang, because the version is too high (17 or 18, and taichi only supports llvm <= 15)
    • and I started using llvm 15 from brew
  • so, it turns out that llvm 15 in brew doesnt define the macro saying the version of mac os x
    • result: __sincospi was not being defined by math.h
  • I added the macro stating the mac os x version, and now math.h defines the __sincospi functions 🙌

Comment thread taichi/rhi/metal/metal_device.mm Outdated
#include "taichi/rhi/device.h"
#include "taichi/rhi/device_capability.h"
#include "taichi/rhi/impl_support.h"
#include "taichi/rhi/metal/metal_device.h"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This looks super clean.

Comment thread CMakeLists.txt
# comes from; probably a XCode preset?
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -ffunction-sections -fdata-sections")
endif()
add_definitions(-D__MAC_OS_X_VERSION_MIN_REQUIRED=140000)
Copy link
Copy Markdown
Contributor

@duburcqa duburcqa Jun 9, 2025

Choose a reason for hiding this comment

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

I think you should rather specify:

              -DCMAKE_OSX_ARCHITECTURES="${OSX_ARCHITECTURES}" \
              -DCMAKE_OSX_DEPLOYMENT_TARGET="${OSX_DEPLOYMENT_TARGET}" \

with

      OSX_DEPLOYMENT_TARGET: "11.0"
      OSX_ARCHITECTURES: "x86_64;arm64"

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thanks! Will give it a shot, and see what happens. (I'm using generic clang, not OS X clang, so it's unclear to me whether these will be honored => will try, and report back on what happens).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Tried this. Here is the new code:

if(LINUX OR APPLE)
    if (NOT IOS)
        # (penguinliong) Not compatible with -fembed-bitcode. Not sure where it
        # comes from; probably a XCode preset?
        set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -ffunction-sections -fdata-sections")
    endif()
    set(CMAKE_OSX_DEPLOYMENT_TARGET "14.0")
    set(CMAKE_OSX_ARCHITECTURES "x86_64;arm64")
endif()

Result:

/Users/hugh/.cache/ti-build-cache/sccache-v041/bin/sccache /opt/homebrew/opt/llvm@15/bin/clang++  -I/Users/hugh/.cache/ti-build-cache/miniforge/envs/3.10/lib/python3.10/site-packages/numpy/_core/include -I/Users/hugh/git/taichi -I/Users/hugh/git/taichi/external/SPIRV-Cross -I/Users/hugh/git/taichi/external/spdlog/include -I/Users/hugh/git/taichi/external/glad/include -I/Users/hugh/git/taichi/external/glfw/include -ffunction-sections -fdata-sections -DTI_ISE_NONE -std=c++17 -fsized-deallocation -Wno-deprecated-declarations -Wno-shorten-64-to-32 -Wall  -Werror  -Wno-ignored-attributes  -Wno-nullability-completeness  -Wno-unused-private-field  -Wno-unneeded-internal-declaration  -Wno-unused-but-set-variable  -DTI_ARCH_ARM -DTI_PASS_EXCEPTION_TO_PYTHON -DTI_INCLUDED -fno-objc-arc -DTI_WITH_LLVM -DTI_WITH_METAL -DTI_WITH_METAL -O3 -DNDEBUG -std=gnu++17 -arch x86_64 -arch arm64 -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX15.5.sdk -mmacosx-version-min=14.0 -fPIC -fvisibility=hidden -fvisibility-inlines-hidden -MD -MT taichi/rhi/metal/CMakeFiles/metal_rhi.dir/metal_device.mm.o -MF taichi/rhi/metal/CMakeFiles/metal_rhi.dir/metal_device.mm.o.d -o taichi/rhi/metal/CMakeFiles/metal_rhi.dir/metal_device.mm.o -c /Users/hugh/git/taichi/taichi/rhi/metal/metal_device.mm
In file included from /Users/hugh/git/taichi/taichi/rhi/metal/metal_device.mm:7:
In file included from /Users/hugh/git/taichi/taichi/rhi/metal/metal_device.h:14:
In file included from /Library/Developer/CommandLineTools/SDKs/MacOSX15.5.sdk/System/Library/Frameworks/Foundation.framework/Headers/Foundation.h:9:
/Library/Developer/CommandLineTools/SDKs/MacOSX15.5.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:626:74: error: unknown type name 'NSUInteger'
FOUNDATION_EXPORT const char *NSGetSizeAndAlignment(const char *typePtr, NSUInteger * _Nullable sizep, NSUInteger * _Nullable alignp);
                                                                         ^
/Library/Developer/CommandLineTools/SDKs/MacOSX15.5.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:626:104: error: unknown type name 'NSUInteger'
FOUNDATION_EXPORT const char *NSGetSizeAndAlignment(const char *typePtr, NSUInteger * _Nullable sizep, NSUInteger * _Nullable alignp);
                                                                                                       ^
/Library/Developer/CommandLineTools/SDKs/MacOSX15.5.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:642:24: error: unknown type name 'NSInteger'
typedef NS_CLOSED_ENUM(NSInteger, NSComparisonResult) {
                       ^
/Library/Developer/CommandLineTools/SDKs/MacOSX15.5.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:642:24: error: unknown type name 'NSInteger'
/Library/Developer/CommandLineTools/SDKs/MacOSX15.5.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:650:20: error: unknown type name 'NSUInteger'
typedef NS_OPTIONS(NSUInteger, NSEnumerationOptions) {
                   ^
/Library/Developer/CommandLineTools/SDKs/MacOSX15.5.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:655:20: error: unknown type name 'NSUInteger'
typedef NS_OPTIONS(NSUInteger, NSSortOptions) {
                   ^
/Library/Developer/CommandLineTools/SDKs/MacOSX15.5.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:660:17: error: unknown type name 'NSInteger'
typedef NS_ENUM(NSInteger, NSQualityOfService) {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

(tested on this branch, https://github.com/genesis-company/taichi/tree/hp/mac-fix-metal-device-change-approach2 (since have to enable mac build etc, which isnt enabled on this PR branch yet))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think you should remove x86 actually because Metal is not available on old Intel MacBook.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

NSUIinteger needs TARGET_OS_OSX to be defined.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

(Note: I'm not intending to cross-compile here. What makes you feel I am cross-compiling? 🤔 )

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

oh -arch x86_64. Alright, I'll remove that. But the errors will persist I predict :)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

with x86 removed:

FAILED: taichi/rhi/metal/CMakeFiles/metal_rhi.dir/metal_device.mm.o
/Users/hugh/.cache/ti-build-cache/sccache-v041/bin/sccache /opt/homebrew/opt/llvm@15/bin/clang++  -I/Users/hugh/.cache/ti-build-cache/miniforge/envs/3.10/lib/python3.10/site-packages/numpy/_core/include -I/Users/hugh/git/taichi -I/Users/hugh/git/taichi/external/SPIRV-Cross -I/Users/hugh/git/taichi/external/spdlog/include -I/Users/hugh/git/taichi/external/glad/include -I/Users/hugh/git/taichi/external/glfw/include -ffunction-sections -fdata-sections -DTI_ISE_NONE -std=c++17 -fsized-deallocation -Wno-deprecated-declarations -Wno-shorten-64-to-32 -Wall  -Werror  -Wno-ignored-attributes  -Wno-nullability-completeness  -Wno-unused-private-field  -Wno-unneeded-internal-declaration  -Wno-unused-but-set-variable  -DTI_ARCH_ARM -DTI_PASS_EXCEPTION_TO_PYTHON -DTI_INCLUDED -fno-objc-arc -DTI_WITH_LLVM -DTI_WITH_METAL -DTI_WITH_METAL -O3 -DNDEBUG -std=gnu++17 -arch arm64 -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX15.5.sdk -mmacosx-version-min=14.0 -fPIC -fvisibility=hidden -fvisibility-inlines-hidden -MD -MT taichi/rhi/metal/CMakeFiles/metal_rhi.dir/metal_device.mm.o -MF taichi/rhi/metal/CMakeFiles/metal_rhi.dir/metal_device.mm.o.d -o taichi/rhi/metal/CMakeFiles/metal_rhi.dir/metal_device.mm.o -c /Users/hugh/git/taichi/taichi/rhi/metal/metal_device.mm
In file included from /Users/hugh/git/taichi/taichi/rhi/metal/metal_device.mm:7:
In file included from /Users/hugh/git/taichi/taichi/rhi/metal/metal_device.h:14:
In file included from /Library/Developer/CommandLineTools/SDKs/MacOSX15.5.sdk/System/Library/Frameworks/Foundation.framework/Headers/Foundation.h:9:
/Library/Developer/CommandLineTools/SDKs/MacOSX15.5.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:626:74: error: unknown type name 'NSUInteger'
FOUNDATION_EXPORT const char *NSGetSizeAndAlignment(const char *typePtr, NSUInteger * _Nullable sizep, NSUInteger * _Nullable alignp);
                                                                         ^
/Library/Developer/CommandLineTools/SDKs/MacOSX15.5.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:626:104: error: unknown type name 'NSUInteger'
FOUNDATION_EXPORT const char *NSGetSizeAndAlignment(const char *typePtr, NSUInteger * _Nullable sizep, NSUInteger * _Nullable alignp);
                                                                                                       ^
/Library/Developer/CommandLineTools/SDKs/MacOSX15.5.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:642:24: error: unknown type name 'NSInteger'
typedef NS_CLOSED_ENUM(NSInteger, NSComparisonResult) {
                       ^
/Library/Developer/CommandLineTools/SDKs/MacOSX15.5.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:642:24: error: unknown type name 'NSInteger'
/Library/Developer/CommandLineTools/SDKs/MacOSX15.5.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:650:20: error: unknown type name 'NSUInteger'
typedef NS_OPTIONS(NSUInteger, NSEnumerationOptions) {
                   ^
/Library/Developer/CommandLineTools/SDKs/MacOSX15.5.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:655:20: error: unknown type name 'NSUInteger'
typedef NS_OPTIONS(NSUInteger, NSSortOptions) {
                   ^
/Library/Developer/CommandLineTools/SDKs/MacOSX15.5.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:660:17: error: unknown type name 'NSInteger'
typedef NS_ENUM(NSInteger, NSQualityOfService) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ok I see. Probably related to using classic clang instead of the one provided with Xcode.

@hughperkins hughperkins enabled auto-merge (squash) June 10, 2025 18:37
@hughperkins hughperkins merged commit 33e39cf into main Jun 10, 2025
16 checks passed
@hughperkins hughperkins deleted the hp/mac-fix-metal-device branch June 10, 2025 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants