From 6ed049fe3642c775e53b3bef2f9bfb6a1de0f325 Mon Sep 17 00:00:00 2001 From: kcbanner Date: Sun, 18 Dec 2022 17:30:52 -0500 Subject: [PATCH] cbe: all behaviour tests now pass on msvc - Fix zig_clz_u128 not respecting the bits argument. This was crashing the compile-rt addxf3 tests with the cbe - Instead of redering a negation for negative 128 bit int literals, render the literal as twos complement. This allows rendering int representations of floats correctly (specifically f80). --- lib/std/math/big/int.zig | 34 +++++++++++++++ lib/zig.h | 89 +++------------------------------------ src/codegen/c.zig | 39 +++++++++++++---- test/behavior/atomics.zig | 3 +- 4 files changed, 73 insertions(+), 92 deletions(-) diff --git a/lib/std/math/big/int.zig b/lib/std/math/big/int.zig index 8410e25864..d222d6913b 100644 --- a/lib/std/math/big/int.zig +++ b/lib/std/math/big/int.zig @@ -1677,6 +1677,40 @@ pub const Mutable = struct { y.shiftRight(y.toConst(), norm_shift); } + /// If a is positive, this passes through to truncate. + /// If a is negative, then r is set to positive with the bit pattern ~(a - 1). + /// + /// Asserts `r` has enough storage to store the result. + /// The upper bound is `calcTwosCompLimbCount(a.len)`. + pub fn convertToTwosComplement(r: *Mutable, a: Const, signedness: Signedness, bit_count: usize) void { + if (a.positive) { + r.truncate(a, signedness, bit_count); + return; + } + + const req_limbs = calcTwosCompLimbCount(bit_count); + if (req_limbs == 0 or a.eqZero()) { + r.set(0); + return; + } + + const bit = @truncate(Log2Limb, bit_count - 1); + const signmask = @as(Limb, 1) << bit; + const mask = (signmask << 1) -% 1; + + r.addScalar(a.abs(), -1); + if (req_limbs > r.len) { + mem.set(Limb, r.limbs[r.len..req_limbs], 0); + } + + assert(r.limbs.len >= req_limbs); + r.len = req_limbs; + + llnot(r.limbs[0..r.len]); + r.limbs[r.len - 1] &= mask; + r.normalize(r.len); + } + /// Truncate an integer to a number of bits, following 2s-complement semantics. /// r may alias a. /// diff --git a/lib/zig.h b/lib/zig.h index b8e64708d5..53e7c81dda 100644 --- a/lib/zig.h +++ b/lib/zig.h @@ -1339,7 +1339,7 @@ static inline zig_u128 zig_shl_u128(zig_u128 lhs, zig_u8 rhs) { static inline zig_i128 zig_shl_i128(zig_i128 lhs, zig_u8 rhs) { if (rhs == zig_as_u8(0)) return lhs; - if (rhs >= zig_as_u8(64)) return (zig_i128){ .hi = lhs.hi << (rhs - zig_as_u8(64)), .lo = zig_minInt_u64 }; // TODO: Fix? + if (rhs >= zig_as_u8(64)) return (zig_i128){ .hi = lhs.lo << rhs, .lo = zig_minInt_u64 }; return (zig_i128){ .hi = lhs.hi << rhs | lhs.lo >> (zig_as_u8(64) - rhs), .lo = lhs.lo << rhs }; } @@ -1681,8 +1681,9 @@ static inline zig_i128 zig_muls_i128(zig_i128 lhs, zig_i128 rhs, zig_u8 bits) { } static inline zig_u8 zig_clz_u128(zig_u128 val, zig_u8 bits) { + if (bits <= zig_as_u8(64)) return zig_clz_u64(zig_lo_u128(val), bits); if (zig_hi_u128(val) != 0) return zig_clz_u64(zig_hi_u128(val), bits - zig_as_u8(64)); - return zig_clz_u64(zig_lo_u128(val), zig_as_u8(64)) + zig_as_u8(64); + return zig_clz_u64(zig_lo_u128(val), zig_as_u8(64)) + (bits - zig_as_u8(64)); } static inline zig_u8 zig_clz_i128(zig_i128 val, zig_u8 bits) { @@ -1942,14 +1943,15 @@ typedef zig_i128 zig_f128; #define zig_has_c_longdouble 1 #define zig_libc_name_c_longdouble(name) name##l #define zig_as_special_constant_c_longdouble(sign, name, arg, repr) zig_as_special_c_longdouble(sign, name, arg, repr) -#if !_MSC_VER // TODO: Is there a better way to detect long double == double on msvc? +#ifdef zig_bitSizeOf_c_longdouble typedef long double zig_c_longdouble; #define zig_as_c_longdouble(fp, repr) fp##l #else #undef zig_has_c_longdouble +#define zig_bitSizeOf_c_longdouble 80 +#define zig_compiler_rt_abbrev_c_longdouble zig_compiler_rt_abbrev_f80 #define zig_has_c_longdouble 0 #define zig_repr_c_longdouble i128 -#define zig_bitSizeOf_c_longdouble 128 typedef zig_i128 zig_c_longdouble; #define zig_as_c_longdouble(fp, repr) repr #undef zig_as_special_c_longdouble @@ -1972,85 +1974,6 @@ zig_float_from_repr(f128, u128) zig_float_from_repr(c_longdouble, u128) #endif -/* #define zig_float_from_repr(Type) *((zig_##Type*)&repr) */ - -/* #define zig_float_inf_builtin_0(Type, ReprType) \ */ -/* static inline zig_##Type zig_as_special_inf_##Type(zig_##ReprType repr) { \ */ -/* return zig_float_from_repr(Type); \ */ -/* } */ -/* #define zig_float_inf_builtin_1(Type, ReprType) \ */ -/* static inline zig_##Type zig_as_special_inf_##Type(zig_##ReprType repr) { \ */ -/* return __builtin_inf(); \ */ -/* } */ -/* #define zig_float_nan_builtin_0(Type, ReprType) \ */ -/* static inline zig_##Type zig_as_special_nan_##Type(const char* arg, zig_##ReprType repr) { \ */ -/* return zig_float_from_repr(Type); \ */ -/* } */ -/* #define zig_float_nan_builtin_1(Type, ReprType) \ */ -/* static inline zig_##Type zig_as_special_nan_##Type(const char* arg, zig_##ReprType repr) { \ */ -/* return __builtin_nan(arg); \ */ -/* } */ -/* #define zig_float_nans_builtin_0(Type, ReprType) \ */ -/* static inline zig_##Type zig_as_special_nans_##Type(const char* arg, zig_##ReprType repr) { \ */ -/* return zig_float_from_repr(Type); \ */ -/* } */ -/* #define zig_float_nans_builtin_1(Type, ReprType) \ */ -/* static inline zig_##Type zig_as_special_nans_##Type(const char* arg, zig_##ReprType repr) { \ */ -/* return __builtin_nans(arg); \ */ -/* } */ - -/* #define zig_float_special_builtins(Type, ReprType) \ */ -/* zig_expand_concat(zig_float_inf_builtin_, zig_has_builtin(inf))(Type, ReprType) \ */ -/* zig_expand_concat(zig_float_nan_builtin_, zig_has_builtin(nan))(Type, ReprType) \ */ -/* zig_expand_concat(zig_float_nans_builtin_, zig_has_builtin(nans))(Type, ReprType) */ - -/* #if zig_has_builtin(nan) */ -/* #define zig_as_special_nan(arg, repr) __builtin_nan(arg); */ -/* #define zig_as_special_nan_f16(arg, repr) __builtin_nan(arg); */ -/* #define zig_as_special_nan_f32(arg, repr) __builtin_nan(arg); */ -/* #define zig_as_special_nan_f64(arg, repr) __builtin_nan(arg); */ -/* #define zig_as_special_nan_f80(arg, repr) __builtin_nan(arg); */ -/* #define zig_as_special_nan_f128(arg, repr) __builtin_nan(arg); */ -/* #else */ -/* zig_float_special_builtins(); */ -/* #endif */ - -/* #if zig_has_f16 */ -/* zig_float_special_builtins(f16, u16) */ -/* #endif */ - -/* #if zig_has_f32 */ -/* zig_float_special_builtins(f32, u32) */ -/* #endif */ - -/* #if zig_has_f64 */ -/* zig_float_special_builtins(f64, u64) */ -/* #endif */ - -/* #if zig_has_f80 */ -/* zig_float_special_builtins(f80, u128) */ -/* #endif */ - -/* #if zig_has_f128 */ -/* zig_float_special_builtins(f128, u128) */ -/* #endif */ - -/* #if zig_has_c_longdouble */ -/* zig_float_special_builtins(c_longdouble, u128) */ -/* #endif */ - -#if zig_bitSizeOf_c_longdouble == 16 -#define zig_compiler_rt_abbrev_c_longdouble zig_compiler_rt_abbrev_f16 -#elif zig_bitSizeOf_c_longdouble == 32 -#define zig_compiler_rt_abbrev_c_longdouble zig_compiler_rt_abbrev_f32 -#elif zig_bitSizeOf_c_longdouble == 64 -#define zig_compiler_rt_abbrev_c_longdouble zig_compiler_rt_abbrev_f64 -#elif zig_bitSizeOf_c_longdouble == 80 -#define zig_compiler_rt_abbrev_c_longdouble zig_compiler_rt_abbrev_f80 -#elif zig_bitSizeOf_c_longdouble == 128 -#define zig_compiler_rt_abbrev_c_longdouble zig_compiler_rt_abbrev_f128 -#endif - #define zig_cast_f16 (zig_f16) #define zig_cast_f32 (zig_f32) #define zig_cast_f64 (zig_f64) diff --git a/src/codegen/c.zig b/src/codegen/c.zig index e0a59e50e5..23d9e1093f 100644 --- a/src/codegen/c.zig +++ b/src/codegen/c.zig @@ -2603,7 +2603,7 @@ pub const DeclGen = struct { dg: *DeclGen, ty: Type, val: Value, - location: ValueRenderLocation, // TODO: Instead add this as optional arg to fmtIntLiteralLoc + location: ValueRenderLocation, // TODO: Instead add this as optional arg to fmtIntLiteral ) !std.fmt.Formatter(formatIntLiteral) { const int_info = ty.intInfo(dg.module.getTarget()); const c_bits = toCIntBits(int_info.bits); @@ -7251,11 +7251,16 @@ fn formatIntLiteral( return writer.print("{s}_{s}", .{ abbrev, if (int.positive) "MAX" else "MIN" }); } + var use_twos_comp = false; if (!int.positive) { if (c_bits > 64) { - // TODO: Could use negate function instead? - // TODO: Use fmtIntLiteral for 0? - try writer.print("zig_sub_{c}{d}(zig_as_{c}{d}(0, 0), ", .{ signAbbrev(int_info.signedness), c_bits, signAbbrev(int_info.signedness), c_bits }); + // TODO: Can this be done for decimal literals as well? + if (fmt.len == 1 and fmt[0] != 'd') { + use_twos_comp = true; + } else { + // TODO: Use fmtIntLiteral for 0? + try writer.print("zig_sub_{c}{d}(zig_as_{c}{d}(0, 0), ", .{ signAbbrev(int_info.signedness), c_bits, signAbbrev(int_info.signedness), c_bits }); + } } else { try writer.writeByte('-'); } @@ -7310,16 +7315,34 @@ fn formatIntLiteral( } else { assert(c_bits == 128); const split = std.math.min(int.limbs.len, limbs_count_64); + var twos_comp_limbs: [BigInt.calcTwosCompLimbCount(128)]BigIntLimb = undefined; + + // Adding a negation in the C code before the doesn't work in all cases: + // - struct versions would require an extra zig_sub_ call to negate, which wouldn't work in constant expressions + // - negating the f80 int representation (i128) doesn't make sense + // Instead we write out the literal as a negative number in twos complement + var limbs = int.limbs; + + if (use_twos_comp) { + var twos_comp = BigInt.Mutable{ + .limbs = &twos_comp_limbs, + .positive = undefined, + .len = undefined, + }; + + twos_comp.convertToTwosComplement(int, .signed, int_info.bits); + limbs = twos_comp.limbs; + } var upper_pl = Value.Payload.BigInt{ .base = .{ .tag = .int_big_positive }, - .data = int.limbs[split..], + .data = limbs[split..], }; const upper_val = Value.initPayload(&upper_pl.base); try formatIntLiteral(.{ .ty = switch (int_info.signedness) { .unsigned => Type.u64, - .signed => Type.i64, + .signed => if (use_twos_comp) Type.u64 else Type.i64, }, .val = upper_val, .mod = data.mod, @@ -7329,7 +7352,7 @@ fn formatIntLiteral( var lower_pl = Value.Payload.BigInt{ .base = .{ .tag = .int_big_positive }, - .data = int.limbs[0..split], + .data = limbs[0..split], }; const lower_val = Value.initPayload(&lower_pl.base); try formatIntLiteral(.{ @@ -7338,7 +7361,7 @@ fn formatIntLiteral( .mod = data.mod, }, fmt, options, writer); - if (!int.positive and c_bits > 64) try writer.writeByte(')'); + if (!int.positive and c_bits > 64 and !use_twos_comp) try writer.writeByte(')'); return writer.writeByte(')'); } diff --git a/test/behavior/atomics.zig b/test/behavior/atomics.zig index 17c4a139ae..f6463ad80a 100644 --- a/test/behavior/atomics.zig +++ b/test/behavior/atomics.zig @@ -251,7 +251,8 @@ test "atomicrmw with ints" { return error.SkipZigTest; } - const bit_values = [_]usize{ 8, 16, 32, 64 }; + // TODO: Use the max atomic bit size for the target, maybe builtin? + const bit_values = [_]usize{ 8 } ++ if (builtin.cpu.arch == .x86_64) [_]usize{ 16, 32, 64 } else [_]usize{ }; inline for (bit_values) |bits| { try testAtomicRmwInt(.unsigned, bits); comptime try testAtomicRmwInt(.unsigned, bits);