Skip to content

Commit

Permalink
[RISC-V] ELT profiler: fix reconstruction of struct args passed parti…
Browse files Browse the repository at this point in the history
…ally on the stack (#91797)

* [RISC-V] Additional corner-cases for ELT profiler argument reading

* [RISC-V] Cover the case where struct is partially spilled on the stack
  • Loading branch information
tomeksowi authored Sep 8, 2023
1 parent e0a8170 commit 520236d
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 4 deletions.
25 changes: 21 additions & 4 deletions src/coreclr/vm/riscv64/profiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,6 @@ LPVOID ProfileArgIterator::GetNextArgAddr()
}

int argOffset = m_argIterator.GetNextOffset();

if (argOffset == TransitionBlock::InvalidOffset)
{
return nullptr;
Expand All @@ -192,21 +191,39 @@ LPVOID ProfileArgIterator::GetNextArgAddr()
}
}

int argSize = m_argIterator.IsArgPassedByRef() ? sizeof(void*) : m_argIterator.GetArgSize();
if (TransitionBlock::IsFloatArgumentRegisterOffset(argOffset))
{
return (LPBYTE)&pData->floatArgumentRegisters + (argOffset - TransitionBlock::GetOffsetOfFloatArgumentRegisters());
int offset = argOffset - TransitionBlock::GetOffsetOfFloatArgumentRegisters();
_ASSERTE(offset + argSize <= sizeof(pData->floatArgumentRegisters));
return (LPBYTE)&pData->floatArgumentRegisters + offset;
}

LPVOID pArg = nullptr;

if (TransitionBlock::IsArgumentRegisterOffset(argOffset))
{
pArg = (LPBYTE)&pData->argumentRegisters + (argOffset - TransitionBlock::GetOffsetOfArgumentRegisters());
int offset = argOffset - TransitionBlock::GetOffsetOfArgumentRegisters();
if (offset + argSize > sizeof(pData->argumentRegisters))
{
// Struct partially spilled on stack
const int regIndex = NUM_ARGUMENT_REGISTERS - 1; // first part of struct must be in last register
_ASSERTE(regIndex == offset / sizeof(pData->argumentRegisters.a[0]));
const int neededSpace = 2 * sizeof(INT64);
_ASSERTE(argSize <= neededSpace);
_ASSERTE(m_bufferPos + neededSpace <= sizeof(pData->buffer));
INT64* dest = (INT64*)&pData->buffer[m_bufferPos];
dest[0] = pData->argumentRegisters.a[regIndex];
// spilled part must be first on stack (if we copy too much, that's ok)
dest[1] = *(INT64*)pData->profiledSp;
m_bufferPos += neededSpace;
return dest;
}
pArg = (LPBYTE)&pData->argumentRegisters + offset;
}
else
{
_ASSERTE(TransitionBlock::IsStackArgumentOffset(argOffset));

pArg = (LPBYTE)pData->profiledSp + (argOffset - TransitionBlock::GetOffsetOfArgs());
}

Expand Down
35 changes: 35 additions & 0 deletions src/tests/profiler/elt/slowpathcommon.cs
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,8 @@ public static int RunTest()

Console.WriteLine($"DoubleRetFunc returned {DoubleRetFunc()}");

Console.WriteLine($"FloatRetFunc returned {FloatRetFunc()}");

Console.WriteLine($"IntegerSseStructFunc returned {IntegerSseStructFunc()}");

Console.WriteLine($"SseIntegerStructFunc returned {SseIntegerStructFunc()}");
Expand All @@ -379,6 +381,19 @@ public static int RunTest()

Console.WriteLine($"MixedMixedStructFunc returned {MixedMixedStructFunc()}");

var s1 = new MixedStruct(1, 1);
var s2 = new MixedStruct(2, 2);
var s3 = new MixedStruct(3, 3);
var s4 = new MixedStruct(4, 4);
var s5 = new MixedStruct(5, 5);
var s6 = new MixedStruct(6, 6);
var s7 = new MixedStruct(7, 7);
var s8 = new MixedStruct(8, 8);
var s9 = new MixedStruct(9, 9);
Console.WriteLine($"IntManyMixedStructFunc returned {IntManyMixedStructFunc(11, s1, s2, s3, s4, s5, s6, s7, s8, s9)}");

Console.WriteLine($"DoubleManyMixedStructFunc returned {DoubleManyMixedStructFunc(11.0, s1, s2, s3, s4, s5, s6, s7, s8, s9)}");

return 100;
}

Expand Down Expand Up @@ -519,6 +534,12 @@ public static double DoubleRetFunc()
return 13.0;
}

[MethodImplAttribute(MethodImplOptions.NoInlining)]
public static float FloatRetFunc()
{
return 13.0f;
}

[MethodImplAttribute(MethodImplOptions.NoInlining)]
public static IntegerSseStruct IntegerSseStructFunc()
{
Expand Down Expand Up @@ -548,5 +569,19 @@ public static MixedMixedStruct MixedMixedStructFunc()
{
return new MixedMixedStruct(1.2f, 3, 5, 6.7f);
}

[MethodImplAttribute(MethodImplOptions.NoInlining)]
public static MixedStruct IntManyMixedStructFunc(int i, MixedStruct s1, MixedStruct s2, MixedStruct s3, MixedStruct s4, MixedStruct s5, MixedStruct s6, MixedStruct s7, MixedStruct s8, MixedStruct s9)
{
Console.WriteLine($"i={i} s1=({s1}) s2=({s2}) s3=({s3}) s4=({s4}) s5=({s5}) s6=({s6}) s7=({s7}) s8=({s8}) s9=({s9})");
return s1;
}

[MethodImplAttribute(MethodImplOptions.NoInlining)]
public static MixedStruct DoubleManyMixedStructFunc(double d, MixedStruct s1, MixedStruct s2, MixedStruct s3, MixedStruct s4, MixedStruct s5, MixedStruct s6, MixedStruct s7, MixedStruct s8, MixedStruct s9)
{
Console.WriteLine($"d={d} s1=({s1}) s2=({s2}) s3=({s3}) s4=({s4}) s5=({s5}) s6=({s6}) s7=({s7}) s8=({s8}) s9=({s9})");
return s1;
}
}
}
49 changes: 49 additions & 0 deletions src/tests/profiler/native/eltprofiler/slowpatheltprofiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,48 @@ HRESULT STDMETHODCALLTYPE SlowPathELTProfiler::EnterCallback(FunctionIDOrClientI

_sawFuncEnter[functionName.ToWString()] = true;
}
else if (functionName == WCHAR("IntManyMixedStructFunc"))
{
int i = 11;
MixedStruct ss[] = {{1, 1.0}, {2, 2.0}, {3, 3.0}, {4, 4.0}, {5, 5.0}, {6, 6.0}, {7, 7.0}, {8, 8.0}, {9, 9.0}};
vector<ExpectedArgValue> expectedValues = {
{ sizeof(int), (void *)&i, [&](UINT_PTR ptr){ return ValidateInt(ptr, i); } },
{ sizeof(MixedStruct), (void *)&ss[0], [&](UINT_PTR ptr){ return ValidateMixedStruct(ptr, ss[0]); } },
{ sizeof(MixedStruct), (void *)&ss[1], [&](UINT_PTR ptr){ return ValidateMixedStruct(ptr, ss[1]); } },
{ sizeof(MixedStruct), (void *)&ss[2], [&](UINT_PTR ptr){ return ValidateMixedStruct(ptr, ss[2]); } },
{ sizeof(MixedStruct), (void *)&ss[3], [&](UINT_PTR ptr){ return ValidateMixedStruct(ptr, ss[3]); } },
{ sizeof(MixedStruct), (void *)&ss[4], [&](UINT_PTR ptr){ return ValidateMixedStruct(ptr, ss[4]); } },
{ sizeof(MixedStruct), (void *)&ss[5], [&](UINT_PTR ptr){ return ValidateMixedStruct(ptr, ss[5]); } },
{ sizeof(MixedStruct), (void *)&ss[6], [&](UINT_PTR ptr){ return ValidateMixedStruct(ptr, ss[6]); } },
{ sizeof(MixedStruct), (void *)&ss[7], [&](UINT_PTR ptr){ return ValidateMixedStruct(ptr, ss[7]); } },
{ sizeof(MixedStruct), (void *)&ss[8], [&](UINT_PTR ptr){ return ValidateMixedStruct(ptr, ss[8]); } },
};

hr = ValidateFunctionArgs(pArgumentInfo, functionName, expectedValues);

_sawFuncEnter[functionName.ToWString()] = true;
}
else if (functionName == WCHAR("DoubleManyMixedStructFunc"))
{
double d = 11.0;
MixedStruct ss[] = {{1, 1.0}, {2, 2.0}, {3, 3.0}, {4, 4.0}, {5, 5.0}, {6, 6.0}, {7, 7.0}, {8, 8.0}, {9, 9.0}};
vector<ExpectedArgValue> expectedValues = {
{ sizeof(double), (void *)&d, [&](UINT_PTR ptr){ return ValidateDouble(ptr, d); } },
{ sizeof(MixedStruct), (void *)&ss[0], [&](UINT_PTR ptr){ return ValidateMixedStruct(ptr, ss[0]); } },
{ sizeof(MixedStruct), (void *)&ss[1], [&](UINT_PTR ptr){ return ValidateMixedStruct(ptr, ss[1]); } },
{ sizeof(MixedStruct), (void *)&ss[2], [&](UINT_PTR ptr){ return ValidateMixedStruct(ptr, ss[2]); } },
{ sizeof(MixedStruct), (void *)&ss[3], [&](UINT_PTR ptr){ return ValidateMixedStruct(ptr, ss[3]); } },
{ sizeof(MixedStruct), (void *)&ss[4], [&](UINT_PTR ptr){ return ValidateMixedStruct(ptr, ss[4]); } },
{ sizeof(MixedStruct), (void *)&ss[5], [&](UINT_PTR ptr){ return ValidateMixedStruct(ptr, ss[5]); } },
{ sizeof(MixedStruct), (void *)&ss[6], [&](UINT_PTR ptr){ return ValidateMixedStruct(ptr, ss[6]); } },
{ sizeof(MixedStruct), (void *)&ss[7], [&](UINT_PTR ptr){ return ValidateMixedStruct(ptr, ss[7]); } },
{ sizeof(MixedStruct), (void *)&ss[8], [&](UINT_PTR ptr){ return ValidateMixedStruct(ptr, ss[8]); } },
};

hr = ValidateFunctionArgs(pArgumentInfo, functionName, expectedValues);

_sawFuncEnter[functionName.ToWString()] = true;
}
return hr;
}

Expand Down Expand Up @@ -489,6 +530,14 @@ HRESULT STDMETHODCALLTYPE SlowPathELTProfiler::LeaveCallback(FunctionIDOrClientI

_sawFuncLeave[functionName.ToWString()] = true;
}
else if (functionName == WCHAR("FloatRetFunc"))
{
float f = 13.0f;
ExpectedArgValue floatRetValue = { sizeof(float), (void *)&f, [&](UINT_PTR ptr){ return ValidateFloat(ptr, f); } };
hr = ValidateOneArgument(pRetvalRange, functionName, 0, floatRetValue);

_sawFuncLeave[functionName.ToWString()] = true;
}
else if (functionName == WCHAR("IntegerSseStructFunc"))
{
IntegerSseStruct val = { 1, 2, 3.5 };
Expand Down
3 changes: 3 additions & 0 deletions src/tests/profiler/native/eltprofiler/slowpatheltprofiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,8 @@ class SlowPathELTProfiler : public Profiler
_sawFuncEnter[L"Fp64x3StructDoubleFp64x3StructDoubleFunc"] = false;
_sawFuncEnter[L"Fp64x4StructFunc"] = false;
_sawFuncEnter[L"Fp64x4StructFp64x4StructFunc"] = false;
_sawFuncEnter[L"IntManyMixedStructFunc"] = false;
_sawFuncEnter[L"DoubleManyMixedStructFunc"] = false;

_sawFuncLeave[L"SimpleArgsFunc"] = false;
_sawFuncLeave[L"MixedStructFunc"] = false;
Expand All @@ -158,6 +160,7 @@ class SlowPathELTProfiler : public Profiler
_sawFuncLeave[L"Fp64x3StructFunc"] = false;
_sawFuncLeave[L"Fp64x4StructFunc"] = false;
_sawFuncLeave[L"DoubleRetFunc"] = false;
_sawFuncLeave[L"FloatRetFunc"] = false;
_sawFuncLeave[L"IntegerSseStructFunc"] = false;
_sawFuncLeave[L"SseIntegerStructFunc"] = false;
_sawFuncLeave[L"MixedSseStructFunc"] = false;
Expand Down

0 comments on commit 520236d

Please sign in to comment.