From 5a100e74e591d5b3c1d40f87adc65d5d3caa6835 Mon Sep 17 00:00:00 2001 From: Jan Philipp Hafer Date: Wed, 9 Feb 2022 15:25:38 +0100 Subject: [PATCH 1/2] builtins: overflow arithmetic perf optimization change signature of arithmetic operations @addwithOverflow, @subWithOverflow, @mulWithOverflow, shlWithOverflow from @operation(comptime T: type, a: T, b: T, result: *T) bool to @operation(comptime T: type, a: T, b: T) anytype with anytype being a tuple struct { res: T, ov: bool } This removes the pointer store and load for efficiency of codegen. Comptime operation is accordingly kept in sync. closes #10248 --- src/Air.zig | 9 +++++---- src/AstGen.zig | 22 ---------------------- src/BuiltinFn.zig | 14 ++++++++++---- src/Zir.zig | 1 - src/print_zir.zig | 2 -- 5 files changed, 15 insertions(+), 33 deletions(-) diff --git a/src/Air.zig b/src/Air.zig index 623da2625543..f0028e3e5a34 100644 --- a/src/Air.zig +++ b/src/Air.zig @@ -135,10 +135,11 @@ pub const Inst = struct { /// is the same as both operands. /// Uses the `bin_op` field. min, + /// DEBUG /// Integer addition with overflow. Both operands are guaranteed to be the same type, - /// and the result is bool. The wrapped value is written to the pointer given by the in - /// operand of the `pl_op` field. Payload is `Bin` with `lhs` and `rhs` the relevant types - /// of the operation. + /// and the result is a tuple with .{res, ov}. The wrapped value is written to res + /// and if an overflow happens, ov if true. Otherwise ov is false. + /// Payload is `Bin` with `lhs` and `rhs` the relevant types of the operation. /// Uses the `pl_op` field with payload `Bin`. add_with_overflow, /// Integer subtraction with overflow. Both operands are guaranteed to be the same type, @@ -934,7 +935,7 @@ pub fn typeOfIndex(air: Air, inst: Air.Inst.Index) Type { .sub_with_overflow, .mul_with_overflow, .shl_with_overflow, - => return Type.initTag(.bool), + => return Type.initTag(.Struct), } } diff --git a/src/AstGen.zig b/src/AstGen.zig index 9bc10f25e8fa..0a165fee1e59 100644 --- a/src/AstGen.zig +++ b/src/AstGen.zig @@ -7186,23 +7186,12 @@ fn builtinCall( .shl_with_overflow => { const int_type = try typeExpr(gz, scope, params[0]); const log2_int_type = try gz.addUnNode(.log2_int_type, int_type, params[0]); - const ptr_type = try gz.add(.{ .tag = .ptr_type_simple, .data = .{ - .ptr_type_simple = .{ - .is_allowzero = false, - .is_mutable = true, - .is_volatile = false, - .size = .One, - .elem_type = int_type, - }, - } }); const lhs = try expr(gz, scope, .{ .ty = int_type }, params[1]); const rhs = try expr(gz, scope, .{ .ty = log2_int_type }, params[2]); - const ptr = try expr(gz, scope, .{ .ty = ptr_type }, params[3]); const result = try gz.addExtendedPayload(.shl_with_overflow, Zir.Inst.OverflowArithmetic{ .node = gz.nodeIndexToRelative(node), .lhs = lhs, .rhs = rhs, - .ptr = ptr, }); return rvalue(gz, rl, result, node); }, @@ -7596,23 +7585,12 @@ fn overflowArithmetic( tag: Zir.Inst.Extended, ) InnerError!Zir.Inst.Ref { const int_type = try typeExpr(gz, scope, params[0]); - const ptr_type = try gz.add(.{ .tag = .ptr_type_simple, .data = .{ - .ptr_type_simple = .{ - .is_allowzero = false, - .is_mutable = true, - .is_volatile = false, - .size = .One, - .elem_type = int_type, - }, - } }); const lhs = try expr(gz, scope, .{ .ty = int_type }, params[1]); const rhs = try expr(gz, scope, .{ .ty = int_type }, params[2]); - const ptr = try expr(gz, scope, .{ .ty = ptr_type }, params[3]); const result = try gz.addExtendedPayload(tag, Zir.Inst.OverflowArithmetic{ .node = gz.nodeIndexToRelative(node), .lhs = lhs, .rhs = rhs, - .ptr = ptr, }); return rvalue(gz, rl, result, node); } diff --git a/src/BuiltinFn.zig b/src/BuiltinFn.zig index 3bf7224fabcd..e34fecd43791 100644 --- a/src/BuiltinFn.zig +++ b/src/BuiltinFn.zig @@ -145,10 +145,13 @@ pub const list = list: { @setEvalBranchQuota(3000); break :list std.ComptimeStringMap(@This(), .{ .{ + // changed from @addWithOverflow(comptime T: type, a: T, b: T, result: *T) bool //DEBUG + // to 1. RetType(ST) = struct {res: T, flag: u8}; //DEBUG + // 2. @addWithOverflow(comptime T: type, a: T, b: T) RetType //DEBUG "@addWithOverflow", .{ .tag = .add_with_overflow, - .param_count = 4, + .param_count = 3, }, }, .{ @@ -596,10 +599,11 @@ pub const list = list: { }, }, .{ + // see @addWithOverflow // DEBUG "@mulWithOverflow", .{ .tag = .mul_with_overflow, - .param_count = 4, + .param_count = 3, }, }, .{ @@ -701,10 +705,11 @@ pub const list = list: { }, }, .{ + // see @addWithOverflow // DEBUG "@shlWithOverflow", .{ .tag = .shl_with_overflow, - .param_count = 4, + .param_count = 3, }, }, .{ @@ -842,10 +847,11 @@ pub const list = list: { }, }, .{ + // see @addWithOverflow // DEBUG "@subWithOverflow", .{ .tag = .sub_with_overflow, - .param_count = 4, + .param_count = 3, }, }, .{ diff --git a/src/Zir.zig b/src/Zir.zig index b7e3e609160b..48c977b55e24 100644 --- a/src/Zir.zig +++ b/src/Zir.zig @@ -2843,7 +2843,6 @@ pub const Inst = struct { node: i32, lhs: Ref, rhs: Ref, - ptr: Ref, }; pub const Cmpxchg = struct { diff --git a/src/print_zir.zig b/src/print_zir.zig index 9c79ad1a37df..8c069c9345e3 100644 --- a/src/print_zir.zig +++ b/src/print_zir.zig @@ -1076,8 +1076,6 @@ const Writer = struct { try self.writeInstRef(stream, extra.lhs); try stream.writeAll(", "); try self.writeInstRef(stream, extra.rhs); - try stream.writeAll(", "); - try self.writeInstRef(stream, extra.ptr); try stream.writeAll(")) "); try self.writeSrc(stream, src); } From bf87a1eb1771773d7dc49a1c0d87536b64c5142e Mon Sep 17 00:00:00 2001 From: Jan Philipp Hafer Date: Thu, 10 Feb 2022 08:11:37 +0100 Subject: [PATCH 2/2] todo: figure out encoding of AIR for the result --- src/BuiltinFn.zig | 6 ------ src/Sema.zig | 10 +++++++--- todos.txt | 31 +++++++++++++++++++++++++++++++ 3 files changed, 38 insertions(+), 9 deletions(-) create mode 100644 todos.txt diff --git a/src/BuiltinFn.zig b/src/BuiltinFn.zig index e34fecd43791..c10098124e16 100644 --- a/src/BuiltinFn.zig +++ b/src/BuiltinFn.zig @@ -145,9 +145,6 @@ pub const list = list: { @setEvalBranchQuota(3000); break :list std.ComptimeStringMap(@This(), .{ .{ - // changed from @addWithOverflow(comptime T: type, a: T, b: T, result: *T) bool //DEBUG - // to 1. RetType(ST) = struct {res: T, flag: u8}; //DEBUG - // 2. @addWithOverflow(comptime T: type, a: T, b: T) RetType //DEBUG "@addWithOverflow", .{ .tag = .add_with_overflow, @@ -599,7 +596,6 @@ pub const list = list: { }, }, .{ - // see @addWithOverflow // DEBUG "@mulWithOverflow", .{ .tag = .mul_with_overflow, @@ -705,7 +701,6 @@ pub const list = list: { }, }, .{ - // see @addWithOverflow // DEBUG "@shlWithOverflow", .{ .tag = .shl_with_overflow, @@ -847,7 +842,6 @@ pub const list = list: { }, }, .{ - // see @addWithOverflow // DEBUG "@subWithOverflow", .{ .tag = .sub_with_overflow, diff --git a/src/Sema.zig b/src/Sema.zig index 3b4f1c6f55e6..4dbb8edcafae 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -7974,15 +7974,13 @@ fn zirOverflowArithmetic( const lhs_src: LazySrcLoc = .{ .node_offset_builtin_call_arg0 = extra.node }; const rhs_src: LazySrcLoc = .{ .node_offset_builtin_call_arg1 = extra.node }; - const ptr_src: LazySrcLoc = .{ .node_offset_builtin_call_arg2 = extra.node }; const lhs = sema.resolveInst(extra.lhs); const rhs = sema.resolveInst(extra.rhs); - const ptr = sema.resolveInst(extra.ptr); const lhs_ty = sema.typeOf(lhs); - // Note, the types of lhs/rhs (also for shifting)/ptr are already correct as ensured by astgen. + // Note, the types of lhs/rhs (also for shifting) are already correct as ensured by astgen. const dest_ty = lhs_ty; if (dest_ty.zigTypeTag() != .Int) { return sema.fail(block, src, "expected integer type, found '{}'", .{dest_ty}); @@ -8117,6 +8115,8 @@ fn zirOverflowArithmetic( }; try sema.requireRuntimeBlock(block, src); + // TODO DEBUG FIXME + // figure out how stuff below is encoded to write it correctly return block.addInst(.{ .tag = air_tag, .data = .{ .pl_op = .{ @@ -8129,8 +8129,12 @@ fn zirOverflowArithmetic( }); }; + // TODO DEBUG FIXME + // this stuff is not needed try sema.storePtr2(block, src, ptr, ptr_src, result.wrapped, src, .store); + // TODO DEBUG FIXME + // figure out how to create the correct handle return switch (result.overflowed) { .yes => Air.Inst.Ref.bool_true, .no => Air.Inst.Ref.bool_false, diff --git a/todos.txt b/todos.txt new file mode 100644 index 000000000000..33561293a563 --- /dev/null +++ b/todos.txt @@ -0,0 +1,31 @@ +UNCHANGED src/arch/aarch64/CodeGen.zig:533:14: .add_with_overflow => try self.airAddWithOverflow(inst), +UNCHANGED src/arch/arm/CodeGen.zig:525:14: .add_with_overflow => try self.airAddWithOverflow(inst), +UNCHANGED src/arch/riscv64/CodeGen.zig:512:14: .add_with_overflow => try self.airAddWithOverflow(inst), +UNCHANGED src/arch/wasm/CodeGen.zig:1701:10: .add_with_overflow, +UNCHANGED src/arch/x86_64/CodeGen.zig:604:14: .add_with_overflow => try self.airAddWithOverflow(inst), +UNCHANGED src/codegen/c.zig:1451:14: .add_with_overflow => try airAddWithOverflow(f, inst), + +// changed from @addWithOverflow(comptime T: type, a: T, b: T, result: *T) bool +// to 1. RetType(ST) = struct {res: T, flag: u8}; +// 2. @addWithOverflow(comptime T: type, a: T, b: T) RetType +for add_with_overflow, sub_with_overflow, mul_with_overflow, shl_with_overflow +DONE src/BuiltinFn.zig:150:25: .tag = .add_with_overflow, +DONE src/BuiltinFn.zig:4:5: add_with_overflow, + + +DONE src/AstGen.zig:7183:10: .add_with_overflow => return overflowArithmetic(gz, scope, rl, node, params, .add_with_overflow), +DONE src/AstGen.zig:7183:87: .add_with_overflow => return overflowArithmetic(gz, scope, rl, node, params, .add_with_overflow), +DONE src/Zir.zig:1566:9: add_with_overflow, +DONE src/print_zir.zig:462:14: .add_with_overflow, +UNCHANGED src/Liveness.zig:420:10: .add_with_overflow, + +change the following inspirated by align_cast +TODO src/Sema.zig:1132:10: .add_with_overflow => return sema.zirOverflowArithmetic(block, extended, extended.opcode), +TODO src/Sema.zig:8001:14: .add_with_overflow => { +TODO src/Sema.zig:8112:14: .add_with_overflow => .add_with_overflow, +TODO src/Sema.zig:8112:36: .add_with_overflow => .add_with_overflow, +TODO src/codegen/llvm.zig:2042:18: .add_with_overflow => try self.airOverflow(inst, "llvm.sadd.with.overflow", "llvm.uadd.with.overflow"), + +DONE src/Air.zig:143:9: add_with_overflow, +DONE src/Air.zig:884:10: .add_with_overflow, +DONE? src/print_air.zig:240:14: .add_with_overflow,