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

RFC: Make struct passing work properly #3466

Closed
wants to merge 17 commits into from
Closed

RFC: Make struct passing work properly #3466

wants to merge 17 commits into from

Conversation

Keno
Copy link
Member

@Keno Keno commented Jun 20, 2013

This is a WIP on the long standing issue of passing Julia structs to C. I have implemented the x86_64 ABI and am working on the others. This is passing the tests included in Jameson's original pull request, though as the ABI is rather complicated, I would appreciate more tests to include.

@Keno
Copy link
Member Author

Keno commented Jun 21, 2013

Now with 100% more ABI compliant cfunctions.
To keep track of issues (from comments) lost during the rebase:

  • Need to add link to LDC
  • Need to figure out what to do with fixed-size arrays.

@timholy
Copy link
Member

timholy commented Jun 21, 2013

Amazing!

@ViralBShah
Copy link
Member

As always - awesome work @loladiro.

@Keno
Copy link
Member Author

Keno commented Jun 23, 2013

two down, one to go.

@timholy
Copy link
Member

timholy commented Jun 23, 2013

That's a ton of great work!

@timholy
Copy link
Member

timholy commented Jun 23, 2013

With regards to tests, do you mean "more examples of structs to pass to C libraries"? If it would help I can cull things from HDF5 (although do you want dependencies like that?).

@quinnj
Copy link
Member

quinnj commented Jun 24, 2013

There are also some simple structs in ODBC that might be good for testing. I've been tracking this issue to implement these structs in my package anyway.

 typedef struct tagDATE_STRUCT {
    SQLSMALLINT year;
    SQLUSMALLINT month;
    SQLUSMALLINT day;
  } DATE_STRUCT;

  typedef struct tagTIME_STRUCT {
    SQLUSMALLINT hour;
    SQLUSMALLINT minute;
    SQLUSMALLINT second;
  } TIME_STRUCT;

  typedef struct tagTIMESTAMP_STRUCT {
    SQLSMALLINT year;
    SQLUSMALLINT month;
    SQLUSMALLINT day;
    SQLUSMALLINT hour;
    SQLUSMALLINT minute;
    SQLUSMALLINT second;
    SQLUINTEGER fraction;
  } TIMESTAMP_STRUCT;

@Keno
Copy link
Member Author

Keno commented Jun 24, 2013

More than the actual structs, I need examples of function signatures, where structs are being passed by value.

@quinnj
Copy link
Member

quinnj commented Jun 24, 2013

Hmmm...yeah, ODBC wouldn't help much here.

@IainNZ
Copy link
Member

IainNZ commented Jun 24, 2013

SDL.jl?

@Keno
Copy link
Member Author

Keno commented Jun 25, 2013

I tried implementing win64, but LLVM seems to be somewhat broken in this case.

@Keno
Copy link
Member Author

Keno commented Jun 27, 2013

Done! :)

@IainNZ
Copy link
Member

IainNZ commented Jun 27, 2013

👍 Good stuff

@Keno
Copy link
Member Author

Keno commented Jun 28, 2013

Hmm interestingly this seems broken on Linux 64bit, but not on Mac 64bit (the parallel tests don't work). @JeffBezanson or @vtjnash are you aware of any ABI differences between Mac and Linux on 64bit?

Also adjust the tests for the fact that Complex is immutable
@vtjnash
Copy link
Member

vtjnash commented Jan 11, 2014

I was discussing that with Jeff offline. Using the C convention internally gets everything (cfunction with structs, and redistributable sys.so image) faster, then we can think about optimizing it more later.

@Keno
Copy link
Member Author

Keno commented Jan 11, 2014

Fine with me.

@JeffBezanson JeffBezanson modified the milestones: 0.3, 0.4 Feb 18, 2014
@Keno Keno modified the milestones: 0.4, 0.3 Mar 19, 2014
@StefanKarpinski
Copy link
Member

This completely fell through the cracks and should have been merged during the 0.3 cycle. See StackOverflow question about this: http://stackoverflow.com/questions/24870010/call-div-c-function-from-julia. I've encountered this occasionally myself – it would be really nice to be able to return structs.

@IainNZ
Copy link
Member

IainNZ commented Jul 21, 2014

0.3.1 milestone?

@StefanKarpinski
Copy link
Member

Maybe. It's technically a breaking change though.

@IainNZ
Copy link
Member

IainNZ commented Jul 21, 2014

Ah in that case, 0.4. I was thinking of it as a purely-feature-adding thing.

@kmsquire
Copy link
Member

I could also really use this. I've been working on wrapping libav/ffmpeg, which has a kajillion structs. My options seem to have been to pass pointers to immutables (which are then mutated by the C code), or use StrPack.

For now, I've gone with using immutables, as it has been the path of least resistance. But it feels rather sketchy to change immutable values (or even declare that a 100-member struct is immutable), so I'm looking forward to types having the right structure.

(I think I'll be able to publish a working version pretty soon.)

@vtjnash
Copy link
Member

vtjnash commented Jul 21, 2014

This is unrelated to immutables or to using pointers to types as structs

Once Keno finishes cleaning this up, or turns it over for someone else to finish, it could be merged.

@kmsquire
Copy link
Member

This is unrelated to immutables or to using pointers to types as structs

Okay, I misinterpreted. This is for passing structs directly, then?

@vtjnash
Copy link
Member

vtjnash commented Jul 22, 2014

Correct, this is for correcting the calling-convention of passing structs by value.

@vtjnash
Copy link
Member

vtjnash commented Jul 22, 2014

Passing pointers to types works the same (and was implemented before) immutable types.

@kmsquire
Copy link
Member

Passing pointers to types works the same (and was implemented before) immutable types.

Okay, thanks. I'm probably doing something wrong, then. Sorry for the noise, will post on julia-dev or a separate issue if I have further problems.

@zenna
Copy link

zenna commented Jan 14, 2015

What is the latest on this?

@vtjnash
Copy link
Member

vtjnash commented Jan 14, 2015

Updated pr in #7906

@vtjnash vtjnash closed this Jan 14, 2015
@tkelman tkelman deleted the kf/ccall3 branch March 22, 2016 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking This change will break code
Projects
None yet
Development

Successfully merging this pull request may close these issues.