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.

Sign up for GitHub

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

[RISC-V] ELT profiler: fix reconstruction of struct args passed partially on the stack #91797

Merged
merged 2 commits into from
Sep 8, 2023
+108 −4
Merged

[RISC-V] ELT profiler: fix reconstruction of struct args passed partially on the stack #91797

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9db0ba4
[RISC-V] Additional corner-cases for ELT profiler argument reading
tomeksowi Sep 6, 2023
533e3cb
[RISC-V] Cover the case where struct is partially spilled on the stack
tomeksowi Sep 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 21 additions & 4 deletions src/coreclr/vm/riscv64/profiler.cpp
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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