Skip to content

[LLVM->SPIRV] Cast the GEP base pointer to source type upon mismatch #3255

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

Merged
merged 3 commits into from
Jul 9, 2025

Conversation

karolzwolak
Copy link
Contributor

@karolzwolak karolzwolak commented Jul 8, 2025

The source element type used in a GEP may differ from the actual type of the pointer operand (e.g., ptr i8 vs. ptr [N x T]).
This mismatch can lead to incorrect address computations during translation to SPIR-V of GEP used in constexpr context, which requires that pointer types match the type of the object being accessed.

This patch inserts an explicit bitcast to convert the GEP pointer operand to the expected type, derived from the GEP’s source element type, before emitting an PtrAccessChain.
This ensures the resulting SPIR-V instruction has a correctly typed base pointer and produces valid indexing behavior.

For example:
Before this change, the following GEP was translated incorrectly:
getelementptr(i8, ptr addrspace(1) @a_var, i64 2)
Whereas this nearly equivalent GEP was handled correctly:
getelementptr inbounds ([2 x i8], ptr @a_var, i64 0, i64 1)

Previously, the first form was incorrectly interpreted as:
getelementptr inbounds ([2 x i8], ptr @a_var, i64 0, i64 2)

The source element type used in a GEP may differ
from the actual type of the pointer operand (e.g., `ptr i8` vs.
`ptr [N x T]`). This mismatch can lead to incorrect address computations
during translation to SPIR-V, which requires that pointer types match the
type of the object being accessed.

This patch inserts an explicit bitcast to convert the GEP pointer operand
to the expected type, derived from the GEP’s source element type, before
emitting an `PtrAccessChain`. This ensures the resulting SPIR-V
instruction has a correctly typed base pointer and produces valid
indexing behavior.

For example:
Before this change, the following GEP was translated incorrectly:
  `getelementptr(i8, ptr addrspace(1) @a_var, i64 2)`
Whereas this nearly equivalent GEP was handled correctly:
  `getelementptr inbounds ([2 x i8], ptr @a_var, i64 0, i64 1)`

Previously, the first form was incorrectly interpreted as:
  `getelementptr inbounds ([2 x i8], ptr @a_var, i64 0, i64 2)`
@karolzwolak karolzwolak force-pushed the gep-cast-operand-to-source branch from 88233ee to 8c5a9d9 Compare July 8, 2025 07:29
@karolzwolak
Copy link
Contributor Author

@MrSidims Could you please take a look at this?
PS: I made this for previous LLVM versions, and I'm not sure what should be done on the SPIR-V untyped pointers.
Should we just skip the bitcast or is this fine?

@karolzwolak karolzwolak force-pushed the gep-cast-operand-to-source branch from dfb2031 to c38c46b Compare July 8, 2025 11:10
Copy link
Contributor

@MrSidims MrSidims left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@karolzwolak if you don't mind, I have changed the description from
This mismatch can lead to incorrect address computations during translation to SPIR-V
to
This mismatch can lead to incorrect address computations during translation to SPIR-V of GEP used in constexpr context

@MrSidims
Copy link
Contributor

MrSidims commented Jul 8, 2025

what should be done on the SPIR-V untyped pointers

If you are asking about SPV_KHR_untyped_pointers - I believe we can skip this corner case for now.

@karolzwolak
Copy link
Contributor Author

Great, I don't mind the description change.

@MrSidims MrSidims merged commit 1be9366 into KhronosGroup:main Jul 9, 2025
9 checks passed
karolzwolak added a commit to karolzwolak/SPIRV-LLVM-Translator that referenced this pull request Jul 9, 2025
…e upon mismatch (KhronosGroup#3255)

The source element type used in a GEP may differ from the actual type of
the pointer operand (e.g., ptr i8 vs. ptr [N x T]).
This mismatch can lead to incorrect address computations during
translation to SPIR-V of GEP used in constexpr context, which requires
that pointer types match the type of the object being accessed.

This patch inserts an explicit bitcast to convert the GEP pointer
operand to the expected type, derived from the GEP’s source element
type, before emitting an PtrAccessChain.
This ensures the resulting SPIR-V instruction has a correctly typed base
pointer and produces valid indexing behavior.

For example:
Before this change, the following GEP was translated incorrectly:
getelementptr(i8, ptr addrspace(1) @a_var, i64 2)
Whereas this nearly equivalent GEP was handled correctly:
getelementptr inbounds ([2 x i8], ptr @a_var, i64 0, i64 1)

Previously, the first form was incorrectly interpreted as:
getelementptr inbounds ([2 x i8], ptr @a_var, i64 0, i64 2)
karolzwolak added a commit to karolzwolak/SPIRV-LLVM-Translator that referenced this pull request Jul 9, 2025
…er to source type upon mismatch (KhronosGroup#3255)

The source element type used in a GEP may differ from the actual type of
the pointer operand (e.g., ptr i8 vs. ptr [N x T]).
This mismatch can lead to incorrect address computations during
translation to SPIR-V of GEP used in constexpr context, which requires
that pointer types match the type of the object being accessed.

This patch inserts an explicit bitcast to convert the GEP pointer
operand to the expected type, derived from the GEP’s source element
type, before emitting an PtrAccessChain.
This ensures the resulting SPIR-V instruction has a correctly typed base
pointer and produces valid indexing behavior.

For example:
Before this change, the following GEP was translated incorrectly:
getelementptr(i8, ptr addrspace(1) @a_var, i64 2)
Whereas this nearly equivalent GEP was handled correctly:
getelementptr inbounds ([2 x i8], ptr @a_var, i64 0, i64 1)

Previously, the first form was incorrectly interpreted as:
getelementptr inbounds ([2 x i8], ptr @a_var, i64 0, i64 2)
MrSidims pushed a commit that referenced this pull request Jul 10, 2025
…er to source type upon mismatch (#3255) (#3263)

The source element type used in a GEP may differ from the actual type of
the pointer operand (e.g., ptr i8 vs. ptr [N x T]). This mismatch can
lead to incorrect address computations during translation to SPIR-V of
GEP used in constexpr context, which requires that pointer types match
the type of the object being accessed.

This patch inserts an explicit bitcast to convert the GEP pointer
operand to the expected type, derived from the GEP’s source element
type, before emitting an PtrAccessChain.
This ensures the resulting SPIR-V instruction has a correctly typed base
pointer and produces valid indexing behavior.

For example:
Before this change, the following GEP was translated incorrectly:
getelementptr(i8, ptr addrspace(1) @a_var, i64 2)
Whereas this nearly equivalent GEP was handled correctly: getelementptr
inbounds ([2 x i8], ptr @a_var, i64 0, i64 1)

Previously, the first form was incorrectly interpreted as: getelementptr
inbounds ([2 x i8], ptr @a_var, i64 0, i64 2)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants