Skip to content

[amd64 Unix|arm64] fastTailCall stack fixup NYI if TYP_STRUCT  #8152

Closed
dotnet/coreclr
#26255

Description

@jashook

The Issue:

Lowering::LowerFastTailCall will fix up the stack before the jump. In the case that the caller's stack frame has a TYP_STRUCT which needs to be overwritten we would create a temp struct and pass it currently according to the windows ABI. Currently we will not execute this code path because we make several choices before lowering the tail call that keep this from being true. First, we will force create a temp object for structs passed on the stack, so we will simply use the def of the temp we made during morph instead of creating a temp later. Second, we will decide not to fast tail call on amd64 unix and arm64 much more liberally than on amd64 Windows see #4941 for more information.

The closest I could come to a repro is:

using System;

public struct A
{
    public int a;
    public int b;
}

class TailCallStructPassing
{
    public static int bar(int count, A temp)
    {
        if (count < 100)
        {
            return count;
        }

        else
        {
            count -= 100;
            return bar(count, temp);
        }
    }

    /// Args
    ///
    /// (0) - A one
    /// (1) - A two
    /// (2) - A three
    /// (3) - A four
    /// (4) - A five
    /// (5) - A six
    /// (6) - int b
    /// (7) - int c
    /// (8) - int d
    /// (9) - int e
    /// (10) - int f
    /// (11) - int g
    /// (12) - int h
    /// (13) - int i
    /// (14) - int j
    /// (15) - A temp
    /// (16) - int k
    /// (17) - int l
    /// (18) - int m
    /// (19) - int n
    /// (20) - int o
    /// (21) - int p
    /// (22) - int q
    /// (23) - int r
    /// (24) - int s
    /// (25) - int t
    /// (26) - int u
    /// (27) - int decision
    /// (28) - int count
    /// (29) - A temp2
    /// (30) - A first
    /// (31) - int
    public static int foo(A one, A two, A three, A four, A five, A six, int b, int c, int d, int e, int f, int g, int h, int i, int j, A temp, int k, int l, int m, int n, int o, int p, int q, int r, int s, int t, int u, int decision, int count, A temp2, A first, int copy)
    {
        if (decision < 100)
        {
            return foo(six, five, four, three, two, one, b, c, u, t, s, r, q, p, o, temp, n, m, l, k, j, i, h, g, f, e, d, 500, 15, temp2, first, copy);
        }

        else
        {
            return bar(count, temp);
        }
    }

    ///
    /// Args
    /// (0) - int decision
    /// (1) - int count
    /// (2) - A one
    /// (3) - A two
    /// (4) - A three
    /// (5) - A four
    /// (6) - A five
    /// (7) - A six
    /// (8) - int a
    /// (9) - int b
    /// (10) - int c 
    /// (11) - int d
    /// (12) - int e
    /// (13) - int f
    /// (14) - int g
    /// (15) - A temp2
    /// (16) - int h
    /// (17) - int i
    /// (18) - int j 
    /// (19) - int k
    /// (20) - int l
    /// (21) - int m
    /// (22) - int n
    /// (23) - int o
    /// (24) - int p
    /// (25) - int q
    /// (26) - int r
    /// (27) - int s
    /// (28) - int t
    /// (29) - A temp
    /// (30) - A first
    /// (31) - A second
    /// (32) - int u
    /// (33) - int v
    /// (34) - int w
    /// (35) - int x
    /// (36) - int y
    /// (37) - int z
    /// (38) - int aa=10
    /// (39) - int ab=11
    /// (40) - int ac=12
    /// (41) - int ad=13
    /// (42) - int ae=14
    /// (43) - int af=15
    /// (44) - int ag=16
    /// (45) - int ah=17
    /// (46) - int ai=18
    /// (47) - int aj=19
    /// (48) - int ak=20
    /// (49) - int al=21
    /// (50) - int am=22
    /// (51) - int an=23
    /// (52) - int ao=24
    /// (53) - int ap=25
    /// (54) - int aq=26
    /// (55) - int ar=27
    /// (56) - int at=29
    /// (57) - int au=30
    public static int foo(int decision, int count, A one, A two, A three, A four, A five, A six, int a, int b, int c, int d, int e, int f, int g, A temp2, int h, int i, int j, int k, int l, int m, int n, int o, int p, int q, int r, int s, int t, A temp, A first, A second, int u, int v, int w, int x, int y, int z, int aa=10, int ab=11, int ac=12, int ad=13, int ae=14, int af=15, int ag=16, int ah=17, int ai=18, int aj=19, int ak=20, int al=21, int am=22, int an=23, int ao=24, int ap=25, int aq=26, int ar=27, int at=29, int au=30)
    {
        if (decision < 100)
        {
            return foo(six, five, four, three, two, first, b, c, u, t, s, r, q, p, o, temp, n, m, l, k, j, i, h, g, f, e, d, 500, 15, temp2, second, decision);
        }

        else
        {
            return bar(count, temp);
        }
    }

    public static void Main()
    {
        A temp = new A();
        temp.a = 50;
        temp.b = 75;
        //temp.c = 100;

        A temp2 = new A();
        temp2.a = 90;
        temp2.b = 75;
        //temp2.c = 10;

        A one = new A();
        A two = new A();
        A three = new A();
        A four = new A();
        A five = new A();
        A six = new A();
        A first = new A();
        A second = new A();

        one.a = 10;
        two.a = 11;
        three.a = 12;
        four.a = 13;
        five.a = 14;
        six.a = 15;

        one.b = 75;
        two.b = 75;
        three.b = 75;
        four.b = 75;
        five.b = 75;
        six.b = 75;

        int ret = foo(50, 19000, one, two, three, four, five, six, 0, 1, 2, 3, 4, 5, 6, temp2, 7, 8, 9, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, temp, first, second, 24, 25, 26, 27, 28, 29);
    } 
}

However, the above example will not create the temp due to the stack forced structs having been already copied in morph.
category:correctness
theme:tail-call
skill-level:intermediate
cost:medium

Activity

  1. jashook commented on May 18, 2017

    @jashook
    ContributorAuthor
  2. AndyAyersMS commented on Feb 23, 2018

    @AndyAyersMS
    Member

    @jashook is this still an issue? Does it need to be addressed for 2.1?

  3. jashook commented on Feb 26, 2018

    @jashook
    ContributorAuthor

    This has been mitigated but not fixed. We currently will not go down the code path that will cause the bug.

  4. jashook commented on Feb 26, 2018

    @jashook
    ContributorAuthor

    So this should safely be able to be switched to future

  5. transferred this issue fromdotnet/coreclron Jan 31, 2020
  6. added this to the Future milestone on Jan 31, 2020
  7. ghost locked as resolved and limited conversation to collaborators on Dec 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

arch-x64area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMIbugos-linuxLinux OS (any supported distro)

Type

No type

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions