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

Support building on AIX 7.2 with Clang #13397

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

Conversation

mustartt
Copy link

This patch supports basic operations needed for building RocksDB on AIX 7.2 with Clang or IBM OpenXL C/C++ for AIX

Changes

  • compile with -mcmodel=large as globals exceeds small table of content.
  • Implement AIX specific headers and macros in env/fs_posix.cc, port/port_posix.h, and toku_time.h.

@mustartt
Copy link
Author

Current test status on AIX 7.2: 98% tests passed, 302 tests failed out of 12159

Copy link

@ZarkoT ZarkoT left a comment

Choose a reason for hiding this comment

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

@@ -43,7 +43,7 @@
#include <sys/types.h>
#define PLATFORM_IS_LITTLE_ENDIAN (BYTE_ORDER == LITTLE_ENDIAN)
#include <alloca.h>
#elif defined(OS_FREEBSD) || defined(OS_OPENBSD) || defined(OS_NETBSD) || \
#elif defined(OS_AIX) || defined(OS_FREEBSD) || defined(OS_OPENBSD) || defined(OS_NETBSD) || \
defined(OS_DRAGONFLYBSD) || defined(OS_ANDROID)
#include <sys/endian.h>
Copy link

Choose a reason for hiding this comment

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

I don't think we have sys/endian.h on AIX. You can use sys/machine.h for the ENDIAN macros.

Copy link
Author

Choose a reason for hiding this comment

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

Good call, added now

@@ -58,7 +58,7 @@ Copyright (c) 2006, 2015, Percona and/or its affiliates. All rights reserved.
#include <stdint.h>
#include <sys/time.h>
#include <time.h>
#if defined(__powerpc__)
#if !defined(OS_AIX) && defined(__powerpc__)
Copy link

Choose a reason for hiding this comment

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

Do we need defined(__powerpc__)?

Copy link
Author

@mustartt mustartt Feb 20, 2025

Choose a reason for hiding this comment

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

This is for Linux on power that does have the header

Copy link

Choose a reason for hiding this comment

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

Oh right, I misread the code changes. Thought you had:

#if defined(__powerpc__)
#if !defined(OS_AIX) && defined(__powerpc__)

What you have looks good.

@mustartt
Copy link
Author

mustartt commented Feb 20, 2025

I think we also need `(OS_AIX) here: https://github.com/facebook/rocksdb/blob/main/env/io_posix.cc#L31

I don't think we uses any of the headers directly. Added just in case

#include <sys/statfs.h>
#include <sys/sysmacros.h>

@mustartt mustartt requested a review from ZarkoT February 20, 2025 16:53
Copy link

@ZarkoT ZarkoT left a comment

Choose a reason for hiding this comment

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

AIX changes look good to me.

I don't have the ability to approve but LGTM.

@@ -58,7 +58,7 @@ Copyright (c) 2006, 2015, Percona and/or its affiliates. All rights reserved.
#include <stdint.h>
#include <sys/time.h>
#include <time.h>
#if defined(__powerpc__)
#if !defined(OS_AIX) && defined(__powerpc__)
Copy link

Choose a reason for hiding this comment

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

Oh right, I misread the code changes. Thought you had:

#if defined(__powerpc__)
#if !defined(OS_AIX) && defined(__powerpc__)

What you have looks good.

Comment on lines +134 to 137
#elif defined(OS_AIX)
return __builtin_ppc_get_timebase();
#elif defined(__powerpc__)
return __ppc_get_timebase();
Copy link

Choose a reason for hiding this comment

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

Suggested change
#elif defined(OS_AIX)
return __builtin_ppc_get_timebase();
#elif defined(__powerpc__)
return __ppc_get_timebase();
#elif defined(__powerpc__)
#if defined(OS_AIX)
return __builtin_ppc_get_timebase();
#else
return __ppc_get_timebase();
#endif

Nit: personal preference to keep the Power targets together.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants