-
Notifications
You must be signed in to change notification settings - Fork 110
DRAFT: OpenMP Target Build Scripts for El Cap #1937
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
base: develop
Are you sure you want to change the base?
Conversation
|
The following linking error occurs: After consultation, John G. found in his own tests that this error occurs when a |
|
It looks like this occurs within an atomic test. This might come from use of memcpy in the implementation of atomics for types that are not natively supported. |
| -DHIP_ROOT_DIR="/opt/rocm-${COMP_VER}/hip" \ | ||
| -DHIP_PATH=/opt/rocm-${COMP_VER}/llvm/bin \ | ||
| -DENABLE_CLANGFORMAT=On \ | ||
| -DCLANGFORMAT_EXECUTABLE=/opt/rocm-5.2.3/llvm/bin/clang-format \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should point at regular clang for clang-format
| set(CMAKE_CXX_FLAGS_RELEASE "-O2" CACHE STRING "") | ||
| set(CMAKE_CXX_FLAGS_RELWITHDEBINFO "-O2 -g" CACHE STRING "") | ||
| set(CMAKE_CXX_FLAGS_DEBUG "-O0 -g" CACHE STRING "") | ||
| set(CMAKE_CXX_FLAGS_RELEASE "--gcc-toolchain=/opt/rh/gcc-toolset-13/root/usr -O2" CACHE STRING "") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should be using the hip_4 host-config file
|
@rchen20 does the regular clang compiler work? Looking through the code, I don't see anything obviously wrong. Could the memcpy be related to the camp EventProxy? |
The regular clang compiler is not available on our El Cap platforms, so I should remove that particular script.
I don't see any memcpy's in Camp::EventProxy, but there are implementations of memcpy in Camp::resource. After removing those implementations, the same memcpy linking error occurs. I may need to do as Jason suggests and dig in to our atomic implementations and see if the memcpy is coming from there. |
|
@rchen20 I skimmed the RAJA atomics and did not see any memcpy calls. A thought....what happens if you remove the CAS atomic check here https://github.com/LLNL/RAJA/blob/develop/test/functional/forall/atomic-basic/tests/test-forall-atomic-basic.hpp#L105? |
That's a good idea, but it still gave the same error after removing the CAS operation. |
|
Maybe try eliminating entries in the list of data types we test to see if that can help pin down the issue? |
…o task/chen59/omptargetelcap
| void * custom_memcpy( void * dest, const void * src, size_t len ) | ||
| { | ||
| char * customdest = (char *) dest; | ||
| const char * customsrc = (const char *) src; | ||
|
|
||
| for ( size_t ii = 0; ii < len; ++ii ) | ||
| { | ||
| customdest[ii] = customsrc[ii]; | ||
| } | ||
|
|
||
| return dest; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't need to do this. Asking AMD about this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you hear anything back? It shouldn't be required, makes me wonder if there's a header or builtin missing that's causing problems.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They reproduced it a couple weeks ago and are looking in to solving it. The Cray compiler has these symbols in libu.a, where they apparently keep a hodgepodge collection of these implementations. I don't know where AMD's implementations lie, but they probably just need to add these memory functions in somewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @rchen20. If you don't get satisfaction on this, let me know. I'm going to try and mention it on our checkin with AMD engineering this afternoon to keep it on their radar.
Summary