Skip to content

Fix crossgen2 ArgIterator for SystemV Amd64 Unix ABI #499

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 2 commits into from
Dec 5, 2019
Merged

Fix crossgen2 ArgIterator for SystemV Amd64 Unix ABI #499

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
9f3bfc4
Fix crossgen2 ArgIterator for amd64 Unix ABI
janvorli Dec 3, 2019
a107392
Remove the SystemV struct classification cache
janvorli Dec 4, 2019
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
5 changes: 2 additions & 3 deletions src/coreclr/src/tools/Common/JitInterface/CorInfoImpl.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2524,13 +2524,12 @@ private uint getMethodHash(CORINFO_METHOD_STRUCT_* ftn)
private byte* findNameOfToken(CORINFO_MODULE_STRUCT_* moduleHandle, mdToken token, byte* szFQName, UIntPtr FQNameCapacity)
{ throw new NotImplementedException("findNameOfToken"); }

SystemVStructClassificator _systemVStructClassificator = new SystemVStructClassificator();

private bool getSystemVAmd64PassStructInRegisterDescriptor(CORINFO_CLASS_STRUCT_* structHnd, SYSTEMV_AMD64_CORINFO_STRUCT_REG_PASSING_DESCRIPTOR* structPassInRegDescPtr)
{
TypeDesc typeDesc = HandleToObject(structHnd);

return _systemVStructClassificator.getSystemVAmd64PassStructInRegisterDescriptor(typeDesc, structPassInRegDescPtr);
SystemVStructClassificator.GetSystemVAmd64PassStructInRegisterDescriptor(typeDesc, out *structPassInRegDescPtr);
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this become a problem when PR #495 that enables multi-threaded compilation is merged?

Copy link
Member

Choose a reason for hiding this comment

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

I think one way to reword this is to ask whether the below classification dictionary should become a ConcurrentDictionary or whether we rather need to put locks around its population.

Copy link
Member Author

Choose a reason for hiding this comment

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

We could also add the classification to the TypeDesc instead of having a separate cache. We could then set a reference to the classification structure using compare exchange (if we allocated it separately) or assign the passedInRegisters as the last member and use a proper memory barrier / volatile access to ensure it is observed as executed as the last by concurrent threads of execution.
That would actually be similar to the runtime where we have the classification stored in the EEClass.

return true;
}

private uint getThreadTLSIndex(ref void* ppIndirection)
Expand Down
49 changes: 20 additions & 29 deletions src/coreclr/src/tools/Common/JitInterface/SystemVStructClassificator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,8 @@ namespace Internal.JitInterface
using static SYSTEMV_AMD64_CORINFO_STRUCT_REG_PASSING_DESCRIPTOR;
using static SystemVClassificationType;

internal class SystemVStructClassificator
internal static class SystemVStructClassificator
{
private Dictionary<TypeDesc, SYSTEMV_AMD64_CORINFO_STRUCT_REG_PASSING_DESCRIPTOR> _classificationCache = new Dictionary<TypeDesc, SYSTEMV_AMD64_CORINFO_STRUCT_REG_PASSING_DESCRIPTOR>();

private struct SystemVStructRegisterPassingHelper
{
internal SystemVStructRegisterPassingHelper(int totalStructSize)
Expand Down Expand Up @@ -93,44 +91,37 @@ internal static IEnumerable<FieldDesc> GetInstanceFields(TypeDesc typeDesc, bool
}
}

public unsafe bool getSystemVAmd64PassStructInRegisterDescriptor(TypeDesc typeDesc, SYSTEMV_AMD64_CORINFO_STRUCT_REG_PASSING_DESCRIPTOR* structPassInRegDescPtr)
public static void GetSystemVAmd64PassStructInRegisterDescriptor(TypeDesc typeDesc, out SYSTEMV_AMD64_CORINFO_STRUCT_REG_PASSING_DESCRIPTOR structPassInRegDescPtr)
{
structPassInRegDescPtr->passedInRegisters = false;
structPassInRegDescPtr = default;
structPassInRegDescPtr.passedInRegisters = false;

int typeSize = typeDesc.GetElementSize().AsInt;
if (typeDesc.IsValueType && (typeSize <= CLR_SYSTEMV_MAX_STRUCT_BYTES_TO_PASS_IN_REGISTERS))
{
Debug.Assert((TypeDef2SystemVClassification(typeDesc) == SystemVClassificationTypeStruct) ||
(TypeDef2SystemVClassification(typeDesc) == SystemVClassificationTypeTypedReference));

if (_classificationCache.TryGetValue(typeDesc, out SYSTEMV_AMD64_CORINFO_STRUCT_REG_PASSING_DESCRIPTOR descriptor))
if ((TypeDef2SystemVClassification(typeDesc) != SystemVClassificationTypeStruct) &&
(TypeDef2SystemVClassification(typeDesc) != SystemVClassificationTypeTypedReference))
{
*structPassInRegDescPtr = descriptor;
return;
}
else

SystemVStructRegisterPassingHelper helper = new SystemVStructRegisterPassingHelper(typeSize);
bool canPassInRegisters = ClassifyEightBytes(typeDesc, ref helper, 0);
if (canPassInRegisters)
{
SystemVStructRegisterPassingHelper helper = new SystemVStructRegisterPassingHelper(typeSize);
bool canPassInRegisters = ClassifyEightBytes(typeDesc, ref helper, 0);
if (canPassInRegisters)
{
structPassInRegDescPtr->passedInRegisters = canPassInRegisters;
structPassInRegDescPtr->eightByteCount = (byte)helper.EightByteCount;
Debug.Assert(structPassInRegDescPtr->eightByteCount <= CLR_SYSTEMV_MAX_EIGHTBYTES_COUNT_TO_PASS_IN_REGISTERS);
structPassInRegDescPtr.passedInRegisters = canPassInRegisters;
structPassInRegDescPtr.eightByteCount = (byte)helper.EightByteCount;
Debug.Assert(structPassInRegDescPtr.eightByteCount <= CLR_SYSTEMV_MAX_EIGHTBYTES_COUNT_TO_PASS_IN_REGISTERS);

structPassInRegDescPtr->eightByteClassifications0 = helper.EightByteClassifications[0];
structPassInRegDescPtr->eightByteSizes0 = (byte)helper.EightByteSizes[0];
structPassInRegDescPtr->eightByteOffsets0 = (byte)helper.EightByteOffsets[0];
structPassInRegDescPtr.eightByteClassifications0 = helper.EightByteClassifications[0];
structPassInRegDescPtr.eightByteSizes0 = (byte)helper.EightByteSizes[0];
structPassInRegDescPtr.eightByteOffsets0 = (byte)helper.EightByteOffsets[0];

structPassInRegDescPtr->eightByteClassifications1 = helper.EightByteClassifications[1];
structPassInRegDescPtr->eightByteSizes1 = (byte)helper.EightByteSizes[1];
structPassInRegDescPtr->eightByteOffsets1 = (byte)helper.EightByteOffsets[1];
}

_classificationCache.Add(typeDesc, *structPassInRegDescPtr);
structPassInRegDescPtr.eightByteClassifications1 = helper.EightByteClassifications[1];
structPassInRegDescPtr.eightByteSizes1 = (byte)helper.EightByteSizes[1];
structPassInRegDescPtr.eightByteOffsets1 = (byte)helper.EightByteOffsets[1];
}
}

return true;
}

private static SystemVClassificationType TypeDef2SystemVClassification(TypeDesc typeDesc)
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/src/tools/ReadyToRun.SuperIlc/BuildFolderSet.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ public FrameworkExclusion(string simpleName, string reason, bool crossgen2Only =
new FrameworkExclusion("Microsoft.CodeAnalysis.CSharp", "Ibc TypeToken 6200019a has type token which resolves to a nil token", crossgen2Only: true),
new FrameworkExclusion("Microsoft.CodeAnalysis", "Ibc TypeToken 620001af unable to find external typedef", crossgen2Only: true),
new FrameworkExclusion("Microsoft.CodeAnalysis.VisualBasic", "Ibc TypeToken 620002ce unable to find external typedef", crossgen2Only: true),

new FrameworkExclusion("System.Runtime.Serialization.Formatters", "Assert in JIT on Linux", crossgen2Only: true)
};

private readonly IEnumerable<BuildFolder> _buildFolders;
Expand Down
Loading