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

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

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.

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
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?

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