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

Revert "Revert Allow some intrinsics in Tier0" #82354

Merged
merged 5 commits into from
Feb 20, 2023

Conversation

EgorBo
Copy link
Member

@EgorBo EgorBo commented Feb 18, 2023

Reverts #82079 revert.

It turned out Morph folded some of those intrinsics to 0/1 (e.g. typeof() comparisons and IsKnownConstant) and we've got JTRUE(0/1) that was never cleaned up in Tier0.

I plan to enable a limited version of constant folding in tier0 (mainly, to remove dead branches = call VM less = better TP), e.g.

static void Test<T>()
{
    if (typeof(T) == typeof(float))
        Console.WriteLine("float");
    if (typeof(T) == typeof(double))
        Console.WriteLine("double");
}

Currently, in tier0 we don't fold these dead branches (but at least this PR folds comparisons to 0/1).
image
so this PR is expected to bring the wins back ^

Will run outerloop CI

@ghost ghost assigned EgorBo Feb 18, 2023
@EgorBo
Copy link
Member Author

EgorBo commented Feb 18, 2023

/azp run runtime-coreclr outerloop, runtime-coreclr pgo, runtime-coreclr libraries-pgo

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jakobbotsch
Copy link
Member

jakobbotsch commented Feb 18, 2023

There are other places that assume that only relops appear under JTRUE:

assert(relop->OperIsCompare());

noway_assert(relop->OperIsCompare());

These may not run under tier 0, but if that is the IR invariant I don't think we should be breaking it in tier 0 (or, we should change the IR invariant simultaneously everywhere). Can we avoid the folding in morph instead? GTF_RELOP_JMP_USED can be used to check if the relop is under a GT_JTRUE.

@EgorBo
Copy link
Member Author

EgorBo commented Feb 19, 2023

@jakobbotsch thanks, I eliminated two sources of such possabilities:

  1. Don't carry IsKnownConstant to morph in Tier0
  2. Run gtFoldTypeCompare earlier in importer - it allows both Tier0 and Tier1 eliminate some type comparisons early and caught diffs even for Tier1

Diffs: https://dev.azure.com/dnceng-public/public/_build/results?buildId=177313&view=ms.vss-build-web.run-extensions-tab

@EgorBo
Copy link
Member Author

EgorBo commented Feb 19, 2023

/azp run runtime-coreclr outerloop, runtime-coreclr pgo, runtime-coreclr libraries-pgo

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@EgorBo
Copy link
Member Author

EgorBo commented Feb 19, 2023

/azp run runtime-coreclr outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@EgorBo
Copy link
Member Author

EgorBo commented Feb 20, 2023

PTAL @dotnet/jit-contrib @jakobbotsch @AndyAyersMS

@EgorBo
Copy link
Member Author

EgorBo commented Feb 20, 2023

Failure is #82352 (comment)

@EgorBo EgorBo merged commit e79694c into dotnet:main Feb 20, 2023
@EgorBo EgorBo deleted the tier0-intrinsics-2 branch February 20, 2023 14:00
@ghost ghost locked as resolved and limited conversation to collaborators Mar 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants