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

Upgrade wrapper generator #226

Merged
merged 6 commits into from
May 6, 2021
Merged

Upgrade wrapper generator #226

merged 6 commits into from
May 6, 2021

Conversation

Gnimuc
Copy link
Contributor

@Gnimuc Gnimuc commented Mar 2, 2021

I'm testing JuliaInterop/Clang.jl#278. Before making a new release for Clang.jl, I'd like to make sure that the code generated by the new generator is not broken.

I got the following test errors with this PR, any clue would be highly appreciated.

UPDATE: looks we're all good now.

@vchuravy
Copy link
Collaborator

vchuravy commented Mar 2, 2021

ERROR: LoadError: LoadError: LoadError: ArgumentError: invalid type expression for Cenum #= /home/runner/work/LLVM.jl/LLVM.jl/lib/libLLVM_h.jl:424 =# @var_str("##Ctag#408")::Int32

lib/libLLVM_h.jl Outdated Show resolved Hide resolved
lib/libLLVM_h.jl Outdated Show resolved Hide resolved
@Gnimuc Gnimuc changed the title WIP: Upgrade wrapper generator Upgrade wrapper generator Mar 2, 2021
@maleadt
Copy link
Owner

maleadt commented Mar 2, 2021

Thanks! Let's maybe wait to merge this until we can record something definite in the Manifest (i.e. a commit on the master branch of Clang.jl).

@Gnimuc
Copy link
Contributor Author

Gnimuc commented Mar 2, 2021

Too bad this now still requires a manual clean-up step; doesn't it make sense for CEnum.jl to support some type of anonymous enum?

@maleadt In Julia 1.3+, this manual clean-up step is actually not mandatory because those anonymous enum "names" should never be used anywhere.

The new generator uses Julia's gensym for generating "anonymous names", which guarantees there is no name collision. The generated names will fluctuate between invocations, which forces end-users to not use these names in their code.

For Julia 1.0-1.2, maybe it's a good idea to add an optional pass to rewrite those var"##..." names to common symbols like __ANONYMOUS_ENUM_NUM.

The new generator is designed to generate native Julia code without any magic, so for those Julia users who do not have decent knowledge about C, the generator can give a simple answer. For example, if a user doesn't know how to translate the following C typedef to a Julia type:

shell> cat c.h
typedef char (*(*x[3])())[5];

The generator just tells:

julia> using Clang.Generators
julia> ctx = create_context("c.h");
julia> build!(ctx);
[ Info: [CollectTopLevelNode]: processing header: c.h
[ Info: Building the DAG...
[ Info: Emit Julia expressions...
# C code:
# typedef char ( * ( * x [ 3 ] ) ( ) ) [ 5 ]
const x = NTuple{3, Ptr{Cvoid}}

[ Info: Done!

x is a 3-element NTuple of pointers to some C opaque types. The result clearly shows C's constant array is mapped to Julia's NTuple and as there is no corresponding Julia construct that can be mapped to C's function proto, the generator just yields a very vague result Ptr{Cvoid} as a workaround.

As of C's anonymous tag-types, there is also no corresponding Julia construct. It's inevitable to do de-anonymizing on the Julia side. I think making CEnum.jl support anonymous enums just hides too many details. That's why I added an option to completely remove the use of CEnum.jl.

In theory, it's easy for the generator to generate C-like wrapper code by using CBinding.jl, but there is already a CBindingGen.jl for that. CBinding.jl is very good at emulating C-syntaxes by featuring Julia's cool macro system, it's highly recommended to those users who like magics and don't wanna know the things under the hood. I guess the only drawback is that without a solid knowledge of both Julia and C, if something goes wrong, users are unlikely to make a fix by themselves.

Not to mention Cxx.jl, one of the most magic packages in the Julia world, lets users write pure C/C++ and do Julia-C/C++ cross optimizations at the LLVM level. It's the easiest one to use but the hardest one to maintain. I'm glad we have all these 3 packages in the Julia ecosystem for Julia/C interop to cover different use cases in the real world.

@Gnimuc
Copy link
Contributor Author

Gnimuc commented Apr 23, 2021

Looks like I need an approval for running tests on the CI.

The script has been simplified even a little bit more with JuliaInterop/Clang.jl#289.

However, there is one thing that concerns me. The symbol off_t is manually defined as Csize_t in the patch file, but the actual definition should depend on the platform.

(to do the test locally, you need to comment @add_def off_t and use Clang.jl PR289)

  1. args = get_default_args("x86_64-apple-darwin14") gives
const __darwin_off_t = Int64

const off_t = __darwin_off_t
  1. args = get_default_args("x86_64-linux-gnu") & args = get_default_args("i686-linux-gnu") gives
const __off_t = Clong

const off_t = __off_t
  1. args = get_default_args("x86_64-w64-mingw32") & args = get_default_args("i686-w64-mingw32") gives
const off32_t = Clong

const off_t = off32_t

I didn't test other platforms, but I guess their definition for off_t could also be different.

cc @vchuravy, @maleadt

@maleadt
Copy link
Owner

maleadt commented Apr 23, 2021

Nice progress! I approved CI. A correct off_t would be better, but it's currently only needed for an API call we don't use, so CI won't spot a difference.

@Gnimuc
Copy link
Contributor Author

Gnimuc commented May 2, 2021

@maleadt I manually defined off_t for different platforms for Julia 1.6 in the latest commit. Now, this PR is ready to merge.

BTW, I've no plan to tag a new version of Clang.jl before the generator scripts in all Clang.jl's downstream projects have been upgraded. So don't forget to checkout Clang.jl#master to run the script.

@maleadt
Copy link
Owner

maleadt commented May 6, 2021

Great, thanks! Looking forward to using this with CUDA.jl. I've already migrated oneAPI.jl, https://github.com/JuliaGPU/oneAPI.jl/tree/master/res, too.

@maleadt maleadt merged commit 9583c67 into maleadt:master May 6, 2021
@Gnimuc Gnimuc deleted the up-generator branch May 6, 2021 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants