Skip to content

Commit

Permalink
Fix Proc#call that takes and returns large extern structs by value (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
HertzDevil committed Feb 27, 2024
1 parent 81fa3a9 commit e3eb178
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 12 deletions.
23 changes: 21 additions & 2 deletions spec/compiler/codegen/extern_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -164,10 +164,29 @@ describe "Codegen: extern struct" do
)).to_i.should eq(3)
end

it "codegens proc that takes and returns large extern struct by value" do
run(<<-CRYSTAL).to_i.should eq(149)
@[Extern]
struct Foo
@unused = uninitialized Int64
def initialize(@x : Int32, @y : Int32, @z : Int32)
end
end
f = ->(foo : Foo) {
Foo.new(foo.@x, foo.@y &* 2, foo.@z &* 3)
}
foo = f.call(Foo.new(100, 20, 3))
foo.@x &+ foo.@y &+ foo.@z
CRYSTAL
end

# These specs *should* also work for 32 bits, but for now we'll
# make sure they work in 64 bits (they probably work in 32 bits too,
# it's just that the specs need to be a bit different)
{% if flag?(:x86_64) %}
{% if flag?(:x86_64) || flag?(:aarch64) %}
it "codegens proc that takes an extern struct with C ABI" do
test_c(
%(
Expand Down Expand Up @@ -427,7 +446,7 @@ describe "Codegen: extern struct" do
), &.to_i.should eq(30))
end

pending_win32 "codegens proc that takes and returns an extern struct with sret" do
it "codegens proc that takes and returns an extern struct with sret" do
test_c(
%(
struct Struct {
Expand Down
2 changes: 1 addition & 1 deletion spec/std/llvm/aarch64_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ class LLVM::ABI
info.return_type.should eq(ArgType.direct(str, cast: ctx.int64.array(2)))
end

test "does with structs between 64 and 128 bits" do |abi, ctx|
test "does with structs larger than 128 bits" do |abi, ctx|
str = ctx.struct([ctx.int64, ctx.int64, ctx.int8])
arg_types = [str]
return_type = str
Expand Down
44 changes: 37 additions & 7 deletions spec/std/llvm/x86_64_abi_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,17 @@ require "llvm"
LLVM.init_x86
{% end %}

private def abi
triple = LLVM.default_target_triple.gsub(/^(.+?)-/, "x86_64-")
private def abi(win64 = false)
triple = win64 ? "x86_64-windows-msvc" : LLVM.default_target_triple.gsub(/^(.+?)-/, "x86_64-")
target = LLVM::Target.from_triple(triple)
machine = target.create_target_machine(triple)
machine.enable_global_isel = false
LLVM::ABI::X86_64.new(machine)
win64 ? LLVM::ABI::X86_Win64.new(machine) : LLVM::ABI::X86_64.new(machine)
end

private def test(msg, &block : LLVM::ABI, LLVM::Context ->)
it msg do
abi = abi()
private def test(msg, *, win64 = false, file = __FILE__, line = __LINE__, &block : LLVM::ABI, LLVM::Context ->)
it msg, file: file, line: line do
abi = abi(win64)
ctx = LLVM::Context.new
block.call(abi, ctx)
end
Expand Down Expand Up @@ -133,7 +133,37 @@ class LLVM::ABI
info.return_type.should eq(ArgType.direct(str, cast: ctx.struct([ctx.int64, ctx.int64])))
end

test "does with structs between 64 and 128 bits" do |abi, ctx|
test "does with structs larger than 128 bits" do |abi, ctx|
str = ctx.struct([ctx.int64, ctx.int64, ctx.int8])
arg_types = [str]
return_type = str

info = abi.abi_info(arg_types, return_type, true, ctx)
info.arg_types.size.should eq(1)

info.arg_types[0].should eq(ArgType.indirect(str, Attribute::ByVal))
info.return_type.should eq(ArgType.indirect(str, Attribute::StructRet))
end
end
{% end %}
end

describe X86_Win64 do
{% if LibLLVM::BUILT_TARGETS.includes?(:x86) %}
describe "abi_info" do
test "does with structs between 64 and 128 bits", win64: true do |abi, ctx|
str = ctx.struct([ctx.int64, ctx.int16])
arg_types = [str]
return_type = str

info = abi.abi_info(arg_types, return_type, true, ctx)
info.arg_types.size.should eq(1)

info.arg_types[0].should eq(ArgType.indirect(str, Attribute::ByVal))
info.return_type.should eq(ArgType.indirect(str, Attribute::StructRet))
end

test "does with structs larger than 128 bits", win64: true do |abi, ctx|
str = ctx.struct([ctx.int64, ctx.int64, ctx.int8])
arg_types = [str]
return_type = str
Expand Down
7 changes: 7 additions & 0 deletions src/compiler/crystal/codegen/call.cr
Original file line number Diff line number Diff line change
Expand Up @@ -597,13 +597,20 @@ class Crystal::CodeGenVisitor
abi_info = abi_info(target_def)
end

sret = abi_info && sret?(abi_info)
arg_offset = is_closure ? 2 : 1
arg_offset += 1 if sret

arg_types = fun_type.try(&.arg_types) || target_def.try &.args.map &.type
arg_types.try &.each_with_index do |arg_type, i|
if abi_info && (abi_arg_type = abi_info.arg_types[i]?) && (attr = abi_arg_type.attr)
@last.add_instruction_attribute(i + arg_offset, attr, llvm_context, abi_arg_type.type)
end
end

if abi_info && sret
@last.add_instruction_attribute(1, LLVM::Attribute::StructRet, llvm_context, abi_info.return_type.type)
end
end

def sret?(abi_info)
Expand Down
5 changes: 4 additions & 1 deletion src/compiler/crystal/codegen/fun.cr
Original file line number Diff line number Diff line change
Expand Up @@ -535,11 +535,14 @@ class Crystal::CodeGenVisitor
value = pointer2
end
else
abi_info = target_def.abi_info? ? abi_info(target_def) : nil
sret = abi_info && sret?(abi_info)

# If it's an extern struct on a def that must be codegened with C ABI
# compatibility, and it's not passed byval, we must cast the value
if target_def.c_calling_convention? && arg.type.extern? && !context.fun.attributes(index + 1).by_val?
# ... unless it's passed indirectly (ie. as a pointer to memory allocated by the caller)
if target_def.abi_info? && abi_info(target_def).arg_types[index].kind.indirect?
if abi_info.try &.arg_types[index - (sret ? 1 : 0)].kind.indirect?
value = declare_debug_for_function_argument(arg.name, var_type, index + 1, value, location) unless target_def.naked?
context.vars[arg.name] = LLVMVar.new(value, var_type)
else
Expand Down
2 changes: 1 addition & 1 deletion src/llvm/abi/x86_win64.cr
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class LLVM::ABI::X86_Win64 < LLVM::ABI::X86
when 2 then ArgType.direct(t, context.int16)
when 4 then ArgType.direct(t, context.int32)
when 8 then ArgType.direct(t, context.int64)
else ArgType.indirect(t, nil)
else ArgType.indirect(t, LLVM::Attribute::ByVal)
end
else
non_struct(t, context)
Expand Down

0 comments on commit e3eb178

Please sign in to comment.