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

Dead code not eliminated after constant folding/propagation #57033

Closed
stephentoub opened this issue Aug 7, 2021 · 5 comments
Closed

Dead code not eliminated after constant folding/propagation #57033

stephentoub opened this issue Aug 7, 2021 · 5 comments
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI help wanted [up-for-grabs] Good issue for external contributors
Milestone

Comments

@stephentoub
Copy link
Member

stephentoub commented Aug 7, 2021

This example is a derivative of one from wikipedia:

[Benchmark]
public int Example()
{
    int a = 30;
    int b = 9 - (a / 5);
    int c;

    c = b * 4;
    if (c > 10)
    {
        c = c - 10;
    }

    return c * (60 / a);
}

The JIT successfully reduces the entire operation to mov eax, 4, but it leaves behind multiple computations that would seem to no longer be necessary:

; .NET 6.0.0 (6.0.21.40103), X64 RyuJIT
; Program.Example()
       mov       ecx,1E
       mov       eax,3C
       xor       edx,edx
       idiv      ecx
       mov       eax,4
       ret
; Total bytes of code 20

Is there some kind of side effect it's relying on from all the earlier gunk, or should it be removing the mov, mov, xor, idiv and just isn't? Note that if I tweak the C# to make int a = 30 be const int a = 30, then all the extra operations go away as expected.

category:cq
theme:value-numbering
skill-level:expert
cost:medium
impact:medium

@dotnet-issue-labeler dotnet-issue-labeler bot added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI untriaged New issue has not been triaged by the area owner labels Aug 7, 2021
@AndyAyersMS
Copy link
Member

During constant prop the jit doesn't clear the exception flag (X in the below) from the divide even though it could; this keeps the divide and computations feeding it alive.

Extracted side effects from a constant tree [000020]:
N003 ( 24,  6) [000019] ---X--------              *  DIV       int    $45
N001 (  1,  1) [000017] ------------              +--*  CNS_INT   int    60 $46
N002 (  3,  2) [000018] ------------              \--*  LCL_VAR   int    V00 loc0         u:2 (last use) $42

Should be an easy fix.

@AndyAyersMS AndyAyersMS removed the untriaged New issue has not been triaged by the area owner label Aug 8, 2021
@AndyAyersMS AndyAyersMS added this to the Future milestone Aug 8, 2021
@AndyAyersMS AndyAyersMS added the help wanted [up-for-grabs] Good issue for external contributors label Aug 8, 2021
@SingleAccretion
Copy link
Contributor

FWIW, I've bumped into the exact same problem before: #50450 (comment), and also found no diffs doing the "easy" fix.

What we'd like to do in these situations is use exception sets, but we cannot because they do not model all exceptions today (canonical example is the lack of NullPtrExc for the GetType call).

@AndyAyersMS
Copy link
Member

For divide at least the operator side effects are modelled.

Wondering if we should consider modelling all the possible effects or consider what it would mean to have some generalized exception VN to model the all exceptions we don't specifically track -- I suppose for the latter we might have a lot of unique exception VNs running around since it might be hard to deduce when two different bits of IR could produce the same set of unmodelled exceptions.

@SingleAccretion
Copy link
Contributor

Wondering if we should consider modelling all the possible effects or consider what it would mean to have some generalized exception VN to model the all exceptions we don't specifically track -- I suppose for the latter we might have a lot of unique exception VNs running around since it might be hard to deduce when two different bits of IR could produce the same set of unmodelled exceptions.

#63559

FWIW, we now model all exception sets precisely, modulo those coming from calls (both helper and user). Once that's fixed, we can start using them a bit more liberally.

There is also the possibility of a more low-tech solution, say enhancing the VN-based constant propagation to clear the exception bits with some kind of GTF_DIV_NONFAULTING, akin to how VN-based non-null prop works (which might be useful on its own anyway).

@EgorBo
Copy link
Member

EgorBo commented Nov 21, 2022

Closed via #78630

@EgorBo EgorBo closed this as completed Nov 21, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Dec 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI help wanted [up-for-grabs] Good issue for external contributors
Projects
Archived in project
Development

No branches or pull requests

4 participants