Skip to content

Commit c7b7245

Browse files
SingleAccretionjkotas
SingleAccretion
and
jkotas
authored
[NativeAOT-LLVM] Stop passing the signatures around for external methods (#3122)
* Add a convenience clr.ilc subset * Clean up external method cell code Address a TODO about not passing the signature around. Remove a TODO about using getAddressOfPInvokeTarget. Fundamentally, the indirection is needed for LLVM reasons, NOT WASM reasons (with straight-line WASM we could simply emit i32.const <sym> call_indirect), and it's cleaner to have this separation between LLVM and not LLVM how it is right now, a separate method. This a no-diff change. * Fix subset name Co-authored-by: Jan Kotas <jkotas@microsoft.com> --------- Co-authored-by: Jan Kotas <jkotas@microsoft.com>
1 parent 2d90d71 commit c7b7245

File tree

11 files changed

+141
-196
lines changed

11 files changed

+141
-196
lines changed

‎eng/Subsets.props

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -178,9 +178,10 @@
178178
</PropertyGroup>
179179

180180
<ItemGroup>
181-
<!-- CoreClr Native AOT -->
182-
<SubsetName Include="NativeAot.Packages" Description="The CoreCLR native AOT compiler." />
181+
<!-- NativeAOT-LLVM subsets. -->
182+
<SubsetName Include="NativeAot.Packages" Description="The CoreCLR native AOT packages." />
183183
<SubsetName Include="NativeAot.Build" Description="The CoreCLR native AOT build integration." />
184+
<SubsetName Include="Clr.Ilc" Description="The CoreCLR native AOT compiler (ILC)." />
184185

185186
<!-- CoreClr -->
186187
<SubsetName Include="Clr" Description="The full CoreCLR runtime. Equivalent to: $(DefaultCoreClrSubsets)" />
@@ -511,6 +512,10 @@
511512
<ProjectToBuild Include="$(CoreClrProjectRoot)nativeaot\BuildIntegration\BuildIntegration.proj" Category="clr" />
512513
</ItemGroup>
513514

515+
<ItemGroup Condition="$(_subset.Contains('+clr.ilc+'))">
516+
<ProjectToBuild Include="$(CoreClrProjectRoot)tools\aot\ILCompiler\ILCompiler.csproj" Category="clr" />
517+
</ItemGroup>
518+
514519
<!-- Mono sets -->
515520
<ItemGroup Condition="$(_subset.Contains('+mono.llvm+')) or $(_subset.Contains('+mono.aotcross+')) or '$(TargetOS)' == 'ios' or '$(TargetOS)' == 'iossimulator' or '$(TargetOS)' == 'tvos' or '$(TargetOS)' == 'tvossimulator' or '$(TargetOS)' == 'maccatalyst' or '$(TargetOS)' == 'android' or '$(TargetOS)' == 'browser' or '$(TargetOS)' == 'wasi' or '$(TargetsLinuxBionic)' == 'true'">
516521
<ProjectToBuild Include="$(MonoProjectRoot)llvm\llvm-init.proj" Category="mono" Condition="'$(RuntimeFlavor)' == 'mono'" />

‎src/coreclr/jit/llvm.cpp

Lines changed: 2 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -687,28 +687,6 @@ CorInfoType Llvm::getLlvmReturnType(CorInfoType sigRetType, CORINFO_CLASS_HANDLE
687687
return toCorInfoType(arg->AbiInfo.ArgType);
688688
}
689689

690-
TargetAbiType Llvm::getAbiTypeForType(var_types type)
691-
{
692-
switch (genActualType(type))
693-
{
694-
case TYP_VOID:
695-
return TargetAbiType::Void;
696-
case TYP_INT:
697-
return TargetAbiType::Int32;
698-
case TYP_LONG:
699-
return TargetAbiType::Int64;
700-
case TYP_REF:
701-
case TYP_BYREF:
702-
return (TARGET_POINTER_SIZE == 4) ? TargetAbiType::Int32 : TargetAbiType::Int64;
703-
case TYP_FLOAT:
704-
return TargetAbiType::Float;
705-
case TYP_DOUBLE:
706-
return TargetAbiType::Double;
707-
default:
708-
unreached();
709-
}
710-
}
711-
712690
CORINFO_GENERIC_HANDLE Llvm::getSymbolHandleForHelperFunc(CorInfoHelpFunc helperFunc)
713691
{
714692
void* pIndirection = nullptr;
@@ -777,10 +755,9 @@ const char* Llvm::GetAlternativeFunctionName()
777755
return CallEEApi<EEAI_GetAlternativeFunctionName, const char*>(m_pEECorInfo);
778756
}
779757

780-
void Llvm::GetExternalMethodAddress(
781-
CORINFO_METHOD_HANDLE methodHandle, const TargetAbiType* sig, int sigLength, CORINFO_CONST_LOOKUP* pLookup)
758+
void Llvm::GetExternalMethodAddress(CORINFO_METHOD_HANDLE methodHandle, CORINFO_CONST_LOOKUP* pLookup)
782759
{
783-
return CallEEApi<EEAI_GetExternalMethodAddress, void>(m_pEECorInfo, methodHandle, sig, sigLength, pLookup);
760+
return CallEEApi<EEAI_GetExternalMethodAddress, void>(m_pEECorInfo, methodHandle, pLookup);
784761
}
785762

786763
void Llvm::GetDebugInfoForCurrentMethod(CORINFO_LLVM_METHOD_DEBUG_INFO* pInfo)

‎src/coreclr/jit/llvm.h

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -54,15 +54,6 @@ const int TARGET_POINTER_BITS = TARGET_POINTER_SIZE * BITS_PER_BYTE;
5454

5555
// Part of the Jit/EE interface, must be kept in sync with the managed versions in "CorInfoImpl.Llvm.cs".
5656
//
57-
enum class TargetAbiType : uint8_t
58-
{
59-
Void,
60-
Int32,
61-
Int64,
62-
Float,
63-
Double
64-
};
65-
6657
enum class CorInfoLlvmEHModel
6758
{
6859
Cpp, // Landingpad-based LLVM IR; compatible with Itanium ABI.
@@ -408,7 +399,6 @@ class Llvm
408399

409400
static CorInfoType toCorInfoType(var_types varType);
410401
static CorInfoType getLlvmArgTypeForCallArg(CallArg* arg);
411-
TargetAbiType getAbiTypeForType(var_types type);
412402

413403
CORINFO_GENERIC_HANDLE getSymbolHandleForHelperFunc(CorInfoHelpFunc helperFunc);
414404
CORINFO_GENERIC_HANDLE getSymbolHandleForClassToken(mdToken token);
@@ -423,8 +413,7 @@ class Llvm
423413
CorInfoType GetPrimitiveTypeForTrivialWasmStruct(CORINFO_CLASS_HANDLE structHandle);
424414
void GetTypeDescriptor(CORINFO_CLASS_HANDLE typeHandle, TypeDescriptor* pTypeDescriptor);
425415
const char* GetAlternativeFunctionName();
426-
void GetExternalMethodAddress(
427-
CORINFO_METHOD_HANDLE methodHandle, const TargetAbiType* callSiteSig, int sigLength, CORINFO_CONST_LOOKUP* pLookup);
416+
void GetExternalMethodAddress(CORINFO_METHOD_HANDLE methodHandle, CORINFO_CONST_LOOKUP* pLookup);
428417
void GetDebugInfoForCurrentMethod(CORINFO_LLVM_METHOD_DEBUG_INFO* pInfo);
429418
SingleThreadedCompilationContext* GetSingleThreadedCompilationContext();
430419
CorInfoLlvmEHModel GetExceptionHandlingModel();

‎src/coreclr/jit/llvmlower.cpp

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -837,23 +837,7 @@ void Llvm::lowerUnmanagedCall(GenTreeCall* callNode)
837837
{
838838
// We cannot easily handle varargs as we do not know which args are the fixed ones.
839839
assert((callNode->gtCallType == CT_USER_FUNC) && !callNode->IsVarargs());
840-
841-
ArrayStack<TargetAbiType> sig(_compiler->getAllocator(CMK_Codegen));
842-
sig.Push(getAbiTypeForType(JITtype2varType(callNode->gtCorInfoType)));
843-
for (CallArg& arg : callNode->gtArgs.Args())
844-
{
845-
sig.Push(getAbiTypeForType(JITtype2varType(getLlvmArgTypeForCallArg(&arg))));
846-
}
847-
848-
// WASM requires the callee and caller signature to match. At the LLVM level, "callee type" is the function
849-
// type attached of the called operand and "caller" - that of its callsite. The problem, then, is that for a
850-
// given module, we can only have one function declaration, thus, one callee type. And we cannot know whether
851-
// this type will be the right one until, in general, runtime (this is the case for WASM imports provided by
852-
// the host environment). Thus, to achieve the experience of runtime erros on signature mismatches, we "hide"
853-
// the target behind an indirection, turning this call into an indirect one.
854-
// TODO-LLVM-Cleanup: switch to using the standard "getAddressOfPInvokeTarget" Jit-EE call, we no longer need
855-
// the signature on the EE side.
856-
GetExternalMethodAddress(callNode->gtCallMethHnd, &sig.BottomRef(), sig.Height(), &callNode->gtEntryPoint);
840+
GetExternalMethodAddress(callNode->gtCallMethHnd, &callNode->gtEntryPoint);
857841
}
858842
}
859843

‎src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_Wasm/WasmAbi.cs

Lines changed: 81 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -12,59 +12,18 @@ public static class WasmAbi
1212
public static WasmValueType GetNaturalIntType(TargetDetails target) =>
1313
target.PointerSize == 4 ? WasmValueType.I32 : WasmValueType.I64;
1414

15-
public static WasmFunctionType GetWasmFunctionType(MethodDesc method) =>
16-
GetWasmFunctionType(method.Signature, method.RequiresInstArg());
17-
18-
public static WasmFunctionType GetWasmFunctionType(MethodSignature signature, bool hasHiddenParam)
15+
public static bool HasWasmFunctionType(MethodDesc method, WasmFunctionType type, WasmFunctionAbiOptions options = 0)
1916
{
20-
WasmValueType wasmPointerType = GetNaturalIntType(signature.Context.Target);
21-
22-
WasmValueType GetWasmArgTypeForArg(TypeDesc argSigType)
23-
{
24-
bool isPassedByRef = false;
25-
TypeDesc argType = argSigType;
26-
if (IsStruct(argSigType))
27-
{
28-
argType = GetPrimitiveTypeForTrivialWasmStruct(argSigType);
29-
if (argType == null)
30-
{
31-
isPassedByRef = true;
32-
}
33-
}
34-
35-
return isPassedByRef ? wasmPointerType : GetWasmTypeForTypeDesc(argType);
36-
}
37-
38-
WasmValueType wasmReturnType = GetWasmReturnType(signature.ReturnType, out bool isReturnByRef);
39-
40-
int maxWasmSigLength = signature.Length + 4;
41-
Span<WasmValueType> signatureTypes =
42-
maxWasmSigLength > 100 ? new WasmValueType[maxWasmSigLength] : stackalloc WasmValueType[maxWasmSigLength];
43-
44-
int index = 0;
45-
signatureTypes[index++] = wasmPointerType; // Shadow stack.
46-
47-
if (!signature.IsStatic) // TODO-LLVM-Bug: doesn't handle explicit 'this'.
48-
{
49-
signatureTypes[index++] = wasmPointerType;
50-
}
51-
52-
if (isReturnByRef)
53-
{
54-
signatureTypes[index++] = wasmPointerType;
55-
}
56-
57-
if (hasHiddenParam)
58-
{
59-
signatureTypes[index++] = wasmPointerType;
60-
}
61-
62-
foreach (TypeDesc type in signature)
63-
{
64-
signatureTypes[index++] = GetWasmArgTypeForArg(type);
65-
}
17+
WasmFunctionType? expectedType = type;
18+
GetWasmFunctionTypeImpl(ref expectedType, method, options);
19+
return expectedType != null;
20+
}
6621

67-
return new WasmFunctionType(wasmReturnType, signatureTypes.Slice(0, index).ToArray());
22+
public static WasmFunctionType GetWasmFunctionType(MethodDesc method, WasmFunctionAbiOptions options = 0)
23+
{
24+
WasmFunctionType? type = null;
25+
GetWasmFunctionTypeImpl(ref type, method, options);
26+
return type.Value;
6827
}
6928

7029
public static WasmValueType GetWasmReturnType(MethodDesc method, out bool isPassedByRef) =>
@@ -114,6 +73,71 @@ public static TypeDesc GetPrimitiveTypeForTrivialWasmStruct(TypeDesc type)
11473
return null;
11574
}
11675

76+
private static void GetWasmFunctionTypeImpl(
77+
ref WasmFunctionType? expectedType, MethodDesc method, WasmFunctionAbiOptions options = 0)
78+
{
79+
MethodSignature signature = method.Signature;
80+
WasmValueType wasmPointerType = GetNaturalIntType(signature.Context.Target);
81+
82+
WasmValueType GetWasmArgTypeForArg(TypeDesc argSigType)
83+
{
84+
bool isPassedByRef = false;
85+
TypeDesc argType = argSigType;
86+
if (IsStruct(argSigType))
87+
{
88+
argType = GetPrimitiveTypeForTrivialWasmStruct(argSigType);
89+
if (argType == null)
90+
{
91+
isPassedByRef = true;
92+
}
93+
}
94+
95+
return isPassedByRef ? wasmPointerType : GetWasmTypeForTypeDesc(argType);
96+
}
97+
98+
WasmValueType wasmReturnType = GetWasmReturnType(signature.ReturnType, out bool isReturnByRef);
99+
100+
int maxWasmSigLength = signature.Length + 4;
101+
Span<WasmValueType> wasmParamTypes =
102+
maxWasmSigLength > 100 ? new WasmValueType[maxWasmSigLength] : stackalloc WasmValueType[maxWasmSigLength];
103+
104+
int index = 0;
105+
if ((options & WasmFunctionAbiOptions.IsNative) == 0)
106+
{
107+
wasmParamTypes[index++] = wasmPointerType; // Shadow stack.
108+
}
109+
110+
if (!signature.IsStatic) // TODO-LLVM-Bug: doesn't handle explicit 'this'.
111+
{
112+
wasmParamTypes[index++] = wasmPointerType;
113+
}
114+
115+
if (isReturnByRef)
116+
{
117+
wasmParamTypes[index++] = wasmPointerType;
118+
}
119+
120+
if (method.RequiresInstArg())
121+
{
122+
wasmParamTypes[index++] = wasmPointerType;
123+
}
124+
125+
foreach (TypeDesc type in signature)
126+
{
127+
wasmParamTypes[index++] = GetWasmArgTypeForArg(type);
128+
}
129+
130+
wasmParamTypes = wasmParamTypes.Slice(0, index);
131+
if (expectedType is null)
132+
{
133+
expectedType = new WasmFunctionType(wasmReturnType, wasmParamTypes.ToArray());
134+
}
135+
else if (!WasmFunctionType.Equals(expectedType.Value, wasmReturnType, wasmParamTypes))
136+
{
137+
expectedType = null;
138+
}
139+
}
140+
117141
private static WasmValueType GetWasmReturnType(TypeDesc sigReturnType, out bool isPassedByRef)
118142
{
119143
TypeDesc returnType = sigReturnType;
@@ -174,4 +198,10 @@ private static WasmValueType GetWasmTypeForTypeDesc(TypeDesc type)
174198

175199
private static bool IsStruct(TypeDesc type) => type.Category is TypeFlags.ValueType or TypeFlags.Nullable;
176200
}
201+
202+
[Flags]
203+
public enum WasmFunctionAbiOptions
204+
{
205+
IsNative = 0x1,
206+
}
177207
}

‎src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_Wasm/WasmFunctionType.cs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,7 @@ public readonly struct WasmFunctionType(WasmValueType result, WasmValueType[] pa
1717
public ReadOnlySpan<WasmValueType> Parameters => _parameters;
1818

1919
public override bool Equals(object obj) => obj is WasmFunctionType type && Equals(type);
20-
21-
public bool Equals(WasmFunctionType other)
22-
{
23-
return Results.SequenceEqual(other.Results) && Parameters.SequenceEqual(other.Parameters);
24-
}
20+
public bool Equals(WasmFunctionType other) => Equals(this, other._result, other._parameters);
2521

2622
public override int GetHashCode()
2723
{
@@ -33,6 +29,9 @@ public override int GetHashCode()
3329

3430
public static bool IsFunction(ISymbolNode symbol) => symbol is ExternSymbolNode or IWasmFunctionNode or IMethodNode { Offset: 0 };
3531

32+
public static bool Equals(WasmFunctionType type, WasmValueType result, ReadOnlySpan<WasmValueType> parameters) =>
33+
type._result == result && type.Parameters.SequenceEqual(parameters);
34+
3635
public static bool operator ==(WasmFunctionType left, WasmFunctionType right) => left.Equals(right);
3736
public static bool operator !=(WasmFunctionType left, WasmFunctionType right) => !(left == right);
3837
}

0 commit comments

Comments
 (0)