authorgravatar for david@vortan.devDavid Rubin <david@vortan.dev> 2024-03-25 05:15:02-07:00
committergravatar for david@vortan.devDavid Rubin <david@vortan.dev> 2024-05-11 02:17:11-07:00
logb28c966e332623dc43b1481b34016d18ce3262fa
treecd000795a0eee123ca83928ce0aedabcfff9165a
parente70584e2f87ae8daab18d9e28f72dac020d7702e

riscv: fix overflow checks in addition.

3 files changed, 61 insertions(+), 7 deletions(-)

src/arch/riscv64/CodeGen.zig+44-4
...@@ -1019,6 +1019,7 @@ fn binOpRegister(...@@ -1019,6 +1019,7 @@ fn binOpRegister(
1019 .add => .add,1019 .add => .add,
1020 .sub => .sub,1020 .sub => .sub,
1021 .cmp_eq => .cmp_eq,1021 .cmp_eq => .cmp_eq,
1022 .cmp_neq => .cmp_neq,
1022 .cmp_gt => .cmp_gt,1023 .cmp_gt => .cmp_gt,
1023 .cmp_gte => .cmp_gte,1024 .cmp_gte => .cmp_gte,
1024 .cmp_lt => .cmp_lt,1025 .cmp_lt => .cmp_lt,
...@@ -1185,6 +1186,8 @@ fn airAddWithOverflow(self: *Self, inst: Air.Inst.Index) !void {...@@ -1185,6 +1186,8 @@ fn airAddWithOverflow(self: *Self, inst: Air.Inst.Index) !void {
1185 const rhs_ty = self.typeOf(extra.rhs);1186 const rhs_ty = self.typeOf(extra.rhs);
11861187
1187 const add_result_mcv = try self.binOp(.add, null, lhs, rhs, lhs_ty, rhs_ty);1188 const add_result_mcv = try self.binOp(.add, null, lhs, rhs, lhs_ty, rhs_ty);
1189 const add_result_lock = self.register_manager.lockRegAssumeUnused(add_result_mcv.register);
1190 defer self.register_manager.unlockReg(add_result_lock);
11881191
1189 const tuple_ty = self.typeOfIndex(inst);1192 const tuple_ty = self.typeOfIndex(inst);
1190 const int_info = lhs_ty.intInfo(mod);1193 const int_info = lhs_ty.intInfo(mod);
...@@ -1196,15 +1199,44 @@ fn airAddWithOverflow(self: *Self, inst: Air.Inst.Index) !void {...@@ -1196,15 +1199,44 @@ fn airAddWithOverflow(self: *Self, inst: Air.Inst.Index) !void {
11961199
1197 const result_offset = tuple_ty.structFieldOffset(0, mod) + offset;1200 const result_offset = tuple_ty.structFieldOffset(0, mod) + offset;
11981201
1199 // set the result first as we don't have a lock on the add_result_mcv register and it will
1200 // get clobbered in the next binOp.
1201 try self.genSetStack(lhs_ty, @intCast(result_offset), add_result_mcv);1202 try self.genSetStack(lhs_ty, @intCast(result_offset), add_result_mcv);
12021203
1203 if (int_info.bits >= 8 and math.isPowerOfTwo(int_info.bits)) {1204 if (int_info.bits >= 8 and math.isPowerOfTwo(int_info.bits)) {
1204 if (int_info.signedness == .unsigned) {1205 if (int_info.signedness == .unsigned) {
1205 const overflow_offset = tuple_ty.structFieldOffset(1, mod) + offset;1206 const overflow_offset = tuple_ty.structFieldOffset(1, mod) + offset;
12061207
1207 const overflow_mcv = try self.binOp(.cmp_lt, null, add_result_mcv, lhs, lhs_ty, lhs_ty);1208 const max_val = std.math.pow(u16, 2, int_info.bits) - 1;
1209
1210 const overflow_reg, const overflow_lock = try self.allocReg();
1211 defer self.register_manager.unlockReg(overflow_lock);
1212
1213 const add_reg, const add_lock = blk: {
1214 if (add_result_mcv == .register) break :blk .{ add_result_mcv.register, null };
1215
1216 const add_reg, const add_lock = try self.allocReg();
1217 try self.genSetReg(lhs_ty, add_reg, add_result_mcv);
1218 break :blk .{ add_reg, add_lock };
1219 };
1220 defer if (add_lock) |lock| self.register_manager.unlockReg(lock);
1221
1222 _ = try self.addInst(.{
1223 .tag = .andi,
1224 .data = .{ .i_type = .{
1225 .rd = overflow_reg,
1226 .rs1 = add_reg,
1227 .imm12 = @intCast(max_val),
1228 } },
1229 });
1230
1231 const overflow_mcv = try self.binOp(
1232 .cmp_neq,
1233 null,
1234 .{ .register = overflow_reg },
1235 .{ .register = add_reg },
1236 lhs_ty,
1237 lhs_ty,
1238 );
1239
1208 try self.genSetStack(Type.u1, @intCast(overflow_offset), overflow_mcv);1240 try self.genSetStack(Type.u1, @intCast(overflow_offset), overflow_mcv);
12091241
1210 break :result result_mcv;1242 break :result result_mcv;
...@@ -3042,7 +3074,15 @@ fn genSetReg(self: *Self, ty: Type, reg: Register, src_val: MCValue) InnerError!...@@ -3042,7 +3074,15 @@ fn genSetReg(self: *Self, ty: Type, reg: Register, src_val: MCValue) InnerError!
30423074
3043fn airIntFromPtr(self: *Self, inst: Air.Inst.Index) !void {3075fn airIntFromPtr(self: *Self, inst: Air.Inst.Index) !void {
3044 const un_op = self.air.instructions.items(.data)[@intFromEnum(inst)].un_op;3076 const un_op = self.air.instructions.items(.data)[@intFromEnum(inst)].un_op;
3045 const result = try self.resolveInst(un_op);3077 const result = result: {
3078 const src_mcv = try self.resolveInst(un_op);
3079 if (self.reuseOperand(inst, un_op, 0, src_mcv)) break :result src_mcv;
3080
3081 const dst_mcv = try self.allocRegOrMem(inst, true);
3082 const dst_ty = self.typeOfIndex(inst);
3083 try self.setValue(dst_ty, dst_mcv, src_mcv);
3084 break :result dst_mcv;
3085 };
3046 return self.finishAir(inst, result, .{ un_op, .none, .none });3086 return self.finishAir(inst, result, .{ un_op, .none, .none });
3047}3087}
30483088
src/arch/riscv64/Emit.zig+12-3
...@@ -58,6 +58,7 @@ pub fn emitMir(...@@ -58,6 +58,7 @@ pub fn emitMir(
58 .@"or" => try emit.mirRType(inst),58 .@"or" => try emit.mirRType(inst),
5959
60 .cmp_eq => try emit.mirRType(inst),60 .cmp_eq => try emit.mirRType(inst),
61 .cmp_neq => try emit.mirRType(inst),
61 .cmp_gt => try emit.mirRType(inst),62 .cmp_gt => try emit.mirRType(inst),
62 .cmp_gte => try emit.mirRType(inst),63 .cmp_gte => try emit.mirRType(inst),
63 .cmp_lt => try emit.mirRType(inst),64 .cmp_lt => try emit.mirRType(inst),
...@@ -68,6 +69,7 @@ pub fn emitMir(...@@ -68,6 +69,7 @@ pub fn emitMir(
68 .bne => try emit.mirBType(inst),69 .bne => try emit.mirBType(inst),
6970
70 .addi => try emit.mirIType(inst),71 .addi => try emit.mirIType(inst),
72 .andi => try emit.mirIType(inst),
71 .jalr => try emit.mirIType(inst),73 .jalr => try emit.mirIType(inst),
72 .abs => try emit.mirIType(inst),74 .abs => try emit.mirIType(inst),
7375
...@@ -201,10 +203,14 @@ fn mirRType(emit: *Emit, inst: Mir.Inst.Index) !void {...@@ -201,10 +203,14 @@ fn mirRType(emit: *Emit, inst: Mir.Inst.Index) !void {
201 .cmp_eq => {203 .cmp_eq => {
202 // rs1 == rs2204 // rs1 == rs2
203205
204 // if equal, write 0 to rd
205 try emit.writeInstruction(Instruction.xor(rd, rs1, rs2));206 try emit.writeInstruction(Instruction.xor(rd, rs1, rs2));
206 // if rd == 0, set rd to 1207 try emit.writeInstruction(Instruction.sltiu(rd, rd, 1)); // seqz
207 try emit.writeInstruction(Instruction.sltiu(rd, rd, 1));208 },
209 .cmp_neq => {
210 // rs1 != rs2
211
212 try emit.writeInstruction(Instruction.xor(rd, rs1, rs2));
213 try emit.writeInstruction(Instruction.sltu(rd, .x0, rd)); // snez
208 },214 },
209 .cmp_lt => {215 .cmp_lt => {
210 // rd = 1 if rs1 < rs2216 // rd = 1 if rs1 < rs2
...@@ -255,6 +261,8 @@ fn mirIType(emit: *Emit, inst: Mir.Inst.Index) !void {...@@ -255,6 +261,8 @@ fn mirIType(emit: *Emit, inst: Mir.Inst.Index) !void {
255 .addi => try emit.writeInstruction(Instruction.addi(rd, rs1, imm12)),261 .addi => try emit.writeInstruction(Instruction.addi(rd, rs1, imm12)),
256 .jalr => try emit.writeInstruction(Instruction.jalr(rd, imm12, rs1)),262 .jalr => try emit.writeInstruction(Instruction.jalr(rd, imm12, rs1)),
257263
264 .andi => try emit.writeInstruction(Instruction.andi(rd, rs1, imm12)),
265
258 .ld => try emit.writeInstruction(Instruction.ld(rd, imm12, rs1)),266 .ld => try emit.writeInstruction(Instruction.ld(rd, imm12, rs1)),
259 .lw => try emit.writeInstruction(Instruction.lw(rd, imm12, rs1)),267 .lw => try emit.writeInstruction(Instruction.lw(rd, imm12, rs1)),
260 .lh => try emit.writeInstruction(Instruction.lh(rd, imm12, rs1)),268 .lh => try emit.writeInstruction(Instruction.lh(rd, imm12, rs1)),
...@@ -515,6 +523,7 @@ fn instructionSize(emit: *Emit, inst: Mir.Inst.Index) usize {...@@ -515,6 +523,7 @@ fn instructionSize(emit: *Emit, inst: Mir.Inst.Index) usize {
515 => 12,523 => 12,
516524
517 .cmp_eq,525 .cmp_eq,
526 .cmp_neq,
518 .cmp_imm_eq,527 .cmp_imm_eq,
519 .cmp_gte,528 .cmp_gte,
520 .load_symbol,529 .load_symbol,
src/arch/riscv64/Mir.zig+5
...@@ -57,9 +57,14 @@ pub const Inst = struct {...@@ -57,9 +57,14 @@ pub const Inst = struct {
57 /// Jumps. Uses `inst` payload.57 /// Jumps. Uses `inst` payload.
58 j,58 j,
5959
60 /// Immediate and, uses i_type payload
61 andi,
62
60 // NOTE: Maybe create a special data for compares that includes the ops63 // NOTE: Maybe create a special data for compares that includes the ops
61 /// Register `==`, uses r_type64 /// Register `==`, uses r_type
62 cmp_eq,65 cmp_eq,
66 /// Register `!=`, uses r_type
67 cmp_neq,
63 /// Register `>`, uses r_type68 /// Register `>`, uses r_type
64 cmp_gt,69 cmp_gt,
65 /// Register `<`, uses r_type70 /// Register `<`, uses r_type