ComPtr<T> improvements, new __uuidof<T> macro

[Sergio0694]

This PR includes a few tweaks and improvements to the ComPtr<T> type, and a port of the __uuidof<T> macro.

🆕 Added a port of the __uuidof() macro, as a new static __uuidof<T>() API in the Windows class. This makes it easy to grab a Guid* value for a given type T, and similarly to the original macro it also can be used to assign a Guid value directly to a local variable. This works through a special UuidOfType type that wraps a Guid* pointer and adds implicit conversions when needed. This also indirectly speeds up a couple of ComPtr<T> APIs as the reflection done in typeof(T).GUID is no longer needed. Tier 1 JIT should also be able to completely replace the code with just a hardcoded address, but even without that it should be better than typeof(T).GUID every time. Also, this new API simplifies a lot having to grab a rIID value to use in various APIs.

🆕 Also added some utility As and CopyTo overloads.

Diff for the API surface:

 namespace TerraFX.Interop
 {
    public static class Windows
    {
+        public static unsafe UuidOfType __uuidof<T>() where T : unmanaged;

+        public readonly unsafe ref struct UuidOfType
+        {
+            public static implicit operator Guid(UuidOfType guid);
+            public static implicit operator Guid*(UuidOfType guid);
+        }
    }

     public unsafe struct ComPtr<T> : IDisposable
         where T : unmanaged
     {
         public ComPtr(T* other);
         public ComPtr(ComPtr<T> other);

         public static implicit operator ComPtr<T>(T* other);
         public static implicit operator T*(ComPtr<T> other);

         public readonly int As<U>(ComPtr<U>* p) where U : unmanaged;
+        public readonly int As<U>(ref ComPtr<U> other) where U : unmanaged;
         public readonly int AsIID(Guid* riid, ComPtr<IUnknown>* other);
+        public readonly int AsIID(Guid* riid, ref ComPtr<IUnknown> other);
         public void Attach(T* other);
         public T* Detach();
         public readonly int CopyTo(T** ptr);
+        public readonly int CopyTo(ComPtr<T>* p);
+        public readonly int CopyTo(ref ComPtr<T> other);
         public readonly int CopyTo<U>(U** ptr) where U : unmanaged;
+        public readonly int CopyTo<U>(ComPtr<U>* p) where U : unmanaged;
+        public readonly int CopyTo<U>(ref ComPtr<U> other) where U : unmanaged;
         public readonly int CopyTo(Guid* riid, void** ptr);
+        public readonly int CopyTo(Guid* riid, ComPtr<IUnknown>* p);
+        public readonly int CopyTo(Guid* riid, ref ComPtr<IUnknown> other);
         public void Dispose();
         public readonly T* Get();
         public readonly T** GetAddressOf();
         public readonly ref T* GetPinnableReference();
         public T** ReleaseAndGetAddressOf();
         public uint Reset();
+        public void Swap(ComPtr<T>* r);
         public void Swap(ref ComPtr<T> other);
     }
 }

🚀 Improved codegen to GetAddressOf and ReleaseAndGetAddressOf. The code was previously using a fixed block, which resulted in poor codegen as the JIT isn't currently able to optimize that away. Fixed that using Unsafe APIs:

; Old codegen
ComPtr`1[[System.Guid, System.Private.CoreLib]].GetAddressOf_Old()
    L0000: push rax
    L0001: xor eax, eax
    L0003: mov [rsp], rax
    L0007: mov [rsp], rcx
    L000b: mov rax, rcx
    L000e: add rsp, 8
    L0012: ret

; New codegen
ComPtr`1[[System.Guid, System.Private.CoreLib]].GetAddressOf_New()
    L0000: mov rax, rcx
    L0003: ret

📖 Minor code tweaks, added complete XML docs to the type

[CLAassistant]

All committers have signed the CLA.

[tannergooding]

What does the codegen look like for this and the other implicit operator?

[Sergio0694]

I'd say it looks quite nice 😄
Plus there's the fact that the tiered JIT could even elide the static constructor check entirely.
But other than this there's basically just the call to the static constructor (which we would've had anyway since Windows has statically initialized fields as well), and then all the rest is completely elided by the JIT.

Guid* converter (click to expand):

public static unsafe Guid* GetAddr()
{
    return __uuidof<ID3D12SomeType>();
}

sub      rsp, 40
mov      rcx, 0xD1FFAB1E
xor      edx, edx
call     CORINFO_HELP_CLASSINIT_SHARED_DYNAMICCLASS
mov      rax, 0xD1FFAB1E
mov      rax, qword ptr [rax]
add      rsp, 40
ret

Guid converter (click to expand):

public static unsafe Guid GetGuid()
{
    return Windows.__uuidof<ID3D12Test>();
}

push     rsi
sub      rsp, 32
vzeroupper 
mov      rsi, rcx
mov      rcx, 0xD1FFAB1E
xor      edx, edx
call     CORINFO_HELP_CLASSINIT_SHARED_DYNAMICCLASS
mov      rax, 0xD1FFAB1E
mov      rax, qword ptr [rax]
vmovdqu  xmm0, xmmword ptr [rax]
vmovdqu  xmmword ptr [rsi], xmm0
mov      rax, rsi
add      rsp, 32
pop      rsi
ret  

In particular for this as we have a direct comparison, the codegen is slightly smaller than just suing static GUIDs from the Windows class, but of course this approach is still much easier to use and less error prone.

Accessing Windows.IID_Foo (click to expand):

public static unsafe Guid GetGuid()
{
    return Windows.IID_Foo;
}

push     rsi
sub      rsp, 32
vzeroupper 
mov      rsi, rcx
mov      rcx, 0xD1FFAB1E
mov      edx, 5
call     CORINFO_HELP_GETSHARED_NONGCSTATIC_BASE
mov      rax, 0xD1FFAB1E
mov      rax, gword ptr [rax]
vmovdqu  xmm0, xmmword ptr [rax+8]
vmovdqu  xmmword ptr [rsi], xmm0
mov      rax, rsi
add      rsp, 32
pop      rsi
ret   

Considering that here we also have the prologue/epilogue that'd just go away once this method is just inlined 🚀

[tannergooding]

I had only called out the doc comment syntax mismatch on the type, but it applies to all added doc comments.

[Sergio0694]

Ah, right. Fixed in b358ae9 🙂

[tannergooding]

We shouldn't need AggressiveInlining on these. They are extremely small and the JIT should handle it already.

[Sergio0694]

Removed in 65b7f00.

[tannergooding]

Unnecessary newline was inserted here and a few other places in the file.

Suggested change

[Sergio0694]

But they looked so nice 😛
Fixed in b2a4be6!

[tannergooding]

The pin could be avoided here by using a local and then internally assigning other.ptr_.

[tannergooding]

Same with other cases like https://github.com/terrafx/terrafx.interop.windows/pull/151/files#diff-a1a828334f363e63b93afdd4a19f4f6ca865a85ceb9411137ecc4073be026014R122

[Sergio0694]

Same as below, removed in e10f572.

[tannergooding]

This pin could also be elided with the right helper. The address isn't actually being used in way that requires it to be fixed, so releasing the value and assigning the field directly is probably better.

[Sergio0694]

Removed all extra fixed blocks in e10f572 🚀

[tannergooding]

Why was this changed from using InternalRelease()?

[Sergio0694]

Here we don't return the ref count so thought doing this manually and simplified could help the JIT with inlining?

[tannergooding]

The names here were matching the ones used in native code. It would be good to keep them where possible. Same for other places as well where changes have been made to the code.

The patterns and names were kept to help do visual validation and ease future ports.

[Sergio0694]

I see, that makes perfect sense 😄
Reverted those changes in 3f93d6e.