-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Cast allocas to default address space #135025
base: master
Are you sure you want to change the base?
Conversation
Pointers for variables all need to be in the same address space for correct compilation. Therefore ensure that even if an `alloca` is created in a different address space, it is casted to the default address space before its value is used. This is necessary for the amdgpu target and others where the default address space for `alloca`s is not 0. For example the following code compiles incorrectly when not casting the address space to the default one: ```rust fn f(p: *const i8 /* addrspace(0) */) -> *const i8 /* addrspace(0) */ { let local = 0i8; /* addrspace(5) */ let res = if cond { p } else { &raw const local }; res } ``` results in ```llvm %local = alloca addrspace(5) i8 %res = alloca addrspace(5) ptr if: ; Store 64-bit flat pointer store ptr %p, ptr addrspace(5) %res else: ; Store 32-bit scratch pointer store ptr addrspace(5) %local, ptr addrspace(5) %res ret: ; Load and return 64-bit flat pointer %res.load = load ptr, ptr addrspace(5) %res ret ptr %res.load ``` For amdgpu, `addrspace(0)` are 64-bit pointers, `addrspace(5)` are 32-bit pointers. The above code may store a 32-bit pointer and read it back as a 64-bit pointer, which is obviously wrong and cannot work. Instead, we need to `addrspacecast %local to ptr addrspace(0)`, then we store and load the correct type.
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @compiler-errors (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
Isn't this going to add a redundant addrspace cast for LLVM targets that don't care about this? |
No, LLVM checks itself if the cast is needed or not. So, if we checked this here, we would check twice in the case the cast is needed. The code in LLVM is here: The code for constant casts is here: |
Pointers for variables all need to be in the same address space for correct compilation. Therefore ensure that even if an
alloca
is created in a different address space, it is casted to the default address space before its value is used.This is necessary for the amdgpu target and others where the default address space for
alloca
s is not 0.For example the following code compiles incorrectly when not casting the address space to the default one:
results in
For amdgpu,
addrspace(0)
are 64-bit pointers,addrspace(5)
are 32-bit pointers.The above code may store a 32-bit pointer and read it back as a 64-bit pointer, which is obviously wrong and cannot work. Instead, we need to
addrspacecast %local to ptr addrspace(0)
, then we store and load the correct type.Tracking issue: #135024