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

ARM64: Optimize pair of "str reg, [reg]" to stp #35133

Closed
kunalspathak opened this issue Apr 17, 2020 · 1 comment · Fixed by #84135
Closed

ARM64: Optimize pair of "str reg, [reg]" to stp #35133

kunalspathak opened this issue Apr 17, 2020 · 1 comment · Fixed by #84135
Labels
arch-arm64 area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
.NET Core CodeGen
Milestone
Future

Comments

@kunalspathak
Copy link
Member

kunalspathak commented Apr 17, 2020 •
edited by BruceForstall 

str     x13, [x12]
str     x14, [x12,#8]

can be combined into stp if the stores are happening in subsequent memory.

stp x13, x14, [x12]

I collected no. of such str pairs in framework libraries and found approx. 1000 pairs in 400 methods.

Details:

str_str_x_to_stp.txt

category:cq
theme:optimization
skill-level:intermediate
cost:small
impact:medium
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-Meta untriaged New issue has not been triaged by the area owner labels Apr 17, 2020
@kunalspathak kunalspathak added arch-arm64 area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI and removed area-Meta labels Apr 17, 2020
@BruceForstall BruceForstall mentioned this issue Apr 18, 2020
Closed
@BruceForstall BruceForstall added this to the Future milestone Apr 20, 2020
@BruceForstall BruceForstall removed the untriaged New issue has not been triaged by the area owner label Apr 20, 2020
@kunalspathak kunalspathak mentioned this issue May 5, 2020
Closed
@BruceForstall BruceForstall added the JitUntriaged CLR JIT issues needing additional triage label Oct 28, 2020
@kunalspathak kunalspathak mentioned this issue Nov 9, 2020
Closed
29 tasks
@kunalspathak kunalspathak modified the milestones: Future, 6.0.0 Nov 9, 2020
@kunalspathak kunalspathak added this to Needs Triage in .NET Core CodeGen via automation Nov 9, 2020
@kunalspathak kunalspathak moved this from Needs Triage to Backlog (General) in .NET Core CodeGen Nov 9, 2020
@kunalspathak kunalspathak removed the JitUntriaged CLR JIT issues needing additional triage label Nov 9, 2020
@JulieLeeMSFT JulieLeeMSFT modified the milestones: 6.0.0, Future Mar 23, 2021
@echesakov echesakov mentioned this issue Jul 8, 2021
Closed
8 tasks
@SwapnilGaikwad SwapnilGaikwad mentioned this issue Oct 26, 2022
Closed
AndyJGraham added a commit to AndyJGraham/runtime that referenced this issue Oct 27, 2022
@AndyJGraham
Replace successive "ldr" and "str" instructions with "ldp" and "stp"
b88ff31 
This change serves to address the following four Github tickets:

    1. ARM64: Optimize pair of "ldr reg, [fp]" to ldp dotnet#35130
    2. ARM64: Optimize pair of "ldr reg, [reg]" to ldp dotnet#35132
    3. ARM64: Optimize pair of "str reg, [reg]" to stp dotnet#35133
    4. ARM64: Optimize pair of "str reg, [fp]" to stp  dotnet#35134

A technique was employed that involved detecting an optimisation
opportunity as instruction sequences were being generated.
The optimised instruction was then generated on top of the previous
instruction, with no second instruction generated. Thus, there were no
changes to instruction group size at “emission time” and no changes to
jump instructions.
@BruceForstall BruceForstall mentioned this issue Oct 27, 2022
Merged
BruceForstall pushed a commit to BruceForstall/runtime that referenced this issue Jan 12, 2023
@AndyJGraham @BruceForstall
Replace successive "ldr" and "str" instructions with "ldp" and "stp"
261c710 
This change serves to address the following four Github tickets:

    1. ARM64: Optimize pair of "ldr reg, [fp]" to ldp dotnet#35130
    2. ARM64: Optimize pair of "ldr reg, [reg]" to ldp dotnet#35132
    3. ARM64: Optimize pair of "str reg, [reg]" to stp dotnet#35133
    4. ARM64: Optimize pair of "str reg, [fp]" to stp  dotnet#35134

A technique was employed that involved detecting an optimisation
opportunity as instruction sequences were being generated.
The optimised instruction was then generated on top of the previous
instruction, with no second instruction generated. Thus, there were no
changes to instruction group size at “emission time” and no changes to
jump instructions.
kunalspathak pushed a commit that referenced this issue Jan 27, 2023
@AndyJGraham @BruceForstall @a74nh
Replace successive "ldr" and "str" instructions with "ldp" and "stp" (#…
acc409e 
…77540)

* Replace successive "ldr" and "str" instructions with "ldp" and "stp"

This change serves to address the following four Github tickets:

    1. ARM64: Optimize pair of "ldr reg, [fp]" to ldp #35130
    2. ARM64: Optimize pair of "ldr reg, [reg]" to ldp #35132
    3. ARM64: Optimize pair of "str reg, [reg]" to stp #35133
    4. ARM64: Optimize pair of "str reg, [fp]" to stp  #35134

A technique was employed that involved detecting an optimisation
opportunity as instruction sequences were being generated.
The optimised instruction was then generated on top of the previous
instruction, with no second instruction generated. Thus, there were no
changes to instruction group size at “emission time” and no changes to
jump instructions.

* No longer use a temporary buffer to build the optimized instruction.

* Addressed assorted review comments.

* Now optimizes ascending locations and decending locations with
consecutive STR and LDR instructions.

* Modification to remove last instructions.

* Ongoing improvements to remove previously-emitted instruction
during ldr / str optimization.

* Stopped optimization of consecutive instructions that straddled an instruction group boundary.

* Addressed code change requests in GitHub.

* Various fixes to ldp/stp optimization

Add code to update IP mappings when an instruction is removed.

* Delete unnecessary and incorrect assert

* Diagnostic change only, to confirm whether a theory is correct or
not when chasing an error.

* Revert "Diagnostic change only, to confirm whether a theory is correct or"

This reverts commit 4b0e51e.

* Do not merge. Temporarily removed calls to
"codeGen->genIPmappingUpdateForRemovedInstruction()".

Also, corrected minor bug in instruction numbering
when removing instructions during optimization.

* Modifications to better update the IP mapping table for a replaced instruction.

* Minor formatting change.

* Check for out of range offsets

* Don't optimise during prolog/epilog

* Fix windows build error

* IGF_HAS_REMOVED_INSTR is ARM64 only

* Add OptimizeLdrStr function

* Fix formatting

* Ensure local variables are tracked

* Don't peephole local variables

Co-authored-by: Bruce Forstall <brucefo@microsoft.com>
Co-authored-by: Alan Hayward <alan.hayward@arm.com>
Co-authored-by: Alan Hayward <a74nh@users.noreply.github.com>
@SwapnilGaikwad SwapnilGaikwad mentioned this issue Mar 30, 2023
Merged
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Mar 30, 2023
.NET Core CodeGen automation moved this from Backlog (General) to Done Mar 31, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Mar 31, 2023
@kunalspathak kunalspathak reopened this Apr 6, 2023
.NET Core CodeGen automation moved this from Done to Needs Triage Apr 6, 2023
@kunalspathak kunalspathak mentioned this issue Apr 19, 2023
Merged
@kunalspathak
Copy link
Member Author

Fixed in #85032

.NET Core CodeGen automation moved this from Needs Triage to Done Apr 27, 2023
@dotnet dotnet locked as resolved and limited conversation to collaborators May 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
arch-arm64 area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
.NET Core CodeGen
  
Done
Milestone
Future
Development

Successfully merging a pull request may close this issue.

Use ldp/stp with SIMD registers on Arm64 SwapnilGaikwad/runtime
4 participants
@BruceForstall @kunalspathak @Dotnet-GitSync-Bot @JulieLeeMSFT