-
Notifications
You must be signed in to change notification settings - Fork 17
Add kmp_* wrapper for gomp environment #79
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
Conversation
@@ -0,0 +1,15 @@ | |||
find_package(OpenMP REQUIRED) | |||
|
|||
if ("iomp" IN_LIST OpenMP_C_LIB_NAMES OR "omp" IN_LIST OpenMP_C_LIB_NAMES OR "omp5" IN_LIST OpenMP_C_LIB_NAMES) |
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.
Does this match "libiomp", "libiomp5", "libomp", "libomp5"?
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.
Yes it should. :)
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.
Yes it should. :)
If so, then the "omp" IN_LIST OpenMP_C_LIB_NAMES
will also match libgomp
? And this is not what we want?
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.
then the "omp" IN_LIST OpenMP_C_LIB_NAMES will also match libgomp
No, I suppose. libgomp has the name gomp
in OpenMP_C_LIB_NAMES
. IN_LIST
of cmake performs a full-name search, instead of finding substr.
"libiomp", "libiomp5", "libomp", "libomp5"
It should only match iomp omp omp5
if (incr == 1) { | ||
trip_count = *pupper - *plower + 1; | ||
} else if (incr == -1) { | ||
trip_count = *plower - *pupper + 1; | ||
} else if (incr > 0) { | ||
// upper-lower can exceed the limit of signed type | ||
trip_count = (UT)(*pupper - *plower) / incr + 1; | ||
} else { | ||
trip_count = (UT)(*plower - *pupper) / (-incr) + 1; |
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.
Seems we can combine all these into a single statement trip_count = (UT)(*pupper - *plower) / incr + 1;
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.
It is copied and simplified from libomp of LLVM. I would like to keep the original code if possible.
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.
I see, then it would be better to add a reference link here?
ping... any further comments? |
if (incr == 1) { | ||
trip_count = *pupper - *plower + 1; | ||
} else if (incr == -1) { | ||
trip_count = *plower - *pupper + 1; | ||
} else if (incr > 0) { | ||
// upper-lower can exceed the limit of signed type | ||
trip_count = (UT)(*pupper - *plower) / incr + 1; | ||
} else { | ||
trip_count = (UT)(*plower - *pupper) / (-incr) + 1; |
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.
I see, then it would be better to add a reference link here?
@ZhennanQin @kurapov-peter any comments from your side? We need to merge this as there's some PR dependency now. |
|
||
extern "C" { | ||
int gc_runtime_keep_alive = 0; | ||
void gc_arrive_at_barrier(barrier_t *b, barrier_idle_func idle_func, |
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.
What's the usage scenario for the barrier?
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.
it is not currently used for now. But we will introduce cpu barrier ops in CPURuntime in near future.
|
||
WIDTH: int = 80 | ||
intel_license: list[str] = [ | ||
intel_license: List[str] = [ |
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.
@WangJialei-A Hi, in my local python (3.8.10), list[str]
is not allowed. I have updated the script here.
omp dialect of MLIR is lowered to
kmp_*
function calls to libomp (and libiomp). These symbols are not provided by gomp. This will force the end-users to use clang instead of g++ to compile the runtime. To avoid this issue, as is recommended by the openmp team, we provide a wrapper over the standard omp primitives to implement the libomp symbols based on gomp. Currently limited features are supported, including static schedule omp-for and omp-section.The wrapper will be disabled when the runtime is linked with libomp/libiomp.