dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.05k stars 4.69k forks source link

[MIPS64] Assertion failed 'NYI: Frame without frame pointer' #329

Closed xiangzhai closed 4 years ago

xiangzhai commented 4 years ago

Hi,

Testcase: JIT/Methodical/divrem/div/u4div_cs_do/u4div_cs_do.exe

Build option:

./build.sh ignorewarnings skipcrossgen

export LANG="zh_CN.UTF-8"
export CLR_ICU_VERSION_OVERRIDE="52.1"

# corefx && aspnetcore ALLIN1
export CORE_LIBRARIES=/home/loongson/zhaixiang/corefx-3.1-Linux.mips64.Debug

# jit
export COMPlus_JitFunctionTrace=1

Assertion failed:

Assert failure(PID 23287 [0x00005af7], Thread: 23301 [0x5b05]): Assertion failed 'NYI: Frame without frame pointer' in 'u4div:ui_f(ref):int' (IL size 18)

    File: /home/loongson/zhaixiang/coreclr-mips64-dev/src/jit/codegencommon.cpp Line: 5780
    Image: /home/loongson/zhaixiang/coreclr-mips64-dev/bin/Product/Linux.mips64.Debug/corerun

MIPS64 ported the CodeGen::genPushCalleeSavedRegisters:

    else
    {
        // No frame pointer (no chaining).
        assert((maskSaveRegsInt & RBM_FP) == 0);
        assert((maskSaveRegsInt & RBM_RA) != 0);

        // Note that there is no pre-indexed save_rapair unwind code variant, so we can't allocate the frame using
        // 'gssq' if we only have one callee-saved register plus RA to save.

        NYI("Frame without frame pointer");
        offset = 0;
    }

Workaround:

diff --git a/src/jit/compiler.cpp b/src/jit/compiler.cpp
index b73d617..eb43f6c 100644
--- a/src/jit/compiler.cpp
+++ b/src/jit/compiler.cpp
@@ -4106,6 +4106,9 @@ void Compiler::compSetOptimizationLevel()

 _SetMinOpts:

+#ifdef _TARGET_MIPS64_
+    theMinOptsValue = true;
+#endif
     // Set the MinOpts value
     opts.SetMinOpts(theMinOptsValue);

The workaround is just able to force codeGen->setFrameRequired(true);

Request for comments.

\cc @BruceForstall

Thanks, Leslie Zhai

xiangzhai commented 4 years ago

\cc @QiaoVanke

xiangzhai commented 4 years ago

BTW COMPlus_JITMinOpts=1 doesn't work without the workaround.

BruceForstall commented 4 years ago

When bringing up the JIT, the first thing is to get everything running with MinOpts, so your "workaround" makes sense. (Setting COMPlus_JITMinOpts=1 should work; I'm not sure why it doesn't for you.)

I'm not familiar with the type of frames that are required by the MIPS ABI. Are there both frame pointer and frame pointer optional frames? I probably would not copy the ARM64 frame types; they are very complicated, and likely do what you want for MIPS. The arm32 ones are probably closer to what you need, depending on the ABI.

BruceForstall commented 4 years ago

btw, I created an arch-mips64 label that we can use to tag MIPS64 architecture issues or PRs, as necessary.

xiangzhai commented 4 years ago

Hi @BruceForstall

Thanks for your teaching!

Setting COMPlus_JITMinOpts=1 should work; I'm not sure why it doesn't for you.

I will double check it for MIPS64.

I probably would not copy the ARM64 frame types; they are very complicated ...

In the very beginning I just followed the ARM64 frameType skeleton:

if (isFramePointerUsed())
{
    if (((compiler->lvaOutgoingArgSpaceSize == 0) && (totalFrameSize <= xxx)) &&
          !genSaveFpRaWithAllCalleeSavedRegisters)
    {
    }
    else if (totalFrameSize <= xxx)
    {
        if (genSaveFpRaWithAllCalleeSavedRegisters)
        {
        }
        else
        {
        }
    }
    else
    {
        if (genSaveFpRaWithAllCalleeSavedRegisters)
        {
        }
        else
        {
        }
    }
}

if (frameType == 1)
{
}
else if (frameType == 2)
{
}
else if (frameType == 3)
{
}
else if (frameType == 4)
{
}
else if (frameType == 5)
{
}

and likely do what you want for MIPS.

Thank @QiaoVanke fixed my complicated porting. He made it suitable for MIPS64.

btw, I created an arch-mips64 label that we can use to tag MIPS64 architecture issues or PRs, as necessary.

Thank you so much!

Thanks, Leslie Zhai

xiangzhai commented 4 years ago

Setting COMPlus_JITMinOpts=1 should work; I'm not sure why it doesn't for you.

I will double check it for MIPS64.

Worked :)