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

[refactor] Remove redundant codegen of BitExtractStmt #6141

Closed

Conversation

strongoier
Copy link
Contributor

Issue: #6134

@netlify
Copy link

netlify bot commented Sep 22, 2022

Deploy Preview for docsite-preview canceled.

Name Link
🔨 Latest commit b32a8f5
🔍 Latest deploy log https://app.netlify.com/sites/docsite-preview/deploys/632c2d324ac26100086af250

@bobcao3
Copy link
Collaborator

bobcao3 commented Sep 22, 2022

There is actually hardware support on both Nvidia and AMD for native bit extract (a lot faster than using lower level bit operations). Maybe we should keep them not demoted?

@strongoier
Copy link
Contributor Author

There is actually hardware support on both Nvidia and AMD for native bit extract (a lot faster than using lower level bit operations). Maybe we should keep them not demoted?

Could you elaborate a bit on what should we do to make use of that? Regarding this PR, demote_operations is just an abstract layer of what current codegen of different backends does.

@bobcao3
Copy link
Collaborator

bobcao3 commented Sep 24, 2022

There is actually hardware support on both Nvidia and AMD for native bit extract (a lot faster than using lower level bit operations). Maybe we should keep them not demoted?

Could you elaborate a bit on what should we do to make use of that? Regarding this PR, demote_operations is just an abstract layer of what current codegen of different backends does.

We would want to be able to optionally demote BFE when backend says it's not supported, and if supported the codegen will want to take the BFE CHI-IR and generate the backend native instruction

@strongoier
Copy link
Contributor Author

We would want to be able to optionally demote BFE when backend says it's not supported, and if supported the codegen will want to take the BFE CHI-IR and generate the backend native instruction

Sounds good. Then I think not performing demotion in CHI IR is a better choice. I'll now close this PR and submit another one in the future.

@strongoier strongoier closed this Sep 26, 2022
@strongoier strongoier deleted the remove-bitextract-codegen branch September 27, 2022 02:43
strongoier added a commit that referenced this pull request Oct 8, 2022
Issue:
#6141 (comment),
#6134

### Brief Summary

In SPIR-V there's actually `OpBitFieldUExtract` which does the job of
`BitExtractStmt` perfectly, so let's stop demoting `BitExtractStmt` and
let codegen handle it with the best instruction(s).
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