[SPIR-V] Stop re-writing SPIR-V type map #3856
Conversation
MrSidims
force-pushed
the
private/MrSidims/spv_pointer_type
branch
from
c00c4dc to
9994c86
Compare
|
I would prefer to push this patch ASAP not waiting for Khronos review and a pull-down. |
There is a map used during forward translation. It links LLVM type
and SPIR-V type, that is being generated, LLVM type here is used as
a key. Due to a bug in type mapping (information is below), it's
possible that for a single pointer type in LLVM, the translator
will create 2 physically-indentical SPIR-V pointer types and with
previous implementation of mapType function the map would be changed
during the second type insertion. This patch prevents it, forcing
to reuse the type that was created first, though we would still have
an unused duplication of this type in SPIR-V module.
Please consider following LLVM IR sample:
%struct._ZTS4Args.Args = type { %struct._ZTS6Object.Object addrspace(4)* }
%struct._ZTS6Object.Object = type { i32 (%struct._ZTS6Object.Object addrspace(4)*, i32)* }
Here we have 2 LLVM type declarations. The translator will recursively
go through structure members and then through element types of the
pointer types, creating the appropriate SPIR-V types and mapping them
on LLVM types. Due to a recursive nature of this algorithm, here we will
have '%struct._ZTS6Object.Object addrspace(4)*' be processed twice with
2 SPIR-V type declaration instruction being created.
Signed-off-by: Dmitry Sidorov <[email protected]>
MrSidims
force-pushed
the
private/MrSidims/spv_pointer_type
branch
from
9994c86 to
95db1e9
Compare
Why? |
There is an assertion firing because of this bug (store instruction validation) and a customer, that uses intel/llvm build wants it be fixed ASAP. |
Signed-off-by: Dmitry Sidorov <[email protected]>
There was a problem hiding this comment.
@MrSidims, could you please report CI failure - it is not the first time I see that something is wrong on some download steps.
The patch looks good to me - we have KhronosGroup/SPIRV-LLVM-Translator#1048 submitted to track the main problem and this patch seems to be a small, but reasonable step towards proper solution
I reported it. Though, while I'm trying to get access to the logs I'm getting a timeout error. |
It's a follow up change for intel#3856 Signed-off-by: Dmitry Sidorov <[email protected]>
It's a follow up change for #3856 Signed-off-by: Dmitry Sidorov <[email protected]>
There is a map used during forward translation. It links LLVM type
and SPIR-V type, that is being generated, LLVM type here is used as
a key. Due to a bug in type mapping (information is below), it's
possible that for a single pointer type in LLVM, the translator
will create 2 physically-indentical SPIR-V pointer types and with
previous implementation of mapType function the map would be changed
during the second type insertion. This patch prevents it, forcing
to reuse the type that was created first, though we would still have
an unused duplication of this type in SPIR-V module.
Please consider following LLVM IR sample:
%struct._ZTS4Args.Args = type { %struct._ZTS6Object.Object addrspace(4)* }
%struct._ZTS6Object.Object = type { i32 (%struct._ZTS6Object.Object addrspace(4), i32) }
Here we have 2 LLVM type declarations. The translator will recursively
go through structure members and then through element types of the
pointer types, creating the appropriate SPIR-V types and mapping them
on LLVM types. Due to a recursive nature of this algorithm, here we will
have '%struct._ZTS6Object.Object addrspace(4)*' be processed twice with
2 SPIR-V type declaration instruction being created.
Signed-off-by: Dmitry Sidorov [email protected]