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

ARM64: Left over dead code for special throw blocks #37675

Closed
kunalspathak opened this issue Jun 9, 2020 · 7 comments
Closed

ARM64: Left over dead code for special throw blocks #37675

kunalspathak opened this issue Jun 9, 2020 · 7 comments
Assignees
Labels
arch-arm64 area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI tenet-performance Performance related issue
Milestone

Comments

@kunalspathak
Copy link
Member

kunalspathak commented Jun 9, 2020

While debugging something on ARM64, I noticed that we end up leaving some of the dead code in place.

private static uint DivTest(uint xr)
{
    return xr  / 4;
}

Generates the following where IG04 block is dead:

G_M26531_IG01:
        A9BF7BFD          stp     fp, lr, [sp,#-16]!
        910003FD          mov     fp, sp
                                                ;; bbWeight=1    PerfScore 1.50
G_M26531_IG02:
        53027C00          lsr     w0, w0, #2
                                                ;; bbWeight=1    PerfScore 1.00
G_M26531_IG03:
        A8C17BFD          ldp     fp, lr, [sp],#16
        D65F03C0          ret     lr
                                                ;; bbWeight=1    PerfScore 2.00
G_M26531_IG04:
        97FF67A9          bl      CORINFO_HELP_THROWDIVZERO
        D43E0000          bkpt

For int parameter, we have 2 blocks that are dead:

G_M26531_IG01:
        A9BF7BFD          stp     fp, lr, [sp,#-16]!
        910003FD          mov     fp, sp
                                                ;; bbWeight=1    PerfScore 1.50
G_M26531_IG02:
        131F7C01          asr     w1, w0, #31
        12000421          and     w1, w1, #3
        0B000020          add     w0, w1, w0
        13027C00          asr     w0, w0, #2
                                                ;; bbWeight=1    PerfScore 3.00
G_M26531_IG03:
        A8C17BFD          ldp     fp, lr, [sp],#16
        D65F03C0          ret     lr
                                                ;; bbWeight=1    PerfScore 2.00
G_M26531_IG04:
        97FF67A2          bl      CORINFO_HELP_OVERFLOW
                                                ;; bbWeight=0    PerfScore 0.00
G_M26531_IG05:
        97FF67A5          bl      CORINFO_HELP_THROWDIVZERO
        D43E0000          bkpt

We don't produce dead BB if I change from division to bit-shift like xr >> 2. I was assuming there be some dead code elimination phase, but looking at dumps I didn't see anything like that happening.

category:cq
theme:dead-code
skill-level:intermediate
cost:medium

@kunalspathak kunalspathak added arch-arm64 tenet-performance Performance related issue labels Jun 9, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Jun 9, 2020
@Dotnet-GitSync-Bot
Copy link
Collaborator

I couldn't figure out the best area label to add to this issue. Please help me learn by adding exactly one area label.

@kunalspathak kunalspathak added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI and removed untriaged New issue has not been triaged by the area owner labels Jun 9, 2020
@kunalspathak
Copy link
Member Author

@dotnet/jit-contrib

@kunalspathak kunalspathak changed the title Left over dead code for special throw blocks ARM64: Left over dead code for special throw blocks Jun 9, 2020
@kunalspathak
Copy link
Member Author

kunalspathak commented Jun 9, 2020

Looks like we add those throwBlocks inside morph for ARM64 and then don't remove internal throw blocks inside flowgraph. We can perhaps eliminate 1 or both those checks in certain situation where e.g. divisor is a constant power of 2 (no need of overflow check) or if it is non-zero constant (no need of divide by zero check).

@AndyAyersMS
Copy link
Member

Marking as future.

@AndyAyersMS AndyAyersMS added this to the Future milestone Jun 11, 2020
@BruceForstall BruceForstall added the JitUntriaged CLR JIT issues needing additional triage label Oct 28, 2020
@BruceForstall BruceForstall modified the milestones: Future, 6.0.0 Nov 7, 2020
@BruceForstall BruceForstall removed the JitUntriaged CLR JIT issues needing additional triage label Nov 7, 2020
@JulieLeeMSFT JulieLeeMSFT added the needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration label Mar 23, 2021
@kunalspathak kunalspathak modified the milestones: 6.0.0, Future Jun 4, 2021
@JulieLeeMSFT JulieLeeMSFT removed the needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration label Jun 7, 2021
@TIHan
Copy link
Contributor

TIHan commented Mar 14, 2023

#82924 and #68945 covered a majority of these cases. There are still cases as it is possible that one of these blocks can be added and then an optimization happens that proves we don't need the blocks.

@EgorBo
Copy link
Member

EgorBo commented May 5, 2024

Let's close it then and re-open/re-create if we find a pattern that is not optimized today. Current asm for the Kunal's snippet:

; Method C:DivTest(uint):uint (FullOpts)
G_M2997_IG01:  ;; offset=0x0000
            stp     fp, lr, [sp, #-0x10]!
            mov     fp, sp
						;; size=8 bbWeight=1 PerfScore 1.50

G_M2997_IG02:  ;; offset=0x0008
            lsr     w0, w0, #2
						;; size=4 bbWeight=1 PerfScore 1.00

G_M2997_IG03:  ;; offset=0x000C
            ldp     fp, lr, [sp], #0x10
            ret     lr
						;; size=8 bbWeight=1 PerfScore 2.00
; Total bytes of code: 20

@EgorBo EgorBo closed this as completed May 5, 2024
@AndyAyersMS
Copy link
Member

I would expect #95379 to catch most cases of this.

@EgorBo EgorBo assigned AndyAyersMS and unassigned kunalspathak May 5, 2024
@EgorBo EgorBo modified the milestones: Future, 9.0.0 May 5, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jun 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-arm64 area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI tenet-performance Performance related issue
Projects
None yet
Development

No branches or pull requests

7 participants