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

[tests] Fix recently added mono test #110682

Merged
merged 1 commit into from
Dec 16, 2024
Merged

[tests] Fix recently added mono test #110682

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
28 changes: 19 additions & 9 deletions src/tests/Loader/classloader/SequentialLayout/ManagedSequential/LargeStructSize_Mono.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ struct BigArray_64_1
{
}

[StructLayout(LayoutKind.Sequential, Size = int.MaxValue - 16 - 1)]
[StructLayout(LayoutKind.Sequential, Size = int.MaxValue - 16 + 1)]
struct BigArray_64_2
{
}
Expand All @@ -65,29 +65,39 @@ struct BigArray_32_1
{
}

[StructLayout(LayoutKind.Sequential, Size = int.MaxValue - 8 - 1)]
[StructLayout(LayoutKind.Sequential, Size = int.MaxValue - 8 + 1)]
struct BigArray_32_2
{
}

[Fact]
public static void TestLargeStructSize()
public static void Test64Bit ()
{
if (Environment.Is64BitProcess)
{
Assert.Equal(int.MaxValue - (IntPtr.Size * 2), sizeof(BigArray_64_1));
Assert.Throws<TypeLoadException>(() => sizeof(BigArray_64_2));
Assert.Throws<TypeLoadException>(() => sizeof(X_64));
Assert.Throws<TypeLoadException>(() => sizeof(X_explicit_64));
Assert.Throws<TypeLoadException>(() => sizeof(Y_64));
}
else
{
}

public static void Test32Bit ()
{
Assert.Equal(int.MaxValue - (IntPtr.Size * 2), sizeof(BigArray_32_1));
Assert.Throws<TypeLoadException>(() => sizeof(BigArray_32_2));
Assert.Throws<TypeLoadException>(() => sizeof(X_32));
Assert.Throws<TypeLoadException>(() => sizeof(X_explicit_32));
Assert.Throws<TypeLoadException>(() => sizeof(Y_32));
}

[Fact]
public static void TestLargeStructSize()
{
if (Environment.Is64BitProcess)
{
Test64Bit ();
Copy link
Member

Choose a reason for hiding this comment

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

Why does moving the code to separate subroutine helps with:

Secondly, inlining of Environment.Is64BitProcess fails due to class initialization not yet being performed.

?

Copy link
Member Author

Choose a reason for hiding this comment

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

Consider code like:

if (true)
    print sizeof (T1)
else
    print sizeof (T2)

sizeof (T1) needs to run some class initialization before being computed. Also the size gets included in the emitted code. In this scenario, during method compilation, sizeof (T1) will be computed and we won't emit any code in the else branch. However if the if condition is not obviously known at method compile time, then both branches will be compiled and we would end up running initialization for T1 as well as for T2. This is not about resolving the condition better, but rather preventing the initialization of both types.

Copy link
Member

Choose a reason for hiding this comment

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

Ah I see, thanks. Everything can be solved by another layer of indirection :)

However, thinking about it more, why would if (Environment.Is64BitProcess) (which is just IntPtr.Size == 8) be unknown, especially during AOT. Maybe we are not optimizing the compilation and always emitting code for both branches and discarding the else branch later?

Copy link
Member Author

Choose a reason for hiding this comment

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

That is not the case. We don't inline any methods from classes that have a cctor that hasn't been run yet. In this particular case, the cctor wouldn't have any consequence on the result.

How does CoreCLR handle Environment.Is64BitProcess for example ? Does it rely on the tiered method to have the optimized code generated while the untiered method doesn't hardcode the value ? @AndyAyersMS

Copy link
Member

Choose a reason for hiding this comment

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

How does CoreCLR handle Environment.Is64BitProcess for example ? Does it rely on the tiered method to have the optimized code generated while the untiered method doesn't hardcode the value ? @AndyAyersMS

I don't see any special treatment (eg intrisic attribute) in CoreCLR for Is64BitProcess, so it will be a call in Tier0 and inlined / folded to a constant in Tier1.

}
else
{
Test32Bit ();
}
}
}
Loading