Commit ac2333ee63

Andrew Kelley <andrew@ziglang.org>
2021-10-04 21:21:31
stage2: fix Type max/min int calculation
This was an attempt to move saturating_arithmetic.zig to the "passing for stage2" section, which did not pan out due to the discovery of 2 prerequisite items that need to be done, but I did make a bug fix along the way of the calculation of max/min integers. This commit also simplifies the saturating arithmetic behavior tests to depend on less of the zig language that is not related to saturating arithmetic.
1 parent a28f2e0
src/type.zig
@@ -3096,7 +3096,7 @@ pub const Type = extern union {
             return Value.initTag(.zero);
         }
 
-        if ((info.bits - 1) <= std.math.maxInt(u6)) {
+        if (info.bits <= 6) {
             const n: i64 = -(@as(i64, 1) << @truncate(u6, info.bits - 1));
             return Value.Tag.int_i64.create(arena, n);
         }
@@ -3117,13 +3117,16 @@ pub const Type = extern union {
         assert(self.zigTypeTag() == .Int);
         const info = self.intInfo(target);
 
-        if (info.signedness == .signed and (info.bits - 1) <= std.math.maxInt(u6)) {
-            const n: i64 = (@as(i64, 1) << @truncate(u6, info.bits - 1)) - 1;
-            return Value.Tag.int_i64.create(arena, n);
-        } else if (info.signedness == .signed and info.bits <= std.math.maxInt(u6)) {
-            const n: u64 = (@as(u64, 1) << @truncate(u6, info.bits)) - 1;
-            return Value.Tag.int_u64.create(arena, n);
-        }
+        if (info.bits <= 6) switch (info.signedness) {
+            .signed => {
+                const n: i64 = (@as(i64, 1) << @truncate(u6, info.bits - 1)) - 1;
+                return Value.Tag.int_i64.create(arena, n);
+            },
+            .unsigned => {
+                const n: u64 = (@as(u64, 1) << @truncate(u6, info.bits)) - 1;
+                return Value.Tag.int_u64.create(arena, n);
+            },
+        };
 
         var res = try std.math.big.int.Managed.init(arena);
         try res.setTwosCompIntLimit(.max, info.signedness, info.bits);
test/behavior/saturating_arithmetic.zig
@@ -1,58 +1,32 @@
 const std = @import("std");
 const builtin = @import("builtin");
-const mem = std.mem;
-const expectEqual = std.testing.expectEqual;
-const Vector = std.meta.Vector;
 const minInt = std.math.minInt;
 const maxInt = std.math.maxInt;
-
-const Op = enum { add, sub, mul, shl };
-fn testSaturatingOp(comptime op: Op, comptime T: type, test_data: [3]T) !void {
-    const a = test_data[0];
-    const b = test_data[1];
-    const expected = test_data[2];
-    {
-        const actual = switch (op) {
-            .add => a +| b,
-            .sub => a -| b,
-            .mul => a *| b,
-            .shl => a <<| b,
-        };
-        try expectEqual(expected, actual);
-    }
-    {
-        var actual = a;
-        switch (op) {
-            .add => actual +|= b,
-            .sub => actual -|= b,
-            .mul => actual *|= b,
-            .shl => actual <<|= b,
-        }
-        try expectEqual(expected, actual);
-    }
-}
+const expect = std.testing.expect;
 
 test "saturating add" {
     const S = struct {
         fn doTheTest() !void {
-            //                             .{a, b, expected a+b}
-            try testSaturatingOp(.add, i8, .{ -3, 10, 7 });
-            try testSaturatingOp(.add, i8, .{ -128, -128, -128 });
-            try testSaturatingOp(.add, i2, .{ 1, 1, 1 });
-            try testSaturatingOp(.add, i64, .{ maxInt(i64), 1, maxInt(i64) });
-            try testSaturatingOp(.add, i128, .{ maxInt(i128), -maxInt(i128), 0 });
-            try testSaturatingOp(.add, i128, .{ minInt(i128), maxInt(i128), -1 });
-            try testSaturatingOp(.add, i8, .{ 127, 127, 127 });
-            try testSaturatingOp(.add, u8, .{ 3, 10, 13 });
-            try testSaturatingOp(.add, u8, .{ 255, 255, 255 });
-            try testSaturatingOp(.add, u2, .{ 3, 2, 3 });
-            try testSaturatingOp(.add, u3, .{ 7, 1, 7 });
-            try testSaturatingOp(.add, u128, .{ maxInt(u128), 1, maxInt(u128) });
+            try testSatAdd(i8, -3, 10, 7);
+            try testSatAdd(i8, -128, -128, -128);
+            try testSatAdd(i2, 1, 1, 1);
+            try testSatAdd(i64, maxInt(i64), 1, maxInt(i64));
+            try testSatAdd(i128, maxInt(i128), -maxInt(i128), 0);
+            try testSatAdd(i128, minInt(i128), maxInt(i128), -1);
+            try testSatAdd(i8, 127, 127, 127);
+            try testSatAdd(u8, 3, 10, 13);
+            try testSatAdd(u8, 255, 255, 255);
+            try testSatAdd(u2, 3, 2, 3);
+            try testSatAdd(u3, 7, 1, 7);
+            try testSatAdd(u128, maxInt(u128), 1, maxInt(u128));
+        }
+
+        fn testSatAdd(comptime T: type, lhs: T, rhs: T, expected: T) !void {
+            try expect((lhs +| rhs) == expected);
 
-            const u8x3 = std.meta.Vector(3, u8);
-            try expectEqual(u8x3{ 255, 255, 255 }, (u8x3{ 255, 254, 1 } +| u8x3{ 1, 2, 255 }));
-            const i8x3 = std.meta.Vector(3, i8);
-            try expectEqual(i8x3{ 127, 127, 127 }, (i8x3{ 127, 126, 1 } +| i8x3{ 1, 2, 127 }));
+            var x = lhs;
+            x +|= rhs;
+            try expect(x == expected);
         }
     };
     try S.doTheTest();
@@ -62,20 +36,24 @@ test "saturating add" {
 test "saturating subtraction" {
     const S = struct {
         fn doTheTest() !void {
-            //                             .{a, b, expected a-b}
-            try testSaturatingOp(.sub, i8, .{ -3, 10, -13 });
-            try testSaturatingOp(.sub, i8, .{ -128, -128, 0 });
-            try testSaturatingOp(.sub, i8, .{ -1, 127, -128 });
-            try testSaturatingOp(.sub, i64, .{ minInt(i64), 1, minInt(i64) });
-            try testSaturatingOp(.sub, i128, .{ maxInt(i128), -1, maxInt(i128) });
-            try testSaturatingOp(.sub, i128, .{ minInt(i128), -maxInt(i128), -1 });
-            try testSaturatingOp(.sub, u8, .{ 10, 3, 7 });
-            try testSaturatingOp(.sub, u8, .{ 0, 255, 0 });
-            try testSaturatingOp(.sub, u5, .{ 0, 31, 0 });
-            try testSaturatingOp(.sub, u128, .{ 0, maxInt(u128), 0 });
+            try testSatSub(i8, -3, 10, -13);
+            try testSatSub(i8, -128, -128, 0);
+            try testSatSub(i8, -1, 127, -128);
+            try testSatSub(i64, minInt(i64), 1, minInt(i64));
+            try testSatSub(i128, maxInt(i128), -1, maxInt(i128));
+            try testSatSub(i128, minInt(i128), -maxInt(i128), -1);
+            try testSatSub(u8, 10, 3, 7);
+            try testSatSub(u8, 0, 255, 0);
+            try testSatSub(u5, 0, 31, 0);
+            try testSatSub(u128, 0, maxInt(u128), 0);
+        }
+
+        fn testSatSub(comptime T: type, lhs: T, rhs: T, expected: T) !void {
+            try expect((lhs -| rhs) == expected);
 
-            const u8x3 = std.meta.Vector(3, u8);
-            try expectEqual(u8x3{ 0, 0, 0 }, (u8x3{ 0, 0, 0 } -| u8x3{ 255, 255, 255 }));
+            var x = lhs;
+            x -|= rhs;
+            try expect(x == expected);
         }
     };
     try S.doTheTest();
@@ -84,26 +62,29 @@ test "saturating subtraction" {
 
 test "saturating multiplication" {
     // TODO: once #9660 has been solved, remove this line
-    if (std.builtin.target.cpu.arch == .wasm32) return error.SkipZigTest;
+    if (builtin.stage2_arch == .wasm32) return error.SkipZigTest;
 
     const S = struct {
         fn doTheTest() !void {
-            //                             .{a, b, expected a*b}
-            try testSaturatingOp(.mul, i8, .{ -3, 10, -30 });
-            try testSaturatingOp(.mul, i4, .{ 2, 4, 7 });
-            try testSaturatingOp(.mul, i8, .{ 2, 127, 127 });
-            // TODO: uncomment these after #9643 has been solved - this should happen at 0.9.0/llvm-13 release
-            // try testSaturatingOp(.mul, i8, .{ -128, -128, 127 });
-            // try testSaturatingOp(.mul, i8, .{ maxInt(i8), maxInt(i8), maxInt(i8) });
-            try testSaturatingOp(.mul, i16, .{ maxInt(i16), -1, minInt(i16) + 1 });
-            try testSaturatingOp(.mul, i128, .{ maxInt(i128), -1, minInt(i128) + 1 });
-            try testSaturatingOp(.mul, i128, .{ minInt(i128), -1, maxInt(i128) });
-            try testSaturatingOp(.mul, u8, .{ 10, 3, 30 });
-            try testSaturatingOp(.mul, u8, .{ 2, 255, 255 });
-            try testSaturatingOp(.mul, u128, .{ maxInt(u128), maxInt(u128), maxInt(u128) });
+            try testSatMul(i8, -3, 10, -30);
+            try testSatMul(i4, 2, 4, 7);
+            try testSatMul(i8, 2, 127, 127);
+            try testSatMul(i8, -128, -128, 127);
+            try testSatMul(i8, maxInt(i8), maxInt(i8), maxInt(i8));
+            try testSatMul(i16, maxInt(i16), -1, minInt(i16) + 1);
+            try testSatMul(i128, maxInt(i128), -1, minInt(i128) + 1);
+            try testSatMul(i128, minInt(i128), -1, maxInt(i128));
+            try testSatMul(u8, 10, 3, 30);
+            try testSatMul(u8, 2, 255, 255);
+            try testSatMul(u128, maxInt(u128), maxInt(u128), maxInt(u128));
+        }
 
-            const u8x3 = std.meta.Vector(3, u8);
-            try expectEqual(u8x3{ 255, 255, 255 }, (u8x3{ 2, 2, 2 } *| u8x3{ 255, 255, 255 }));
+        fn testSatMul(comptime T: type, lhs: T, rhs: T, expected: T) !void {
+            try expect((lhs *| rhs) == expected);
+
+            var x = lhs;
+            x *|= rhs;
+            try expect(x == expected);
         }
     };
 
@@ -114,21 +95,24 @@ test "saturating multiplication" {
 test "saturating shift-left" {
     const S = struct {
         fn doTheTest() !void {
-            //                             .{a, b, expected a<<b}
-            try testSaturatingOp(.shl, i8, .{ 1, 2, 4 });
-            try testSaturatingOp(.shl, i8, .{ 127, 1, 127 });
-            try testSaturatingOp(.shl, i8, .{ -128, 1, -128 });
+            try testSatShl(i8, 1, 2, 4);
+            try testSatShl(i8, 127, 1, 127);
+            try testSatShl(i8, -128, 1, -128);
             // TODO: remove this check once #9668 is completed
-            if (std.builtin.target.cpu.arch != .wasm32) {
+            if (builtin.stage2_arch != .wasm32) {
                 // skip testing ints > 64 bits on wasm due to miscompilation / wasmtime ci error
-                try testSaturatingOp(.shl, i128, .{ maxInt(i128), 64, maxInt(i128) });
-                try testSaturatingOp(.shl, u128, .{ maxInt(u128), 64, maxInt(u128) });
+                try testSatShl(i128, maxInt(i128), 64, maxInt(i128));
+                try testSatShl(u128, maxInt(u128), 64, maxInt(u128));
             }
-            try testSaturatingOp(.shl, u8, .{ 1, 2, 4 });
-            try testSaturatingOp(.shl, u8, .{ 255, 1, 255 });
+            try testSatShl(u8, 1, 2, 4);
+            try testSatShl(u8, 255, 1, 255);
+        }
+        fn testSatShl(comptime T: type, lhs: T, rhs: T, expected: T) !void {
+            try expect((lhs <<| rhs) == expected);
 
-            const u8x3 = std.meta.Vector(3, u8);
-            try expectEqual(u8x3{ 255, 255, 255 }, (u8x3{ 255, 255, 255 } <<| u8x3{ 1, 1, 1 }));
+            var x = lhs;
+            x <<|= rhs;
+            try expect(x == expected);
         }
     };
     try S.doTheTest();
test/behavior/vector.zig
@@ -647,3 +647,53 @@ test "mask parameter of @shuffle is comptime scope" {
     });
     _ = shuffled;
 }
+
+test "saturating add" {
+    const S = struct {
+        fn doTheTest() !void {
+            const u8x3 = std.meta.Vector(3, u8);
+            try expectEqual(u8x3{ 255, 255, 255 }, (u8x3{ 255, 254, 1 } +| u8x3{ 1, 2, 255 }));
+            const i8x3 = std.meta.Vector(3, i8);
+            try expectEqual(i8x3{ 127, 127, 127 }, (i8x3{ 127, 126, 1 } +| i8x3{ 1, 2, 127 }));
+        }
+    };
+    try S.doTheTest();
+    comptime try S.doTheTest();
+}
+
+test "saturating subtraction" {
+    const S = struct {
+        fn doTheTest() !void {
+            const u8x3 = std.meta.Vector(3, u8);
+            try expectEqual(u8x3{ 0, 0, 0 }, (u8x3{ 0, 0, 0 } -| u8x3{ 255, 255, 255 }));
+        }
+    };
+    try S.doTheTest();
+    comptime try S.doTheTest();
+}
+
+test "saturating multiplication" {
+    // TODO: once #9660 has been solved, remove this line
+    if (std.builtin.target.cpu.arch == .wasm32) return error.SkipZigTest;
+
+    const S = struct {
+        fn doTheTest() !void {
+            const u8x3 = std.meta.Vector(3, u8);
+            try expectEqual(u8x3{ 255, 255, 255 }, (u8x3{ 2, 2, 2 } *| u8x3{ 255, 255, 255 }));
+        }
+    };
+
+    try S.doTheTest();
+    comptime try S.doTheTest();
+}
+
+test "saturating shift-left" {
+    const S = struct {
+        fn doTheTest() !void {
+            const u8x3 = std.meta.Vector(3, u8);
+            try expectEqual(u8x3{ 255, 255, 255 }, (u8x3{ 255, 255, 255 } <<| u8x3{ 1, 1, 1 }));
+        }
+    };
+    try S.doTheTest();
+    comptime try S.doTheTest();
+}
test/behavior.zig
@@ -34,8 +34,8 @@ test {
     _ = @import("behavior/underscore.zig");
     _ = @import("behavior/union.zig");
     _ = @import("behavior/usingnamespace.zig");
-    _ = @import("behavior/widening.zig");
     _ = @import("behavior/while.zig");
+    _ = @import("behavior/widening.zig");
 
     if (builtin.zig_is_stage2) {
         // When all comptime_memory.zig tests pass, #9646 can be closed.
@@ -140,7 +140,12 @@ test {
         _ = @import("behavior/pub_enum.zig");
         _ = @import("behavior/ref_var_in_if_after_if_2nd_switch_prong.zig");
         _ = @import("behavior/reflection.zig");
-        _ = @import("behavior/saturating_arithmetic.zig");
+        {
+            // Checklist for getting saturating_arithmetic.zig passing for stage2:
+            // * add __muloti4 to compiler-rt
+            // * implement comptime saturating shift-left
+            _ = @import("behavior/saturating_arithmetic.zig");
+        }
         _ = @import("behavior/shuffle.zig");
         _ = @import("behavior/select.zig");
         _ = @import("behavior/sizeof_and_typeof_stage1.zig");