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

struct passing bug(?) #2149

Closed
ihnorton opened this issue Jan 28, 2013 · 12 comments
Closed

struct passing bug(?) #2149

ihnorton opened this issue Jan 28, 2013 · 12 comments
Assignees

Comments

@ihnorton
Copy link
Member

@vtjnash knows what I am talking about from IRC. Struct return seems to be working, but struct passing is problematic.

debugging lines:
https://github.com/ihnorton/CIndex.jl/blob/purejl/lib/wrapcindex.cpp#L79-L89

debugging output:
https://gist.github.com/4653523#file-gist1-L4-L12

all of the hashes should match.

the branch with attempt to use pure julia structs is here:
https://github.com/ihnorton/CIndex.jl/tree/purejl

@vtjnash
Copy link
Member

vtjnash commented Jan 28, 2013

recording this for my reference:

%struct.CXCursor = type { i32, i32, [3 x i8*] }
%struct.CXCursor = type { i32, i32, i8*,i8*,i8* }
declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture, i8* nocapture, i64, i32, i1) nounwind
declare i32 @clang_hashCursor(%struct.CXCursor* byval align 8)
define void @test_cu2(%struct.CXCursor* byval align 8 %cu) uwtable {
  %1 = alloca %struct.CXCursor, align 8
  call void @llvm.dbg.declare(metadata !{%struct.CXCursor* %cu}, metadata !2262), !dbg !2263
  %2 = bitcast %struct.CXCursor* %1 to i8*, !dbg !2264
  %3 = bitcast %struct.CXCursor* %cu to i8*, !dbg !2264
  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %2, i8* %3, i64 32, i32 8, i1 false), !dbg !2264
  %4 = call i32 @clang_hashCursor(%struct.CXCursor* byval align 8 %1), !dbg !2264
  %5 = getelementptr inbounds %struct.CXCursor* %cu, i32 0, i32 0, !dbg !2264
  %6 = load i32* %5, align 4, !dbg !2264
  %7 = call i32 (i8*, ...)* @printf(i8* getelementptr inbounds ([25 x i8]* @.str3, i32 0, i32 0), i3
2 %4, i32 %6), !dbg !2264
  ret void, !dbg !2266
}

@ghost ghost assigned vtjnash Jan 28, 2013
@vtjnash vtjnash reopened this Jan 28, 2013
@vtjnash vtjnash reopened this Jan 29, 2013
@ihnorton
Copy link
Member Author

75f8321 fixes it.

julia> using cindex; tu = tu_init("Index.h")
Ptr{Void} @0x00000000032a1560

julia> cu = ccall( (:test_cu1, "../lib/libwrapcindex"), CXCursor, (Ptr{Void},), tu)
hash out: ef5bd483 kind out: 300
CXCursor(300,0,Ptr3(0x00000000032a1560000000000000000100000000032a01d0))

julia> ccall( (:test_cu2, "../lib/libwrapcindex"), Void, (CXCursor,), cu)
hash in: ef5bd483 kind in: 300

julia> cindex.hashCursor(cu)
0xef5bd483

julia> cindex.hashCursor(cu)
0xef5bd483

vtjnash added a commit that referenced this issue Jan 29, 2013
… struct return (disabled). unfortunately this breaks ComplexPair{Int} (ComplexPair{Float64} seems OK)""

This reverts commit 4fa9364.
vtjnash added a commit that referenced this issue Jan 29, 2013
ComplexPair{Int32} and similar small, integer structs are broken for
ccall pass-by-value (not a new problem)
@vtjnash
Copy link
Member

vtjnash commented Feb 2, 2013

sigh. apparently passing structs to functions is not actually supported by the LLVM StructType. Clang has about 20k lines of code apparently just for this purpose. I can try to put something simple together that works for 90% of cases, but I won't be sure if I've got everything, and it will only be for x86 and x86_64. In the long term, we may need to leverage more from clang (it has over 1.5k lines of code just for x86_64 calling convention rules themselves), but that will be really annoying too, since we'll have to rebuild all of the llvm integer and floating points types in clang's type system.

oddly, clang also distinguishes between complex float and struct { float re; float im; } when generating LLVM op-codes (although the x86 asm is the same)

@ihnorton
Copy link
Member Author

ihnorton commented Feb 3, 2013

Probably not useful for Julia, but the LuaJIT ffi implementation is really impressive (as is LuaJIT in general):
http://repo.or.cz/w/luajit-2.0.git/blob/master:/src/lj_ccall.c
The x86 rules are <100 lines, with no libffi anywhere. Maybe could be useful for building unit tests. Certainly easier than wading through what libffi and clang are doing.

(a few other notes about struct ffi approaches: https://github.com/ihnorton/notes/wiki/Notes-on-struct-passing )

@vtjnash
Copy link
Member

vtjnash commented Feb 3, 2013

those are some great references. it's worth noting that a struct of 2 floats != complex float on x86_64

this is probably the most possible to port: http://repo.or.cz/w/luajit-2.0.git/blob/master:/src/lj_ccall.c

for v0.1, I would like to ensure that passing complex numbers, structs larger than 2 pointers, and possibly structs of only integer types works on x86 and x86_64 linux. the remaining options should be an error (for now). then we can go from there.

@JeffBezanson
Copy link
Member

3 years and counting waiting for support for this: http://llvm.org/bugs/show_bug.cgi?id=4246

vtjnash added a commit that referenced this issue Feb 18, 2013
… struct return (disabled). unfortunately this breaks ComplexPair{Int} (ComplexPair{Float64} seems OK)""

This reverts commit 4fa9364.
@vtjnash
Copy link
Member

vtjnash commented Mar 18, 2013

well, the clang approach seems to have died in the waters. however, hope is not to be forsaken (not yet anyways)

The D language looks like it has this awesome llvm implementation which supports everything (of course it has to). What's better, its only a few thousands lines of C++ code -- unlike the attempt from clang -- and it looks reasonably intelligible (although there's a comment in the code claiming this part is a bit of a mess :).
https://github.com/ldc-developers/ldc/tree/master/gen

if anyone can point me to the Doxygen files or wants to try asking them to split that part off into a library (saving me the work of doing just that), it would be much appreciated!

The license for the relevant files is "Artistic license or the GPL version 2 or later." which seems OK, although perhaps not great? Definitely seems like an argument for making this a standalone library.

@pao
Copy link
Member

pao commented Mar 18, 2013

"Artistic license or the GPL version 2 or later."

Kind of ugly. Artistic is complicated and really only supposed to show up in conjunction with Perl.

@JeffBezanson
Copy link
Member

The comments in the files themselves refer mostly to the "BSD-style LDC license", which is fine.
This would be part of the core language, so needs to be BSD-flavor. The artistic is probably not acceptable, since it adds strange requirements about describing your changes and such.

@vtjnash
Copy link
Member

vtjnash commented Mar 18, 2013

Oops, Jeff is right, I was tired and looking at the wrong file's license. It is in fact "three-clause BSD" LDC license.

Copyright (c) 2007-2012 LDC Team.
All rights reserved.

Redistribution and use in source and binary forms, with or without
modification, are permitted provided that the following conditions are met:

    * Redistributions of source code must retain the above copyright notice,
      this list of conditions and the following disclaimer.
    * Redistributions in binary form must reproduce the above copyright notice,
      this list of conditions and the following disclaimer in the documentation
      and/or other materials provided with the distribution.
    * Neither the name of the LDC Team nor the names of its contributors may be
      used to endorse or promote products derived from this software without
      specific prior written permission.

THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
(INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

@vtjnash vtjnash reopened this Jun 15, 2013
@ghost ghost assigned Keno Jun 20, 2013
@vtjnash
Copy link
Member

vtjnash commented Dec 13, 2013

will be addressed by #3466

@vtjnash vtjnash closed this as completed Dec 13, 2013
@eschnett
Copy link
Contributor

"oddly, clang also distinguishes between complex float and struct { float re; float im; } when generating LLVM op-codes (although the x86 asm is the same)"

@vtjnash These are probably the same on 64-bit x86; they differ on 32-bit x86. This also means that C's complex number (which use a built-in type) return complex numbers different from C++ (which defines them essentially as struct).

One simple way to handle this in Julia, which uses the C++ approach and defines complex numbers in a library instead of requiring them as a built-in type, would be to wrap any external routines that return complex numbers in C++ code with extern "C", so that they are returned as structs.

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

No branches or pull requests

7 participants