Skip to content

Commit

Permalink
sat-arithmetic: make tests actually run at runtime
Browse files Browse the repository at this point in the history 
- rework test cases which were skipping runtime evaluation. this uncovered
  bugs related to *mul.fix.sat intrinsic.
- removed extra type args in ZigLLVMBuild*MulFixSat so mul.fix.sat intrinsic
  is called correctly
- replaced panics in ir.cpp with compile errors.
- added case to test/compile_errors.zig given floats
- updated langref: indcate future `a +| b` syntax
  • Loading branch information
@travisstaloch
travisstaloch committed Aug 29, 2021
1 parent 437fba2 commit 6b21164
Show file tree
Hide file tree
Showing 6 changed files with 84 additions and 100 deletions.
10 changes: 9 additions & 1 deletion doc/langref.html.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7035,6 +7035,8 @@ fn readFile(allocator: *Allocator, filename: []const u8) ![]u8 {
<pre>{#syntax#}@addWithSaturation(a: T, b: T) T{#endsyntax#}</pre>
<p>
Returns {#syntax#}a + b{#endsyntax#}. The result will be clamped between the type maximum and minimum.
Once <a href="https://github.com/ziglang/zig/issues/1284">Saturating arithmetic</a>.
is completed, the syntax {#syntax#}a +| b{#endsyntax#} will be equivalent to calling @addWithSaturation.
</p>
{#header_close#}
{#header_open|@alignCast#}
Expand Down Expand Up @@ -8154,6 +8156,8 @@ test "@wasmMemoryGrow" {
<pre>{#syntax#}@mulWithSaturation(a: T, b: T) T{#endsyntax#}</pre>
<p>
Returns {#syntax#}a * b{#endsyntax#}. The result will be clamped between the type maximum and minimum.
Once <a href="https://github.com/ziglang/zig/issues/1284">Saturating arithmetic</a>.
is completed, the syntax {#syntax#}a *| b{#endsyntax#} will be equivalent to calling @mulWithSaturation.
</p>
{#header_close#}

Expand Down Expand Up @@ -8402,6 +8406,8 @@ test "@setRuntimeSafety" {
<pre>{#syntax#}@shlWithSaturation(a: T, shift_amt: T) T{#endsyntax#}</pre>
<p>
Returns {#syntax#}a << b{#endsyntax#}. The result will be clamped between type minimum and maximum.
Once <a href="https://github.com/ziglang/zig/issues/1284">Saturating arithmetic</a>.
is completed, the syntax {#syntax#}a <<| b{#endsyntax#} will be equivalent to calling @shlWithSaturation.
</p>
<p>
Unlike other @shl builtins, shift_amt doesn't need to be a Log2T as saturated overshifting is well defined.
Expand Down Expand Up @@ -8722,7 +8728,9 @@ fn doTheTest() !void {
{#header_open|@subWithSaturation#}
<pre>{#syntax#}@subWithSaturation(a: T, b: T) T{#endsyntax#}</pre>
<p>
Returns {#syntax#}a - b{#endsyntax#}. The result will be clamped between the type maximum and minimum.
Returns {#syntax#}a - b{#endsyntax#}. The result will be clamped between the type maximum and minimum.
Once <a href="https://github.com/ziglang/zig/issues/1284">Saturating arithmetic</a>.
is completed, the syntax {#syntax#}a -| b{#endsyntax#} will be equivalent to calling @subWithSaturation.
</p>
{#header_close#}

Expand Down
2 changes: 1 addition & 1 deletion src/Zir.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3619,7 +3619,7 @@ const Writer = struct {
try stream.writeAll(", ");
try self.writeInstRef(stream, extra.rhs);
try stream.writeAll(", ");
try stream.writeAll(")) ");
try stream.writeAll(") ");
try self.writeSrc(stream, src);
}

Expand Down
12 changes: 4 additions & 8 deletions src/stage1/ir.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9824,32 +9824,28 @@ static ErrorMsg *ir_eval_math_op_scalar(IrAnalyze *ira, Scope *scope, AstNode *s
if (is_int) {
bigint_add_sat(&out_val->data.x_bigint, &op1_val->data.x_bigint, &op2_val->data.x_bigint, type_entry->data.integral.bit_count, type_entry->data.integral.is_signed);
} else {
// TODO: support floats?
zig_panic("float types not supported by @addWithSaturation");
zig_unreachable();
}
break;
case IrBinOpSatSub:
if (is_int) {
bigint_sub_sat(&out_val->data.x_bigint, &op1_val->data.x_bigint, &op2_val->data.x_bigint, type_entry->data.integral.bit_count, type_entry->data.integral.is_signed);
} else {
// TODO: support floats?
zig_panic("float types not supported by @subWithSaturation");
zig_unreachable();
}
break;
case IrBinOpSatMul:
if (is_int) {
bigint_mul_sat(&out_val->data.x_bigint, &op1_val->data.x_bigint, &op2_val->data.x_bigint, type_entry->data.integral.bit_count, type_entry->data.integral.is_signed);
} else {
// TODO: support floats?
zig_panic("float types not supported by @mulWithSaturation");
zig_unreachable();
}
break;
case IrBinOpSatShl:
if (is_int) {
bigint_shl_sat(&out_val->data.x_bigint, &op1_val->data.x_bigint, &op2_val->data.x_bigint, type_entry->data.integral.bit_count, type_entry->data.integral.is_signed);
} else {
// TODO: support floats?
zig_panic("float types not supported by @shlWithSaturation");
zig_unreachable();
}
break;
}
Expand Down
12 changes: 4 additions & 8 deletions src/zig_llvm.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -509,25 +509,21 @@ LLVMValueRef ZigLLVMBuildUSubSat(LLVMBuilderRef B, LLVMValueRef LHS, LLVMValueRe
}

LLVMValueRef ZigLLVMBuildSMulFixSat(LLVMBuilderRef B, LLVMValueRef LHS, LLVMValueRef RHS, const char *name) {
// pass scale = 0 as third argument
llvm::Type* types[3] = {
llvm::Type* types[1] = {
unwrap(LHS)->getType(),
unwrap(RHS)->getType(),
llvm::Type::getInt32PtrTy(unwrap(LHS)->getContext())
};
// pass scale = 0 as third argument
llvm::Value* values[3] = {unwrap(LHS), unwrap(RHS), unwrap(B)->getInt32(0)};

CallInst *call_inst = unwrap(B)->CreateIntrinsic(Intrinsic::smul_fix_sat, types, values, nullptr, name);
return wrap(call_inst);
}

LLVMValueRef ZigLLVMBuildUMulFixSat(LLVMBuilderRef B, LLVMValueRef LHS, LLVMValueRef RHS, const char *name) {
// pass scale = 0 as third argument
llvm::Type* types[3] = {
llvm::Type* types[1] = {
unwrap(LHS)->getType(),
unwrap(RHS)->getType(),
llvm::Type::getInt32PtrTy(unwrap(LHS)->getContext())
};
// pass scale = 0 as third argument
llvm::Value* values[3] = {unwrap(LHS), unwrap(RHS), unwrap(B)->getInt32(0)};

CallInst *call_inst = unwrap(B)->CreateIntrinsic(Intrinsic::umul_fix_sat, types, values, nullptr, name);
Expand Down
140 changes: 58 additions & 82 deletions test/behavior/saturating_arithmetic.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,32 +6,36 @@ const Vector = std.meta.Vector;
const minInt = std.math.minInt;
const maxInt = std.math.maxInt;

const Op = enum { add, sub, mul, shl };
fn testSaturatingOp(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 => @addWithSaturation(a, b),
.sub => @subWithSaturation(a, b),
.mul => @mulWithSaturation(a, b),
.shl => @shlWithSaturation(a, b),
};
try expectEqual(expected, actual);
}

test "@addWithSaturation" {
const S = struct {
fn doTheTest() !void {
const test_data = .{
// { a, b, expected a+b }
[_]i8{ -3, 10, 7 },
[_]i8{ -128, -128, -128 },
[_]i2{ 1, 1, 1 },
[_]i64{ maxInt(i64), 1, maxInt(i64) },
[_]i128{ maxInt(i128), -maxInt(i128), 0 },
[_]i128{ minInt(i128), maxInt(i128), -1 },
[_]i8{ 127, 127, 127 },
[_]u8{ 3, 10, 13 },
[_]u8{ 255, 255, 255 },
[_]u2{ 3, 2, 3 },
[_]u3{ 7, 1, 7 },
[_]u128{ maxInt(u128), 1, maxInt(u128) },
};

inline for (test_data) |array| {
const a = array[0];
const b = array[1];
const expected = array[2];
const actual = @addWithSaturation(a, b);
try expectEqual(expected, actual);
}
// .{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) });

const u8x3 = std.meta.Vector(3, u8);
try expectEqual(u8x3{ 255, 255, 255 }, @addWithSaturation(
Expand All @@ -52,27 +56,17 @@ test "@addWithSaturation" {
test "@subWithSaturation" {
const S = struct {
fn doTheTest() !void {
const test_data = .{
// { a, b, expected a-b }
[_]i8{ -3, 10, -13 },
[_]i8{ -128, -128, 0 },
[_]i8{ -1, 127, -128 },
[_]i64{ minInt(i64), 1, minInt(i64) },
[_]i128{ maxInt(i128), -1, maxInt(i128) },
[_]i128{ minInt(i128), -maxInt(i128), -1 },
[_]u8{ 10, 3, 7 },
[_]u8{ 0, 255, 0 },
[_]u5{ 0, 31, 0 },
[_]u128{ 0, maxInt(u128), 0 },
};

inline for (test_data) |array| {
const a = array[0];
const b = array[1];
const expected = array[2];
const actual = @subWithSaturation(a, b);
try expectEqual(expected, actual);
}
// .{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 });

const u8x3 = std.meta.Vector(3, u8);
try expectEqual(u8x3{ 0, 0, 0 }, @subWithSaturation(
Expand All @@ -88,26 +82,18 @@ test "@subWithSaturation" {
test "@mulWithSaturation" {
const S = struct {
fn doTheTest() !void {
const test_data = .{
// { a, b, expected a*b }
[_]i8{ -3, 10, -30 },
[_]i8{ -128, -128, 127 },
[_]i8{ 2, 127, 127 },
[_]i128{ maxInt(i128), maxInt(i128), maxInt(i128) },
[_]i128{ maxInt(i128), -1, minInt(i128) },
[_]i128{ minInt(i128), -1, maxInt(i128) },
[_]u8{ 10, 3, 30 },
[_]u8{ 2, 255, 255 },
[_]u128{ maxInt(u128), maxInt(u128), maxInt(u128) },
};

inline for (test_data) |array| {
const a = array[0];
const b = array[1];
const expected = array[2];
const actual = @mulWithSaturation(a, b);
try expectEqual(expected, actual);
}
// .{a, b, expected a*b}
try testSaturatingOp(.mul, i8, .{ -3, 10, -30 });
try testSaturatingOp(.mul, i4, .{ 2, 4, 7 });
// TODO: add these tests cases back after implementing workaround for #9643
// try testSaturatingOp(.mul, i8, .{ -128, -128, 127 });
try testSaturatingOp(.mul, i8, .{ 2, 127, 127 });
// try testSaturatingOp(.mul, i128, .{ maxInt(i128), maxInt(i128), maxInt(i128) });
// try testSaturatingOp(.mul, i128, .{ maxInt(i128), -1, minInt(i128) });
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) });

const u8x3 = std.meta.Vector(3, u8);
try expectEqual(u8x3{ 255, 255, 255 }, @mulWithSaturation(
Expand All @@ -123,24 +109,14 @@ test "@mulWithSaturation" {
test "@shlWithSaturation" {
const S = struct {
fn doTheTest() !void {
const test_data = .{
// { a, b, expected a<<b }
[_]i8{ 1, 2, 4 },
[_]i8{ 127, 1, 127 },
[_]i8{ -128, 1, -128 },
[_]i128{ maxInt(i128), 64, maxInt(i128) },
[_]u8{ 1, 2, 4 },
[_]u8{ 255, 1, 255 },
[_]u128{ maxInt(u128), 64, maxInt(u128) },
};

inline for (test_data) |array| {
const a = array[0];
const b = array[1];
const expected = array[2];
const actual = @shlWithSaturation(a, b);
try expectEqual(expected, actual);
}
// .{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 testSaturatingOp(.shl, i128, .{ maxInt(i128), 64, maxInt(i128) });
try testSaturatingOp(.shl, u8, .{ 1, 2, 4 });
try testSaturatingOp(.shl, u8, .{ 255, 1, 255 });
try testSaturatingOp(.shl, u128, .{ maxInt(u128), 64, maxInt(u128) });

const u8x3 = std.meta.Vector(3, u8);
try expectEqual(u8x3{ 255, 255, 255 }, @shlWithSaturation(
Expand Down
8 changes: 8 additions & 0 deletions test/compile_errors.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8838,4 +8838,12 @@ pub fn addCases(ctx: *TestContext) !void {
"tmp.zig:2:9: note: declared mutable here",
"tmp.zig:3:12: note: crosses namespace boundary here",
});

ctx.objErrStage1("Issue #9619: saturating arithmetic builtins should fail to compile when given floats",
\\pub fn main() !void {
\\ _ = @addWithSaturation(@as(f32, 1.0), @as(f32, 1.0));
\\}
, &[_][]const u8{
"error: invalid operands to binary expression: 'f32' and 'f32'",
});
}

0 comments on commit 6b21164

Please sign in to comment.