[clang] [HIP] fix host min/max in header (PR #82956)

Artem Belevich via cfe-commits cfe-commits at lists.llvm.org
Mon Feb 26 11:36:42 PST 2024


================
@@ -1306,15 +1306,68 @@ float min(float __x, float __y) { return __builtin_fminf(__x, __y); }
 __DEVICE__
 double min(double __x, double __y) { return __builtin_fmin(__x, __y); }
 
-#if !defined(__HIPCC_RTC__) && !defined(__OPENMP_AMDGCN__)
-__host__ inline static int min(int __arg1, int __arg2) {
-  return __arg1 < __arg2 ? __arg1 : __arg2;
+// Define host min/max functions.
+#if !defined(__HIPCC_RTC__) && !defined(__OPENMP_AMDGCN__) &&                  \
+    !defined(__HIP_NO_HOST_MIN_MAX_IN_GLOBAL_NAMESPACE__)
+
+#pragma push_macro("DEFINE_MIN_MAX_FUNCTIONS")
+#pragma push_macro("DEFINE_MIN_MAX_FUNCTIONS")
+#define DEFINE_MIN_MAX_FUNCTIONS(ret_type, type1, type2)                       \
+  static inline ret_type min(const type1 __a, const type2 __b) {               \
+    return (__a < __b) ? __a : __b;                                            \
+  }                                                                            \
+  static inline ret_type max(const type1 __a, const type2 __b) {               \
+    return (__a > __b) ? __a : __b;                                            \
+  }
+
+// Define min and max functions for same type comparisons
+DEFINE_MIN_MAX_FUNCTIONS(int, int, int)
+DEFINE_MIN_MAX_FUNCTIONS(unsigned int, unsigned int, unsigned int)
+DEFINE_MIN_MAX_FUNCTIONS(long, long, long)
+DEFINE_MIN_MAX_FUNCTIONS(unsigned long, unsigned long, unsigned long)
+DEFINE_MIN_MAX_FUNCTIONS(long long, long long, long long)
+DEFINE_MIN_MAX_FUNCTIONS(unsigned long long, unsigned long long,
+                         unsigned long long)
+
+// Define min and max functions for all mixed type comparisons
+DEFINE_MIN_MAX_FUNCTIONS(unsigned int, int, unsigned int)
+DEFINE_MIN_MAX_FUNCTIONS(unsigned int, unsigned int, int)
+DEFINE_MIN_MAX_FUNCTIONS(unsigned long, long, unsigned long)
+DEFINE_MIN_MAX_FUNCTIONS(unsigned long, unsigned long, long)
+DEFINE_MIN_MAX_FUNCTIONS(unsigned long long, long long, unsigned long long)
+DEFINE_MIN_MAX_FUNCTIONS(unsigned long long, unsigned long long, long long)
----------------
Artem-B wrote:

Not everything CUDA does is the right model to follow. This may be one of the cases where we should improve things, if we can, instead of just copying the broken behavior. Not adding problematic things is easier than removing them later, when they are used, intentionally or not.

Considering that HIP currently does not have those functions, it would suggest that there is probably no existing HIP code depending on them. Existing cuda code which may need those functions will need some amount of porting to HIP, anyway, so fixing the source code could be done as part of the porting effort.

We could put those mixed min/max functions under some preprocessor guard, which would keep them disabled by default. If someone desperately needs them, they would have to specify `-DPLEASE_ENABLE_BROKEN_MINMAX_ON_MIXED_SIGNED_UNSIGNED_TYPES`.



https://github.com/llvm/llvm-project/pull/82956


More information about the cfe-commits mailing list