Module: Mesa Branch: staging/21.2 Commit: b44451ad2039b1191ab70617638a79fac2067aee URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=b44451ad2039b1191ab70617638a79fac2067aee
Author: Ian Romanick <[email protected]> Date: Mon Aug 2 16:43:52 2021 -0700 util: Add and use functions to calculate min and max int for a size Many places need to know the maximum or minimum possible value for a given size integer... so everyone just open-codes their favorite version. There is some potential to hit either undefined or implementation-defined behavior, so having one version that Just Works seems beneficial. v2: Fix copy-and-pasted bug (INT64_MAX instead of INT64_MIN) in u_intmin. Noticed by CI. Lol. Rename functions `s/u_(uint|int)(min|max)/u_\1N_\2/g`. Suggested by Jason. Add some unit tests that would have caught the copy-and-paste bug before wasting CI time. Change the implementation of u_intN_min to use the same pattern as stdint.h. This avoids the integer division. Noticed by Jason. v3: Add changes to convert_clear_color (src/gallium/drivers/iris/iris_clear.c). Suggested by Nanley. Reviewed-by: Jason Ekstrand <[email protected]> Suggested-by: Jason Ekstrand <[email protected]> Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12177> (cherry picked from commit 72259a870f6047c83b3f95ac205aca7ed5bdc049) --- .pick_status.json | 2 +- src/compiler/nir/nir_constant_expressions.py | 2 - src/compiler/nir/nir_opcodes.py | 2 +- src/compiler/nir/nir_search.c | 2 +- src/gallium/drivers/iris/iris_clear.c | 4 +- src/intel/isl/isl_format.c | 10 +-- src/util/fast_idiv_by_const.c | 4 +- src/util/format/format_utils.h | 53 +++++++--------- src/util/macros.h | 26 ++++++++ src/util/meson.build | 9 +++ .../fast_idiv_by_const/fast_idiv_by_const_test.cpp | 9 +-- src/util/tests/int_min_max.cpp | 73 ++++++++++++++++++++++ 12 files changed, 147 insertions(+), 49 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index b5226fd6a8b..c7dd303cfc2 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -7627,7 +7627,7 @@ "description": "util: Add and use functions to calculate min and max int for a size", "nominated": false, "nomination_type": null, - "resolution": 4, + "resolution": 1, "main_sha": null, "because_sha": null }, diff --git a/src/compiler/nir/nir_constant_expressions.py b/src/compiler/nir/nir_constant_expressions.py index 606e974353f..c1097de7c67 100644 --- a/src/compiler/nir/nir_constant_expressions.py +++ b/src/compiler/nir/nir_constant_expressions.py @@ -68,8 +68,6 @@ template = """\ #include "util/bigmath.h" #include "nir_constant_expressions.h" -#define MAX_UINT_FOR_SIZE(bits) (UINT64_MAX >> (64 - (bits))) - /** * \brief Checks if the provided value is a denorm and flushes it to zero. */ diff --git a/src/compiler/nir/nir_opcodes.py b/src/compiler/nir/nir_opcodes.py index f9330c0af2c..e2172d7a26e 100644 --- a/src/compiler/nir/nir_opcodes.py +++ b/src/compiler/nir/nir_opcodes.py @@ -634,7 +634,7 @@ binop("iadd_sat", tint, _2src_commutative, """ (src0 < src0 + src1 ? (1ull << (bit_size - 1)) : src0 + src1) """) binop("uadd_sat", tuint, _2src_commutative, - "(src0 + src1) < src0 ? MAX_UINT_FOR_SIZE(sizeof(src0) * 8) : (src0 + src1)") + "(src0 + src1) < src0 ? u_uintN_max(sizeof(src0) * 8) : (src0 + src1)") binop("isub_sat", tint, "", """ src1 < 0 ? (src0 - src1 < src0 ? (1ull << (bit_size - 1)) - 1 : src0 - src1) : diff --git a/src/compiler/nir/nir_search.c b/src/compiler/nir/nir_search.c index 272a3f6444a..437a24b9b02 100644 --- a/src/compiler/nir/nir_search.c +++ b/src/compiler/nir/nir_search.c @@ -368,7 +368,7 @@ match_value(const nir_search_value *value, nir_alu_instr *instr, unsigned src, case nir_type_uint: case nir_type_bool: { unsigned bit_size = nir_src_bit_size(instr->src[src].src); - uint64_t mask = bit_size == 64 ? UINT64_MAX : (1ull << bit_size) - 1; + uint64_t mask = u_uintN_max(bit_size); for (unsigned i = 0; i < num_components; ++i) { uint64_t val = nir_src_comp_as_uint(instr->src[src].src, new_swizzle[i]); diff --git a/src/gallium/drivers/iris/iris_clear.c b/src/gallium/drivers/iris/iris_clear.c index cc619b46a0c..a59ba735cbc 100644 --- a/src/gallium/drivers/iris/iris_clear.c +++ b/src/gallium/drivers/iris/iris_clear.c @@ -185,8 +185,8 @@ convert_clear_color(enum pipe_format format, unsigned bits = util_format_get_component_bits( format, UTIL_FORMAT_COLORSPACE_RGB, i); if (bits > 0 && bits < 32) { - int32_t max = (1 << (bits - 1)) - 1; - int32_t min = -(1 << (bits - 1)); + int32_t max = u_intN_max(bits); + int32_t min = u_intN_min(bits); override_color.i32[i] = CLAMP(override_color.i32[i], min, max); } } diff --git a/src/intel/isl/isl_format.c b/src/intel/isl/isl_format.c index 60f4409e2b0..bd9c63d140c 100644 --- a/src/intel/isl/isl_format.c +++ b/src/intel/isl/isl_format.c @@ -1188,11 +1188,11 @@ pack_channel(const union isl_color_value *value, unsigned i, } break; case ISL_UINT: - packed = MIN(value->u32[i], MAX_UINT(layout->bits)); + packed = MIN(value->u32[i], u_uintN_max(layout->bits)); break; case ISL_SINT: - packed = MIN(MAX(value->u32[i], MIN_INT(layout->bits)), - MAX_INT(layout->bits)); + packed = MIN(MAX(value->u32[i], u_intN_min(layout->bits)), + u_intN_max(layout->bits)); break; default: @@ -1202,7 +1202,7 @@ pack_channel(const union isl_color_value *value, unsigned i, unsigned dword = layout->start_bit / 32; unsigned bit = layout->start_bit % 32; assert(bit + layout->bits <= 32); - data_out[dword] |= (packed & MAX_UINT(layout->bits)) << bit; + data_out[dword] |= (packed & u_uintN_max(layout->bits)) << bit; } /** @@ -1264,7 +1264,7 @@ unpack_channel(union isl_color_value *value, unsigned dword = layout->start_bit / 32; unsigned bit = layout->start_bit % 32; assert(bit + layout->bits <= 32); - uint32_t packed = (data_in[dword] >> bit) & MAX_UINT(layout->bits); + uint32_t packed = (data_in[dword] >> bit) & u_uintN_max(layout->bits); union { uint32_t u32; diff --git a/src/util/fast_idiv_by_const.c b/src/util/fast_idiv_by_const.c index 4f0f6b769b8..b9f0b9cb760 100644 --- a/src/util/fast_idiv_by_const.c +++ b/src/util/fast_idiv_by_const.c @@ -39,6 +39,7 @@ #include "fast_idiv_by_const.h" #include "u_math.h" +#include "util/macros.h" #include <limits.h> #include <assert.h> @@ -65,8 +66,7 @@ util_compute_fast_udiv_info(uint64_t D, unsigned num_bits, unsigned UINT_BITS) } else { /* Dividing by 1. */ /* Assuming: floor((num + 1) * (2^32 - 1) / 2^32) = num */ - result.multiplier = UINT_BITS == 64 ? UINT64_MAX : - (1ull << UINT_BITS) - 1; + result.multiplier = u_uintN_max(UINT_BITS); result.pre_shift = 0; result.post_shift = 0; result.increment = 1; diff --git a/src/util/format/format_utils.h b/src/util/format/format_utils.h index fa1d30060d9..0768a26ecce 100644 --- a/src/util/format/format_utils.h +++ b/src/util/format/format_utils.h @@ -34,29 +34,24 @@ #include "util/half_float.h" #include "util/rounding.h" -/* Only guaranteed to work for BITS <= 32 */ -#define MAX_UINT(BITS) ((BITS) == 32 ? UINT32_MAX : ((1u << (BITS)) - 1)) -#define MAX_INT(BITS) ((int)MAX_UINT((BITS) - 1)) -#define MIN_INT(BITS) ((BITS) == 32 ? INT32_MIN : (-(1 << (BITS - 1)))) - /* Extends an integer of size SRC_BITS to one of size DST_BITS linearly */ #define EXTEND_NORMALIZED_INT(X, SRC_BITS, DST_BITS) \ - (((X) * (int)(MAX_UINT(DST_BITS) / MAX_UINT(SRC_BITS))) + \ + (((X) * (int)(u_uintN_max(DST_BITS) / u_uintN_max(SRC_BITS))) + \ ((DST_BITS % SRC_BITS) ? ((X) >> (SRC_BITS - DST_BITS % SRC_BITS)) : 0)) static inline float _mesa_unorm_to_float(unsigned x, unsigned src_bits) { - return x * (1.0f / (float)MAX_UINT(src_bits)); + return x * (1.0f / (float)u_uintN_max(src_bits)); } static inline float _mesa_snorm_to_float(int x, unsigned src_bits) { - if (x <= -MAX_INT(src_bits)) + if (x <= -u_intN_max(src_bits)) return -1.0f; else - return x * (1.0f / (float)MAX_INT(src_bits)); + return x * (1.0f / (float)u_intN_max(src_bits)); } static inline uint16_t @@ -77,9 +72,9 @@ _mesa_float_to_unorm(float x, unsigned dst_bits) if (x < 0.0f) return 0; else if (x > 1.0f) - return MAX_UINT(dst_bits); + return u_uintN_max(dst_bits); else - return _mesa_i64roundevenf(x * MAX_UINT(dst_bits)); + return _mesa_i64roundevenf(x * u_uintN_max(dst_bits)); } static inline unsigned @@ -98,10 +93,10 @@ _mesa_unorm_to_unorm(unsigned x, unsigned src_bits, unsigned dst_bits) if (src_bits + dst_bits > sizeof(x) * 8) { assert(src_bits + dst_bits <= sizeof(uint64_t) * 8); - return (((uint64_t) x * MAX_UINT(dst_bits) + src_half) / - MAX_UINT(src_bits)); + return (((uint64_t) x * u_uintN_max(dst_bits) + src_half) / + u_uintN_max(src_bits)); } else { - return (x * MAX_UINT(dst_bits) + src_half) / MAX_UINT(src_bits); + return (x * u_uintN_max(dst_bits) + src_half) / u_uintN_max(src_bits); } } else { return x; @@ -121,11 +116,11 @@ static inline int _mesa_float_to_snorm(float x, unsigned dst_bits) { if (x < -1.0f) - return -MAX_INT(dst_bits); + return -u_intN_max(dst_bits); else if (x > 1.0f) - return MAX_INT(dst_bits); + return u_intN_max(dst_bits); else - return _mesa_lroundevenf(x * MAX_INT(dst_bits)); + return _mesa_lroundevenf(x * u_intN_max(dst_bits)); } static inline int @@ -143,8 +138,8 @@ _mesa_unorm_to_snorm(unsigned x, unsigned src_bits, unsigned dst_bits) static inline int _mesa_snorm_to_snorm(int x, unsigned src_bits, unsigned dst_bits) { - if (x < -MAX_INT(src_bits)) - return -MAX_INT(dst_bits); + if (x < -u_intN_max(src_bits)) + return -u_intN_max(dst_bits); else if (src_bits < dst_bits) return EXTEND_NORMALIZED_INT(x, src_bits - 1, dst_bits - 1); else @@ -154,25 +149,25 @@ _mesa_snorm_to_snorm(int x, unsigned src_bits, unsigned dst_bits) static inline unsigned _mesa_unsigned_to_unsigned(unsigned src, unsigned dst_size) { - return MIN2(src, MAX_UINT(dst_size)); + return MIN2(src, u_uintN_max(dst_size)); } static inline int _mesa_unsigned_to_signed(unsigned src, unsigned dst_size) { - return MIN2(src, (unsigned)MAX_INT(dst_size)); + return MIN2(src, (unsigned)u_intN_max(dst_size)); } static inline int _mesa_signed_to_signed(int src, unsigned dst_size) { - return CLAMP(src, MIN_INT(dst_size), MAX_INT(dst_size)); + return CLAMP(src, u_intN_min(dst_size), u_intN_max(dst_size)); } static inline unsigned _mesa_signed_to_unsigned(int src, unsigned dst_size) { - return CLAMP(src, 0, MAX_UINT(dst_size)); + return CLAMP(src, 0, u_uintN_max(dst_size)); } static inline unsigned @@ -180,18 +175,18 @@ _mesa_float_to_unsigned(float src, unsigned dst_bits) { if (src < 0.0f) return 0; - if (src > (float)MAX_UINT(dst_bits)) - return MAX_UINT(dst_bits); + if (src > (float)u_uintN_max(dst_bits)) + return u_uintN_max(dst_bits); return _mesa_signed_to_unsigned(src, dst_bits); } static inline unsigned _mesa_float_to_signed(float src, unsigned dst_bits) { - if (src < (float)(-MAX_INT(dst_bits))) - return -MAX_INT(dst_bits); - if (src > (float)MAX_INT(dst_bits)) - return MAX_INT(dst_bits); + if (src < (float)(-u_intN_max(dst_bits))) + return -u_intN_max(dst_bits); + if (src > (float)u_intN_max(dst_bits)) + return u_intN_max(dst_bits); return _mesa_signed_to_signed(src, dst_bits); } diff --git a/src/util/macros.h b/src/util/macros.h index 1fc9e23355b..4bd18f55ec0 100644 --- a/src/util/macros.h +++ b/src/util/macros.h @@ -30,6 +30,8 @@ #include "c99_compat.h" #include "c11_compat.h" +#include <stdint.h> + /* Compute the size of an array */ #ifndef ARRAY_SIZE # define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) @@ -392,6 +394,30 @@ do { \ #define BITFIELD64_RANGE(b, count) \ (BITFIELD64_MASK((b) + (count)) & ~BITFIELD64_MASK(b)) +static inline int64_t +u_intN_max(unsigned bit_size) +{ + assert(bit_size <= 64 && bit_size > 0); + return INT64_MAX >> (64 - bit_size); +} + +static inline int64_t +u_intN_min(unsigned bit_size) +{ + /* On 2's compliment platforms, which is every platform Mesa is likely to + * every worry about, stdint.h generally calculated INT##_MIN in this + * manner. + */ + return (-u_intN_max(bit_size)) - 1; +} + +static inline uint64_t +u_uintN_max(unsigned bit_size) +{ + assert(bit_size <= 64 && bit_size > 0); + return UINT64_MAX >> (64 - bit_size); +} + /* TODO: In future we should try to move this to u_debug.h once header * dependencies are reorganised to allow this. */ diff --git a/src/util/meson.build b/src/util/meson.build index aa5bfef5dbc..319b22d9bf7 100644 --- a/src/util/meson.build +++ b/src/util/meson.build @@ -383,6 +383,15 @@ if with_tests env: ['BUILD_FULL_PATH='+process_test_exe_full_path] ) + test('int_min_max', + executable('int_min_max_test', + files('tests/int_min_max.cpp'), + include_directories : [inc_include, inc_src], + dependencies : [idep_mesautil, idep_gtest], + ), + suite : ['util'], + ) + subdir('tests/cache') subdir('tests/fast_idiv_by_const') subdir('tests/fast_urem_by_const') diff --git a/src/util/tests/fast_idiv_by_const/fast_idiv_by_const_test.cpp b/src/util/tests/fast_idiv_by_const/fast_idiv_by_const_test.cpp index 330f90fa464..abf6079944f 100644 --- a/src/util/tests/fast_idiv_by_const/fast_idiv_by_const_test.cpp +++ b/src/util/tests/fast_idiv_by_const/fast_idiv_by_const_test.cpp @@ -30,9 +30,6 @@ #define RAND_TEST_ITERATIONS 100000 -#define MAX_UINT(bits) \ - (((bits) == 64) ? UINT64_MAX : ((1ull << (bits)) - 1)) - static inline uint64_t utrunc(uint64_t x, unsigned num_bits) { @@ -82,7 +79,7 @@ uadd_sat(uint64_t a, uint64_t b, unsigned num_bits) return sum < a ? UINT64_MAX : sum; } else { /* Check if sum is more than num_bits */ - return (sum >> num_bits) ? MAX_UINT(num_bits) : sum; + return (sum >> num_bits) ? u_uintN_max(num_bits) : sum; } } @@ -201,7 +198,7 @@ rand_uint(unsigned bits, unsigned min) if (k == 17) { return min + (rand() % 16); } else if (k == 42) { - return MAX_UINT(bits) - (rand() % 16); + return u_uintN_max(bits) - (rand() % 16); } else if (k == 9) { uint64_t r; do { @@ -230,7 +227,7 @@ rand_sint(unsigned bits, unsigned min_abs) { /* Make sure we hit MIN_INT every once in a while */ if (rand() % 64 == 37) - return INT64_MIN >> (64 - bits); + return u_intN_min(bits); int64_t s = rand_uint(bits - 1, min_abs); return rand() & 1 ? s : -s; diff --git a/src/util/tests/int_min_max.cpp b/src/util/tests/int_min_max.cpp new file mode 100644 index 00000000000..8d74ecb7d33 --- /dev/null +++ b/src/util/tests/int_min_max.cpp @@ -0,0 +1,73 @@ +/* + * Copyright © 2021 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + */ + +#include <gtest/gtest.h> +#include "util/macros.h" + +#define MESA_UINT24_MAX 16777215 +#define MESA_INT24_MAX 8388607 +#define MESA_INT24_MIN (-8388607-1) + +#define MESA_UINT12_MAX 4095 +#define MESA_INT12_MAX 2047 +#define MESA_INT12_MIN (-2047-1) + +#define MESA_UINT10_MAX 1023 +#define MESA_INT10_MAX 511 +#define MESA_INT10_MIN (-511-1) + +TEST(int_min_max, u_intN_min) +{ + EXPECT_EQ(INT64_MIN, u_intN_min(64)); + EXPECT_EQ(INT32_MIN, u_intN_min(32)); + EXPECT_EQ(INT16_MIN, u_intN_min(16)); + EXPECT_EQ(INT8_MIN, u_intN_min(8)); + + EXPECT_EQ(MESA_INT24_MIN, u_intN_min(24)); + EXPECT_EQ(MESA_INT12_MIN, u_intN_min(12)); + EXPECT_EQ(MESA_INT10_MIN, u_intN_min(10)); +} + +TEST(int_min_max, u_intN_max) +{ + EXPECT_EQ(INT64_MAX, u_intN_max(64)); + EXPECT_EQ(INT32_MAX, u_intN_max(32)); + EXPECT_EQ(INT16_MAX, u_intN_max(16)); + EXPECT_EQ(INT8_MAX, u_intN_max(8)); + + EXPECT_EQ(MESA_INT24_MAX, u_intN_max(24)); + EXPECT_EQ(MESA_INT12_MAX, u_intN_max(12)); + EXPECT_EQ(MESA_INT10_MAX, u_intN_max(10)); +} + +TEST(int_min_max, u_uintN_max) +{ + EXPECT_EQ(UINT64_MAX, u_uintN_max(64)); + EXPECT_EQ(UINT32_MAX, u_uintN_max(32)); + EXPECT_EQ(UINT16_MAX, u_uintN_max(16)); + EXPECT_EQ(UINT8_MAX, u_uintN_max(8)); + + EXPECT_EQ(MESA_UINT24_MAX, u_uintN_max(24)); + EXPECT_EQ(MESA_UINT12_MAX, u_uintN_max(12)); + EXPECT_EQ(MESA_UINT10_MAX, u_uintN_max(10)); +}
