Skip to content
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 LLVM 19.1.x support for LDC #4772

Merged
merged 12 commits into from
Nov 27, 2024
Merged

Add LLVM 19.1.x support for LDC #4772

merged 12 commits into from
Nov 27, 2024

Conversation

liushuyu
Copy link
Contributor

@liushuyu liushuyu commented Nov 5, 2024

This pull request adds LLVM 19.1.x support for LDC. It is based on previous work by @kinke and @thewilsonator in #4763 and #4735.

Some CI setup is required because it needs 19.1.3 binaries in the forked LLVM repository.

@kinke
Copy link
Member

kinke commented Nov 5, 2024

Cheers! I'm preparing the LLVM fork based on v19.1.3.

@kinke
Copy link
Member

kinke commented Nov 5, 2024

Okay not looking bad, just lit-test failures to look into, most apparently intrinsic-related too.

@liushuyu liushuyu force-pushed the llvm-19 branch 4 times, most recently from 8edf863 to 5265dcb Compare November 6, 2024 19:33
@liushuyu
Copy link
Contributor Author

liushuyu commented Nov 6, 2024

Done; now all tests passed on CI systems. I also "accidentally" fixed a D-Compute regression introduced in the LLVM 18 port.

@kinke
Copy link
Member

kinke commented Nov 6, 2024

Awesome! - I've released https://github.com/ldc-developers/llvm-project/releases/tag/ldc-v19.1.3, so we can switch back to v19.1.3 in the YAMLs.

@liushuyu
Copy link
Contributor Author

liushuyu commented Nov 6, 2024

Awesome! - I've released https://github.com/ldc-developers/llvm-project/releases/tag/ldc-v19.1.3, so we can switch back to v19.1.3 in the YAMLs.

Done. I have edited your commit to exclude the commit hash changes.

@liushuyu
Copy link
Contributor Author

liushuyu commented Nov 7, 2024

Done. CI passed again.

@liushuyu
Copy link
Contributor Author

liushuyu commented Nov 7, 2024

Oh, I think I accidentally shadowed #4726

#define SPIR_DATALAYOUT64 \
"e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128" \
"-v192:256-v256:256-v512:512-v1024:1024-G1"
#endif
Copy link
Member

Choose a reason for hiding this comment

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

@thewilsonator: Please take a look.

Copy link
Member

Choose a reason for hiding this comment

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

@thewilsonator: Is your approval a definitive this-is-OK, or rather a I-guess-it's-OK? :) - Just wondering wrt. #4772 (comment), the missing n parts that clang uses.

gen/dibuilder.cpp Outdated Show resolved Hide resolved
gen/functions.cpp Outdated Show resolved Hide resolved
gen/trycatchfinally.cpp Outdated Show resolved Hide resolved
@liushuyu
Copy link
Contributor Author

liushuyu commented Nov 7, 2024

xcode-select: error: invalid developer directory '/Applications/Xcode_16.app

I don't know why the macOS ARM64 CI runner had a problem. According to https://github.com/actions/runner-images/blob/main/images/macos/macos-14-arm64-Readme.md, the CI runner image did not change. Maybe it is a temporary failure that can be fixed by a retry.

@kinke
Copy link
Member

kinke commented Nov 7, 2024

Yeah don't worry about that apparent GHA hickup. And note that I can take care of the remaining nits, I'll add a changelog entry anyway. Just waiting for @thewilsonator to confirm the SPIRV stuff. - Thanks!

@liushuyu liushuyu force-pushed the llvm-19 branch 2 times, most recently from f5eb0e0 to 90c43e0 Compare November 7, 2024 15:34
@liushuyu
Copy link
Contributor Author

liushuyu commented Nov 7, 2024

Yeah don't worry about that apparent GHA hickup. And note that I can take care of the remaining nits,

I will need to rebase the pull request against the latest master branch anyways, I will just address those.

I'll add a changelog entry anyway. Just waiting for @thewilsonator to confirm the SPIRV stuff. - Thanks!

For the context, the new data layout string comes from: https://github.com/llvm/llvm-project/blob/85eec89600085a054650585d3a3287a6e0a93a50/clang/lib/Basic/Targets/SPIR.h#L248-L276

@kinke
Copy link
Member

kinke commented Nov 7, 2024

For the context, the new data layout string comes from: https://github.com/llvm/llvm-project/blob/85eec89600085a054650585d3a3287a6e0a93a50/clang/lib/Basic/Targets/SPIR.h#L248-L276

Ah thx. I see there are similar strings for SPIRV targets a bit below: https://github.com/llvm/llvm-project/blob/85eec89600085a054650585d3a3287a6e0a93a50/clang/lib/Basic/Targets/SPIR.h#L321-L367. Those have an extra n8:16:32:64- before the G1 suffix.

@liushuyu
Copy link
Contributor Author

liushuyu commented Nov 7, 2024

For the context, the new data layout string comes from: https://github.com/llvm/llvm-project/blob/85eec89600085a054650585d3a3287a6e0a93a50/clang/lib/Basic/Targets/SPIR.h#L248-L276

Ah thx. I see there are similar strings for SPIRV targets a bit below: https://github.com/llvm/llvm-project/blob/85eec89600085a054650585d3a3287a6e0a93a50/clang/lib/Basic/Targets/SPIR.h#L321-L367. Those have an extra n8:16:32:64- before the G1 suffix.

I have tested both data layouts; the one with the n8:16:32:64- part will make LDC explode due to "mismatching data layout." I believe Clang have atomics enabled inside their configurations, while LDC doesn't. Correction: nxx part should mean the target has native registers for the sizes specified after the n character. I guess it's a format issue?

@liushuyu
Copy link
Contributor Author

Rebased the pull request against the latest master branch.

@liushuyu
Copy link
Contributor Author

The CI blew up because CircleCI recently upgraded their VM host to Ubuntu 24.04 which uses 6.8.0-1018-aws that changed how ASLR (Address Space Layout Randomization) works. This breaks the sanitizers in Ubuntu 20.04 container images because they can't function correctly in a high entropy ASLR environment.
Please see google/sanitizers#1716 (comment)

@kinke
Copy link
Member

kinke commented Nov 25, 2024

Thx for digging wrt. Circle! Keeping CI working can be tedious at times indeed. I lean towards trying to use the latest Ubuntu image (24.04 or 24.10) for Circle (the 2 jobs don't offer much value, not covering a lot that isn't covered by other CI jobs); hopefully their gdb is recent enough to not hit #4389, which is the main (only?) blocker for using more recent distros for CI.

@liushuyu
Copy link
Contributor Author

Thx for digging wrt. Circle! Keeping CI working can be tedious at times indeed. I lean towards trying to use the latest Ubuntu image (24.04 or 24.10) for Circle (the 2 jobs don't offer much value, not covering a lot that isn't covered by other CI jobs); hopefully their gdb is recent enough to not hit #4389, which is the main (only?) blocker for using more recent distros for CI.

I see, Ubuntu 24.04 is the first release that got GDB regression fixed. Also Ubuntu 22.04 will also get this regression fixed very soon: https://launchpad.net/ubuntu/+source/gdb/12.1-0ubuntu1~22.04.3

@kinke
Copy link
Member

kinke commented Nov 26, 2024

Feel free to add a changelog entry yourself, analogous to

- Support for [LLVM 18](https://releases.llvm.org/18.1.0/docs/ReleaseNotes.html). The prebuilt packages use v18.1.5 (except for macOS arm64). (#4599, #4605, #4607, #4604, #4628, #4642)
and

ldc/CHANGELOG.md

Lines 37 to 38 in 00a4493

#### Platform support
- Supports LLVM 11 - 18.

Note that there's this already, please remove it (but maybe keep the PR reference):

- LLVM for prebuilt packages bumped to v18.1.8 (incl. macOS arm64). (#4712)

@liushuyu
Copy link
Contributor Author

Done. Changelog entries added.

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@kinke
Copy link
Member

kinke commented Nov 27, 2024

Thanks mate; I'll merge manually after CI has run - we've run out of free credits for Cirrus CI for this month.

@liushuyu
Copy link
Contributor Author

Thanks mate; I'll merge manually after CI has run - we've run out of free credits for Cirrus CI for this month.

There's https://github.blog/news-insights/product-news/arm64-on-github-actions-powering-faster-more-efficient-build-systems/#github-and-arm-a-new-class-of-image; maybe we will be able to use these (hopefully) next year.

@kinke
Copy link
Member

kinke commented Nov 27, 2024

Yep that's exactly the announcement I'm waiting for; I assumed/hoped they'd open it to the public in this year still.

@kinke kinke merged commit 64dd9c0 into ldc-developers:master Nov 27, 2024
18 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants