Skip to content
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

Emit vector constant for vector index #921

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

nkoskelo
Copy link
Contributor

Fixes #870.

@nkoskelo nkoskelo changed the title Enable vectorization of ternary operators. Emit vector constant for vector index Feb 20, 2025
@nkoskelo nkoskelo force-pushed the enable_vectorization_of_select branch from 9b6a609 to 141d9d7 Compare February 24, 2025 22:12
@nkoskelo nkoskelo requested a review from inducer February 24, 2025 22:28
# is the number of elements in the vector which is the same as in src.
# https://registry.khronos.org/OpenCL/specs/3.0-unified/html/OpenCL_C.html#explicit-casts
if self.codegen_state.target.is_vector_dtype(actual_type) or \
actual_type.dtype.kind == "b":
Copy link
Owner

Choose a reason for hiding this comment

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

What is this bool handling doing here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The type, actual_type is computed using the expression before the vector literal is inserted. I think the better solution would be to update the type inference system to use the vectorized version of the expression instead.

vector_type = self.codegen_state.target.vector_dtype(index_type,
length)
conditional_needed_loopy_type = to_loopy_type(vector_type)
except UnvectorizableError:
Copy link
Owner

Choose a reason for hiding this comment

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

Why are we suppressing exceptions here in the first place?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We know that VectorizabilityChecker has succeeded at least once before. However, it is unclear that it will pass for the current expression which may be for a different part of the code. So out of caution, I thought it would be best to rerun the VectorizabilityChecker. If the VectorizabilityChecker succeeds then we need to ensure the proper typing of the vector conditional. If it fails, then the expression is not a vector and so we just handle the case like normal.

@nkoskelo nkoskelo marked this pull request as draft March 12, 2025 22:13
@nkoskelo nkoskelo marked this pull request as ready for review March 14, 2025 15:18
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.

Allow vectorization of ternaries, indices
2 participants