Skip to content

Implement sizeof() in the interpreter #115745

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

Merged
merged 1 commit into from
May 20, 2025
Merged

Implement sizeof() in the interpreter #115745

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 10 additions & 0 deletions src/coreclr/interpreter/compiler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3500,6 +3500,16 @@ int InterpCompiler::GenerateCode(CORINFO_METHOD_INFO* methodInfo)
m_pLastNewIns->SetDVar(m_pStackPointer[-1].var);
m_ip++;
break;
case CEE_SIZEOF:
{
CORINFO_CLASS_HANDLE clsHnd = ResolveClassToken(getU4LittleEndian(m_ip + 1));
Copy link
Member

@janvorli janvorli May 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems it would make sense to add CHECK_STACK(1);

Copy link
Member Author

@kg kg May 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, I thought CHECK_STACK was "make sure this many items are on the stack"

Copy link
Member

@janvorli janvorli May 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I guess I am wrong in this case, I haven't realized the token is not on the stack.

AddIns(INTOP_LDC_I4);
m_pLastNewIns->data[0] = m_compHnd->getClassSize(clsHnd);
PushStackType(StackTypeI4, NULL);
m_pLastNewIns->SetDVar(m_pStackPointer[-1].var);
Copy link

Copilot AI May 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Consider adding a comment to explain the reason for incrementing m_ip by 5, which can help clarify the instruction length and avoid confusion.

Suggested change
m_pLastNewIns->SetDVar(m_pStackPointer[-1].var);
m_pLastNewIns->SetDVar(m_pStackPointer[-1].var);
// Increment the instruction pointer by 5 to account for the 1-byte opcode
// and the 4-byte operand (e.g., a token or offset) of the CEE_SIZEOF instruction.

Copilot uses AI. Check for mistakes.
m_ip += 5;
break;
}
default:
assert(0);
break;
Expand Down
14 changes: 14 additions & 0 deletions src/tests/JIT/interpreter/Interpreter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,9 @@ public static void RunInterpreterTests()
if (!TestArray())
Environment.FailFast(null);

if (!TestSizeof())
Environment.FailFast(null);
Copy link

Copilot AI May 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider providing a descriptive error message in Environment.FailFast to aid debugging if TestSizeof fails.

Suggested change
Environment.FailFast(null);
Environment.FailFast("TestSizeof failed.");

Copilot uses AI. Check for mistakes.

System.GC.Collect();
}

Expand Down Expand Up @@ -945,4 +948,15 @@ public static bool ArrayDouble(int length, double value)

return true;
}

public static unsafe bool TestSizeof()
{
if (sizeof(int) != 4)
return false;
if (sizeof(double) != 8)
return false;
if (sizeof(MyStruct) != 4)
Copy link

Copilot AI May 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Consider adding a comment explaining the assumptions behind MyStruct's size (e.g. packing or field types) to clarify why the expected size is 4.

Copilot uses AI. Check for mistakes.
return false;
return true;
}
}
Loading