aarch64: check lo/cc flag for unsigned sub_with_overflow

kubkon · kubkon · commit 97816e3cb826 · 2022-05-25T23:43:45.000+02:00
With this change, we are now correctly lowering `sub_with_overflow`
for signed and unsigned integers of register-sized integers (32-
or 64-bit precisely). We also match LLVM's behavior and so, the
condition flags we now set are:

* unsigned:
  - `add_with_overflow`: `hs`/`cs` (carry set)
  - `sub_with_overflow`: `lo`/`cc` (carry clear)
* signed:
  - `add_with_overflow`/`sub_with_overflow`: `vs` (overflow)
diff --git a/src/arch/aarch64/CodeGen.zig b/src/arch/aarch64/CodeGen.zig
@@ -128,18 +128,11 @@ const MCValue = union(enum) {
     register: Register,
     /// The value is a tuple { wrapped: u32, overflow: u1 } where
     /// wrapped is stored in the register and the overflow bit is
-    /// stored in the C flag of the CPSR.
+    /// stored in the C (signed) or V (unsigned) flag of the CPSR.
     ///
     /// This MCValue is only generated by a add_with_overflow or
-    /// sub_with_overflow instruction operating on u32.
-    register_c_flag: Register,
-    /// The value is a tuple { wrapped: i32, overflow: u1 } where
-    /// wrapped is stored in the register and the overflow bit is
-    /// stored in the V flag of the CPSR.
-    ///
-    /// This MCValue is only generated by a add_with_overflow or
-    /// sub_with_overflow instruction operating on i32.
-    register_v_flag: Register,
+    /// sub_with_overflow instruction operating on 32- or 64-bit values.
+    register_with_overflow: struct { reg: Register, flag: bits.Instruction.Condition },
     /// The value is in memory at a hard-coded address.
     ///
     /// If the type is a pointer, it means the pointer address is at
@@ -760,10 +753,8 @@ fn processDeath(self: *Self, inst: Air.Inst.Index) void {
         .register => |reg| {
             self.register_manager.freeReg(reg);
         },
-        .register_c_flag,
-        .register_v_flag,
-        => |reg| {
-            self.register_manager.freeReg(reg);
+        .register_with_overflow => |rwo| {
+            self.register_manager.freeReg(rwo.reg);
             self.compare_flags_inst = null;
         },
         .compare_flags_signed, .compare_flags_unsigned => {
@@ -905,10 +896,8 @@ pub fn spillInstruction(self: *Self, reg: Register, inst: Air.Inst.Index) !void
     log.debug("spilling {d} to stack mcv {any}", .{ inst, stack_mcv });
     const reg_mcv = self.getResolvedInstValue(inst);
     switch (reg_mcv) {
-        .register,
-        .register_c_flag,
-        .register_v_flag,
-        => |r| assert(reg.id() == r.id()),
+        .register => |r| assert(reg.id() == r.id()),
+        .register_with_overflow => |rwo| assert(rwo.reg.id() == reg.id()),
         else => unreachable, // not a register
     }
     const branch = &self.branch_stack.items[self.branch_stack.items.len - 1];
@@ -925,9 +914,7 @@ fn spillCompareFlagsIfOccupied(self: *Self) !void {
             .compare_flags_signed,
             .compare_flags_unsigned,
             => try self.allocRegOrMem(inst_to_save, true),
-            .register_c_flag,
-            .register_v_flag,
-            => try self.allocRegOrMem(inst_to_save, false),
+            .register_with_overflow => try self.allocRegOrMem(inst_to_save, false),
             else => unreachable, // mcv doesn't occupy the compare flags
         };
 
@@ -942,9 +929,7 @@ fn spillCompareFlagsIfOccupied(self: *Self) !void {
         // TODO consolidate with register manager and spillInstruction
         // this call should really belong in the register manager!
         switch (mcv) {
-            .register_c_flag,
-            .register_v_flag,
-            => |reg| self.register_manager.freeReg(reg),
+            .register_with_overflow => |rwo| self.register_manager.freeReg(rwo.reg),
             else => {},
         }
     }
@@ -1932,14 +1917,18 @@ fn airOverflow(self: *Self, inst: Air.Inst.Index) !void {
                             }
                         };
 
-                        if (tag == .sub_with_overflow) {
-                            break :result MCValue{ .register_v_flag = dest.register };
-                        }
-
-                        switch (int_info.signedness) {
-                            .unsigned => break :result MCValue{ .register_c_flag = dest.register },
-                            .signed => break :result MCValue{ .register_v_flag = dest.register },
-                        }
+                        const flag: bits.Instruction.Condition = switch (int_info.signedness) {
+                            .unsigned => switch (tag) {
+                                .add_with_overflow => bits.Instruction.Condition.cs,
+                                .sub_with_overflow => bits.Instruction.Condition.cc,
+                                else => unreachable,
+                            },
+                            .signed => .vs,
+                        };
+                        break :result MCValue{ .register_with_overflow = .{
+                            .reg = dest.register,
+                            .flag = flag,
+                        } };
                     },
                     else => return self.fail("TODO overflow operations on integers > u32/i32", .{}),
                 }
@@ -2648,8 +2637,7 @@ fn load(self: *Self, dst_mcv: MCValue, ptr: MCValue, ptr_ty: Type) InnerError!vo
         .dead => unreachable,
         .compare_flags_unsigned,
         .compare_flags_signed,
-        .register_c_flag,
-        .register_v_flag,
+        .register_with_overflow,
         => unreachable, // cannot hold an address
         .immediate => |imm| try self.setRegOrMem(elem_ty, dst_mcv, .{ .memory = imm }),
         .ptr_stack_offset => |off| try self.setRegOrMem(elem_ty, dst_mcv, .{ .stack_offset = off }),
@@ -2871,8 +2859,7 @@ fn store(self: *Self, ptr: MCValue, value: MCValue, ptr_ty: Type, value_ty: Type
         .dead => unreachable,
         .compare_flags_unsigned,
         .compare_flags_signed,
-        .register_c_flag,
-        .register_v_flag,
+        .register_with_overflow,
         => unreachable, // cannot hold an address
         .immediate => |imm| {
             try self.setRegOrMem(value_ty, .{ .memory = imm }, value);
@@ -2993,13 +2980,11 @@ fn airStructFieldVal(self: *Self, inst: Air.Inst.Index) !void {
             .memory => |addr| {
                 break :result MCValue{ .memory = addr + struct_field_offset };
             },
-            .register_c_flag,
-            .register_v_flag,
-            => |reg| {
+            .register_with_overflow => |rwo| {
                 switch (index) {
                     0 => {
                         // get wrapped value: return register
-                        break :result MCValue{ .register = reg };
+                        break :result MCValue{ .register = rwo.reg };
                     },
                     1 => {
                         // TODO return special MCValue condition flags
@@ -3008,17 +2993,11 @@ fn airStructFieldVal(self: *Self, inst: Air.Inst.Index) !void {
                         const raw_dest_reg = try self.register_manager.allocReg(null, gp);
                         const dest_reg = raw_dest_reg.to32();
 
-                        // C flag: cset reg, cs
-                        // V flag: cset reg, vs
                         _ = try self.addInst(.{
                             .tag = .cset,
                             .data = .{ .r_cond = .{
                                 .rd = dest_reg,
-                                .cond = switch (mcv) {
-                                    .register_c_flag => .cs,
-                                    .register_v_flag => .vs,
-                                    else => unreachable,
-                                },
+                                .cond = rwo.flag,
                             } },
                         });
 
@@ -4073,14 +4052,12 @@ fn genSetStack(self: *Self, ty: Type, stack_offset: u32, mcv: MCValue) InnerErro
                 else => return self.fail("TODO implement storing other types abi_size={}", .{abi_size}),
             }
         },
-        .register_c_flag,
-        .register_v_flag,
-        => |reg| {
-            const reg_lock = self.register_manager.lockReg(reg);
+        .register_with_overflow => |rwo| {
+            const reg_lock = self.register_manager.lockReg(rwo.reg);
             defer if (reg_lock) |locked_reg| self.register_manager.unlockReg(locked_reg);
 
             const wrapped_ty = ty.structFieldType(0);
-            try self.genSetStack(wrapped_ty, stack_offset, .{ .register = reg });
+            try self.genSetStack(wrapped_ty, stack_offset, .{ .register = rwo.reg });
 
             const overflow_bit_ty = ty.structFieldType(1);
             const overflow_bit_offset = @intCast(u32, ty.structFieldOffset(1, self.target.*));
@@ -4090,17 +4067,11 @@ fn genSetStack(self: *Self, ty: Type, stack_offset: u32, mcv: MCValue) InnerErro
                 @intCast(u32, overflow_bit_ty.abiSize(self.target.*)),
             );
 
-            // C flag: cset reg, cs
-            // V flag: cset reg, vs
             _ = try self.addInst(.{
                 .tag = .cset,
                 .data = .{ .r_cond = .{
                     .rd = cond_reg,
-                    .cond = switch (mcv) {
-                        .register_c_flag => .cs,
-                        .register_v_flag => .vs,
-                        else => unreachable,
-                    },
+                    .cond = rwo.flag,
                 } },
             });
 
@@ -4270,9 +4241,7 @@ fn genSetReg(self: *Self, ty: Type, reg: Register, mcv: MCValue) InnerError!void
                 .data = .{ .rr = .{ .rd = reg, .rn = src_reg } },
             });
         },
-        .register_c_flag,
-        .register_v_flag,
-        => unreachable, // doesn't fit into a register
+        .register_with_overflow => unreachable, // doesn't fit into a register
         .got_load,
         .direct_load,
         => |sym_index| {
