-
-
Notifications
You must be signed in to change notification settings - Fork 267
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
tree-wide: port LDC to LLVM 18 #4599
Conversation
If you could add |
Will do. I cannot add LLVM 18 CI at the moment due to LLVM upstream missing binary packages for arm64 macOS. |
@thewilsonator Done. Added files from LLVM upstream source. |
Thx a lot, much appreciated! |
#else | ||
val = llvm::ConstantFoldCastOperand(llvm::Instruction::ZExt, val, I8PtrTy, *gDataLayout); | ||
#endif | ||
} |
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.
This could probably be simplified to something like val = DtoConstUbyte(val->isNullValue() ? 0 : 1);
.
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.
Replaced.
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.
@JohanEngelen: I just remembered #4559 - I guess this might be dangerous here too?
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.
yeah, good point. At the very least, this change should be in its own PR, with careful checking.
Keep this (big) PR about LLVM18 compatibility only.
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.
Should I just revert the changes to use LLVM constant folding here?
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.
Yeah please, and sorry about the wrong hint.
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.
Yeah please, and sorry about the wrong hint.
No problem, just a partial revert away. Now pushed.
There's also ldc/cmake/Modules/FindLLVM.cmake Lines 30 to 63 in ef72ded
llvm-config* executable.
|
b56613e
to
b330225
Compare
CI failures seems unrelated and can be retried |
9f4849f
to
4bf2630
Compare
https://github.com/ldc-developers/ldc/actions/runs/8410785498/job/23029649005?pr=4599 This CI job needs a retry. |
... LLVM upstream release v18.1.2
... upstream release v18.1.2
* Use getVoidPtrType helper * Replace #define with constexpr
[Following up with some fixes & additions in #4604.] |
This pull request adds LLVM 18.1.x support to LDC.
LLVM recently switched to a new versioning scheme, where LLVM 18.1.0 was the first stable release of LLVM 18 series.
Aside from the usual API renaming,
zext
can not be used directly now and needs to be utilized through a constant folding mechanism.