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

The x86 path for UnmanagedCallersOnlyAttribute should be improved. #33582

Closed
AaronRobinsonMSFT opened this issue Mar 14, 2020 · 6 comments
Closed

Comments

@AaronRobinsonMSFT
Copy link
Member

AaronRobinsonMSFT commented Mar 14, 2020

The making public of NativeCallableAttribute UnmanagedCallersOnlyAttribute done in #33005, provided some additional optimizations. However the Windows-x86 path was deemed lower priority so wasn't optimized. There are various locations where the Windows-x86 platform opted out of the optimization.

Examples:

#if defined(TARGET_X86) && defined(TARGET_WINDOWS) && !defined(CROSSGEN_COMPILE)
// Deferring X86 support until a need is observed or
// time permits investigation into all the potential issues.
if (pMD->HasNativeCallableAttribute())
{
pResult->addr = (void*)COMDelegate::ConvertToCallback(pMD);
}
else
{
pResult->addr = (void*)pMD->GetMultiCallableAddrOfCode();
}
#else

#if !defined(TARGET_X86) || !defined(TARGET_WINDOWS)
if (ftn->HasNativeCallableAttribute())
flags.Set(CORJIT_FLAGS::CORJIT_FLAG_REVERSE_PINVOKE);
#endif // !TARGET_X86 || !TARGET_WINDOWS

if (targetArchitecture == TargetArchitecture.X86
&& _compilation.TypeSystemContext.Target.OperatingSystem == TargetOS.Windows)
{
throw new RequiresRuntimeJitException("ReadyToRun: Methods with NativeCallableAttribute not implemented");
}

var targetDetails = _compilation.TypeSystemContext.Target;
if (targetDetails.Architecture == TargetArchitecture.X86
&& targetDetails.OperatingSystem == TargetOS.Windows
&& targetMethod.IsNativeCallable)
{
throw new RequiresRuntimeJitException("ReadyToRun: References to methods with NativeCallableAttribute not implemented");
}

@AaronRobinsonMSFT AaronRobinsonMSFT added area-Interop-coreclr untriaged New issue has not been triaged by the area owner labels Mar 14, 2020
@jkotas jkotas changed the title The Windows x86 path for NativeCallableAttribute should be improved. The x86 path for NativeCallableAttribute should be improved. Mar 29, 2020
@jkotas
Copy link
Member

jkotas commented Mar 29, 2020

Generalizing the issue to both Windows and Linux. It turns our both Windows x86 and Linux x86 need work.

@AaronRobinsonMSFT
Copy link
Member Author

See #34251 (comment) for Linux x86 issues.

@jeffschwMSFT jeffschwMSFT removed the untriaged New issue has not been triaged by the area owner label Apr 2, 2020
@jeffschwMSFT jeffschwMSFT added this to the 5.0 milestone Apr 2, 2020
@jkotas
Copy link
Member

jkotas commented Apr 6, 2020

See #34548 for (subset of) places that are affected.

@AaronRobinsonMSFT
Copy link
Member Author

@jkoritzinsky I assume some of the work you are doing is addressing this.

@jkoritzinsky
Copy link
Member

Kind of. I’ll make sure to link to this issue as I do the work related to it.

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Dec 19, 2020
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jan 20, 2021
@jkoritzinsky
Copy link
Member

Fixed by #46238

@ghost ghost locked as resolved and limited conversation to collaborators May 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants