OpenMPToolsInterface / LLVM-openmp

This is a stale repository and only there for the commit history. All development moved over to the llvm-project repository! Was: LLVM OpenMP runtime with experimental changes for OMPT (Preview of TR7 features in *_tr branches)
Other
16 stars 13 forks source link

Return address for omp_init_lock #36

Closed sconvent closed 7 years ago

sconvent commented 7 years ago

The changes in https://github.com/OpenMPToolsInterface/LLVM-openmp/commit/31483d0467188324d6f22aee7478226e0f7ae72a seem to be not compatible with intel compiler. The binary calls __kmpc_init_lock directly without going through omp_init_lock. Is this code generated by the compiler, or inlined from the library? The value of the 4th argument codeptr_ra seems to be the same as for the 3rd argument.

hansangbae commented 7 years ago

Intel compiler generates direct calls to _kmpc entry functions if it is compiling for Intel architecture for some of the OpenMP runtime routines, so I think the return address needs to be computed within _kmpc entries in such cases.

jmellorcrummey commented 7 years ago

I agree with @sconvent and @hansangbae. The kmpc_* routines should not have codeptr_ra as an argument. Instead, they should determine the codeptr_ra themselves internally by calling builtinreturn_address().

jprotze commented 7 years ago

Currently a call to omp_init_lock/__kmpc_init_lock does not initialize the runtime. This means, that a tool would not get an event for the initialization of the lock. Should the call trigger runtime initialization, so that a tool has the chance to get loaded?

jmellorcrummey commented 7 years ago

I don't think that we shouldn't trigger runtime initialization for a lock. I think the language committee would agree with my point of view.

hansangbae commented 7 years ago

The runtime is actually initialized by calling kmp_entry_gtid if omp_init_lock is called, and Intel compiler inserts a call to kmpc_global_thread_num instead for direct calls to kmpc_init_lock, which eventually calls kmp_entry_gtid.

jprotze commented 7 years ago

Fixed by 9131ddbf5620342501ffa707aadb2ec77d486f77