WillAyd commented on code in PR #47958:
URL: https://github.com/apache/arrow/pull/47958#discussion_r2466378822
##########
cpp/src/arrow/util/int_util_overflow.h:
##########
@@ -38,49 +35,141 @@ namespace internal {
// On overflow, these functions return true. Otherwise, false is returned
// and `out` is updated with the result of the operation.
-#define OP_WITH_OVERFLOW(_func_name, _psnip_op, _type, _psnip_type) \
- [[nodiscard]] static inline bool _func_name(_type u, _type v, _type* out) { \
- return !psnip_safe_##_psnip_type##_##_psnip_op(out, u, v); \
+#define SAFE_INT_OP_WITH_OVERFLOW(_func_name, _op_name, _c_type, _type)
\
+ [[nodiscard]] static inline bool _func_name(_c_type u, _c_type v, _c_type*
out) { \
+ return !check_##_op_name##_##_type##_##_type(u, v, out);
\
}
-#define OPS_WITH_OVERFLOW(_func_name, _psnip_op) \
- OP_WITH_OVERFLOW(_func_name, _psnip_op, int8_t, int8) \
- OP_WITH_OVERFLOW(_func_name, _psnip_op, int16_t, int16) \
- OP_WITH_OVERFLOW(_func_name, _psnip_op, int32_t, int32) \
- OP_WITH_OVERFLOW(_func_name, _psnip_op, int64_t, int64) \
- OP_WITH_OVERFLOW(_func_name, _psnip_op, uint8_t, uint8) \
- OP_WITH_OVERFLOW(_func_name, _psnip_op, uint16_t, uint16) \
- OP_WITH_OVERFLOW(_func_name, _psnip_op, uint32_t, uint32) \
- OP_WITH_OVERFLOW(_func_name, _psnip_op, uint64_t, uint64)
+#define SAFE_INT_OPS_WITH_OVERFLOW(_func_name, _op_name) \
+ SAFE_INT_OP_WITH_OVERFLOW(_func_name, _op_name, int32_t, int32) \
+ SAFE_INT_OP_WITH_OVERFLOW(_func_name, _op_name, int64_t, int64) \
+ SAFE_INT_OP_WITH_OVERFLOW(_func_name, _op_name, uint32_t, uint32) \
+ SAFE_INT_OP_WITH_OVERFLOW(_func_name, _op_name, uint64_t, uint64)
+
+SAFE_INT_OPS_WITH_OVERFLOW(SafeIntAddWithOverflow, add)
+SAFE_INT_OPS_WITH_OVERFLOW(SafeIntSubtractWithOverflow, sub)
+SAFE_INT_OPS_WITH_OVERFLOW(SafeIntMultiplyWithOverflow, mul)
+
+#undef SAFE_INT_OP_WITH_OVERFLOW
+#undef SAFE_INT_OPS_WITH_OVERFLOW
+
+template <typename Int, typename SignedRet, typename UnsignedRet>
+using transformed_int_t =
+ std::conditional_t<std::is_signed_v<Int>, SignedRet, UnsignedRet>;
+
+template <typename Int>
+using upscaled_int32_t = transformed_int_t<Int, int32_t, uint32_t>;
+
+// Use GCC/CLang builtins for checked arithmetic, promising better performance
Review Comment:
Does SafeInt say anything about its comparison to the Windows intsafe.h
functions? Not a blocker here by any means
##########
cpp/src/arrow/util/int_util_overflow.h:
##########
@@ -38,49 +35,141 @@ namespace internal {
// On overflow, these functions return true. Otherwise, false is returned
// and `out` is updated with the result of the operation.
-#define OP_WITH_OVERFLOW(_func_name, _psnip_op, _type, _psnip_type) \
- [[nodiscard]] static inline bool _func_name(_type u, _type v, _type* out) { \
- return !psnip_safe_##_psnip_type##_##_psnip_op(out, u, v); \
+#define SAFE_INT_OP_WITH_OVERFLOW(_func_name, _op_name, _c_type, _type)
\
+ [[nodiscard]] static inline bool _func_name(_c_type u, _c_type v, _c_type*
out) { \
+ return !check_##_op_name##_##_type##_##_type(u, v, out);
\
}
-#define OPS_WITH_OVERFLOW(_func_name, _psnip_op) \
- OP_WITH_OVERFLOW(_func_name, _psnip_op, int8_t, int8) \
- OP_WITH_OVERFLOW(_func_name, _psnip_op, int16_t, int16) \
- OP_WITH_OVERFLOW(_func_name, _psnip_op, int32_t, int32) \
- OP_WITH_OVERFLOW(_func_name, _psnip_op, int64_t, int64) \
- OP_WITH_OVERFLOW(_func_name, _psnip_op, uint8_t, uint8) \
- OP_WITH_OVERFLOW(_func_name, _psnip_op, uint16_t, uint16) \
- OP_WITH_OVERFLOW(_func_name, _psnip_op, uint32_t, uint32) \
- OP_WITH_OVERFLOW(_func_name, _psnip_op, uint64_t, uint64)
+#define SAFE_INT_OPS_WITH_OVERFLOW(_func_name, _op_name) \
+ SAFE_INT_OP_WITH_OVERFLOW(_func_name, _op_name, int32_t, int32) \
+ SAFE_INT_OP_WITH_OVERFLOW(_func_name, _op_name, int64_t, int64) \
+ SAFE_INT_OP_WITH_OVERFLOW(_func_name, _op_name, uint32_t, uint32) \
+ SAFE_INT_OP_WITH_OVERFLOW(_func_name, _op_name, uint64_t, uint64)
+
+SAFE_INT_OPS_WITH_OVERFLOW(SafeIntAddWithOverflow, add)
+SAFE_INT_OPS_WITH_OVERFLOW(SafeIntSubtractWithOverflow, sub)
+SAFE_INT_OPS_WITH_OVERFLOW(SafeIntMultiplyWithOverflow, mul)
+
+#undef SAFE_INT_OP_WITH_OVERFLOW
+#undef SAFE_INT_OPS_WITH_OVERFLOW
+
+template <typename Int, typename SignedRet, typename UnsignedRet>
+using transformed_int_t =
+ std::conditional_t<std::is_signed_v<Int>, SignedRet, UnsignedRet>;
+
+template <typename Int>
+using upscaled_int32_t = transformed_int_t<Int, int32_t, uint32_t>;
+
+// Use GCC/CLang builtins for checked arithmetic, promising better performance
+// than SafeInt's hand-written implementations.
+#if defined __has_builtin
+# if __has_builtin(__builtin_object_size)
+# define USE_CHECKED_ARITHMETIC_BUILTINS 1
+# else
+# define USE_CHECKED_ARITHMETIC_BUILTINS 0
+# endif
+#endif
+
+template <typename Int>
+[[nodiscard]] bool AddWithOverflowGeneric(Int u, Int v, Int* out) {
+#if USE_CHECKED_ARITHMETIC_BUILTINS
Review Comment:
Maybe it would make things easier if this was a part of the
`SAFE_INT_OPS_WITH_OVERFLOW` macro
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
