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

[Codegen][ROCm] Drop assume alignments #18951

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

qedawkins
Copy link
Contributor

This reportedly hurts the LLVM optimizer. We may want to find a better way to annotate alignments.

This reportedly hurts the LLVM optimizer. We may want to find a better
way to annotate alignments.
@benvanik
Copy link
Collaborator

benvanik commented Oct 30, 2024

If this lands it should be tagged with an upstream issue to fix that - a backend should not pessimize codegen by having more information about pointer alignment. This is also a hard thing to profile as in many cases dynamically shaped models will be hurt the worst (where offsets are all dynamic values) - our suite definitely doesn't cover that.

@qedawkins
Copy link
Contributor Author

Correctness issues without these assumes is scary...

Failed
Expected near equality of these values. Contents does not match.
  lhs:
    2x2x2xf32=[[3 6][15 18]][[21 24][78 84]]
  rhs:
    2x2x2xf32=[[1 1][1 1]][[2 2][2 2]]

@benvanik
Copy link
Collaborator

Methinks that codegen target needs some improvement :)

@qedawkins
Copy link
Contributor Author

methinks so too :(

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