Fix SPIR-V global to function replacement for differing load types #2160

sarnex · 2023-09-19T19:09:11Z

In some cases, we will see IR with the following

@__spirv_BuiltInGlobalInvocationId = external dso_local local_unnamed_addr addrspace(1) constant <3 x i64>, align 32

...

%0 = load <6 x i32>, ptr addrspace(1) @__spirv_BuiltInGlobalInvocationId, align 32
%1 = extractelement <6 x i32> %0, i64 0

Note the global type and load type are different. Change the handling of vector loads from vector globals to reconstruct the global vector type and then bitcast to the load type.

Thanks to @jcranmer-intel for helping me find the simplest solution.

@jcranmer-intel

In some cases, we will see IR with the following ``` @__spirv_BuiltInGlobalInvocationId = external dso_local local_unnamed_addr addrspace(1) constant <3 x i64>, align 32 ... %0 = load <6 x i32>, ptr addrspace(1) @__spirv_BuiltInGlobalInvocationId, align 32 %1 = extractelement <6 x i32> %0, i64 0 ``` Note the global type and load type are different. Change the handling of vector loads from vector globals to reconstruct the global vector type and then bitcast to the load type. Thanks to @jcranmer-intel for helping me find the simpliest solution. Signed-off-by: Sarnie, Nick <nick.sarnie@intel.com>

Signed-off-by: Sarnie, Nick <nick.sarnie@intel.com>

sarnex · 2023-09-19T21:13:38Z

@jcranmer-intel Looks like I don't have permission to add reviewers, but here's the PR.

lib/SPIRV/SPIRVUtil.cpp

Signed-off-by: Sarnie, Nick <nick.sarnie@intel.com>

lib/SPIRV/SPIRVUtil.cpp

asudarsa · 2023-09-29T04:22:12Z

Hi @sarnex

I am wondering why the input LLVM IR would try to extract a 32-bit value from a 64-bit vector. Can you please point out the scenario in which such code is generated and is valid?

Thanks

lib/SPIRV/SPIRVUtil.cpp

Signed-off-by: Sarnie, Nick <nick.sarnie@intel.com>

sarnex · 2023-09-29T14:57:33Z

Hi @asudarsa,
Yes, the input IR is valid. In this case it is the result of an optimization inside InstCombine. It's in the combineLoadToOperationType function where it changes the load type to be the use type in some cases. We have a <3 x i64> load with a single use which is a bitcast from <3 x i64> to <6 x i32>, so it just changes the load type and gets rid of the cast.

asudarsa

Thanks for answers, LGTM

sarnex · 2023-09-29T20:41:34Z

@MrSidims I think we can merge this bad boy, thanks

@jcranmer-intel

…ng load types (KhronosGroup#2160) In some cases, we will see IR with the following @__spirv_BuiltInGlobalInvocationId = external dso_local local_unnamed_addr addrspace(1) constant <3 x i64>, align 32 ... %0 = load <6 x i32>, ptr addrspace(1) @__spirv_BuiltInGlobalInvocationId, align 32 %1 = extractelement <6 x i32> %0, i64 0 Note the global type and load type are different. Change the handling of vector loads from vector globals to reconstruct the global vector type and then bitcast to the load type. Thanks to @jcranmer-intel for helping me find the simplest solution.

@jcranmer-intel

…ing load types (KhronosGroup#2160) In some cases, we will see IR with the following @__spirv_BuiltInGlobalInvocationId = external dso_local local_unnamed_addr addrspace(1) constant <3 x i64>, align 32 ... %0 = load <6 x i32>, ptr addrspace(1) @__spirv_BuiltInGlobalInvocationId, align 32 %1 = extractelement <6 x i32> %0, i64 0 Note the global type and load type are different. Change the handling of vector loads from vector globals to reconstruct the global vector type and then bitcast to the load type. Thanks to @jcranmer-intel for helping me find the simplest solution.

@jcranmer-intel

…ing load types (KhronosGroup#2160) In some cases, we will see IR with the following @__spirv_BuiltInGlobalInvocationId = external dso_local local_unnamed_addr addrspace(1) constant <3 x i64>, align 32 ... %0 = load <6 x i32>, ptr addrspace(1) @__spirv_BuiltInGlobalInvocationId, align 32 %1 = extractelement <6 x i32> %0, i64 0 Note the global type and load type are different. Change the handling of vector loads from vector globals to reconstruct the global vector type and then bitcast to the load type. Thanks to @jcranmer-intel for helping me find the simplest solution.

@jcranmer-intel

…ing load types (KhronosGroup#2160) In some cases, we will see IR with the following @__spirv_BuiltInGlobalInvocationId = external dso_local local_unnamed_addr addrspace(1) constant <3 x i64>, align 32 ... %0 = load <6 x i32>, ptr addrspace(1) @__spirv_BuiltInGlobalInvocationId, align 32 %1 = extractelement <6 x i32> %0, i64 0 Note the global type and load type are different. Change the handling of vector loads from vector globals to reconstruct the global vector type and then bitcast to the load type. Thanks to @jcranmer-intel for helping me find the simplest solution.

@jcranmer-intel

…ing load types (KhronosGroup#2160) In some cases, we will see IR with the following @__spirv_BuiltInGlobalInvocationId = external dso_local local_unnamed_addr addrspace(1) constant <3 x i64>, align 32 ... %0 = load <6 x i32>, ptr addrspace(1) @__spirv_BuiltInGlobalInvocationId, align 32 %1 = extractelement <6 x i32> %0, i64 0 Note the global type and load type are different. Change the handling of vector loads from vector globals to reconstruct the global vector type and then bitcast to the load type. Thanks to @jcranmer-intel for helping me find the simplest solution.

@jcranmer-intel

…ng load types (KhronosGroup#2160) In some cases, we will see IR with the following @__spirv_BuiltInGlobalInvocationId = external dso_local local_unnamed_addr addrspace(1) constant <3 x i64>, align 32 ... %0 = load <6 x i32>, ptr addrspace(1) @__spirv_BuiltInGlobalInvocationId, align 32 %1 = extractelement <6 x i32> %0, i64 0 Note the global type and load type are different. Change the handling of vector loads from vector globals to reconstruct the global vector type and then bitcast to the load type. Thanks to @jcranmer-intel for helping me find the simplest solution.

@jcranmer-intel

…ing load types (KhronosGroup#2160) In some cases, we will see IR with the following @__spirv_BuiltInGlobalInvocationId = external dso_local local_unnamed_addr addrspace(1) constant <3 x i64>, align 32 ... %0 = load <6 x i32>, ptr addrspace(1) @__spirv_BuiltInGlobalInvocationId, align 32 %1 = extractelement <6 x i32> %0, i64 0 Note the global type and load type are different. Change the handling of vector loads from vector globals to reconstruct the global vector type and then bitcast to the load type. Thanks to @jcranmer-intel for helping me find the simplest solution.

@jcranmer-intel

…ing load types (KhronosGroup#2160) In some cases, we will see IR with the following @__spirv_BuiltInGlobalInvocationId = external dso_local local_unnamed_addr addrspace(1) constant <3 x i64>, align 32 ... %0 = load <6 x i32>, ptr addrspace(1) @__spirv_BuiltInGlobalInvocationId, align 32 %1 = extractelement <6 x i32> %0, i64 0 Note the global type and load type are different. Change the handling of vector loads from vector globals to reconstruct the global vector type and then bitcast to the load type. Thanks to @jcranmer-intel for helping me find the simplest solution.

@jcranmer-intel

…ing load types (#2160) (#2242) In some cases, we will see IR with the following @__spirv_BuiltInGlobalInvocationId = external dso_local local_unnamed_addr addrspace(1) constant <3 x i64>, align 32 ... %0 = load <6 x i32>, ptr addrspace(1) @__spirv_BuiltInGlobalInvocationId, align 32 %1 = extractelement <6 x i32> %0, i64 0 Note the global type and load type are different. Change the handling of vector loads from vector globals to reconstruct the global vector type and then bitcast to the load type. Thanks to @jcranmer-intel for helping me find the simplest solution. Co-authored-by: Nick Sarnie <sarnex@users.noreply.github.com>

@jcranmer-intel

…ing load types (#2160) (#2243) In some cases, we will see IR with the following @__spirv_BuiltInGlobalInvocationId = external dso_local local_unnamed_addr addrspace(1) constant <3 x i64>, align 32 ... %0 = load <6 x i32>, ptr addrspace(1) @__spirv_BuiltInGlobalInvocationId, align 32 %1 = extractelement <6 x i32> %0, i64 0 Note the global type and load type are different. Change the handling of vector loads from vector globals to reconstruct the global vector type and then bitcast to the load type. Thanks to @jcranmer-intel for helping me find the simplest solution. Co-authored-by: Nick Sarnie <sarnex@users.noreply.github.com>

@jcranmer-intel

…ing load types (#2160) (#2244) In some cases, we will see IR with the following @__spirv_BuiltInGlobalInvocationId = external dso_local local_unnamed_addr addrspace(1) constant <3 x i64>, align 32 ... %0 = load <6 x i32>, ptr addrspace(1) @__spirv_BuiltInGlobalInvocationId, align 32 %1 = extractelement <6 x i32> %0, i64 0 Note the global type and load type are different. Change the handling of vector loads from vector globals to reconstruct the global vector type and then bitcast to the load type. Thanks to @jcranmer-intel for helping me find the simplest solution. Co-authored-by: Nick Sarnie <sarnex@users.noreply.github.com>

@jcranmer-intel

…ing load types (#2160) (#2245) In some cases, we will see IR with the following @__spirv_BuiltInGlobalInvocationId = external dso_local local_unnamed_addr addrspace(1) constant <3 x i64>, align 32 ... %0 = load <6 x i32>, ptr addrspace(1) @__spirv_BuiltInGlobalInvocationId, align 32 %1 = extractelement <6 x i32> %0, i64 0 Note the global type and load type are different. Change the handling of vector loads from vector globals to reconstruct the global vector type and then bitcast to the load type. Thanks to @jcranmer-intel for helping me find the simplest solution. Co-authored-by: Nick Sarnie <sarnex@users.noreply.github.com>

sarnex force-pushed the vec branch from 7605b18 to af8eb11 Compare September 19, 2023 19:40

sarnex added 2 commits September 19, 2023 12:49

fix bad clang format

e60dcd9

Signed-off-by: Sarnie, Nick <nick.sarnie@intel.com>

formatting again

ba3ac51

Signed-off-by: Sarnie, Nick <nick.sarnie@intel.com>

sarnex marked this pull request as ready for review September 19, 2023 21:13

jcranmer-intel reviewed Sep 27, 2023

View reviewed changes

lib/SPIRV/SPIRVUtil.cpp Show resolved Hide resolved

add assert

669e0de

Signed-off-by: Sarnie, Nick <nick.sarnie@intel.com>

sarnex requested a review from jcranmer-intel September 28, 2023 17:48

jcranmer-intel reviewed Sep 28, 2023

View reviewed changes

lib/SPIRV/SPIRVUtil.cpp Show resolved Hide resolved

jcranmer-intel approved these changes Sep 28, 2023

View reviewed changes

lib/SPIRV/SPIRVUtil.cpp Show resolved Hide resolved

asudarsa reviewed Sep 29, 2023

View reviewed changes

lib/SPIRV/SPIRVUtil.cpp Outdated Show resolved Hide resolved

update comment

cd5902e

Signed-off-by: Sarnie, Nick <nick.sarnie@intel.com>

asudarsa approved these changes Sep 29, 2023

View reviewed changes

MrSidims merged commit c7ec63a into KhronosGroup:main Sep 29, 2023
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix SPIR-V global to function replacement for differing load types #2160

Fix SPIR-V global to function replacement for differing load types #2160

sarnex commented Sep 19, 2023 •

edited

Loading

sarnex commented Sep 19, 2023

asudarsa commented Sep 29, 2023

sarnex commented Sep 29, 2023 •

edited

Loading

asudarsa left a comment

sarnex commented Sep 29, 2023

Fix SPIR-V global to function replacement for differing load types #2160

Fix SPIR-V global to function replacement for differing load types #2160

Conversation

sarnex commented Sep 19, 2023 • edited Loading

sarnex commented Sep 19, 2023

asudarsa commented Sep 29, 2023

sarnex commented Sep 29, 2023 • edited Loading

asudarsa left a comment

Choose a reason for hiding this comment

sarnex commented Sep 29, 2023

sarnex commented Sep 19, 2023 •

edited

Loading

sarnex commented Sep 29, 2023 •

edited

Loading