authorgravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2022-03-01 13:30:25-07:00
committergravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2022-03-01 14:58:37-07:00
log1b194931b0df08db0f38a284bb10b89cc00a8817
tree9b6c1f4a7f265e36f02dc7a0c03780f22ddd1713
parent18e42661dc9b8199311ee086b24ef5c85cf4708f

LLVM: fix when sret and isByRef ret_ty disagree

This can happen functions use the C ABI.

2 files changed, 56 insertions(+), 27 deletions(-)

src/codegen/llvm.zig+53-25
......@@ -450,10 +450,22 @@ pub const Object = struct {
450450 DeclGen.removeFnAttr(llvm_func, "cold");
451451 }
452452
453 // Remove all the basic blocks of a function in order to start over, generating
454 // LLVM IR from an empty function body.
455 while (llvm_func.getFirstBasicBlock()) |bb| {
456 bb.deleteBasicBlock();
457 }
458
459 const builder = dg.context.createBuilder();
460
461 const entry_block = dg.context.appendBasicBlock(llvm_func, "Entry");
462 builder.positionBuilderAtEnd(entry_block);
463
453464 // This gets the LLVM values from the function and stores them in `dg.args`.
454465 const fn_info = decl.ty.fnInfo();
455 const ret_ty_by_ref = isByRef(fn_info.return_type);
456 const ret_ptr = if (ret_ty_by_ref) llvm_func.getParam(0) else null;
466 const target = dg.module.getTarget();
467 const sret = firstParamSRet(fn_info, target);
468 const ret_ptr = if (sret) llvm_func.getParam(0) else null;
457469
458470 var args = std.ArrayList(*const llvm.Value).init(dg.gpa);
459471 defer args.deinit();
......@@ -466,17 +478,6 @@ pub const Object = struct {
466478 try args.append(llvm_func.getParam(llvm_arg_i));
467479 }
468480
469 // Remove all the basic blocks of a function in order to start over, generating
470 // LLVM IR from an empty function body.
471 while (llvm_func.getFirstBasicBlock()) |bb| {
472 bb.deleteBasicBlock();
473 }
474
475 const builder = dg.context.createBuilder();
476
477 const entry_block = dg.context.appendBasicBlock(llvm_func, "Entry");
478 builder.positionBuilderAtEnd(entry_block);
479
480481 var fg: FuncGen = .{
481482 .gpa = dg.gpa,
482483 .air = air,
......@@ -485,7 +486,7 @@ pub const Object = struct {
485486 .dg = &dg,
486487 .builder = builder,
487488 .ret_ptr = ret_ptr,
488 .args = args.toOwnedSlice(),
489 .args = args.items,
489490 .arg_index = 0,
490491 .func_inst_table = .{},
491492 .llvm_func = llvm_func,
......@@ -1977,20 +1978,22 @@ pub const FuncGen = struct {
19771978 air: Air,
19781979 liveness: Liveness,
19791980 context: *const llvm.Context,
1980
19811981 builder: *const llvm.Builder,
19821982
19831983 /// This stores the LLVM values used in a function, such that they can be referred to
19841984 /// in other instructions. This table is cleared before every function is generated.
19851985 func_inst_table: std.AutoHashMapUnmanaged(Air.Inst.Ref, *const llvm.Value),
19861986
1987 /// If the return type isByRef, this is the result pointer. Otherwise null.
1987 /// If the return type is sret, this is the result pointer. Otherwise null.
1988 /// Note that this can disagree with isByRef for the return type in the case
1989 /// of C ABI functions.
19881990 ret_ptr: ?*const llvm.Value,
19891991 /// These fields are used to refer to the LLVM value of the function parameters
19901992 /// in an Arg instruction.
19911993 /// This list may be shorter than the list according to the zig type system;
1992 /// it omits 0-bit types.
1993 args: []*const llvm.Value,
1994 /// it omits 0-bit types. If the function uses sret as the first parameter,
1995 /// this slice does not include it.
1996 args: []const *const llvm.Value,
19941997 arg_index: usize,
19951998
19961999 llvm_func: *const llvm.Value,
......@@ -2010,7 +2013,6 @@ pub const FuncGen = struct {
20102013 fn deinit(self: *FuncGen) void {
20112014 self.builder.dispose();
20122015 self.func_inst_table.deinit(self.gpa);
2013 self.gpa.free(self.args);
20142016 self.blocks.deinit(self.gpa);
20152017 }
20162018
......@@ -2283,12 +2285,35 @@ pub const FuncGen = struct {
22832285 if (return_type.isNoReturn()) {
22842286 _ = self.builder.buildUnreachable();
22852287 return null;
2286 } else if (self.liveness.isUnused(inst) or !return_type.hasRuntimeBits()) {
2288 }
2289
2290 if (self.liveness.isUnused(inst) or !return_type.hasRuntimeBits()) {
22872291 return null;
2288 } else if (sret) {
2292 }
2293
2294 if (ret_ptr) |rp| {
22892295 const llvm_ret_ty = try self.dg.llvmType(return_type);
22902296 call.setCallSret(llvm_ret_ty);
2291 return ret_ptr;
2297 if (isByRef(return_type)) {
2298 return rp;
2299 } else {
2300 // our by-ref status disagrees with sret so we must load.
2301 const loaded = self.builder.buildLoad(rp, "");
2302 loaded.setAlignment(return_type.abiAlignment(target));
2303 return loaded;
2304 }
2305 }
2306
2307 if (isByRef(return_type)) {
2308 // our by-ref status disagrees with sret so we must allocate, store,
2309 // and return the allocation pointer.
2310 const llvm_ret_ty = try self.dg.llvmType(return_type);
2311 const rp = self.buildAlloca(llvm_ret_ty);
2312 const alignment = return_type.abiAlignment(target);
2313 rp.setAlignment(alignment);
2314 const store_inst = self.builder.buildStore(call, rp);
2315 store_inst.setAlignment(alignment);
2316 return rp;
22922317 } else {
22932318 return call;
22942319 }
......@@ -2321,12 +2346,14 @@ pub const FuncGen = struct {
23212346 const un_op = self.air.instructions.items(.data)[inst].un_op;
23222347 const ptr_ty = self.air.typeOf(un_op);
23232348 const ret_ty = ptr_ty.childType();
2324 if (!ret_ty.hasRuntimeBits() or isByRef(ret_ty)) {
2349 if (!ret_ty.hasRuntimeBits() or self.ret_ptr != null) {
23252350 _ = self.builder.buildRetVoid();
23262351 return null;
23272352 }
2353 const target = self.dg.module.getTarget();
23282354 const ptr = try self.resolveInst(un_op);
23292355 const loaded = self.builder.buildLoad(ptr, "");
2356 loaded.setAlignment(ret_ty.abiAlignment(target));
23302357 _ = self.builder.buildRet(loaded);
23312358 return null;
23322359 }
......@@ -5456,11 +5483,12 @@ fn firstParamSRet(fn_info: Type.Payload.Function.Data, target: std.Target) bool
54565483 .C => {},
54575484 else => return false,
54585485 }
5486 const x86_64_abi = @import("../arch/x86_64/abi.zig");
54595487 switch (target.cpu.arch) {
54605488 .mips, .mipsel => return false,
54615489 .x86_64 => switch (target.os.tag) {
5462 .windows => return @import("../arch/x86_64/abi.zig").classifyWindows(fn_info.return_type, target) == .memory,
5463 else => return @import("../arch/x86_64/abi.zig").classifySystemV(fn_info.return_type, target)[0] == .memory,
5490 .windows => return x86_64_abi.classifyWindows(fn_info.return_type, target) == .memory,
5491 else => return x86_64_abi.classifySystemV(fn_info.return_type, target)[0] == .memory,
54645492 },
54655493 else => return false, // TODO investigate C ABI for other architectures
54665494 }
test/behavior/struct.zig+3-2
......@@ -833,12 +833,13 @@ test "packed struct with fp fields" {
833833}
834834
835835test "fn with C calling convention returns struct by value" {
836 if (builtin.zig_backend != .stage1) return error.SkipZigTest; // TODO
836 if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO
837 if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO
837838
838839 const S = struct {
839840 fn entry() !void {
840841 var x = makeBar(10);
841 try expectEqual(@as(i32, 10), x.handle);
842 try expect(@as(i32, 10) == x.handle);
842843 }
843844
844845 const ExternBar = extern struct {