Skip to content

[OpenMP][test]Flip bit-fields in 'struct flags' for big-endian in test cases #79895

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Feb 7, 2024
Merged

[OpenMP][test]Flip bit-fields in 'struct flags' for big-endian in test cases #79895

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
60e8aa5
Flip bitfields of "struct flags" for big-endian in test cases to matc…
xingxue-ibm Jan 29, 2024
1053a58
git-clang-format changes.
xingxue-ibm Jan 29, 2024
c26507f
Flip bit-fields in 'struct kmp_depend_info' for big-endian in test in…
xingxue-ibm Jan 30, 2024
27e7826
Addressed comments.
xingxue-ibm Feb 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion openmp/runtime/src/kmp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2494,7 +2494,8 @@ typedef struct kmp_dephash_entry kmp_dephash_entry_t;
#define KMP_DEP_MTX 0x4
#define KMP_DEP_SET 0x8
#define KMP_DEP_ALL 0x80
// Compiler sends us this info:
// Compiler sends us this info. Note: some test cases contain an explicit copy
// of this struct and should be in sync with any changes here.
typedef struct kmp_depend_info {
kmp_intptr_t base_addr;
size_t len;
Expand Down
21 changes: 15 additions & 6 deletions openmp/runtime/test/tasking/bug_nested_proxy_task.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,21 @@ typedef struct kmp_depend_info {
union {
kmp_uint8 flag; // flag as an unsigned char
struct { // flag as a set of 8 bits
unsigned in : 1;
unsigned out : 1;
unsigned mtx : 1;
unsigned set : 1;
unsigned unused : 3;
unsigned all : 1;
#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
Copy link
Collaborator

@jprotze jprotze Feb 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I was wondering whether this test is compiler(clang) specific, or is this portable?
Since kmp.h has the same code, it make sense to replicate the pattern here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was wondering whether this test something that is compiler(clang) specific, or is this portable?

Checked compilers in Compiler Explorer, the test works with gcc, clang, icc, icx, and zig cc. Does not seem to work with msvc.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is something that would also need to be addressed in kmp.h.

Does this versioning have a correctness implication, or is this a performance optimization?
In the latter case, from my perspective, a viable approach would be hardcode the endianess, if the two macros are undefined.

It's your choice to fix it as part of this patch or whether you prefer to fix it in a separate pr:)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would do it in a separate PR.

unsigned all : 1;
unsigned unused : 3;
unsigned set : 1;
unsigned mtx : 1;
unsigned out : 1;
unsigned in : 1;
#else
unsigned in : 1;
unsigned out : 1;
unsigned mtx : 1;
unsigned set : 1;
unsigned unused : 3;
unsigned all : 1;
#endif
} flags;
};
} kmp_depend_info_t;
Expand Down
21 changes: 15 additions & 6 deletions openmp/runtime/test/tasking/bug_proxy_task_dep_waiting.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,21 @@ typedef struct kmp_depend_info {
union {
kmp_uint8 flag; // flag as an unsigned char
struct { // flag as a set of 8 bits
unsigned in : 1;
unsigned out : 1;
unsigned mtx : 1;
unsigned set : 1;
unsigned unused : 3;
unsigned all : 1;
#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
unsigned all : 1;
unsigned unused : 3;
unsigned set : 1;
unsigned mtx : 1;
unsigned out : 1;
unsigned in : 1;
#else
unsigned in : 1;
unsigned out : 1;
unsigned mtx : 1;
unsigned set : 1;
unsigned unused : 3;
unsigned all : 1;
#endif
} flags;
};
} kmp_depend_info_t;
Expand Down
18 changes: 15 additions & 3 deletions openmp/runtime/test/tasking/hidden_helper_task/common.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,21 @@ typedef struct kmp_depend_info {
union {
unsigned char flag;
struct {
bool in : 1;
bool out : 1;
bool mtx : 1;
#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
unsigned all : 1;
unsigned unused : 3;
unsigned set : 1;
unsigned mtx : 1;
unsigned out : 1;
unsigned in : 1;
#else
unsigned in : 1;
unsigned out : 1;
unsigned mtx : 1;
unsigned set : 1;
unsigned unused : 3;
unsigned all : 1;
#endif
} flags;
};
} kmp_depend_info_t;
Expand Down