Skip to content

Commit

Permalink
Codegen: use malloc_atomic whenever possible. Fixes #4081
Browse files Browse the repository at this point in the history
  • Loading branch information
Ary Borenszweig committed Mar 3, 2017
1 parent dc0d365 commit f1998de
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 8 deletions.
51 changes: 44 additions & 7 deletions src/compiler/crystal/codegen/codegen.cr
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ module Crystal
MAIN_NAME = "__crystal_main"
RAISE_NAME = "__crystal_raise"
MALLOC_NAME = "__crystal_malloc"
MALLOC_ATOMIC_NAME = "__crystal_malloc_atomic"
REALLOC_NAME = "__crystal_realloc"
PERSONALITY_NAME = "__crystal_personality"
GET_EXCEPTION_NAME = "__crystal_get_exception"
Expand Down Expand Up @@ -129,6 +130,7 @@ module Crystal
@empty_md_list : LLVM::Value
@rescue_block : LLVM::BasicBlock?
@malloc_fun : LLVM::Function?
@malloc_atomic_fun : LLVM::Function?
@sret_value : LLVM::Value?
@cant_pass_closure_to_c_exception_call : Call?
@realloc_fun : LLVM::Function?
Expand Down Expand Up @@ -1746,7 +1748,12 @@ module Crystal
if type.passed_by_value?
@last = alloca struct_type
else
@last = malloc struct_type
if type.is_a?(InstanceVarContainer) && !type.struct? &&
type.all_instance_vars.each_value.any? &.type.has_inner_pointers?
@last = malloc struct_type
else
@last = malloc_atomic struct_type
end
end
memset @last, int8(0), struct_type.size
type_ptr = @last
Expand Down Expand Up @@ -1806,9 +1813,15 @@ module Crystal
end

def malloc(type)
@malloc_fun ||= @main_mod.functions[MALLOC_NAME]?
if malloc_fun = @malloc_fun
malloc_fun = check_main_fun MALLOC_NAME, malloc_fun
generic_malloc(type) { malloc_fun }
end

def malloc_atomic(type)
generic_malloc(type) { malloc_atomic_fun }
end

def generic_malloc(type)
if malloc_fun = yield
size = trunc(type.size, llvm_context.int32)
pointer = call malloc_fun, size
bit_cast pointer, type.pointer
Expand All @@ -1818,12 +1831,18 @@ module Crystal
end

def array_malloc(type, count)
@malloc_fun ||= @main_mod.functions[MALLOC_NAME]?
generic_array_malloc(type, count) { malloc_fun }
end

def array_malloc_atomic(type, count)
generic_array_malloc(type, count) { malloc_atomic_fun }
end

def generic_array_malloc(type, count)
size = trunc(type.size, llvm_context.int32)
count = trunc(count, llvm_context.int32)
size = builder.mul size, count
if malloc_fun = @malloc_fun
malloc_fun = check_main_fun MALLOC_NAME, malloc_fun
if malloc_fun = yield
pointer = call malloc_fun, size
memset pointer, int8(0), size
bit_cast pointer, type.pointer
Expand All @@ -1835,6 +1854,24 @@ module Crystal
end
end

def malloc_fun
@malloc_fun ||= @main_mod.functions[MALLOC_NAME]?
if malloc_fun = @malloc_fun
check_main_fun MALLOC_NAME, malloc_fun
else
nil
end
end

def malloc_atomic_fun
@malloc_atomic_fun ||= @main_mod.functions[MALLOC_ATOMIC_NAME]?
if malloc_fun = @malloc_atomic_fun
check_main_fun MALLOC_ATOMIC_NAME, malloc_fun
else
nil
end
end

def memset(pointer, value, size)
pointer = cast_to_void_pointer pointer
call @program.memset(@llvm_mod, llvm_context), [pointer, value, trunc(size, llvm_context.int32), int32(4), int1(0)]
Expand Down
6 changes: 5 additions & 1 deletion src/compiler/crystal/codegen/primitives.cr
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,11 @@ class Crystal::CodeGenVisitor
def codegen_primitive_pointer_malloc(node, target_def, call_args)
type = node.type.as(PointerInstanceType)
llvm_type = llvm_embedded_type(type.element_type)
last = array_malloc(llvm_type, call_args[1])
if type.element_type.has_inner_pointers?
last = array_malloc(llvm_type, call_args[1])
else
last = array_malloc_atomic(llvm_type, call_args[1])
end
last
end

Expand Down
43 changes: 43 additions & 0 deletions src/compiler/crystal/codegen/types.cr
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,49 @@ module Crystal
end
end

# Returns `true` if the type has inner pointers.
# This is useful to know because if a type doesn't have
# inner pointers we can use `malloc_atomic` instead of
# `malloc` in `Pointer.malloc` for a tiny performance boost.
def has_inner_pointers?
case self
when void?

This comment has been minimized.

Copy link
@RX14

RX14 Mar 3, 2017

Contributor

Shouldn't this be .void?

This comment has been minimized.

Copy link
@asterite

asterite Mar 3, 2017

Member

Oh, right. In any case it still works because void? resolves to self.void? so it's the same, but it's true that I meant the other one. I'll fix it. Thanks!

This comment has been minimized.

Copy link
@RX14

RX14 Mar 3, 2017

Contributor

Am I correct in saying that it only works because self is truthy?

This comment has been minimized.

Copy link
@asterite

asterite Mar 3, 2017

Member

Oh, actually, you are right, the code is wrong. The above translates to void? === self which is never true. Maybe that explains the segfaults in travis... not sure.

# We consider Void to have pointers, so doing
# Pointer(Void).malloc(...).as(ReferenceType)
# will consider potential inner pointers as such.
true
when PointerInstanceType
true
when ProcInstanceType
# A proc can have closure data which might have pointers
true
when StaticArrayInstanceType
self.element_type.has_inner_pointers?
when TupleInstanceType
self.tuple_types.any? &.has_inner_pointers?
when NamedTupleInstanceType
self.entries.any? &.type.has_inner_pointers?
when PrimitiveType
false
when EnumType
false
when UnionType
self.union_types.any? &.has_inner_pointers?
when AliasType
self.aliased_type.has_inner_pointers?
when TypeDefType
self.typedef.has_inner_pointers?
when InstanceVarContainer
if struct?
all_instance_vars.each_value.any? &.type.has_inner_pointers?
else
true
end
else
true
end
end

def llvm_name
String.build do |io|
llvm_name io
Expand Down

0 comments on commit f1998de

Please sign in to comment.