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

Common/fast lookupHWIntrinsic(...) API #9628

Closed
sdmaclea opened this issue Jan 30, 2018 · 5 comments
Closed

Common/fast lookupHWIntrinsic(...) API #9628

sdmaclea opened this issue Jan 30, 2018 · 5 comments
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@sdmaclea
Copy link
Contributor

NamedIntrinsic Compiler::lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method)

Currently uses this code to lookup intrinsics

#ifdef FEATURE_HW_INTRINSICS
#if defined(_TARGET_XARCH_)
    if ((namespaceName != nullptr) && strcmp(namespaceName, "System.Runtime.Intrinsics.X86") == 0)
    {
        InstructionSet isa = lookupHWIntrinsicISA(className);
        result             = lookupHWIntrinsic(methodName, isa);
    }
#elif defined(_TARGET_ARM64_)
    if ((namespaceName != nullptr) && strcmp(namespaceName, "System.Runtime.Intrinsics.Arm.Arm64") == 0)
    {
        result = lookupHWIntrinsic(className, methodName);
    }
#else // !defined(_TARGET_XARCH_) && !defined(_TARGET_ARM64_)
#error Unsupported platform
#endif // !defined(_TARGET_XARCH_) && !defined(_TARGET_ARM64_)
#endif // FEATURE_HW_INTRINSICS

It would be nice to simplify this to

#ifdef FEATURE_HW_INTRINSICS
    if (result == NI_Illegal)
    {
        result = lookupHWIntrinsic(namespaceName, className, methodName, method);
    }
#endif // FEATURE_HW_INTRINSICS

The actual parameters to lookupHWIntrinsic are arguable.

I proposed including method so that getMethodHash(CORINFO_METHOD_HANDLE) could be used for fast lookup from an unordered dictionary. Of course a hash of the strings could also be used, but I suspect method hash would be faster.

@4creators @tannergooding @CarolEidt @fiigii

category:implementation
theme:jit-coding-style
skill-level:intermediate
cost:small

@tannergooding
Copy link
Member

This sounds reasonable to me, provided we can still keep it performant.

I believe current investigations lead us to believe we would need something like a binary search (or better), given the sheer number of intrinsics some ISAs are providing.

@CarolEidt
Copy link
Contributor

This sounds reasonable to me, provided we can still keep it performant.

It's hard to imagine that it could be any worse than the current approach.

I believe current investigations lead us to believe we would need something like a binary search (or better), given the sheer number of intrinsics some ISAs are providing.

Binary search seems reasonable, since all the keys are known at compile-time. Whatever we do, we don't have to pay setup cost for the case where there are no intrinsics being used.

@fiigii
Copy link
Contributor

fiigii commented Jan 31, 2018

I remember @AndyAyersMS suggested that the faster-lookup should handle all the new-style named intrinsic.

@4creators
Copy link
Contributor

Binary search seems reasonable, since all the keys are known at compile-time

Than it should be possible to go for O(1) - a bit unconventional switch table.

@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@msftgits msftgits added this to the Future milestone Jan 31, 2020
@jkotas
Copy link
Member

jkotas commented Jul 21, 2020

Duplicate of #13617

@jkotas jkotas marked this as a duplicate of #13617 Jul 21, 2020
@jkotas jkotas closed this as completed Jul 21, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

No branches or pull requests

7 participants