Skip to content

[mlir][vector] Linearize vector.create_mask (flatten to 1D) #138760

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

Closed
wants to merge 3 commits into from

Conversation

newling
Copy link
Contributor

@newling newling commented May 6, 2025

Alternative to #138214

Rather than using an arith.select, this PR computes a new rank-1 mask argument directly with scalars before splatting. This PR also supports more mask shapes.

FYI @nbpatel I started trying to suggest this approach in #138214 but thought it'd be clearer to just implement it. Please let me know what you think

@newling newling changed the title [mlir][vector] Linearize vector.create_mask [mlir][vector] Linearize vector.create_mask (flatten to 1D) May 6, 2025
@nbpatel
Copy link
Contributor

nbpatel commented May 7, 2025

why do we need an alternative for PRs already up for review? My PR already got a LGTM and some feedback. I guess if there is some feedback you have we can always discuss or you can review and leave feedback and I can incorporate it in my PR, also, you can create a iterative PR later to incorporate the changes you require. I don't feel duplicating work is the correct way forward

@newling
Copy link
Contributor Author

newling commented May 7, 2025

Hi @nbpatel I don't intend to pursue this PR (it is a draft, so I don't expect review of it) but as I mentioned in the summary I thought it'd be quicker to describe my thoughts on #138214 by trying it code first. I intended to add comments in #138214 summarizing this, my apologies for not being clearer

@newling newling closed this May 13, 2025
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