Repository navigation
Add AIX support - #1065
Add AIX support#1065KV2773 wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds IBM AIX build support across the core shared library build and several bundled third-party dependencies, including handling of AIX-specific macro collisions and big-endian data parsing.
Changes:
- Add AIX-specific exported-symbol list support for the shared library and wire it into the build.
- Patch external dependencies (SentencePiece, OpenCV/libpng, dlib, dr_libs, BlingFire) to compile on AIX.
- Update the UGM tokenizer’s precompiled charsmap parsing to avoid strict-aliasing and handle big-endian hosts.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| shared/ortcustomops.exp | Adds an AIX export list for shared-library symbol exports. |
| shared/api/nemo_mel_spectrogram.h | Undefines hz on AIX to avoid macro collisions with parameter names. |
| operators/tokenizer/ugm_kernels.hpp | Uses memcpy for header reads and swaps bytes on big-endian systems. |
| operators/text/vector_to_string.hpp | Renames the std::hash parameter (no functional change). |
| CMakeLists.txt | Uses .exp exports on AIX and .ver version scripts on Linux/Android. |
| cmake/externals/sentencepieceproject.cmake | Adds AIX/enum patches to the SentencePiece patch set. |
| cmake/externals/sentencepieceproject_enum.patch | Forces ScriptType enum underlying type to int32_t. |
| cmake/externals/sentencepieceproject_aix.patch | Defines endianness macro handling for AIX in SentencePiece. |
| cmake/externals/rand_aix.patch | Adds AIX fixes to OpenCV sources (macro collision + ELF/auxv guards). |
| cmake/externals/opencv.cmake | Applies additional AIX-specific OpenCV patches during fetch. |
| cmake/externals/opencv_libpng_aix.patch | Enables libpng PowerPC VSX optimizations on AIX. |
| cmake/externals/dr_libs.cmake | Applies an AIX patch to dr_libs during fetch. |
| cmake/externals/dr_libs_aix.patch | Undefines hz on AIX for dr_mp3. |
| cmake/externals/dlib.cmake | Applies an AIX patch to dlib during fetch. |
| cmake/externals/dlib_aix.patch | Updates dlib’s embedded libpng VSX detection to include AIX. |
| cmake/externals/blingfire.cmake | Applies an AIX patch and links Iconv on AIX. |
| cmake/externals/blingfire-aix.patch | Adjusts BlingFire headers for AIX (but currently breaks macOS). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sayan Shaw (sayanshaw24)
left a comment
There was a problem hiding this comment.
CIs are still failing due to a corrupt patch, please try to repro by building locally and ensure all errors are fixed:
Building Custom Rule E:/_work/1/s/out/Windows/_deps/dr_libs-subbuild/CMakeLists.txt
No update step for 'dr_libs-populate'
Performing patch step for 'dr_libs-populate'
CUSTOMBUILD : error : corrupt patch at line 6 [E:\_work\1\s\out\Windows\_deps\dr_libs-subbuild\dr_libs-populate.vcxproj]
Hii Sayan Shaw (@sayanshaw24) is it possible for the community to add a .gitattribute file similar to |
|
Hii Sayan Shaw (@sayanshaw24) , may you review it once. |
|
Hii Sayan Shaw (@sayanshaw24) , may you point out some things, that I am missing so I can improve my patches and recontribute them, so that it doesn't break the CI and can be merged asap. |
ef66725 to
e2da7a3
Compare
|
Hii Sayan Shaw (@sayanshaw24) may you review it one last time, and may point out mistakes if any, and if not may you merge it.. |
e2da7a3 to
6bb9ec0
Compare
|
Hii Justin Chu (@justinchuby) may you look into this if you get some time.. |
|
Could you resolve conflict? |
There was a problem hiding this comment.
🔵 Needs a closer look
The AIX linker/export configuration, dependency patches, and big-endian tokenizer paths require native build and runtime validation that is absent from the supplied evidence.
2 open issues
8 resolved since last review
The bounds checkxcda_blob_size + charsmap_offset >= charsmap_data_.size()can overflow (e.g.,…std::memcpy(&xcda_blob_size, &charsmap_data_[0], ...)reads 4 bytes unconditionally; if… This patch file appears to contain CRLF line endings (visible as trailing\\rin the diff). Some… This is within anamespace std { template<> struct hash<std::vector<T>> ... }specialization.…#undef hzin a public API header changes the preprocessor state for all downstream includes in… Swapping endianness on everyoperator[]access can add a measurable cost if this sits on a hot… The byte-order detection (__BYTE_ORDER__/__ORDER_BIG_ENDIAN__) and swap primitive…#include <cstring>is now included twice (also included earlier in this header). Please remove…
Review effort: Balanced
|
heyy Justin Chu (@justinchuby) resolved the conflicts.. |



Patches for building ort-extension on AIX, related to big endianness and some aix specific headers.