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

Champion "Compiler intrinsics (calli, ldftn, etc)" #191

Closed
1 of 5 tasks
Tracked by #829
gafter opened this issue Feb 26, 2017 · 48 comments
Closed
1 of 5 tasks
Tracked by #829

Champion "Compiler intrinsics (calli, ldftn, etc)" #191

gafter opened this issue Feb 26, 2017 · 48 comments
Assignees
Milestone

Comments

@gafter
Copy link
Member

gafter commented Feb 26, 2017

  • Proposal added
  • Discussed in LDM
  • Decision in LDM
  • Finalized (done, rejected, inactive)
  • Spec'ed

See also

@gafter gafter added this to the 7.X candidate milestone Feb 26, 2017
@Unknown6656
Copy link
Contributor

What are the advantages of compiler intrinsics for specific IL-opcodes instead of directly supporting inline-IL?

@orthoxerox
Copy link

@Unknown6656 you don't have to modify the parser, only the emitter, so you get to reuse type checking.

@tannergooding
Copy link
Member

tannergooding commented Apr 29, 2017

At least for calli and ldftn, the static delegate proposal (#302) would cover a way of using it outside of intrinsic support. However, the intrinsic support would still be useful for cases where users can't or don't want to change their API surface.

@MadsTorgersen MadsTorgersen modified the milestones: 7.X candidate, X.X candidate Oct 9, 2017
@MadsTorgersen
Copy link
Contributor

Need more motivation to prioritize this

@MichalStrehovsky
Copy link
Member

Need more motivation to prioritize this

@MadsTorgersen What kind of motivation? I can buy beers to anyone who implements it if they need to motivate that way. There's plenty of use cases for this - see all the people referencing the issue at dotnet/roslyn#11475.

Could implementing this be an up for grabs issue in the Roslyn repo?

@4creators
Copy link

@MichalStrehovsky @MadsTorgersen
Without any beer I could volunteer to implement it, however, due to my time limitations it could be only free time job.

@jaredpar
Copy link
Member

jaredpar commented Dec 5, 2017

Could implementing this be an up for grabs issue in the Roslyn repo?

Unlikely. We are still uncertain this is the design we want to take. I personally have a lot of reservations about the approach.

@4creators
Copy link

@jaredpar
Can you describe what kind of reservations C# team has about this proposal?

@GeirGrusom
Copy link

IL as a language feature is a little bit of a cop-out I think.

@jaredpar
Copy link
Member

jaredpar commented Dec 6, 2017

@4creators the LDM notes capture most of the push back. Mostly though it's a feature that essentially lets you write IL in C#. If we want that as a feature I'd rather just support inline IL than almost IL for a few small cases.

But more importantly the main use cases for these features can be done with firstn class language features which are already under consideration. Would rather support static delegates for instance and get the calli support + have an allocation free delegate experience.

@tannergooding
Copy link
Member

I would also think that these would be more useful as a general API (exposed in CoreFX and powered by CoreCLR, just like the hardware intrinsics), rather than as a language specific feature.

Being able to write IL code that isn't supported by the language in a easy to use/performant manner isn't just limited to C#.

@tmat
Copy link
Member

tmat commented Dec 6, 2017

IIRC, the LDM approved the feature as proposed in dotnet/roslyn#11475.

@MichalStrehovsky
Copy link
Member

MichalStrehovsky commented Dec 7, 2017

@jaredpar If this has LDM approval, can we mark this as up for grabs? I also remember it being LDM approved from hallway conversations. The LDM meeting notes discussing this stop mid sentence, unfortunately.

@jaredpar
Copy link
Member

jaredpar commented Dec 7, 2017

I don't remember that we approved it directly. Instead we wanted to see if other features were going to get done and then come back to this if not. It's been a while since we discussed this and possible I'm misremembering.

If this has LDM approval, can we mark this as up for grabs?

The process for contributing language features is described here: Developing a Language Feature. In summary though language features are done with oversight / in conjunction with the compiler team. They are sufficiently complex, and must update so many parts of the language experience, that it's not realistic to have an "up for grabs model".

@tmat
Copy link
Member

tmat commented Dec 7, 2017

@gafter @MadsTorgersen who might recall the LDM meeting.

@svick
Copy link
Contributor

svick commented Dec 8, 2017

@tannergooding

I would also think that these would be more useful as a general API (exposed in CoreFX and powered by CoreCLR, just like the hardware intrinsics), rather than as a language specific feature.

As I understand it, the main point of this proposal is to support IL opcodes that can't be reasonably expressed as library methods.

Specifically, how do you imagine the API for calli could look like?

@HaloFour
Copy link
Contributor

HaloFour commented Dec 8, 2017

@svick

Specifically, how do you imagine the API for calli could look like?

Like this: Static Delegates

Seems the point is that rather than providing awkward facilities to access the specific IL opcodes directly that the use cases for those opcodes would be determined and proper language features offered instead. That largely makes sense, although there will likely always be weird edge cases that couldn't be handled. Not to mention the burden would be on the team to identify those cases and design APIs around them.

@tannergooding
Copy link
Member

calli is effectively a method call on a function ptr with a specific signature, so

TResult CallIndirect<TArg1, TArg2, TResult>(T1 arg1, T2 arg2, void* functionPtr)

It would effectively have the same signature as however C# would expose it if it had an intrinsic for it.

@tannergooding
Copy link
Member

But yes, for the relatively few keywords C# doesn't support, I would personally prefer language features (Static Delegates, rather than calli and ldftn, for example).

And for il intrinsics in general, I feel like a managed API that has runtime support would be better suited.

We can currently emit IL dynamically, but it is expensive and (AFAIK) can't readily be used inline with existing methods. Having an API where the JIT sees an API call and interprets it as the equivalent IL instruction should be reasonably fast, output the same code, and work on all languages. (it would just require justification as to why doing so is worthwhile)

@HaloFour
Copy link
Contributor

HaloFour commented Dec 8, 2017

@tannergooding

Having an API where the JIT sees an API call and interprets it as the equivalent IL instruction should be reasonably fast

I assumed that the interpreting and replacement would be done by the compiler? Why put the onus onto the JIT to identify, validate and replace IL intrinsics?

@tannergooding
Copy link
Member

In the same vein, why put any burden on the compiler to implement this when the corresponding language proposals to expose the functionality would likely be better.

I think this would be better as a runtime feature because:

  • C# is not (strictly speaking) tied to the CLR and there aren't really any features that don't translate to non-il based runtimes. IL intrinsics would be highly CLR specific
  • The functionality is useful for essentially any language targeting the CLR. It can be used for interop with other languages or for expressing code patterns not currently expressible in the language
    • There isn't a good story for writing a type partially in a high level language and partially in IL (such as if you only want a single method writing in IL)
  • I'm not sure this qualifies as a language feature and wonder if it would actually make it into the spec or if it would be a compiler implementation detail
    • If it was the latter, then I can't necessarily take my C# code files and compile it on another compiler that supports C# v8.0 (or whatever language version the Roslyn compiler would ship with when this functionality shipped)

That being said, I think there are better investments for the runtime as well and that this wouldn't be very high on the priority list (if it wasn't rejected right away anyways)

@HaloFour
Copy link
Contributor

HaloFour commented Dec 8, 2017

@tannergooding

Maybe. I think it depends a great deal on the feature, especially since unless the API is shaped very specifically following CLS it would be up in the air as to whether or not other CLR-based languages could exploit it anyway. And even then it might not feel natural in those languages. I think that approach makes significantly more sense when dealing with instrinsics specific to machine language for specific architectures and less with IL. As a compiler feature it skips an unnecessary step and could have better integration and validation within the language.

@MichalStrehovsky
Copy link
Member

C# already has several of these "IL intrinsics" that are not part of the official language, but are there for people who need them for advanced scenarios. Think __arglist, __makeref, and friends. The proposal is not much different from __addrof, __call, __stdcall, etc. (an unadvertised feature for power scenarios that is not expected to be used by 99% of developers).

We keep coming back to the "let's make this a JIT feature instead", but that would require JIT to pattern match delegate creation sequence with the intrinsic (for the addrof scenario) and those things are not great (e.g. it took several attempts to get RuntimeHelpers.InitializeArray an intrinsic that would be guaranteed to expand because it goes against the single pass nature of the IL importer). I don't see why JIT would have to concern itself with it, especially since there's perfectly fine pure IL representation for the concept. Also, as a JIT intrinsic, it would have to be implemented by half a dozen codegen backends we have in .NET (CLR JIT, .NET Native UTC, Mono's JIT, Mono's LLVM backend, and experimental CppCodegen and WebAssembly backends in CoreRT).

@tmat
Copy link
Member

tmat commented Dec 10, 2017

Agreed. We have discussed all the options and their pros and cons presented on this thread already at the said LDM and decided that the proposal dotnet/roslyn#11475 is the optimal in terms of complexity (low) and amount of impact (no impact) on the rest of the stack (IDE, analyzers, tools, JIT, runtime) etc. Any feature that requires runtime changes will take very long time to get implemented in all relevant runtimes -- Desktop .NET, Core CLR, Mono, LLVM, etc. and it will take even longer for a significant amount of customers to upgrade to the new versions of these runtimes. Besides not all the intrinsics are covered by proper language/runtime features like static delegates so the tools that need these intrinsics will need them even if static delegates are implemented everywhere.

@fschmied
Copy link

  1. What's the status of this issue? https://github.com/dotnet/roslyn/blob/master/docs/Language%20Feature%20Status.md mentions that it has been merged into master, but this issue says it's only a proposal, not yet discussed or decided in LDM?
  2. Assuming the proposal has been accepted, isn't handleof, when combined with MethodBase.GetMethodFromHandle, Type.GetTypeFromHandle, and FieldInfo.GetFieldFromHandle, effectively the infoof (aka memberof) operator? (See also Proposal: memberof operator keyword #712, Add memberof operator  roslyn#1653, infoof / propertyof / methodof (and reflection objects in attributes?) roslyn#128.) I'm surprised, but not unhappy, that (judging from the LDM notes linked in the OP) this was accepted without all the discussion that usually goes along with infoof.

@weltkante
Copy link

your link (1) only says the function pointer aspect has been merged (that would mean calli and ldftn are no longer needed as "compiler intrinsics" since they now have other low level representations)

I think there were other discussions for function pointers, maybe linking here was just a mistake

@Unknown6656
Copy link
Contributor

The following code defintily works:

public unsafe static class Program
{
    static delegate*<int> pointer = &Main;
    
    public static int Main() {
        return 0;
    }
}

and it correctly outputs:

.class public auto ansi abstract sealed beforefieldinit Program extends [System.Private.CoreLib]System.Object
{
    .field private static method int32 *() pointer

    .method public hidebysig static int32 Main () cil managed 
    {
        .maxstack 8
        ldc.i4.0
        ret
    }

    .method private hidebysig specialname rtspecialname static void .cctor () cil managed
    {
        .maxstack 8
        ldftn int32 Program::Main()
        stsfld method int32 *() Program::pointer
        ret
    }
}

https://sharplab.io/#v2:EYLgtghgzgLgpgJwDQxASwDYB8ACBmAAgFcA7KCAMzgJwEYA2GgJhtoHYBYAKAG9uCBrRgBM4GOAHMI8AFQAeNCRgA+AgAcA9ovgICAXgIAyALIRFAbn6CrA/EILaCpxQAoAlAT5dBPmmwIADJbeggC+3KFAA===

@weltkante
Copy link

The following code defintily works:

@Unknown6656 yes but that wasn't the question, nor is that what was described in this proposal. There were other proposals for function pointers that got implemented as you described them.

net effect:

  • this proposal no longer needs calli/ldftn (or can be updated to match what was implemented)
  • this proposal requests low level access to ldtoken which is not covered by function pointers, so this proposal is not redundant or completed yet
  • I don't know if ldvirtftn (part of this proposal) is covered by function pointers

@Unknown6656
Copy link
Contributor

Fair point, I only mentioned it for completeness sake - not really as an answering comment.

@jaredpar
Copy link
Member

What's the status of this issue?

The feature function pointers is now merged into the main branch but remains under preview. The plan is to ship it with C# 9. This issue is separate from function pointers but has significant overlap. My expectation is that when we ship function pointers we will resolve this issue at the same time we close down function pointers.

There are a few items, like tokenof, that won't be implemented. At the point function pointers ships though if there is still significant need for them we can track them as a separate issue.

@333fred
Copy link
Member

333fred commented Jun 16, 2020

  • I don't know if ldvirtftn (part of this proposal) is covered by function pointers

It is not.

@newtype-cybernetics
Copy link

Please help me get this to the right place.

I am experiencing a bug with function pointers.

Steps to repro:
Create console app. Switch build to .net 5 and enable unsafe code. Replace contents of Program.cs with the following:

public struct Host { Extractor<Host> uhoh; }
public unsafe struct Extractor<T> { delegate*<ref T, ref int> extract; }
class Program { static void Main( string[] args ) { Host h; } }

Run.

Result:
An unhandled exception of type 'System.TypeLoadException' occurred in Unknown Module.
Could not load type 'bug.Host' from assembly 'bug, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null'.

I am guessing this has something to do with Extractor uhoh being a member of Host, and also using Host as an argument for it's function pointer?

Love the new feature otherwise :D. Is full documentation in the works?

@333fred
Copy link
Member

333fred commented Jan 27, 2021

I don't believe that has anything to do with function pointers, it looks like dotnet/runtime#6924.

@newtype-cybernetics
Copy link

newtype-cybernetics commented Jan 27, 2021

@333fred It's very likely that that is my bug. Thank you.

@dellamonica
Copy link

In my code I have something like this in a very hot path:

interface IFoo { int DoSomething(args); }

class FooGroup
{
     private readonly ImmutableArray<IFoo> _foos;

     public int HotMethod(args)
     {
          int local = 0;
          foreach (var foo in _foos)
          {
              local += foo.DoSomething(args)
          }
          return local
     }
}

The IFoo's are all distinct types, so I guess the JIT can't do much with respect to guarded devirtualization

Every time HotMethod gets called, callvirt is used to invoke each of the DoSomething's implementations. If the function pointer could be assigned an instance method, I could then cache those pointers in FooGroup's constructor and HotMethod would use them instead of a virtual call.

Is there any plans to support this (function pointer to instance/interface methods)?

@333fred
Copy link
Member

333fred commented Nov 12, 2021

What's the status of this issue?

The feature function pointers is now merged into the main branch but remains under preview. The plan is to ship it with C# 9. This issue is separate from function pointers but has significant overlap. My expectation is that when we ship function pointers we will resolve this issue at the same time we close down function pointers.

There are a few items, like tokenof, that won't be implemented. At the point function pointers ships though if there is still significant need for them we can track them as a separate issue.

As Jared indicated, I'm closing this out.

@333fred 333fred closed this as completed Nov 12, 2021
@333fred 333fred modified the milestones: Backlog, Likely Never Nov 12, 2021
@fandrei
Copy link

fandrei commented Nov 12, 2021

@333fred function pointers are only a small subset of possible use cases proposed here.

@333fred
Copy link
Member

333fred commented Nov 12, 2021

Yes, I'm aware. As Jared said, if there is significant need we can track those as separate issues/discussions. As of right now, there is no LDM champion for future intrinsics work.

@jaredpar
Copy link
Member

I personally viewed the other aspects of this proposal as add-ons. The primary motivation of the proposal was function pointers and the suggestion was il-intrinsics as a solution. If we chose to go that route then it made sense to consider a few other intrinsics as they were essentially incremental cost that point. The team would already be doing the heavy lifting to expose a few of the instructions, it made sense to take a bigger view and grab the other missing pieces. I didn't view them as standing on their own here.

I'm more than happy to help review a proposal for the work that didn't get done here and the motivations for adding them in. But I do think it should be tracked as a new issue.

@jerviscui
Copy link

@svick

Specifically, how do you imagine the API for calli could look like?

Like this: Static Delegates

Seems the point is that rather than providing awkward facilities to access the specific IL opcodes directly that the use cases for those opcodes would be determined and proper language features offered instead. That largely makes sense, although there will likely always be weird edge cases that couldn't be handled. Not to mention the burden would be on the team to identify those cases and design APIs around them.

Static Delegates link has changed.
Static Delegates superseded by function-pointers.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests