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

fix compiling errors on AIX #2181

Merged
merged 3 commits into from
Nov 20, 2021
Merged

fix compiling errors on AIX #2181

merged 3 commits into from
Nov 20, 2021

Conversation

lisr
Copy link
Contributor

@lisr lisr commented Nov 19, 2021

What

spdlog failed to compile on AIX 7.2

In file included from /~/spdlog/src/spdlog.cpp:12:
/~/spdlog/include/spdlog/details/os-inl.h:237:16: error: expected unqualified-id
    int fd = ::fileno(f);
               ^
/usr/include/stdio.h:515:25: note: expanded from macro 'fileno'
#define fileno(__p)     ((__p)->_file)
                        ^
In file included from /~spdlog/src/spdlog.cpp:12:
/~/spdlog/include/spdlog/details/os-inl.h:340:34: error: no member named 'pthread_getthreadid_np' in the global namespace
    return static_cast<size_t>(::pthread_getthreadid_np());
                               ~~^
1 warning and 2 errors generated.

with following configurations

export OBJECT_MODE=64
cmake .. -DCMAKE_C_COMPILER=xlclang -DCMAKE_CXX_COMPILER=xlclang++ -DCMAKE_POSITION_INDEPENDENT_CODE=ON

Code Change

  • fileno is a macro on AIX, so we cannot use :: scope decorator for this function.
  • AIX use pthread_self + pthread_getthrds_np instead of pthread_getthreadid_np See AIX Doc

Environment

# xlclang++ --version
IBM XL C/C++ for AIX, V16.1.0  (5725-C72, 5765-J12)
Version: 16.01.0000.0009
System Model: IBM,9119-MHE
Machine Serial Number: 10D945P
Processor Type: PowerPC_POWER8
Processor Implementation Mode: POWER 8
Processor Version: PV_8_Compat
Number Of Processors: 2
Processor Clock Speed: 4024 MHz
CPU Type: 64-bit
Kernel Type: 64-bit
LPAR Info: 24 paix871
Memory Size: 8192 MB
Good Memory Size: 8192 MB
Platform Firmware level: SC860_226
Firmware Version: IBM,FW860.90 (SC860_226)
Console Login: enable
Auto Restart: true
Full Core: false
NX Crypto Acceleration: Capable and Enabled
In-Core Crypto Acceleration: Capable, but not Enabled

@gabime
Copy link
Owner

gabime commented Nov 19, 2021

The doc states that pthread_getthreadid_np() is much far better for our needs:


The pthread_self() function does NOT return the integral thread of the calling thread. You must use pthread_getthreadid_np() to return an integral identifier for the thread.

If your code requires the unique integer identifier for the calling thread often, or in a loop, the pthread_getthreadid_np() function can provide significant performance improvements over the combination of pthread_self(), and pthread_getunique_np() calls that provide equivalent behavior.

@lisr
Copy link
Contributor Author

lisr commented Nov 20, 2021

Hi @gabime , The doc link you referred to is for IBM i 7.3, not AIX, both AIX 7.2 and future AIX 7.3 do not support pthread_getthreadid_np for now.

I've made a new commit to use pthread_getthrds_np in our case https://www.ibm.com/docs/en/aix/7.2?topic=p-pthread-getthrds-np-subroutine

pthread_getthrds_np gives a more meaningful id like this:

[20:38:38 -05:00] [I] [thread 25297381] This an info message with custom format

it's better than pthread_getunique_np (which was deprecated on AIX https://www.ibm.com/docs/en/aix/7.2?topic=p-pthread-getunique-np-subroutine)

This subroutine is not POSIX compliant and is provided only for compatibility with DCE threads. It should not be used when writing new applications.

@gabime
Copy link
Owner

gabime commented Nov 20, 2021

Thanks for clarifying.It would be interesting to perform a quick bench for this implementation:

mkdir build && cd build && cmake -DSPDLOG_BUILD_BENCH=1 ..
cd bench && ./formatter-bench %t

@lisr
Copy link
Contributor Author

lisr commented Nov 20, 2021

On same aforementioned AIX environment, here is the command line to compile

export OBJECT_MODE=64
mkdir build && cd build && cmake -DCMAKE_C_COMPILER=xlclang -DCMAKE_CXX_COMPILER=xlclang++ -DSPDLOG_BUILD_BENCH=1 ..
make
cd bench && ./formatter-bench %t

and this is the output:

bash-5.1# cd bench && ./formatter-bench %t
failed to open /proc/cpuinfo
2021-11-20T09:46:40+00:00
Running ./formatter-bench
Run on (-1 X 512.016 MHz CPU )
***WARNING*** Library was built as DEBUG. Timings may be affected.
-----------------------------------------------------
Benchmark           Time             CPU   Iterations
-----------------------------------------------------
%t               62.1 ns         62.0 ns     11136364

BTW, I just pushed another commit as the async_bench.cpp failed to compile on AIX due to:

~~/spdlog/bench/async_bench.cpp:163:12: error: reference to 'thread' is ambiguous
    vector<thread> threads;
           ^
/usr/include/sys/thread.h:106:8: note: candidate found by name lookup is 'thread'
struct  thread {
        ^
/opt/IBM/xlC/16.1.0/include2/c++/thread:301:24: note: candidate found by name lookup is 'std::__1::thread'
class _LIBCPP_TYPE_VIS thread
                       ^
~~/spdlog/bench/async_bench.cpp:163:12: error: reference to 'thread' is ambiguous
    vector<thread> threads;
           ^
/usr/include/sys/thread.h:106:8: note: candidate found by name lookup is 'thread'
struct  thread {
        ^
/opt/IBM/xlC/16.1.0/include2/c++/thread:301:24: note: candidate found by name lookup is 'std::__1::thread'
class _LIBCPP_TYPE_VIS thread
                       ^

@gabime gabime merged commit 2ab86a4 into gabime:v1.x Nov 20, 2021
@gabime
Copy link
Owner

gabime commented Nov 20, 2021

Merged. Thanks @lisr

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.

2 participants