cg_llvm: Clean up FFI calls for operand bundles #132342
Conversation
|
rustbot has assigned @compiler-errors. Use |
rustbot
added
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
|
added in LLVM 19: llvm/llvm-project@584253c |
|
Oh boo, that makes things a bit more complicated. Thanks for the pointer. |
|
@rustbot author |
rustbot
added
S-waiting-on-author
Zalathar
force-pushed
the
operand-bundle
branch
from
9bd7c15
to
c307159
Compare
|
Rather than back out the switch to So after the next LLVM bump when 19 becomes baseline, the polyfill can just be deleted. |
|
In discussions elsewhere there was some brief confusion over whether the requirement should be 19.0 or 19.1. It turns out that LLVM has switched to a policy of making their first release in a series X.1, to distinguish it from development builds prior to the release process: https://discourse.llvm.org/t/rfc-name-the-first-release-from-a-branch-n-1-0-instead-of-n-0-0/75384. I'm going to stick with <19.0 as the condition for the polyfill, because I'd rather err on the side of not having the polyfill. If someone really wants to try building against prerelease LLVM 19 for some reason, whether or not that works is their problem. |
|
Looks like the polyfill works in PR CI. @rustbot ready |
rustbot
added
S-waiting-on-review
| /// Owns an [`OperandBundle`], and will dispose of it when dropped. | ||
| pub(crate) struct OperandBundleOwned<'a> { | ||
| raw: ptr::NonNull<OperandBundle<'a>>, |
There was a problem hiding this comment.
ah, and the dependence on the &[&'a Value] is what means we can't just make OperandBundleOwned be actually this, right?
type OperandBundleOwned = OperandBundle<'static>;There was a problem hiding this comment.
Yeah, from what I can tell an operand bundle holds copies of the value pointers internally, so it would be unsound for the bundle to outlive those values.
(In practice, I think the lifetime of values is always 'll, i.e. the lifetime of the LLVM context.)
There was a problem hiding this comment.
Oh one other relevant point, OperandBundleOwned is closer to Box<OperandBundle>, since its representation is a pointer to the underlying bundle, and the owner is responsible for deallocating on drop.
So the two types would still have to be different, unless we hypothetically had &own references to handle dropping.
|
I'm satisfied. @bors r+ |
bors
added
S-waiting-on-bors
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#129394 (Don't lint `irrefutable_let_patterns` on leading patterns if `else if` let-chains) - rust-lang#131856 (TypingMode: merge intercrate, reveal, and defining_opaque_types) - rust-lang#132322 (powerpc64-ibm-aix: update maintainters) - rust-lang#132327 (Point to Fuchsia team in platform support docs) - rust-lang#132332 (Use `token_descr` more in error messages) - rust-lang#132338 (Remove `Engine`) - rust-lang#132340 (cg_llvm: Consistently use safe wrapper function `set_section`) - rust-lang#132342 (cg_llvm: Clean up FFI calls for operand bundles) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#132342 - Zalathar:operand-bundle, r=workingjubilee cg_llvm: Clean up FFI calls for operand bundles All of these FFI functions have equivalents in the stable LLVM-C API, though `LLVMBuildCallBr` requires a temporary polyfill on LLVM 18. This PR also creates a clear split between `OperandBundleOwned` and `OperandBundle`, and updates the internals of the owner to be a little less terrifying.
All of these FFI functions have equivalents in the stable LLVM-C API, though
LLVMBuildCallBrrequires a temporary polyfill on LLVM 18.This PR also creates a clear split between
OperandBundleOwnedandOperandBundle, and updates the internals of the owner to be a little less terrifying.