DRAFT: Fix for spir-v implicit address space cast #651
Conversation
It is legal for spirv to implicitly cast the address space within the bitcast instruction, but we don't handle this case. This adds a check for this and creates an AddrSpaceCast instruction.
coldav
changed the title
| //value = new llvm::AddrSpaceCastInst(value, type); | ||
| //IRBuilder.Insert(value); | ||
| value = IRBuilder.CreateAddrSpaceCast(value, type); | ||
| } |
There was a problem hiding this comment.
The effect of a bitcast is supposed to be that of a store to memory as one type, and a load from the same memory as another type. I could be wrong, but I believe in general an addrspacecast is not guaranteed to be bit-pattern-preserving and this could be a wrong translation, unless I am missing a guarantee that we make somewhere that it doesn't matter?
There was a problem hiding this comment.
https://llvm.org/docs/LangRef.html#addrspacecast-to-instruction "(i.e. casting the result back to the original address space should yield the original bit pattern)." - are you sure it's not reasonable to do this? How else would we get the same effect?
There was a problem hiding this comment.
Casting the result back may yield the original bit pattern, but in the different address space it may have a different bit pattern. Also from that link: "It can be a no-op cast or a complex value modification, depending on the target and the address space pair." I'm not sure what the best way to translate this is, one possibility is actually doing an alloca to have somewhere to store and load from, another possibility is it may be okay to do ptrtoint and then inttoptr to the other address space, there may yet be more options that I'm not seeing right now.
Overview
Fix for spir-v implicit address space cast
Reason for change
It is legal for spirv to implicitly cast the address space within the bitcast instruction, but we don't handle this case.
Description of change
This adds a check for this and creates an AddrSpaceCast instruction.