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

CoreRT shows a problem with inlined tail calls. #8258

Closed
sandreenko opened this issue May 30, 2017 · 1 comment
Closed

CoreRT shows a problem with inlined tail calls. #8258

sandreenko opened this issue May 30, 2017 · 1 comment
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI bug
Milestone

Comments

@sandreenko
Copy link
Contributor

Run ILC in Release mode with Debug RyuJit with such test:

using System;
using System.Reflection;
using System.Runtime.CompilerServices;

internal class ConstructorAttribute : Attribute
{
}

internal class Program
{
    [MethodImpl(MethodImplOptions.NoInlining)]
    private bool GetAttributeConstructor(ConstructorInfo c)
    {
        return CustomAttributeExtensions.IsDefined(c, typeof(ConstructorAttribute), true);
    }

    private static void Main(string[] args)
    {
        new Program().GetAttributeConstructor(typeof(Program).GetTypeInfo().GetConstructor(Array.Empty<Type>()));
    }
}

Set ngenDump for GetAttributeConstructor, you will see:

Invoking compiler for the inlinee method Internal.LowLevelLinq.LowLevelEnumerable:Any(ref):bool :
IL to import:
IL_0000  02                ldarg.0
IL_0001  6f 83 00 00 0a    callvirt     0xA000083
IL_0006  0a                stloc.0
IL_0007  06                ldloc.0
IL_0008  6f d3 1d 00 06    callvirt     0x6001DD3
IL_000d  2a                ret

and

 
***** BB01, stmt 7
               [000076] ------------             *  stmtExpr  void  (IL   ???...  ???)
               [000073] --C---------             |  /--*  cast      int <- bool <- int
               [000067] --C-G-------             |  |  \--*  call      int    Internal.LowLevelLinq.LowLevelEnumerable.Any
               [000068] ------------ arg0        |  |     +--*  const(h)  long   0x427DA0 method
               [000066] ------------ arg1        |  |     \--*  lclVar    ref    V07 tmp5
               [000075] -AC---------             \--*  =         bool
               [000074] D------N----                \--*  lclVar    bool   V05 tmp3

***** BB01, stmt 8
               [000025] ------------             *  stmtExpr  void  (IL   ???...  ???)
               [000024] --C---------             \--*  return    int
               [000023] --C---------                \--*  cast      int <- bool <- int
               [000077] ------------                   \--*  lclVar    int    V05 tmp3

Call 67 will be marked as tail call, but the morph condition for the return statement doesn't expect a cast and will blow up.

It works fine with the release mode of the jit.
I am not yet sure about the right fix, but there is strange code in fgMorphCall:

            // Peel off casts
            while (treeWithCall->gtOper == GT_CAST)
            {
                noway_assert(!treeWithCall->gtOverflow());
                treeWithCall = treeWithCall->gtGetOp1();
            }

It is inside the #ifdef Debug branch, that initially contained only assert checks, but some real code was added recently (#9405). @AndyAyersMS Is it right that we peel casts only in debug and only for the call node, but not for the return statement?

cc @MichalStrehovsky @dotnet/jit-contrib.

@AndyAyersMS
Copy link
Member

The call side peeling is ok -- it only feeds an assert that the call is found in the right kind of tree.

If the return gets split from the call as you show above then the logic on the return side -- assuming you're hitting the final noway_assert below:

        // Delete GT_RETURN  if any
        if (nextMorphStmt != nullptr)
        {
            GenTreePtr retExpr = nextMorphStmt->gtStmtExpr;
            noway_assert(retExpr->gtOper == GT_RETURN);

            // If var=call, then the next stmt must be a GT_RETURN(TYP_VOID) or GT_RETURN(var).
            // This can occur if impSpillStackEnsure() has introduced an assignment to a temp.
            if (stmtExpr->gtOper == GT_ASG && info.compRetType != TYP_VOID)
            {
                noway_assert(stmtExpr->gtGetOp1()->OperIsLocal());
                noway_assert(stmtExpr->gtGetOp1()->AsLclVarCommon()->gtLclNum ==
                             retExpr->gtGetOp1()->AsLclVarCommon()->gtLclNum);
            }

-- will need a similar update: you will need to tunnel through casts on the retExpr op1 side to find a non-cast and then assert it is the expected local var.

sandreenko referenced this issue in sandreenko/coreclr May 30, 2017
sandreenko referenced this issue in dotnet/coreclr May 30, 2017
@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@msftgits msftgits added this to the 2.1.0 milestone Jan 31, 2020
@dotnet dotnet locked as resolved and limited conversation to collaborators Dec 22, 2020
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 bug
Projects
None yet
Development

No branches or pull requests

3 participants