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.

Sign up for GitHub

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

Jump to bottom

codegen: fix union layout alignment and padding #24197

Merged
merged 2 commits into from
Oct 19, 2017
Merged

codegen: fix union layout alignment and padding #24197

merged 2 commits into from
Oct 19, 2017

Conversation

vtjnash
Copy link
Sponsor Member

@vtjnash vtjnash commented Oct 18, 2017

No description provided.

@vtjnash vtjnash requested a review from yuyichao October 18, 2017 19:02
@kshyatt kshyatt added the compiler:codegen Generation of LLVM IR and native code label Oct 18, 2017
yuyichao
yuyichao approved these changes Oct 19, 2017
View reviewed changes
Copy link
Contributor

@yuyichao yuyichao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

src/cgutils.cpp
// (also requires change to emit_new_struct to not assume 0 == 0)
if (!isTuple && latypes.size() > 1) {
Type *NoopType = ArrayType::get(T_int1, 0);
latypes.insert(latypes.begin(), NoopType);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

=)

@vtjnash vtjnash merged commit 40ee27a into master Oct 19, 2017
@vtjnash vtjnash deleted the jn/codegen-union-layout branch October 19, 2017 17:38
@yuyichao yuyichao mentioned this pull request Oct 21, 2017
Closed
@nalimilan
Copy link
Member

This introduced a crash when building Julia on #23642. If you pull that branch and cherry-pick e3cd320 (which I have removed from the branch), make gives this:

[...]
/home/milan/Dev/julia/base/precompile.jl

signal (11): Segmentation fault
in expression starting at /home/milan/Dev/julia/base/precompile.jl:17
_ZN4llvm11PointerType3getEPNS_4TypeEj at /home/milan/Dev/julia/usr/bin/../lib/libLLVM-3.9.so (unknown line)
getGEPReturnType at /home/milan/Dev/julia/usr/include/llvm/IR/Instructions.h:991 [inlined]
GetElementPtrInst at /home/milan/Dev/julia/usr/include/llvm/IR/Instructions.h:1063 [inlined]
Create at /home/milan/Dev/julia/usr/include/llvm/IR/Instructions.h:868 [inlined]
CreateInBounds at /home/milan/Dev/julia/usr/include/llvm/IR/Instructions.h:899 [inlined]
CreateConstInBoundsGEP2_32 at /home/milan/Dev/julia/usr/include/llvm/IR/IRBuilder.h:1200
emit_struct_gep at /home/milan/Dev/julia/src/cgutils.cpp:422
emit_getfield_knownidx at /home/milan/Dev/julia/src/cgutils.cpp:1518
emit_getfield at /home/milan/Dev/julia/src/codegen.cpp:2051 [inlined]
emit_builtin_call at /home/milan/Dev/julia/src/codegen.cpp:2588
emit_call at /home/milan/Dev/julia/src/codegen.cpp:3066
emit_expr at /home/milan/Dev/julia/src/codegen.cpp:3780
emit_assignment at /home/milan/Dev/julia/src/codegen.cpp:3476 [inlined]
emit_expr at /home/milan/Dev/julia/src/codegen.cpp:3794
emit_stmtpos at /home/milan/Dev/julia/src/codegen.cpp:3703 [inlined]
emit_function at /home/milan/Dev/julia/src/codegen.cpp:5791
jl_compile_linfo at /home/milan/Dev/julia/src/codegen.cpp:1156
emit_invoke at /home/milan/Dev/julia/src/codegen.cpp:3016
emit_expr at /home/milan/Dev/julia/src/codegen.cpp:3777
emit_assignment at /home/milan/Dev/julia/src/codegen.cpp:3476 [inlined]
emit_expr at /home/milan/Dev/julia/src/codegen.cpp:3794
emit_stmtpos at /home/milan/Dev/julia/src/codegen.cpp:3703 [inlined]
emit_function at /home/milan/Dev/julia/src/codegen.cpp:5791
jl_compile_linfo at /home/milan/Dev/julia/src/codegen.cpp:1156
emit_invoke at /home/milan/Dev/julia/src/codegen.cpp:3016
emit_expr at /home/milan/Dev/julia/src/codegen.cpp:3777
emit_assignment at /home/milan/Dev/julia/src/codegen.cpp:3476 [inlined]
emit_expr at /home/milan/Dev/julia/src/codegen.cpp:3794
emit_stmtpos at /home/milan/Dev/julia/src/codegen.cpp:3703 [inlined]
emit_function at /home/milan/Dev/julia/src/codegen.cpp:5791
jl_compile_linfo at /home/milan/Dev/julia/src/codegen.cpp:1156
jl_compile_hint at /home/milan/Dev/julia/src/gf.c:1771
precompile at ./essentials.jl:403
unknown function (ip: 0x7ff78c672932)
jl_call_fptr_internal at /home/milan/Dev/julia/src/julia_internal.h:366 [inlined]
jl_call_method_internal at /home/milan/Dev/julia/src/julia_internal.h:385 [inlined]
jl_apply_generic at /home/milan/Dev/julia/src/gf.c:2003
do_call at /home/milan/Dev/julia/src/interpreter.c:70
eval at /home/milan/Dev/julia/src/interpreter.c:262
jl_interpret_toplevel_expr_in at /home/milan/Dev/julia/src/interpreter.c:50
jl_toplevel_eval_flex at /home/milan/Dev/julia/src/toplevel.c:640
jl_parse_eval_all at /home/milan/Dev/julia/src/ast.c:798
jl_load at /home/milan/Dev/julia/src/toplevel.c:687
include_relative at ./loading.jl:533
jl_call_fptr_internal at /home/milan/Dev/julia/src/julia_internal.h:366 [inlined]
jl_call_method_internal at /home/milan/Dev/julia/src/julia_internal.h:385 [inlined]
jl_apply_generic at /home/milan/Dev/julia/src/gf.c:2003
include at ./sysimg.jl:15
jl_call_fptr_internal at /home/milan/Dev/julia/src/julia_internal.h:366 [inlined]
jl_call_method_internal at /home/milan/Dev/julia/src/julia_internal.h:385 [inlined]
jl_apply_generic at /home/milan/Dev/julia/src/gf.c:2003
do_call at /home/milan/Dev/julia/src/interpreter.c:70
eval at /home/milan/Dev/julia/src/interpreter.c:262
jl_interpret_toplevel_expr_in at /home/milan/Dev/julia/src/interpreter.c:50
jl_toplevel_eval_flex at /home/milan/Dev/julia/src/toplevel.c:640
jl_eval_module_expr at /home/milan/Dev/julia/src/toplevel.c:227
jl_toplevel_eval_flex at /home/milan/Dev/julia/src/toplevel.c:509
jl_parse_eval_all at /home/milan/Dev/julia/src/ast.c:798
jl_load at /home/milan/Dev/julia/src/toplevel.c:687
exec_program at /home/milan/Dev/julia/ui/repl.c:36
true_main at /home/milan/Dev/julia/ui/repl.c:119
main at /home/milan/Dev/julia/ui/repl.c:237
__libc_start_main at /lib64/libc.so.6 (unknown line)
_start at /home/milan/Dev/julia/usr/bin/julia (unknown line)
Allocations: 87586922 (Pool: 87584519; Big: 2403); GC: 200
/bin/sh : ligne 1 : 28591 Segmentation fault      (core dumped)/home/milan/Dev/julia/usr/bin/julia -O3 -C "native" --output-o /home/milan/Dev/julia/usr/lib/julia/sys.o.tmp --startup-file=no --warn-overwrite=yes --sysimage /home/milan/Dev/julia/usr/lib/julia/inference.ji sysimg.jl
*** This error is usually fixed by running `make clean`. If the error persists, try `make cleanall`. ***
make[1]: *** [Makefile:226: /home/milan/Dev/julia/usr/lib/julia/sys.o] Error 1
make: *** [Makefile:106: julia-sysimg-release] Error 2

I've traced it to isclusterlazy() = get(PGRP.lazy, false), with lazy::Union{Some{Bool}, Void}.

@nalimilan
Copy link
Member

The Travis build gives more details thanks to LLVM assertions:

julia: /home/travis/build/JuliaLang/julia/usr/include/llvm/Support/Casting.h:237: typename llvm::cast_retty<X, Y*>::ret_type llvm::cast(Y*) [with X = llvm::StructType; Y = llvm::Type; typename llvm::cast_retty<X, Y*>::ret_type = llvm::StructType*]: Assertion `isa<X>(Val) && "cast<Ty>() argument of incompatible type!"' failed.

signal (6): Aborted

in expression starting at /home/travis/build/JuliaLang/julia/base/precompile.jl:17

gsignal at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)

abort at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)

unknown function (ip: 0x2b8f78518bf5)

__assert_fail at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)

cast<llvm::StructType, llvm::Type> at /home/travis/build/JuliaLang/julia/usr/include/llvm/Support/Casting.h:237 [inlined]

convert_struct_offset at /home/travis/build/JuliaLang/julia/src/cgutils.cpp:410 [inlined]

convert_struct_offset at /home/travis/build/JuliaLang/julia/src/cgutils.cpp:416 [inlined]

emit_struct_gep at /home/travis/build/JuliaLang/julia/src/cgutils.cpp:421
[...]

@yuyichao
Copy link
Contributor

yuyichao commented Oct 21, 2017 •
edited
Loading

I'm guessing it hits an i8 array?

@nalimilan
Copy link
Member

How can I know that?

nalimilan added a commit that referenced this pull request Oct 21, 2017
@nalimilan
Temporary: Revert "Merge pull request #24197 from JuliaLang/jn/codege…
d6d77a5 
…n-union-layout"

This reverts commit 40ee27a, reversing
changes made to a1c9549.
nalimilan added a commit that referenced this pull request Oct 21, 2017
@nalimilan
Temporary: Revert "Merge pull request #24197 from JuliaLang/jn/codege…
63f5dc6 
…n-union-layout"

This reverts commit 40ee27a, reversing
changes made to a1c9549.
nalimilan added a commit that referenced this pull request Oct 21, 2017
@nalimilan
Temporary: Revert "Merge pull request #24197 from JuliaLang/jn/codege…
76fd5f3 
…n-union-layout"

This reverts commit 40ee27a, reversing
changes made to a1c9549.
@nalimilan nalimilan mentioned this pull request Oct 21, 2017
Merged
nalimilan added a commit that referenced this pull request Oct 21, 2017
@nalimilan
Temporary: Revert "Merge pull request #24197 from JuliaLang/jn/codege…
55b4f39 
…n-union-layout"

This reverts commit 40ee27a, reversing
changes made to a1c9549.
@yuyichao
Copy link
Contributor

Actually it comes from mutable structs with union fields in which case the lt is %jl_value_t* and not a struct. Reproducible with

julia> mutable struct A
           x::Union{Int,Void}
       end

julia> a = A(1)
A(1)

julia> f(a) = a.x
f (generic function with 1 method)

julia> @code_llvm f(a)

@quinnj quinnj mentioned this pull request Oct 27, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yuyichao yuyichao yuyichao approved these changes

Assignees
No one assigned
Labels
compiler:codegen Generation of LLVM IR and native code
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@vtjnash @nalimilan @yuyichao @kshyatt