Moving openmp to a runtime in newer versions of LLVM #3799
Conversation
|
Test report by @Crivella Overview of tested easyconfigs (in order)
Build succeeded for 0 out of 1 (1 easyconfigs in total) |
|
Test report by @Crivella Overview of tested easyconfigs (in order)
Build succeeded for 1 out of 1 (1 easyconfigs in total) NOTESOMPT is now being properly enabled |
|
@boegelbot please test @ jsc-zen3 |
|
@Crivella: Request for testing this PR well received on jsczen3l1.int.jsc-zen3.fz-juelich.de PR test command '
Test results coming soon (I hope)... - notification for comment with ID 3008838690 processed Message to humans: this is just bookkeeping information for me, |
|
Test report by @Crivella Overview of tested easyconfigs (in order)
Build succeeded for 1 out of 1 (1 easyconfigs in total) |
|
Test report by @boegelbot Overview of tested easyconfigs (in order)
Build succeeded for 1 out of 1 (1 easyconfigs in total) |
|
@boegelbot please test @ jsc-zen3 |
|
@Crivella: Request for testing this PR well received on jsczen3l1.int.jsc-zen3.fz-juelich.de PR test command '
Test results coming soon (I hope)... - notification for comment with ID 3009445052 processed Message to humans: this is just bookkeeping information for me, |
|
Test report by @boegelbot Overview of tested easyconfigs (in order)
Build succeeded for 1 out of 1 (1 easyconfigs in total) |
|
@boegelbot please test @ jsc-zen3 |
|
@Crivella: Request for testing this PR well received on jsczen3l1.int.jsc-zen3.fz-juelich.de PR test command '
Test results coming soon (I hope)... - notification for comment with ID 3010274800 processed Message to humans: this is just bookkeeping information for me, |
|
Test report by @boegelbot Overview of tested easyconfigs (in order)
Build succeeded for 1 out of 1 (1 easyconfigs in total) |
|
While my LLVM build is running, here's a code snippet to verify that the fix works: Click to open#include <assert.h>
#include <omp.h>
#include <omp-tools.h>
#include <stdio.h>
/* This callback will only be invoked when OMPT is built for offload */
static int device_initialize_called = 0;
void
callback_device_initialize( int device_num,
const char* type,
ompt_device_t* device,
ompt_function_lookup_t lookup,
const char* documentation )
{
printf( "[%s] device_num = %d | type = %s | device = %p | lookup = %p | documentation = %s\n",
__FUNCTION__,
device_num,
type,
device,
lookup,
documentation );
device_initialize_called = 1;
}
int
tool_initialize( ompt_function_lookup_t lookup,
int initial_device_num,
ompt_data_t* tool_data )
{
ompt_set_callback_t ompt_set_callback = ( ompt_set_callback_t )lookup( "ompt_set_callback" );
assert( ompt_set_callback && "Could not find ompt_set_callback" );
ompt_set_result_t result = ompt_set_callback( ompt_callback_device_initialize, ( ompt_callback_t )callback_device_initialize );
assert( result == ompt_set_always && "Callback could not be registered!" );
return 1; /* non-zero indicates success */
}
ompt_start_tool_result_t *
ompt_start_tool( unsigned int omp_version,
const char* runtime_version )
{
static ompt_start_tool_result_t tool;
tool.initialize = &tool_initialize;
return &tool;
}
int main( void )
{
int ran_on_host = 1;
#pragma omp target map( tofrom: ran_on_host )
{
ran_on_host = omp_is_initial_device();
}
assert( device_initialize_called && "Device init. callback was not called" );
return ran_on_host;
}The code can be tested with (requires CUDA and a working GPU): $ clang -fopenmp --offload-arch=sm_75 code.c
$ OMP_TARGET_OFFLOAD=mandatory ./a.out |
|
Forgot Here's a manual upload: https://gist.github.com/Thyre/1407f397f09e8ee168b162e9f2a8be97 |
|
Test run with the LLVM: |
|
@boegel I think this is ready for review |
|
USED WRONG EASYBLOCK, see easybuilders/easybuild-framework#4929 Build with ROCm-LLVM on MI250X nodes: Test report by @Thyre Overview of tested easyconfigs (in order)
Build succeeded for 1 out of 1 (1 easyconfigs in total) ROCm-LLVM still has support disabled. However, it's quite hard to figure out if the LLVM EasyBlock was even picked up correctly. The component EasyBlocks are not stored in the build artifacts, see also easybuilders/easybuild-framework#4862 |
Signed-off-by: Jan André Reuter <jan@zyten.de>
Manual test report
|
Manual test report
|
|
With ROCm-LLVM (on Linux, i.e. my Arch Linux machine) in a single stage build: The full log is unfortunately too large to link... |
|
With ROCm-LLVM (on jrc0850.jureca, i.e. Rocky Linux 9.5 VM) in multi stage build: |
|
Looks like the install script is looking in the entirely incorrect folder. The file is at: instead of |
|
Maybe in the same spirit as the symlink i am doing for |
Yeah... I'm trying to figure out where LLVM 19 & ROCm 6.4.1 diverge. I feel like this is just one or two commits that differ and cause these issues. |
Since their CI runners seemingly don't show the issue, they either do things differently, or do not build both
Since the installed LLVM has the expected infrastructure / folder & file layout again, the issue won't occur. Clang should know where to find its libraries. Only the intermediate testing is the issue, because Clang doesn't expect this. This is at least my understanding. |
In theory yes, in practice if we want things moving forward and the fix is small i'd say we can patch it.
This is all stuff we work around ourselves in the EB or in the patch files To my understanding of There is no way to set manually the variables that lead to
Yes that is the only difference i've observed and probably a bug in the build system. When building openmp as a project There are other stuff we are patching out like re-adding |
Might be worth exploring more. Might not be surprised if the are building the projects separately (as standalones) and at that point the |
|
Another possibility for the CI is that they are using customized config files for lit. In that sense our patches, kinda do something similar |
A cache variable is global. So for the non-standalone build it will be set already by the time this code is entered. For the standalone build the possible support has to be determined from the installed files. We are not doing the standalone build so the cache variable is what is going to be used.
Right, you cannot set However I see an issue with the TLDR: We should pass |
I think the logic behind that section is the following: |
I guess yes. But that means they ignore the users choice and should rather error as they do for the host OMPT. I opened a PR with them: llvm/llvm-project#146865 |
I think that even with this, it would not work without having openmp in the runtimes build. The CMake cache of the runtimes build is not shared with the one of the projects to my understanding, which was the entire reason the checks for OMP + OMPT where off causing LIBOMPTARGET to not have OMPT support. I really do not see why we should not follow the preferred upstream build procedures with a minor fix to get 200 tests pass because libomp is not in the expected location for clang rather than change the entire build system |
|
I haven't submitted my last message:
Ok, found it: LLVM_PROJECTS are added via So if openmp and offload are both runtimes they can see each others cache and the OMPT variable(s) is correctly set such that support is enabled. TLDR: You are right with
That wasn't my intention. We should definitely go with this approach of moving openmp to runtimes. And in addition set |
|
@boegelbot please test @ jsc-zen3 |
|
@Crivella: Request for testing this PR well received on jsczen3l1.int.jsc-zen3.fz-juelich.de PR test command '
Test results coming soon (I hope)... - notification for comment with ID 3056409667 processed Message to humans: this is just bookkeeping information for me, |
|
Test report by @boegelbot Overview of tested easyconfigs (in order)
Build succeeded for 4 out of 4 (4 easyconfigs in total) |
|
Test report by @Thyre Overview of tested easyconfigs (in order)
Build succeeded for 1 out of 1 (1 easyconfigs in total) |
@Flamefire I'm looking at that as a nice-to-have in case you want to add a follow-up PR (or open an issue) |
Done #3853 |
With newer versions of LLVM, openmp has been moved from a project to a runtime.
Currently both version can be used but
projectsbuild are being deprecated llvm/llvm-project#136314This change is needed also in order to fix
as building
openmpas a project makes it so the proper CMake variablesLIBOMP_OMPT_SUPPORTandLIBOMPTARGET_OMPT_SUPPORTare not set during the runtime builds, causing OMPT to not be enabled when buildingoffloadTODO
It seems also in
18.1.xopenmpcan be built as a runtime, will defer to the result of the test builds...WORKED also with
18.1.8libomptargettestsWith this fix
libomptargettests are failing again. In this caselibomp.sois not being found as it is not anymore insilde.lib/libomp.sobut./runtimes/runtimes-bins/openmp/runtime/src/libomp.soLIBRARY_PATH/LD_LIBRARY_PATHduring testinglibomp.soto the expected one in the build directory