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

[mono] Improvement: avoid deep recursion in mono_ssa_rename_vars #60266

Closed
radekdoulik opened this issue Oct 11, 2021 · 2 comments
Closed

[mono] Improvement: avoid deep recursion in mono_ssa_rename_vars #60266

radekdoulik opened this issue Oct 11, 2021 · 2 comments
Assignees
@radekdoulik
Labels
area-Codegen-AOT-mono
Milestone
8.0.0

Comments

@radekdoulik
Copy link
Member

In #57141 I see that mono_ssa_rename_vars is entering deep recursion and was resulting in stack overflow on windows.

Consider to modify it to avoid recursion.
@radekdoulik radekdoulik self-assigned this Oct 11, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Oct 11, 2021
@radekdoulik radekdoulik removed the untriaged New issue has not been triaged by the area owner label Oct 11, 2021
@radekdoulik radekdoulik added this to the 7.0.0 milestone Oct 11, 2021
@radekdoulik radekdoulik mentioned this issue Oct 13, 2021
Merged
radekdoulik added a commit to radekdoulik/runtime that referenced this issue Nov 16, 2021
@radekdoulik
[mono] Don't use recursion in mono_ssa_rename_vars
b747f3b 
Implements dotnet#60266

Avoid deep recursion in mono_ssa_rename_vars, change the way we traverse
dominated bb's. Instead of using recursion, use stack like array to
store information about stack history and the traversal.

The performance remains the same (or is slightly better) than before.
Times for the compilation of repro from dotnet#57141
(JIT time minimum from 5 runs):

Before:

    LLVM output file: './Uno.UI.FluentTheme.dll.bc.tmp'.
    JIT time: 4810 ms, Generation time: 2040 ms, Assembly+Link time: 2 ms.

After:

    LLVM output file: './Uno.UI.FluentTheme.dll.bc.tmp'.
    JIT time: 4781 ms, Generation time: 2017 ms, Assembly+Link time: 2 ms.
@radekdoulik radekdoulik mentioned this issue Nov 16, 2021
Merged
radekdoulik added a commit that referenced this issue Nov 23, 2021
@radekdoulik @imhameed
[mono] Don't use recursion in mono_ssa_rename_vars (#61677)
b35c0ad 
Implements #60266

Avoid deep recursion in `mono_ssa_rename_vars`, change the way we traverse
dominated bb's. Instead of using recursion, use stack-like array to
store information about stack history and the traversal.

The performance remains the same (or is slightly better) than before.
Times for the compilation of repro from #57141
(JIT time minimum from 5 runs):

Before:

    LLVM output file: './Uno.UI.FluentTheme.dll.bc.tmp'.
    JIT time: 4810 ms, Generation time: 2040 ms, Assembly+Link time: 2 ms.

After:

    LLVM output file: './Uno.UI.FluentTheme.dll.bc.tmp'.
    JIT time: 4781 ms, Generation time: 2017 ms, Assembly+Link time: 2 ms.

* Fix assignment

* Apply suggestions from code review

Co-authored-by: imhameed <[email protected]>

* Remove casts we don't need

Co-authored-by: imhameed <[email protected]>
@SamMonoRT
Copy link
Member

@radekdoulik -any more work remaining here ?

@SamMonoRT SamMonoRT modified the milestones: 7.0.0, 8.0.0 Aug 9, 2022
@radekdoulik
Copy link
Member Author

It's done :-)

@ghost ghost locked as resolved and limited conversation to collaborators Aug 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@radekdoulik radekdoulik

Labels
area-Codegen-AOT-mono
Projects
None yet
Milestone
8.0.0
Development

No branches or pull requests
2 participants
@radekdoulik @SamMonoRT