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

Add cmake option AOTRITON_NAME_SUFFIX to resolve name conflicts #42

Merged
merged 10 commits into from
Sep 30, 2024

Conversation

xinyazhang
Copy link
Collaborator

This adds suffix to namespace aotriton and the library file prefix libaotriton. For example, build with -DAOTRITON_NAME_SUFFIX=123 will generate the library with namespace aotrition123 and libaotriton123_v2.so.

This is to address potential name conflicts with PyTorch since PyTorch will bundle AOTriton shared library starting 2.4+

Tested with
```
cmake .. -DCMAKE_INSTALL_PREFIX=./install_dir -DCMAKE_BUILD_TYPE=Release
-DAOTRITON_GPU_BUILD_TIMEOUT=0 -G Ninja -DTARGET_GPUS=MI200
-DAOTRITON_NAME_SUFFIX="123";
```
xinyazhang and others added 3 commits September 11, 2024 23:08
Otherwise pyaotriton may call the libaotriton_v2.so shipped with
PyTorch and cause weird bugs.
#if AOTRITON_ENABLE_SUFFIX
#define xstr(s) str(s)
#define str(s) #s
[]() -> std::string { return xstr(AOTRITON_NAME_SUFFIX); }
Copy link
Contributor

Choose a reason for hiding this comment

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

@xinyazhang I'm sure there's some nuance with the preprocessing flow here, but can you elaborate why we need two macros xstr and str?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

# target_link_libraries(aotriton INTERFACE ${AOTRITON_V2_BUILD_DIR}/${AOTRITON_LIBRARY_FILE})
# target_include_directories(aotriton INTERFACE ${CMAKE_CURRENT_SOURCE_PARENT_DIR}/include)
# target_include_directories(aotriton INTERFACE ${CMAKE_CURRENT_SOURCE_PARENT_DIR}/include)
target_link_libraries(aotriton INTERFACE aotriton_v2)
Copy link
Contributor

Choose a reason for hiding this comment

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

@xinyazhang Are these changes related to the name suffix feature? Can you please explain this changeset?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the suffix is not long added to the symbols, but also added to the output files. Linking aotriton INTERFACE can eliminate the necessary to construct the output file name with suffix again.

This forces find_package(Python3) select Python >= 3.8 by default.
User may override this to pick a higher version of Python.
@jithunnair-amd
Copy link
Contributor

Build was successful and produced following outputs:

#ls -lart /usr//lib/aotriton/lib/
total 274288
-rw-r--r-- 1 root root 280861184 Sep 27 20:40 libaotritonmy_suffix_v2.so

#grep suffix /usr//lib/aotriton/include/aotriton/ -r
/usr//lib/aotriton/include/aotriton/config.h:#define AOTRITON_NAME_SUFFIX my_suffix
/usr//lib/aotriton/include/aotriton/config.h:#define AOTRITON_NS aotritonmy_suffix

@xinyazhang xinyazhang merged commit 2ed786c into main Sep 30, 2024
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