Regenerate LLVMSharp from the 6.0.0 sources, based on the ClangSharp rewrite #93

tannergooding · 2019-04-26T15:22:10Z

It looks like LLVMSharp is still on 5.0.0, so I've regenerated the sources using the ClangSharp rewrite (dotnet/ClangSharp#34) to help validate things.

As per the original PR, the notable changes are:

The visitation logic now considers Location.IsInMainFile, this ultimately means the order is slightly different and that we don't visit #includes. This changes the output order slightly but also means we don't need to keep track of already visited nodes.
We return uint for uint rather than int
We no longer prefix @ to everything, and instead only prefix it to reserved C# keywords
We better follow typedefs and resolve the correct type to output. For example, we will now write out public CXCompletionString CompletionString rather than public IntPtr @CompletionString. The latter (IntPtr) will still be done if there is a double indirection.
We no longer explicitly declarate an enum as : int, since that is the default
We output IntPtr rather than ulong for size_t, to make the code produced more portable

This should also help minimize diffs when moving to 8.0.0

tannergooding · 2019-04-26T15:22:42Z

(still reviewing the changes myself to make sure nothing surprising changed since the ordering makes diffing difficult)

src/Generated.cs

tannergooding · 2019-04-26T15:28:42Z

src/Generated.cs

    }

-    public enum lto_symbol_attributes : int
+    public enum (anonymous enum at E:\Users\tagoo\Repos\llvm\include/llvm-c/Core.h:342:1)


Need to handle anonymous top level declarations...

Looks like the original binding generator code has some logic to try (ForwardDeclarationVisitor) and find the typedef in this scenario. In the rewrite, that logic was dropped in favor of just getting the associated typedef directly from clang...

However, this enum in particular is setup like:

enum { LLVMAttributeReturnIndex = 0U, LLVMAttributeFunctionIndex = -1, }; typedef unsigned LLVMAttributeIndex;

So the ForwardDeclarationVisitor would find it, but not the rewrite. However, I also don't think the original code was doing the write thing for more general usage, since there is no guarantee it isn't a truly anonymous enum and that the next typedef is actually related...

That's correct. This is better, although when I did this it was for LLVMSharp. In any case, this is better but how do you resolve it?

For the general use case, I wouldn't expect this is that common (truly anonymous top level declarations) so it is possibly one of those things worth manually handling..

I dont think there will ever be a way to do this reliably and if it isnt common, it might not be worth the having a switch to support it (but having a switch would be a good option if it was common).

I've done a manual fixup as part of the PR for now.

src/Generated.cs

mjsabby · 2019-04-26T15:44:54Z

I think llvm 6 but it may be mostly identical.

tannergooding · 2019-04-28T16:57:31Z

Fixed a few issues and updated to use LLVM 6

tannergooding · 2019-04-28T17:51:12Z

src/LLVMSharp.csproj

    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+    <TargetFramework>netstandard2.0</TargetFramework>


This is required to support the custom UTF8 string marshaler and puts it at the same target as ClangSharp currently has.

mjsabby · 2019-04-28T23:07:09Z

@tannergooding did you see the build failures?

tannergooding · 2019-04-29T14:03:46Z

Yes. There are some fixups still required in the wrappers. Those will take a bit longer...

…rewrite

tannergooding · 2019-04-29T18:11:06Z

I think it should be all resolved now. All tests are passing locally.

tannergooding · 2019-04-29T18:17:49Z

dotnet/ClangSharp#40 contains the last of the changes that were made when generating this locally.

tannergooding · 2019-04-29T18:18:23Z

src/Api/DisasmContext.cs


    public sealed class DisasmContext : IDisposableWrapper<LLVMDisasmContextRef>, IDisposable
    {
        LLVMDisasmContextRef IWrapper<LLVMDisasmContextRef>.ToHandleType => this._instance;
        void IDisposableWrapper<LLVMDisasmContextRef>.MakeHandleOwner() => this._owner = true;
-
-        private class SymbolLookupClosure


Delegates are generated such that these wrappers are no longer needed.

tannergooding · 2019-04-29T18:18:51Z

src/Api/DisasmContext.cs

@@ -111,16 +85,10 @@ private void Dispose(bool disposing)

        public unsafe Tuple<string, int> DisasmInstruction(byte[] instructionBytes, ulong programCounter)
        {
-            fixed(byte* iptr = &instructionBytes[0])


Method signatures are largely generated so explicit fixing isn't needed.

tannergooding · 2019-04-29T18:19:21Z

src/Api/ExecutionEngine.cs


        public unsafe static ExecutionEngine CreateMCJITCompiler(Module module, int optionsSize)
        {
            LLVMMCJITCompilerOptions options;
-            var size = new size_t(new IntPtr(optionsSize));


No more size_t, since the new generator knows to handle it directly as IntPtr

tannergooding · 2019-04-29T18:20:15Z

src/Api/ExecutionEngine.cs

-        public int RunAsMain(Function f, uint argC, string[] argV, string[] envP) => LLVM.RunFunctionAsMain(this.Unwrap(), f.Unwrap(), argC, argV, envP);
+        public int RunAsMain(Function f, uint argC, string[] argV, string[] envP)
+        {
+            var marshaledArgv = StringMarshaler.MarshalArray(argV);


These weren't being marshaled as LPStr before, so it's possible they were only handling one character...

They are now each marshaled and through the custom marshaler that handles UTF8 data.

tannergooding · 2019-04-29T18:23:26Z

src/Generated.Custom.cs

+        public static extern IntPtr DisasmInstruction(LLVMDisasmContextRef DC, out byte Bytes, ulong BytesSize, ulong PC, out byte OutString, IntPtr OutStringSize);
+
+        [DllImport(libraryPath, EntryPoint = "LLVMGetBufferStart", CallingConvention = CallingConvention.Cdecl)]
+        public static extern IntPtr GetBufferStart(LLVMMemoryBufferRef MemBuf);


A "downside" to the new generator is a few more methods need to be custom handled. This is because LLVM works with "string handles" for most of the inputs and marshaling looks to break that in a few places.

Relying on purely unsafe code would "resolve" that, but would require more explicit work in the wrapper types. That might be beneficial if the wrappers dealt in terms of ROSpan<byte>, but that would place a higher minimum TFM for the library.

tannergooding · 2019-04-29T19:03:53Z

I don't have permissions to merge on this repo.

mjsabby · 2019-04-29T19:29:00Z

You should now.

tannergooding commented Apr 26, 2019

View reviewed changes

src/Generated.cs Outdated Show resolved Hide resolved

tannergooding commented Apr 26, 2019

View reviewed changes

src/Generated.cs Outdated Show resolved Hide resolved

tannergooding commented Apr 26, 2019

View reviewed changes

src/Generated.cs Outdated Show resolved Hide resolved

tannergooding commented Apr 26, 2019

View reviewed changes

src/Generated.cs Outdated Show resolved Hide resolved

tannergooding mentioned this pull request Apr 26, 2019

Adding some additional fixups allow LLVMSharp to be regenerated dotnet/ClangSharp#37

Merged

tannergooding force-pushed the regenerate branch from bec6742 to 5e68675 Compare April 28, 2019 16:56

tannergooding force-pushed the regenerate branch from 5e68675 to e2e2dd3 Compare April 28, 2019 17:44

tannergooding changed the title ~~Regenerate LLVMSharp from the 5.0.0 sources, based on the ClangSharp rewrite~~ Regenerate LLVMSharp from the 6.0.0 sources, based on the ClangSharp rewrite Apr 28, 2019

tannergooding commented Apr 28, 2019

View reviewed changes

tannergooding added 2 commits April 29, 2019 11:07

Regenerate LLVMSharp from the 6.0.0 sources, based on the ClangSharp …

cb14af4

…rewrite

Manual fixups to the generated LLVMSharp code

92c0ca5

tannergooding force-pushed the regenerate branch from e2e2dd3 to 92c0ca5 Compare April 29, 2019 18:10

Rerun the generator excluding LLVMInitializeCore

f86f9cb

tannergooding mentioned this pull request Apr 29, 2019

More improvements to the P/Invoke Binding Generator dotnet/ClangSharp#40

Merged

mjsabby approved these changes Apr 29, 2019

View reviewed changes

tannergooding commented Apr 29, 2019

View reviewed changes

tannergooding merged commit d9fd646 into dotnet:master Apr 29, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Regenerate LLVMSharp from the 6.0.0 sources, based on the ClangSharp rewrite #93

Regenerate LLVMSharp from the 6.0.0 sources, based on the ClangSharp rewrite #93

tannergooding commented Apr 26, 2019

tannergooding commented Apr 26, 2019

tannergooding Apr 26, 2019

tannergooding Apr 26, 2019

mjsabby Apr 26, 2019

tannergooding Apr 26, 2019

tannergooding Apr 28, 2019

mjsabby commented Apr 26, 2019

tannergooding commented Apr 28, 2019

tannergooding Apr 28, 2019

mjsabby commented Apr 28, 2019

tannergooding commented Apr 29, 2019

tannergooding commented Apr 29, 2019

tannergooding commented Apr 29, 2019

tannergooding Apr 29, 2019

tannergooding Apr 29, 2019

tannergooding Apr 29, 2019

tannergooding Apr 29, 2019

tannergooding Apr 29, 2019

tannergooding commented Apr 29, 2019

mjsabby commented Apr 29, 2019

		<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
		<TargetFramework>netstandard2.0</TargetFramework>

Regenerate LLVMSharp from the 6.0.0 sources, based on the ClangSharp rewrite #93

Regenerate LLVMSharp from the 6.0.0 sources, based on the ClangSharp rewrite #93

Conversation

tannergooding commented Apr 26, 2019

tannergooding commented Apr 26, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjsabby commented Apr 26, 2019

tannergooding commented Apr 28, 2019

Choose a reason for hiding this comment

mjsabby commented Apr 28, 2019

tannergooding commented Apr 29, 2019

tannergooding commented Apr 29, 2019

tannergooding commented Apr 29, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tannergooding commented Apr 29, 2019

mjsabby commented Apr 29, 2019