Skip to content

Commit

Permalink
store tuple and vector types to the stack eagerly [ci skip]
Browse files Browse the repository at this point in the history
fix #11187, fix #11450, fix #11026, ref #10525, fix #11003
TODO: confirm all of those numbers were fixed
TODO: ensure the lazy-loaded objects have gc-roots
TODO: re-enable VectorType objects, so small objects still end up in
registers in the calling convention
TODO: allow moving pointers sometimes rather than copying
TODO: teach the GC how it can re-use an existing pointer as a box

this also changes the julia specSig calling convention to pass
non-primitive types by pointer instead of by-value

this additionally fixes a bug in gen_cfunction that could be exposed by
turning off specSig

this additionally moves the alloca calls in ccall (and other places) to
the entry BasicBlock in the function, ensuring that llvm detects them as
static allocations and moves them into the function prologue

this additionally fixes some undefined behavior from changing
a variable's size through a alloca-cast instead of zext/sext/trunc
  • Loading branch information
vtjnash committed Jun 9, 2015
1 parent 43d5a9f commit 244f1cb
Show file tree
Hide file tree
Showing 7 changed files with 347 additions and 190 deletions.
5 changes: 3 additions & 2 deletions base/inference.jl
Original file line number Diff line number Diff line change
Expand Up @@ -3038,6 +3038,7 @@ end
function remove_redundant_temp_vars(ast, sa)
varinfo = ast.args[2][2]
gensym_types = ast.args[2][4]
body = ast.args[3]
for (v,init) in sa
if ((isa(init,Symbol) || isa(init,SymbolNode)) &&
any(vi->symequal(vi[1],init), varinfo) &&
Expand All @@ -3046,7 +3047,7 @@ function remove_redundant_temp_vars(ast, sa)
# this transformation is not valid for vars used before def.
# we need to preserve the point of assignment to know where to
# throw errors (issue #4645).
if !occurs_undef(v, ast.args[3], varinfo)
if !occurs_undef(v, body, varinfo)

# the transformation is not ideal if the assignment
# is present for the auto-unbox functionality
Expand All @@ -3055,7 +3056,7 @@ function remove_redundant_temp_vars(ast, sa)
# everywhere later in the function
if (isa(init,SymbolNode) ? (init.typ <: (isa(v,GenSym)?gensym_types[(v::GenSym).id+1]:local_typeof(v, varinfo))) : true)
delete_var!(ast, v)
sym_replace(ast.args[3], [v], [], [init], [])
sym_replace(body, Any[v], Void[], Any[init], Void[])
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion src/builtins.c
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ static int NOINLINE compare_fields(jl_value_t *a, jl_value_t *b,
return 1;
}

int jl_egal(jl_value_t *a, jl_value_t *b)
int jl_egal(jl_value_t *a, jl_value_t *b) // warning: a,b may NOT have been gc-rooted by the caller
{
if (a == b)
return 1;
Expand Down
Loading

6 comments on commit 244f1cb

@JeffBezanson
Copy link
Member

Choose a reason for hiding this comment

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

👍 Exciting.

@vtjnash
Copy link
Member Author

@vtjnash vtjnash commented on 244f1cb Jun 9, 2015

Choose a reason for hiding this comment

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

i tried turning back on vector types as part of this (since the alignment issues should be fixed now), but so far, that seems to have inhibited one of the harder tests:

$ make core linalg1
    JULIA test/core
     * core
    SUCCESS
    JULIA test/linalg1
     * linalg1
    SUCCESS
$ ../julia -q
julia> Base.Test.@test true
ERROR: test failed: false
 in expression: true
 in error at ./error.jl:21
 in default_handler at test.jl:29
 in do_test at test.jl:52

@JeffBezanson
Copy link
Member

Choose a reason for hiding this comment

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

That is too funny.

@carnaval
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's fine. no one will notice, go ahead.

@vtjnash
Copy link
Member Author

@vtjnash vtjnash commented on 244f1cb Jun 9, 2015

Choose a reason for hiding this comment

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

i found the issue: i was missing a trunc i8 %x to i1 after extractvalue <2 x i8> %x, i32 0, so llvm replaced the result with undef

on to the next issue: llvm appears to assume our gc returns addresses with 32-byte alignment (e.g. it emit vmovaps %ymm0, (%rax), where %rax is the register that holds the return value from the call to allocobj).

completely unrelated finding: did you know that git checkout - does a checkout of the previous branch? (similar idea as cd -)

@yuyichao
Copy link
Contributor

Choose a reason for hiding this comment

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

completely unrelated finding: did you know that git checkout - does a checkout of the previous branch? (similar idea as cd -)

Didn't know that before !

Related reading

Please sign in to comment.