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.

Sign up for GitHub

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

Jump to bottom

rewrite_stack_ptr post process pass #3046

Closed
ESI-SYD opened this issue Dec 19, 2024 · 1 comment · Fixed by #3497
Closed

rewrite_stack_ptr post process pass #3046

ESI-SYD opened this issue Dec 19, 2024 · 1 comment · Fixed by #3497
Assignees
@ESI-SYD
Labels
code quality
Milestone
3. [Triton] Langu...

Comments

@ESI-SYD
Copy link
Contributor

ESI-SYD commented Dec 19, 2024 •
edited
Loading

Describe the bug

Changes from that PR #3010 introduced some duplicate code, should revisit and clean, then upstream.

Environment details

PVC
@ESI-SYD ESI-SYD added the bug label Dec 19, 2024
@ESI-SYD ESI-SYD self-assigned this Jan 6, 2025
@whitneywhtsang whitneywhtsang mentioned this issue Jan 10, 2025
Open
@ESI-SYD ESI-SYD linked a pull request Jan 24, 2025 that will close this issue
Refactorring on getScrathMemoryPtr for SLM and GLM after #3010 #3224
Closed
@ESI-SYD
Copy link
Contributor Author

ESI-SYD commented Feb 18, 2025 •
edited
Loading

According to upstream reviewer's latest suggestion from triton-lang/triton#5878, we change to try add a intel post process pass instead of current targetInfo solution #3224 .

Status: Code drafted locally branch

@ESI-SYD ESI-SYD mentioned this issue Feb 24, 2025
Merged
@ESI-SYD ESI-SYD changed the title Code changes clean after #3010 rewrite_stack_ptr post process pass Feb 24, 2025
@ESI-SYD ESI-SYD linked a pull request Feb 24, 2025 that will close this issue
Add rewrite_stack_ptr post process pass #3497
Merged
ESI-SYD added a commit that referenced this issue Mar 7, 2025
@ESI-SYD
Add rewrite_stack_ptr post process pass (#3497)
7375302 
This PR have following changes:

1. Discard adding `TargetInfo::getStackPointer` that only exist in this
fork(then we keep the same as upstream), replace it with an intel pass
per discussion in
#3046 (comment).

2. Use public `ControlFlowOpToLLVM` and sync public `funcOpToLLVM` to
`PipelineManger` (difficult to use the public `FuncOpToLLVM` directly as
it contains Nv-specific codes) to match calling convention.

3. Lit tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ESI-SYD ESI-SYD

Labels
code quality
Projects
None yet
Milestone
3. [Triton] Language and Runtime
2 participants
@ESI-SYD @vlad-penkin