-
Notifications
You must be signed in to change notification settings - Fork 100
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
Add OpenMP builder for s390x #67
Conversation
Heads up: I'm reproducing issue #40769 when building libomp using clang:
Am I missing anything? |
This should be fixed by llvm/llvm-project#69405. What version of clang do you have? |
@iii-i Interesting... I checked out git commit cf7d4f543c73c2707e0c53bae1e7b8419e12b871, which has 8e810dc7d93bebe5e2d3980d4db084f58248b37f. For the record, more details is available at https://copr.fedorainfracloud.org/coprs/g/fedora-llvm-team/llvm-snapshots-incubator-20231106/build/6601238/ Build log is available at: https://download.copr.fedorainfracloud.org/results/@fedora-llvm-team/llvm-snapshots-incubator-20231106/fedora-rawhide-s390x/06601238-libomp/builder-live.log.gz |
Thanks for the logs! Apparently this has to do with LTO, since I can now reproduce this using a trivial example:
|
I don't think this is necessarily related to LTO, I'm seeing the failure also without LTO. The problem seems to be that the
while the
but the latter isn't actually supported by the SystemZ back-end. Not sure exactly what the best fix here is, but this seems to require at least some back-end changes. |
Is there a downside to adding a |
I agree, that seems to be the best solution. |
@tuliom your issue should be now fixed by llvm/llvm-project#71668. |
@iii-i Thank you! But the build is still failing.
The complete log is available at: https://download.copr.fedorainfracloud.org/results/@fedora-llvm-team/llvm-snapshots-incubator-20231109/fedora-37-s390x/06614614-libomp/builder-live.log.gz I'm wondering if another openmp function is missing the backchain attribute... Please let me know if you'd like to move this discussion to its own issue. |
Thanks for the analysis, I think you are correct that we need to compile another function with backchain. I posted llvm/llvm-project#71834. |
@iii-i It worked! Thank you! |
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.
LGTM.
No description provided.