Skip to content

builtins: overflow arithmetic perf optimization #10854

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/Air.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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),
}
}

Expand Down
22 changes: 0 additions & 22 deletions src/AstGen.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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);
},
Expand Down Expand Up @@ -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);
}
Expand Down
8 changes: 4 additions & 4 deletions src/BuiltinFn.zig
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ pub const list = list: {
"@addWithOverflow",
.{
.tag = .add_with_overflow,
.param_count = 4,
.param_count = 3,
},
},
.{
Expand Down Expand Up @@ -599,7 +599,7 @@ pub const list = list: {
"@mulWithOverflow",
.{
.tag = .mul_with_overflow,
.param_count = 4,
.param_count = 3,
},
},
.{
Expand Down Expand Up @@ -704,7 +704,7 @@ pub const list = list: {
"@shlWithOverflow",
.{
.tag = .shl_with_overflow,
.param_count = 4,
.param_count = 3,
},
},
.{
Expand Down Expand Up @@ -845,7 +845,7 @@ pub const list = list: {
"@subWithOverflow",
.{
.tag = .sub_with_overflow,
.param_count = 4,
.param_count = 3,
},
},
.{
Expand Down
10 changes: 7 additions & 3 deletions src/Sema.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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});
Expand Down Expand Up @@ -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 = .{
Expand All @@ -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,
Expand Down
1 change: 0 additions & 1 deletion src/Zir.zig
Original file line number Diff line number Diff line change
Expand Up @@ -2843,7 +2843,6 @@ pub const Inst = struct {
node: i32,
lhs: Ref,
rhs: Ref,
ptr: Ref,
};

pub const Cmpxchg = struct {
Expand Down
2 changes: 0 additions & 2 deletions src/print_zir.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
31 changes: 31 additions & 0 deletions todos.txt
Original file line number Diff line number Diff line change
@@ -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,