Skip to content

[wasm][coreclr] 2nd part of call helpers generator, reverse calls #122007

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

[wasm][coreclr] 2nd part of call helpers generator, reverse calls #122007

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c8c3fc0
[wasm][coreclr] 2nd part of call helpers generator, reverse calls
radekdoulik Nov 18, 2025
a3f20dc
Do not target NETFramework anymore
radekdoulik Nov 18, 2025
89556d8
Update keys calculation
radekdoulik Nov 26, 2025
270a506
Feedback
radekdoulik Nov 26, 2025
f4d47bf
Feedback
radekdoulik Nov 27, 2025
c1a754f
Feedback
radekdoulik Nov 27, 2025
fb8d531
Merge remote-tracking branch 'remotes/origin/main' into clr-interp-wa…
radekdoulik Nov 27, 2025
2fdf1e7
Update after merge
radekdoulik Nov 27, 2025
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
1 change: 0 additions & 1 deletion src/coreclr/vm/wasm/callhelpers-interp-to-managed.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,6 @@ const StringToWasmSigThunk g_wasmThunks[] = {
{ "viiinni", (void*)&CallFunc_I32_I32_I32_IND_IND_I32_RetVoid },
{ "viin", (void*)&CallFunc_I32_I32_IND_RetVoid },
{ "viinni", (void*)&CallFunc_I32_I32_IND_IND_I32_RetVoid },
{ "viin", (void*)&CallFunc_I32_I32_IND_RetVoid },
{ "viinnii", (void*)&CallFunc_I32_I32_IND_IND_I32_I32_RetVoid },
{ "vin", (void*)&CallFunc_I32_IND_RetVoid },
{ "vini", (void*)&CallFunc_I32_IND_I32_RetVoid },
Expand Down
217 changes: 206 additions & 11 deletions src/coreclr/vm/wasm/callhelpers-reverse.cpp
View file
Open in desktop

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions src/coreclr/vm/wasm/callhelpers.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ struct ReverseThunkMapValue
struct ReverseThunkMapEntry
{
ULONG key;
ULONG fallbackKey;
Copy link
Member

@BrzVlad BrzVlad Dec 2, 2025

Choose a reason for hiding this comment

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

If I understand correctly, having trimming enabled makes the primary key irrelevant. I'm assuming that having trimming enabled is a must have default on wasm. So does it even make sense to have 2 keys in the first place ?

ReverseThunkMapValue value;
};

Expand Down
77 changes: 62 additions & 15 deletions src/coreclr/vm/wasm/helpers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -618,10 +618,33 @@ namespace
if (!GetSignatureKey(sig, keyBuffer, keyBufferLen))
return NULL;

return LookupThunk(keyBuffer);
void* thunk = LookupThunk(keyBuffer);
#ifdef _DEBUG
if (thunk == NULL)
printf("WASM calli missing for key: %s\n", keyBuffer);
#endif
return thunk;
}

ULONG CreateFallbackKey(MethodDesc* pMD)
{
_ASSERTE(pMD != nullptr);

// the fallback key is in the form $"{MethodName}#{Method.GetParameters().Length}:{AssemblyName}:{Namespace}:{TypeName}";
LPCUTF8 pszNamespace = nullptr;
LPCUTF8 pszName = pMD->GetMethodTable()->GetFullyQualifiedNameInfo(&pszNamespace);
MetaSig sig(pMD);
SString strFullName;
strFullName.Printf("%s#%d:%s:%s:%s",
pMD->GetName(),
sig.NumFixedArgs(),
pMD->GetAssembly()->GetSimpleName(),
pszNamespace != nullptr ? pszNamespace : "",
pszName);

return strFullName.Hash();
}

// TODO: This hashing function should be replaced.
ULONG CreateKey(MethodDesc* pMD)
{
_ASSERTE(pMD != nullptr);
Expand All @@ -640,30 +663,54 @@ namespace

typedef MapSHash<ULONG, const ReverseThunkMapValue*> HashToReverseThunkHash;
HashToReverseThunkHash* reverseThunkCache = nullptr;
HashToReverseThunkHash* reverseThunkFallbackCache = nullptr;

HashToReverseThunkHash* CreateReverseThunkHashTable(bool fallback)
{
HashToReverseThunkHash* newTable = new HashToReverseThunkHash();
newTable->Reallocate(g_ReverseThunksCount * HashToReverseThunkHash::s_density_factor_denominator / HashToReverseThunkHash::s_density_factor_numerator + 1);
for (size_t i = 0; i < g_ReverseThunksCount; i++)
{
newTable->Add(fallback ? g_ReverseThunks[i].fallbackKey : g_ReverseThunks[i].key, &g_ReverseThunks[i].value);
}

HashToReverseThunkHash **ppCache = fallback ? &reverseThunkFallbackCache : &reverseThunkCache;
if (InterlockedCompareExchangeT(ppCache, newTable, nullptr) != nullptr)
{
// Another thread won the race, discard ours
delete newTable;
}
return *ppCache;
}

const ReverseThunkMapValue* LookupThunk(MethodDesc* pMD)
{
HashToReverseThunkHash* table = VolatileLoad(&reverseThunkCache);
if (table == nullptr)
{
HashToReverseThunkHash* newTable = new HashToReverseThunkHash();
newTable->Reallocate(g_ReverseThunksCount * HashToReverseThunkHash::s_density_factor_denominator / HashToReverseThunkHash::s_density_factor_numerator + 1);
for (size_t i = 0; i < g_ReverseThunksCount; i++)
{
newTable->Add(g_ReverseThunks[i].key, &g_ReverseThunks[i].value);
}

if (InterlockedCompareExchangeT(&reverseThunkCache, newTable, nullptr) != nullptr)
{
// Another thread won the race, discard ours
delete newTable;
}
table = reverseThunkCache;
table = CreateReverseThunkHashTable(false /* fallback */);
}

ULONG key = CreateKey(pMD);

// Try primary key, it is based on Assembly fully qualified name and method token
const ReverseThunkMapValue* thunk;
if (table->Lookup(key, &thunk))
{
return thunk;
}

// Try fallback key, that is based on method properties and assembly name
// The fallback is used when the assembly is trimmed and the token and assembly fully qualified name
// may change.
table = VolatileLoad(&reverseThunkFallbackCache);
if (table == nullptr)
{
table = CreateReverseThunkHashTable(true /* fallback */);
}

key = CreateFallbackKey(pMD);

Comment on lines +701 to +718
Copy link
Member

@kg kg Dec 1, 2025

Choose a reason for hiding this comment

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

What happens if we get a key collision? Is it mechanically prevented from happening somewhere?

Copy link
Member

@kg kg Dec 1, 2025

Choose a reason for hiding this comment

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

Looking deeper, I'm concerned that we're using a low quality 32-bit hash here, so I would hope that at least if we get a hash collision during thunk lookup, it will fail deterministically with a clear error message so we can go back and fix it. Ideally we would use a more robust 64-bit hash to make the odds of a collision way lower, or do an actual string check. But I don't know if thunk lookup is on a hot path to the point that we can't afford it.

bool success = table->Lookup(key, &thunk);
return success ? thunk : nullptr;
}
Expand Down
7 changes: 3 additions & 4 deletions src/tasks/WasmAppBuilder/WasmAppBuilder.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>$(NetCoreAppToolCurrent);$(NetFrameworkToolCurrent)</TargetFrameworks>
<TargetFramework>$(NetCoreAppToolCurrent)</TargetFramework>
<Nullable>enable</Nullable>
<NoWarn>$(NoWarn),CA1050</NoWarn>
<!-- Ignore nullable warnings on net4* -->
<NoWarn Condition="$([MSBuild]::GetTargetFrameworkIdentifier('$(TargetFramework)')) == '.NETFramework'">$(NoWarn),CS8604,CS8602</NoWarn>
<NoWarn Condition="$([MSBuild]::GetTargetFrameworkIdentifier('$(TargetFramework)')) != '.NETFramework'">$(NoWarn),CA1850</NoWarn>
<EnableSingleFileAnalyzer>false</EnableSingleFileAnalyzer>
<CopyLocalLockFileAssemblies>true</CopyLocalLockFileAssemblies>
Expand Down Expand Up @@ -58,7 +57,7 @@
</ItemGroup>
</Target>

<UsingTask TaskName="ManagedToNativeGenerator" Runtime="NET" TaskFactory="TaskHostFactory" AssemblyFile="$(OutputPath)$(NetCoreAppToolCurrent)\WasmAppBuilder.dll" />
<UsingTask TaskName="ManagedToNativeGenerator" Runtime="NET" TaskFactory="TaskHostFactory" AssemblyFile="$(OutputPath)\WasmAppBuilder.dll" />

<Target Name="RunGenerator" DependsOnTargets="Build" Condition="'$(AssembliesScanPath)' != ''">
<ItemGroup>
Expand All @@ -73,7 +72,7 @@
<ManagedToNativeGenerator
Assemblies="@(WasmPInvokeAssembly)"
PInvokeModules="@(WasmPInvokeModule)"
PInvokeOutputPath="$(GeneratorOutputPath)todo-pinvoke-helpers.cpp"
PInvokeOutputPath="$(GeneratorOutputPath)callhelpers-reverse.cpp"
InterpToNativeOutputPath="$(GeneratorOutputPath)callhelpers-interp-to-managed.cpp">
<Output TaskParameter="FileWrites" ItemName="FileWrites" />
</ManagedToNativeGenerator>
Expand Down
Loading
Loading