authorgravatar for 94326797+riverbl@users.noreply.github.comriverbl <94326797+riverbl@users.noreply.github.com> 2021-12-21 12:45:48+00:00
committergravatar for git@vexu.euVeikka Tuominen <git@vexu.eu> 2022-01-29 18:12:28+02:00
logaa29f4a8037ae74fb2d97793312ef8c5262d025a
tree440bd0236e200476ba5292b2d661a6546907fbb4
parentaca665cebd2b6ec9ec3db669cf5446ee45bbb5d0

stage1: fix saturating arithmetic producing incorrect results on type comptime_int, allow saturating left shift on type comptime int

3 files changed, 159 insertions(+), 15 deletions(-)

src/stage1/ir.cpp+23-15
...@@ -10230,13 +10230,7 @@ static Stage1AirInst *ir_analyze_bit_shift(IrAnalyze *ira, Stage1ZirInstBinOp *b...@@ -10230,13 +10230,7 @@ static Stage1AirInst *ir_analyze_bit_shift(IrAnalyze *ira, Stage1ZirInstBinOp *b
10230 // comptime_int has no finite bit width10230 // comptime_int has no finite bit width
10231 casted_op2 = op2;10231 casted_op2 = op2;
1023210232
10233 if (op_id == IrBinOpShlSat) {10233 if (op_id == IrBinOpBitShiftLeftLossy || op_id == IrBinOpShlSat) {
10234 ir_add_error_node(ira, bin_op_instruction->base.source_node,
10235 buf_sprintf("saturating shift on a comptime_int which has unlimited bits"));
10236 return ira->codegen->invalid_inst_gen;
10237 }
10238
10239 if (op_id == IrBinOpBitShiftLeftLossy) {
10240 op_id = IrBinOpBitShiftLeftExact;10234 op_id = IrBinOpBitShiftLeftExact;
10241 }10235 }
1024210236
...@@ -10398,6 +10392,25 @@ static bool ok_float_op(IrBinOp op) {...@@ -10398,6 +10392,25 @@ static bool ok_float_op(IrBinOp op) {
10398 zig_unreachable();10392 zig_unreachable();
10399}10393}
1040010394
10395static IrBinOp map_comptime_arithmetic_op(IrBinOp op) {
10396 switch (op) {
10397 case IrBinOpAddWrap:
10398 case IrBinOpAddSat:
10399 return IrBinOpAdd;
10400
10401 case IrBinOpSubWrap:
10402 case IrBinOpSubSat:
10403 return IrBinOpSub;
10404
10405 case IrBinOpMultWrap:
10406 case IrBinOpMultSat:
10407 return IrBinOpMult;
10408
10409 default:
10410 return op;
10411 }
10412}
10413
10401static bool is_pointer_arithmetic_allowed(ZigType *lhs_type, IrBinOp op) {10414static bool is_pointer_arithmetic_allowed(ZigType *lhs_type, IrBinOp op) {
10402 switch (op) {10415 switch (op) {
10403 case IrBinOpAdd:10416 case IrBinOpAdd:
...@@ -10620,15 +10633,10 @@ static Stage1AirInst *ir_analyze_bin_op_math(IrAnalyze *ira, Stage1ZirInstBinOp...@@ -10620,15 +10633,10 @@ static Stage1AirInst *ir_analyze_bin_op_math(IrAnalyze *ira, Stage1ZirInstBinOp
10620 if (type_is_invalid(casted_op2->value->type))10633 if (type_is_invalid(casted_op2->value->type))
10621 return ira->codegen->invalid_inst_gen;10634 return ira->codegen->invalid_inst_gen;
1062210635
10623 // Comptime integers have no fixed size10636 // Comptime integers have no fixed size, so wrapping or saturating operations should be mapped
10637 // to their non wrapping or saturating equivalents
10624 if (scalar_type->id == ZigTypeIdComptimeInt) {10638 if (scalar_type->id == ZigTypeIdComptimeInt) {
10625 if (op_id == IrBinOpAddWrap) {10639 op_id = map_comptime_arithmetic_op(op_id);
10626 op_id = IrBinOpAdd;
10627 } else if (op_id == IrBinOpSubWrap) {
10628 op_id = IrBinOpSub;
10629 } else if (op_id == IrBinOpMultWrap) {
10630 op_id = IrBinOpMult;
10631 }
10632 }10640 }
1063310641
10634 if (instr_is_comptime(casted_op1) && instr_is_comptime(casted_op2)) {10642 if (instr_is_comptime(casted_op1) && instr_is_comptime(casted_op2)) {
test/behavior/saturating_arithmetic.zig+26
...@@ -29,8 +29,14 @@ test "saturating add" {...@@ -29,8 +29,14 @@ test "saturating add" {
29 try expect(x == expected);29 try expect(x == expected);
30 }30 }
31 };31 };
32
32 try S.doTheTest();33 try S.doTheTest();
33 comptime try S.doTheTest();34 comptime try S.doTheTest();
35
36 comptime try S.testSatAdd(comptime_int, 0, 0, 0);
37 comptime try S.testSatAdd(comptime_int, 3, 2, 5);
38 comptime try S.testSatAdd(comptime_int, 651075816498665588400716961808225370057, 468229432685078038144554201546849378455, 1119305249183743626545271163355074748512);
39 comptime try S.testSatAdd(comptime_int, 7, -593423721213448152027139550640105366508, -593423721213448152027139550640105366501);
34}40}
3541
36test "saturating subtraction" {42test "saturating subtraction" {
...@@ -56,8 +62,14 @@ test "saturating subtraction" {...@@ -56,8 +62,14 @@ test "saturating subtraction" {
56 try expect(x == expected);62 try expect(x == expected);
57 }63 }
58 };64 };
65
59 try S.doTheTest();66 try S.doTheTest();
60 comptime try S.doTheTest();67 comptime try S.doTheTest();
68
69 comptime try S.testSatSub(comptime_int, 0, 0, 0);
70 comptime try S.testSatSub(comptime_int, 3, 2, 1);
71 comptime try S.testSatSub(comptime_int, 651075816498665588400716961808225370057, 468229432685078038144554201546849378455, 182846383813587550256162760261375991602);
72 comptime try S.testSatSub(comptime_int, 7, -593423721213448152027139550640105366508, 593423721213448152027139550640105366515);
61}73}
6274
63test "saturating multiplication" {75test "saturating multiplication" {
...@@ -90,6 +102,11 @@ test "saturating multiplication" {...@@ -90,6 +102,11 @@ test "saturating multiplication" {
90102
91 try S.doTheTest();103 try S.doTheTest();
92 comptime try S.doTheTest();104 comptime try S.doTheTest();
105
106 comptime try S.testSatMul(comptime_int, 0, 0, 0);
107 comptime try S.testSatMul(comptime_int, 3, 2, 6);
108 comptime try S.testSatMul(comptime_int, 651075816498665588400716961808225370057, 468229432685078038144554201546849378455, 304852860194144160265083087140337419215516305999637969803722975979232817921935);
109 comptime try S.testSatMul(comptime_int, 7, -593423721213448152027139550640105366508, -4153966048494137064189976854480737565556);
93}110}
94111
95test "saturating shift-left" {112test "saturating shift-left" {
...@@ -107,6 +124,7 @@ test "saturating shift-left" {...@@ -107,6 +124,7 @@ test "saturating shift-left" {
107 try testSatShl(u8, 1, 2, 4);124 try testSatShl(u8, 1, 2, 4);
108 try testSatShl(u8, 255, 1, 255);125 try testSatShl(u8, 255, 1, 255);
109 }126 }
127
110 fn testSatShl(comptime T: type, lhs: T, rhs: T, expected: T) !void {128 fn testSatShl(comptime T: type, lhs: T, rhs: T, expected: T) !void {
111 try expect((lhs <<| rhs) == expected);129 try expect((lhs <<| rhs) == expected);
112130
...@@ -115,8 +133,14 @@ test "saturating shift-left" {...@@ -115,8 +133,14 @@ test "saturating shift-left" {
115 try expect(x == expected);133 try expect(x == expected);
116 }134 }
117 };135 };
136
118 try S.doTheTest();137 try S.doTheTest();
119 comptime try S.doTheTest();138 comptime try S.doTheTest();
139
140 comptime try S.testSatShl(comptime_int, 0, 0, 0);
141 comptime try S.testSatShl(comptime_int, 1, 2, 4);
142 comptime try S.testSatShl(comptime_int, 13, 150, 18554220005177478453757717602843436772975706112);
143 comptime try S.testSatShl(comptime_int, -582769, 180, -893090893854873184096635538665358532628308979495815656505344);
120}144}
121145
122test "saturating shl uses the LHS type" {146test "saturating shl uses the LHS type" {
...@@ -139,4 +163,6 @@ test "saturating shl uses the LHS type" {...@@ -139,4 +163,6 @@ test "saturating shl uses the LHS type" {
139 try expect((@as(u8, 1) <<| 8) == 255);163 try expect((@as(u8, 1) <<| 8) == 255);
140 try expect((@as(u8, 1) <<| rhs_const) == 255);164 try expect((@as(u8, 1) <<| rhs_const) == 255);
141 try expect((@as(u8, 1) <<| rhs_var) == 255);165 try expect((@as(u8, 1) <<| rhs_var) == 255);
166
167 try expect((1 <<| @as(u8, 200)) == 1606938044258990275541962092341162602522202993782792835301376);
142}168}
test/behavior/wrapping_arithmetic.zig created+110
...@@ -0,0 +1,110 @@
1const std = @import("std");
2const builtin = @import("builtin");
3const minInt = std.math.minInt;
4const maxInt = std.math.maxInt;
5const expect = std.testing.expect;
6
7test "wrapping add" {
8 const S = struct {
9 fn doTheTest() !void {
10 try testWrapAdd(i8, -3, 10, 7);
11 try testWrapAdd(i8, -128, -128, 0);
12 try testWrapAdd(i2, 1, 1, -2);
13 try testWrapAdd(i64, maxInt(i64), 1, minInt(i64));
14 try testWrapAdd(i128, maxInt(i128), -maxInt(i128), 0);
15 try testWrapAdd(i128, minInt(i128), maxInt(i128), -1);
16 try testWrapAdd(i8, 127, 127, -2);
17 try testWrapAdd(u8, 3, 10, 13);
18 try testWrapAdd(u8, 255, 255, 254);
19 try testWrapAdd(u2, 3, 2, 1);
20 try testWrapAdd(u3, 7, 1, 0);
21 try testWrapAdd(u128, maxInt(u128), 1, minInt(u128));
22 }
23
24 fn testWrapAdd(comptime T: type, lhs: T, rhs: T, expected: T) !void {
25 try expect((lhs +% rhs) == expected);
26
27 var x = lhs;
28 x +%= rhs;
29 try expect(x == expected);
30 }
31 };
32
33 try S.doTheTest();
34 comptime try S.doTheTest();
35
36 comptime try S.testWrapAdd(comptime_int, 0, 0, 0);
37 comptime try S.testWrapAdd(comptime_int, 3, 2, 5);
38 comptime try S.testWrapAdd(comptime_int, 651075816498665588400716961808225370057, 468229432685078038144554201546849378455, 1119305249183743626545271163355074748512);
39 comptime try S.testWrapAdd(comptime_int, 7, -593423721213448152027139550640105366508, -593423721213448152027139550640105366501);
40}
41
42test "wrapping subtraction" {
43 const S = struct {
44 fn doTheTest() !void {
45 try testWrapSub(i8, -3, 10, -13);
46 try testWrapSub(i8, -128, -128, 0);
47 try testWrapSub(i8, -1, 127, -128);
48 try testWrapSub(i64, minInt(i64), 1, maxInt(i64));
49 try testWrapSub(i128, maxInt(i128), -1, minInt(i128));
50 try testWrapSub(i128, minInt(i128), -maxInt(i128), -1);
51 try testWrapSub(u8, 10, 3, 7);
52 try testWrapSub(u8, 0, 255, 1);
53 try testWrapSub(u5, 0, 31, 1);
54 try testWrapSub(u128, 0, maxInt(u128), 1);
55 }
56
57 fn testWrapSub(comptime T: type, lhs: T, rhs: T, expected: T) !void {
58 try expect((lhs -% rhs) == expected);
59
60 var x = lhs;
61 x -%= rhs;
62 try expect(x == expected);
63 }
64 };
65
66 try S.doTheTest();
67 comptime try S.doTheTest();
68
69 comptime try S.testWrapSub(comptime_int, 0, 0, 0);
70 comptime try S.testWrapSub(comptime_int, 3, 2, 1);
71 comptime try S.testWrapSub(comptime_int, 651075816498665588400716961808225370057, 468229432685078038144554201546849378455, 182846383813587550256162760261375991602);
72 comptime try S.testWrapSub(comptime_int, 7, -593423721213448152027139550640105366508, 593423721213448152027139550640105366515);
73}
74
75test "wrapping multiplication" {
76 // TODO: once #9660 has been solved, remove this line
77 if (builtin.cpu.arch == .wasm32) return error.SkipZigTest;
78
79 const S = struct {
80 fn doTheTest() !void {
81 try testWrapMul(i8, -3, 10, -30);
82 try testWrapMul(i4, 2, 4, -8);
83 try testWrapMul(i8, 2, 127, -2);
84 try testWrapMul(i8, -128, -128, 0);
85 try testWrapMul(i8, maxInt(i8), maxInt(i8), 1);
86 try testWrapMul(i16, maxInt(i16), -1, minInt(i16) + 1);
87 try testWrapMul(i128, maxInt(i128), -1, minInt(i128) + 1);
88 try testWrapMul(i128, minInt(i128), -1, minInt(i128));
89 try testWrapMul(u8, 10, 3, 30);
90 try testWrapMul(u8, 2, 255, 254);
91 try testWrapMul(u128, maxInt(u128), maxInt(u128), 1);
92 }
93
94 fn testWrapMul(comptime T: type, lhs: T, rhs: T, expected: T) !void {
95 try expect((lhs *% rhs) == expected);
96
97 var x = lhs;
98 x *%= rhs;
99 try expect(x == expected);
100 }
101 };
102
103 try S.doTheTest();
104 comptime try S.doTheTest();
105
106 comptime try S.testWrapMul(comptime_int, 0, 0, 0);
107 comptime try S.testWrapMul(comptime_int, 3, 2, 6);
108 comptime try S.testWrapMul(comptime_int, 651075816498665588400716961808225370057, 468229432685078038144554201546849378455, 304852860194144160265083087140337419215516305999637969803722975979232817921935);
109 comptime try S.testWrapMul(comptime_int, 7, -593423721213448152027139550640105366508, -4153966048494137064189976854480737565556);
110}