-
Notifications
You must be signed in to change notification settings - Fork 15k
[NFC][OpenMP] Update a test that was failing on aarch64. #164456
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
[NFC][OpenMP] Update a test that was failing on aarch64. #164456
Conversation
The failure was reported here: llvm#164039 (comment) The test is checking for the "bad" behavior so as to keep track of it. It could just be updated to check the "good" behavior and marked as XFAIL, but then it would not be as informative. The update for now is to fix the pointer arithmetic. If that isn't sufficient, we can fall back to doing the above.
|
@llvm/pr-subscribers-offload Author: Abhinav Gaba (abhinavgaba) ChangesThe failure was reported here: The test is checking for the "bad" behavior so as to keep track of it. It could just be updated to check the "good" behavior and marked as XFAIL, but then it would not be as informative. The update for now is to fix the pointer arithmetic. If that isn't sufficient, we can fall back to doing the above. Full diff: https://github.com/llvm/llvm-project/pull/164456.diff 1 Files Affected:
diff --git a/offload/test/mapping/use_device_addr/target_data_use_device_addr_class_member_ref_with_map.cpp b/offload/test/mapping/use_device_addr/target_data_use_device_addr_class_member_ref_with_map.cpp
index 5e8769eb3079d..204e1c3407724 100644
--- a/offload/test/mapping/use_device_addr/target_data_use_device_addr_class_member_ref_with_map.cpp
+++ b/offload/test/mapping/use_device_addr/target_data_use_device_addr_class_member_ref_with_map.cpp
@@ -16,7 +16,7 @@ struct ST {
int m = 0;
void f6() {
- uintptr_t offset = (uintptr_t)&d - n;
+ intptr_t offset = (uintptr_t)&d - n;
#pragma omp target data map(to : m, d)
{
void *mapped_ptr = omp_get_mapped_ptr(&d, omp_get_default_device());
@@ -34,10 +34,11 @@ struct ST {
// ref/attach modifiers:
// &ref_ptee(this[0].[d])), &ref_ptee(this[0].d), TO | FROM
// &ref_ptr(this[0].d), &ref_ptee(this[0].d), 4, ATTACH
- // EXPECTED: 1 0
- // CHECK: 0 1
- printf("%d %d\n", &d == mapped_ptr,
- (uintptr_t)&d == (uintptr_t)mapped_ptr - offset);
+ // EXPECTED: 1 0
+ // CHECK-NEXT: 0 1
+ intptr_t offset_device = (intptr_t)mapped_ptr - (intptr_t)&d;
+ printf("%d %d\n", &d == mapped_ptr, offset == offset_device);
+ printf("%lu %ld\n", offset, offset_device);
}
}
}
|
offload/test/mapping/use_device_addr/target_data_use_device_addr_class_member_ref_with_map.cpp
Outdated
Show resolved
Hide resolved
This comment was marked as outdated.
This comment was marked as outdated.
…urrent-bad-behavior-from-uda-tests
This comment was marked as outdated.
This comment was marked as outdated.
|
Maybe I messed up the rebuild while in a hurry. I'm doing a fresh one now, so should know soon. |
|
Yeah, that was a fluke. Now it's failing with: |
|
That's interesting. The difference is that somehow on ARM, in addition to having the same difference as offset, the pointers have a difference of 0xffff in the top few bytes. Could you try one last time with HEAD commit? I would like to see if using Thanks for your help! |
|
|
Thanks for helping with the experiments! I relaxed the check for now. If the issues continue affecting the test even after the codegen fix, we might have to debug further. Thanks again for your help and patience :) |
|
Thanks, the latest version seems to work now. I'll do a full rebuild to make sure. |
The failure was reported here:
#164039 (comment)
The test was checking for the "bad" behavior so as to keep track of it, but there seem to be some issues with the pointer arithmetic specific to aarch64.
The update for now is to not check for the "bad" behavior fully.
We may need to debug further if similar issues are encountered eventually once the codegen has been fixed.