Fix reverse translation of OpPtrDiff with untyped pointers
#2858
Conversation
We should not lose operand type.
lib/SPIRV/SPIRVReader.cpp
Outdated
| Type *ElemTy = nullptr; | ||
| // Should we better get the type info from alloca instruction or from the | ||
| // OpUntypedVariableKHR DataType field? | ||
| if (auto *AI = dyn_cast<AllocaInst>(Operand1)) |
There was a problem hiding this comment.
What happens if we remove this if, leaving only else?
There was a problem hiding this comment.
We'll get:
ptrtoint (ptr getelementptr (ptr, ptr null, i32 1) to i64)
instead of this on the line 49 of the test:
ptrtoint (ptr getelementptr (float, ptr null, i32 1) to i64)
There was a problem hiding this comment.
I was wondering, if the pointers we compare by ptrdiif come not from an alloca instructions, but from operand of a function. In this case the type can be restored by looking at accessing it loads/stores or may be demangling the function. I'm not asking to do such analysis here, but lets say we have the worst case scenario and we can't do anything to restore the type. In this case shouldn't we get ptrtoint (ptr getelementptr (i8, ptr null, i32 1) to i64) instead of ptrtoint (ptr getelementptr (ptr, ptr null, i32 1) to i64)?
There was a problem hiding this comment.
May be resolving this can go to a different patch. This patch already improves the code base.
There was a problem hiding this comment.
I see, I believe it'd be a good idea to unify this piece of code https://github.com/KhronosGroup/SPIRV-LLVM-Translator/pull/2857/files#diff-7990d99acb2f8325d245dfecdab758eae5aebae2b54c04fd8e906212eb80ae10R3463-R3473 to be used in every place where we use untyped instructions to restore correct type.
lib/SPIRV/SPIRVReader.cpp
Outdated
| Value *Operand1 = Ops[0]->stripPointerCasts(); | ||
|
|
||
| Type *ElemTy = nullptr; | ||
| // Should we better get the type info from alloca instruction or from the |
There was a problem hiding this comment.
Both is fine IMO. May be it would be more clean to get it from OpUntypedVariableKHR as we are in SPIR-V consumer part of the translator. Please don't forget to remove the comment.
We should not lose operand type when
SPV_KHR_untyped_pointersis used.