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.

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

CG2 Add lazy string literal loading support #47183

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
3 changes: 1 addition & 2 deletions src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2755,8 +2755,7 @@ private void getFunctionFixedEntryPoint(CORINFO_METHOD_STRUCT_* ftn, ref CORINFO

private CorInfoHelpFunc getLazyStringLiteralHelper(CORINFO_MODULE_STRUCT_* handle)
{
// TODO: Lazy string literal helper
return CorInfoHelpFunc.CORINFO_HELP_UNDEF;
return CorInfoHelpFunc.CORINFO_HELP_STRCNS_CURRENT_MODULE;
nattress marked this conversation as resolved.
Show resolved Hide resolved
}

private CORINFO_MODULE_STRUCT_* embedModuleHandle(CORINFO_MODULE_STRUCT_* handle, ref void* ppIndirection)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -799,6 +799,9 @@ private ISymbolNode GetHelperFtnUncached(CorInfoHelpFunc ftnNum)
case CorInfoHelpFunc.CORINFO_HELP_JIT_REVERSE_PINVOKE_EXIT:
id = ReadyToRunHelper.ReversePInvokeExit;
break;
case CorInfoHelpFunc.CORINFO_HELP_STRCNS_CURRENT_MODULE:
id = ReadyToRunHelper.GetString;
Copy link
Member

Choose a reason for hiding this comment

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

I do not think it is as simple as this. There needs to be a code to add the current module argument for the helper (it is what ZapLazyHelperThunk does in crossgen1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I thought the CORINFO_HELP_STRCNS_CURRENT_MODULE meant we didn't need that. I'll look into adding that lazy thunk.

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 that ImportThunk already has a Lazy variant that I think was supposed to cater for lazy strings, among others. Having said that, it hasn't been tested until now so there's a high chance of it being broken in some way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks Tomas - I corrected things to call the lazy thunk and fixed an x64 bug. I'll kick off a CG2 run to make sure we're okay on other architectures.

nattress marked this conversation as resolved.
Show resolved Hide resolved
break;

case CorInfoHelpFunc.CORINFO_HELP_INITCLASS:
case CorInfoHelpFunc.CORINFO_HELP_INITINSTCLASS:
Expand Down