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

Update LLVM intrinsic names for LLVM 15 #1340

Closed
wants to merge 2 commits into from

Conversation

Amanieu
Copy link
Member

@Amanieu Amanieu commented Oct 13, 2022

LLVM 15 removes the pointer type from intrinsic suffixes, effectively replacing .p0i8 (and other similar types) with just .p0.

cc rust-lang/rust#102738

LLVM 15 removes the pointer type from intrinsic suffixes, effectively
replacing `.p0i8` (and other similar types) with just `.p0`.

cc rust-lang/rust#102738
@rust-highfive
Copy link

@Amanieu: no appropriate reviewer found, use r? to override

@bjorn3
Copy link
Member

bjorn3 commented Oct 13, 2022

This breaks rustc linked against older LLVM, right? We still support LLVM 13 and up.

@Amanieu
Copy link
Member Author

Amanieu commented Oct 13, 2022

It does unfortunately. According to @nikic LLVM should be automatically translating the builtins to their new name, but this doesn't seem to work correctly in some cases. Maybe this should be fixed in LLVM instead?

@bjorn3
Copy link
Member

bjorn3 commented Oct 13, 2022

Does that translation only apply for loading bitcode generated by older LLVM versions maybe?

@nikic
Copy link
Contributor

nikic commented Oct 13, 2022

Does that translation only apply for loading bitcode generated by older LLVM versions maybe?

Generally yes, but we also apply it manually here: https://github.com/rust-lang/rust/blob/fa0ca783f89a83046e6ce0383385ba5b28296435/compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp#L880-L882

@Amanieu
Copy link
Member Author

Amanieu commented Oct 13, 2022

I just looked at the code for UpgradeCallsToIntrinsic and it doesn't seem to do any renaming to remove pointer types from the intrinsic name. Pointer types seem to work for some intrinsics, like llvm.arm.ldrex.p0i32, but not others like llvm.arm.neon.vld1x2.v16i8.p0i8.

@nikic
Copy link
Contributor

nikic commented Oct 13, 2022

I just looked at the code for UpgradeCallsToIntrinsic and it doesn't seem to do any renaming to remove pointer types from the intrinsic name.

The renaming happens here: https://github.com/llvm/llvm-project/blob/096f93e73dc3f88636cdcb57515e3732385b452d/llvm/lib/IR/AutoUpgrade.cpp#L1088

Pointer types seem to work for some intrinsics, like llvm.arm.ldrex.p0i32, but not others like llvm.arm.neon.vld1x2.v16i8.p0i8.

Hm, possibly somehow related to the latter using a LLVMAnyPointerType<LLVMMatchType<0>> type, while the former uses llvm_anyptr_ty.

@nikic
Copy link
Contributor

nikic commented Oct 13, 2022

Auto-upgrade seems to work fine with opt, both with and without a simultaneous upgrade of the return type: https://llvm.godbolt.org/z/Mz6cKbPh1

@Amanieu
Copy link
Member Author

Amanieu commented Oct 13, 2022

It seems that this should be handled in LLVM. I'm going to close this PR, discussion should continue in rust-lang/rust#102738.

@Amanieu Amanieu closed this Oct 13, 2022
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.

4 participants