[clang] [HIP] fix host min/max in header (PR #82956)
yxsamliu wrote: found another library using mixed min: https://github.com/ROCm/Tensile/issues/1977 https://github.com/llvm/llvm-project/pull/82956 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HIP] fix host min/max in header (PR #82956)
yxsamliu wrote: > > > Probably I need to define those functions with mixed args by default to > > > avoid regressions. > > > > > > Are there any other regressions? Can hupCUB be fixed instead? While their > > use case is probably benign, I'd rather fix the user code, than propagate > > CUDA bugs into HIP. > > So far we only found this issue in our internal CI. I will try asking hipCUB > to fix it on their side. hipCUB issue opened https://github.com/ROCm/hipCUB/issues/343 https://github.com/llvm/llvm-project/pull/82956 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HIP] fix host min/max in header (PR #82956)
yxsamliu wrote: > > Probably I need to define those functions with mixed args by default to > > avoid regressions. > > Are there any other regressions? Can hupCUB be fixed instead? While their use > case is probably benign, I'd rather fix the user code, than propagate CUDA > bugs into HIP. So far we only found this issue in our internal CI. I will try asking hipCUB to fix it on their side. https://github.com/llvm/llvm-project/pull/82956 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HIP] fix host min/max in header (PR #82956)
Artem-B wrote: > Probably I need to define those functions with mixed args by default to avoid > regressions. Are there any other regressions? Can hupCUB be fixed intsead? While their use case is probably benign, I'd rather fix the user code, than propagate CUDA bugs into HIP. https://github.com/llvm/llvm-project/pull/82956 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HIP] fix host min/max in header (PR #82956)
@@ -1306,15 +1306,73 @@ 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)
\
+ inline ret_type min(const type1 __a, const type2 __b) {
\
+return (__a < __b) ? __a : __b;
\
+ }
\
+ 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)
+
+// CUDA defines host min/max functions with mixed signed/unsgined integer
+// parameters where signed integers are casted to unsigned integers. However,
+// this may not be users' intention. Therefore do not define them by default
+// unless users specify -D__HIP_DEFINE_MIXED_HOST_MIN_MAX__.
yxsamliu wrote:
I reverted this PR due to regression in hipCUB:
https://github.com/ROCm/hipCUB/blob/develop/hipcub/include/hipcub/backend/rocprim/device/device_spmv.hpp#L142
hipCUB/hipcub/include/hipcub/backend/rocprim/device/device_spmv.hpp:142:33:
error: call to 'min' is ambiguous
it is a call of min(int, unsigned int). The code is likely ported from CUDA.
Probably I need to define those functions with mixed args by default to avoid
regressions.
https://github.com/llvm/llvm-project/pull/82956
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HIP] fix host min/max in header (PR #82956)
https://github.com/yxsamliu closed https://github.com/llvm/llvm-project/pull/82956 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HIP] fix host min/max in header (PR #82956)
https://github.com/yxsamliu updated
https://github.com/llvm/llvm-project/pull/82956
>From ebad3ba006445f290d17c338cc1b39293c18cdad Mon Sep 17 00:00:00 2001
From: "Yaxun (Sam) Liu"
Date: Sun, 25 Feb 2024 11:13:40 -0500
Subject: [PATCH] [HIP] fix host min/max in header
CUDA defines min/max functions for host in global namespace.
HIP header needs to define them too to be compatible.
Currently only min/max(int, int) is defined. This causes
wrong result for arguments that are out of range for int.
This patch defines host min/max functions to be compatible
with CUDA.
Also allows users to define
`__HIP_NO_HOST_MIN_MAX_IN_GLOBAL_NAMESPACE__` to disable
host max/min in global namespace.
min/max functions with mixed signed/unsigned integer
parameters are not defined unless
`__HIP_DEFINE_MIXED_HOST_MIN_MAX__` is defined.
Fixes: SWDEV-446564
---
clang/lib/Headers/__clang_hip_math.h | 72 +---
1 file changed, 66 insertions(+), 6 deletions(-)
diff --git a/clang/lib/Headers/__clang_hip_math.h
b/clang/lib/Headers/__clang_hip_math.h
index 11e1e7d032586f..34d1de0a060055 100644
--- a/clang/lib/Headers/__clang_hip_math.h
+++ b/clang/lib/Headers/__clang_hip_math.h
@@ -1306,15 +1306,75 @@ 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)
\
+ inline ret_type min(const type1 __a, const type2 __b) {
\
+return (__a < __b) ? __a : __b;
\
+ }
\
+ 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)
+
+// The host min/max functions below accept mixed signed/unsigned integer
+// parameters and perform unsigned comparisons, which may produce unexpected
+// results if a signed integer was passed unintentionally. To avoid this
+// happening silently, these overloaded functions are not defined by default.
+// However, for compatibility with CUDA, they will be defined if users define
+// __HIP_DEFINE_MIXED_HOST_MIN_MAX__.
+#ifdef __HIP_DEFINE_MIXED_HOST_MIN_MAX__
+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)
+#endif // ifdef __HIP_DEFINE_MIXED_HOST_MIN_MAX__
+
+// Floating-point comparisons using built-in functions
+inline float min(float const __a, float const __b) {
+ return __builtin_fminf(__a, __b);
+}
+inline double min(double const __a, double const __b) {
+ return __builtin_fmin(__a, __b);
+}
+inline double min(float const __a, double const __b) {
+ return __builtin_fmin(__a, __b);
+}
+inline double min(double const __a, float const __b) {
+ return __builtin_fmin(__a, __b);
}
-__host__ inline static int max(int __arg1, int __arg2) {
- return __arg1 > __arg2 ? __arg1 : __arg2;
+inline float max(float const __a, float const __b) {
+ return __builtin_fmaxf(__a, __b);
+}
+inline double max(double const __a, double const __b) {
+ return __builtin_fmax(__a, __b);
+}
+inline double max(float const __a, double const __b) {
+ return __builtin_fmax(__a, __b);
}
-#endif // !defined(__HIPCC_RTC__) && !defined(__OPENMP_AMDGCN__)
+inline double max(double const __a, float const __b) {
+ return __builtin_fmax(__a, __b);
+}
+
+#pragma pop_macro("DEFINE_MIN_MAX_FUNCTIONS")
+
+#endif // !defined(__HIPCC_RTC__) && !defined(__OPENMP_AMDGCN__) &&
+ // !defined(__HIP_NO_HOST_MIN_MAX_IN_GLOBAL_NAMESPACE__)
#endif
#pragma pop_macro("__DEVICE__")
___
cfe-commits mailing list
[clang] [HIP] fix host min/max in header (PR #82956)
@@ -1306,15 +1306,73 @@ 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)
\
+ inline ret_type min(const type1 __a, const type2 __b) {
\
+return (__a < __b) ? __a : __b;
\
+ }
\
+ 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)
+
+// CUDA defines host min/max functions with mixed signed/unsgined integer
+// parameters where signed integers are casted to unsigned integers. However,
+// this may not be users' intention. Therefore do not define them by default
+// unless users specify -D__HIP_DEFINE_MIXED_HOST_MIN_MAX__.
yxsamliu wrote:
will do
https://github.com/llvm/llvm-project/pull/82956
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HIP] fix host min/max in header (PR #82956)
https://github.com/Artem-B approved this pull request. https://github.com/llvm/llvm-project/pull/82956 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HIP] fix host min/max in header (PR #82956)
@@ -1306,15 +1306,73 @@ 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)
\
+ inline ret_type min(const type1 __a, const type2 __b) {
\
+return (__a < __b) ? __a : __b;
\
+ }
\
+ 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)
+
+// CUDA defines host min/max functions with mixed signed/unsgined integer
+// parameters where signed integers are casted to unsigned integers. However,
+// this may not be users' intention. Therefore do not define them by default
+// unless users specify -D__HIP_DEFINE_MIXED_HOST_MIN_MAX__.
Artem-B wrote:
Nit: signed integers are implicitly promoted to unsigned ones due to the
integer promotion rules. Cast would imply intentional cast and we're not doing
that.
I'd rephrase it a bit along the lines of:
The routines below will perform unsigned comparison, which may produce invalid
results if a signed integer was passed unintentionally. We do not want it
happen silently, and do not provide these overloads by default. However for
compatibility with CUDA, we allow them, if explicitly requested by the user by
defining `__HIP_DEFINE_MIXED_HOST_MIN_MAX__`.
https://github.com/llvm/llvm-project/pull/82956
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HIP] fix host min/max in header (PR #82956)
https://github.com/Artem-B edited https://github.com/llvm/llvm-project/pull/82956 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HIP] fix host min/max in header (PR #82956)
@@ -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)
yxsamliu wrote:
OK. will define them only if `__HIP_DEFINE_MIXED_HOST_MIN_MAX__` is defined.
https://github.com/llvm/llvm-project/pull/82956
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HIP] fix host min/max in header (PR #82956)
https://github.com/yxsamliu updated
https://github.com/llvm/llvm-project/pull/82956
>From f7471303abf989ceb1bdbce0d580d74097572dec Mon Sep 17 00:00:00 2001
From: "Yaxun (Sam) Liu"
Date: Sun, 25 Feb 2024 11:13:40 -0500
Subject: [PATCH] [HIP] fix host min/max in header
CUDA defines min/max functions for host in global namespace.
HIP header needs to define them too to be compatible.
Currently only min/max(int, int) is defined. This causes
wrong result for arguments that are out of range for int.
This patch defines host min/max functions to be compatible
with CUDA.
Also allows users to define
`__HIP_NO_HOST_MIN_MAX_IN_GLOBAL_NAMESPACE__` to disable
host max/min in global namespace.
min/max functions with mixed signed/unsigned integer
parameters are not defined unless
`__HIP_DEFINE_MIXED_HOST_MIN_MAX__` is defined.
Fixes: SWDEV-446564
---
clang/lib/Headers/__clang_hip_math.h | 70 +---
1 file changed, 64 insertions(+), 6 deletions(-)
diff --git a/clang/lib/Headers/__clang_hip_math.h
b/clang/lib/Headers/__clang_hip_math.h
index 11e1e7d032586f..c5b07effeceb44 100644
--- a/clang/lib/Headers/__clang_hip_math.h
+++ b/clang/lib/Headers/__clang_hip_math.h
@@ -1306,15 +1306,73 @@ 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)
\
+ inline ret_type min(const type1 __a, const type2 __b) {
\
+return (__a < __b) ? __a : __b;
\
+ }
\
+ 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)
+
+// CUDA defines host min/max functions with mixed signed/unsgined integer
+// parameters where signed integers are casted to unsigned integers. However,
+// this may not be users' intention. Therefore do not define them by default
+// unless users specify -D__HIP_DEFINE_MIXED_HOST_MIN_MAX__.
+#ifdef __HIP_DEFINE_MIXED_HOST_MIN_MAX__
+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)
+#endif // ifdef __HIP_DEFINE_MIXED_HOST_MIN_MAX__
+
+// Floating-point comparisons using built-in functions
+inline float min(float const __a, float const __b) {
+ return __builtin_fminf(__a, __b);
+}
+inline double min(double const __a, double const __b) {
+ return __builtin_fmin(__a, __b);
+}
+inline double min(float const __a, double const __b) {
+ return __builtin_fmin(__a, __b);
+}
+inline double min(double const __a, float const __b) {
+ return __builtin_fmin(__a, __b);
}
-__host__ inline static int max(int __arg1, int __arg2) {
- return __arg1 > __arg2 ? __arg1 : __arg2;
+inline float max(float const __a, float const __b) {
+ return __builtin_fmaxf(__a, __b);
+}
+inline double max(double const __a, double const __b) {
+ return __builtin_fmax(__a, __b);
+}
+inline double max(float const __a, double const __b) {
+ return __builtin_fmax(__a, __b);
}
-#endif // !defined(__HIPCC_RTC__) && !defined(__OPENMP_AMDGCN__)
+inline double max(double const __a, float const __b) {
+ return __builtin_fmax(__a, __b);
+}
+
+#pragma pop_macro("DEFINE_MIN_MAX_FUNCTIONS")
+
+#endif // !defined(__HIPCC_RTC__) && !defined(__OPENMP_AMDGCN__) &&
+ // !defined(__HIP_NO_HOST_MIN_MAX_IN_GLOBAL_NAMESPACE__)
#endif
#pragma pop_macro("__DEVICE__")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HIP] fix host min/max in header (PR #82956)
@@ -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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HIP] fix host min/max in header (PR #82956)
@@ -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)
yxsamliu wrote:
> I assume these are needed in order to avoid errors about ambiguous overload
> resolution when we pass signed/unsigned arguments.
>
> Normally, if we were to use `std::min()` function, the user would have to
> explicitly cast arguments or use `std::min()` to resolve the issue.
>
> Implicitly converting int->unsigned under the hood is probably not a good
> idea here as we do not know what the user needs/wants and whether it's a WAI
> or an error. For min/max converting a negative argument into an unsigned
> would probably be an error. I think we do need to force users to use one of
> the all-signed or all-unsigned variants here, too, same as with std::min/max.
You are right about that std::min/max does not allow mixed signed/unsigned
arguments.
Unfortunately, that is how CUDA defines its host min/max functions, e.g.
https://godbolt.org/z/Wxxq9dv91
Not following this could cause incompatibility and regressions.
https://github.com/llvm/llvm-project/pull/82956
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HIP] fix host min/max in header (PR #82956)
https://github.com/yxsamliu updated
https://github.com/llvm/llvm-project/pull/82956
>From c8331bffa27011b953747d3ad4f7b423cf73b4a4 Mon Sep 17 00:00:00 2001
From: "Yaxun (Sam) Liu"
Date: Sun, 25 Feb 2024 11:13:40 -0500
Subject: [PATCH] [HIP] fix host min/max in header
CUDA defines min/max functions for host in global namespace.
HIP header needs to define them too to be compatible.
Currently only min/max(int, int) is defined. This causes
wrong result for arguments that are out of range for int.
This patch defines host min/max functions to be compatible
with CUDA.
Also allows users to define
`__HIP_NO_HOST_MIN_MAX_IN_GLOBAL_NAMESPACE__` to disable
host max/min in global namespace.
Fixes: SWDEV-446564
---
clang/lib/Headers/__clang_hip_math.h | 65 +---
1 file changed, 59 insertions(+), 6 deletions(-)
diff --git a/clang/lib/Headers/__clang_hip_math.h
b/clang/lib/Headers/__clang_hip_math.h
index 11e1e7d032586f..f606587c7a8ee8 100644
--- a/clang/lib/Headers/__clang_hip_math.h
+++ b/clang/lib/Headers/__clang_hip_math.h
@@ -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__) && __HIP__
+
+#pragma push_macro("DEFINE_MIN_MAX_FUNCTIONS")
+#pragma push_macro("DEFINE_MIN_MAX_FUNCTIONS")
+#define DEFINE_MIN_MAX_FUNCTIONS(ret_type, type1, type2)
\
+ inline ret_type min(const type1 __a, const type2 __b) {
\
+return (__a < __b) ? __a : __b;
\
+ }
\
+ 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)
+
+// Floating-point comparisons using built-in functions
+inline float min(float const __a, float const __b) {
+ return __builtin_fminf(__a, __b);
+}
+inline double min(double const __a, double const __b) {
+ return __builtin_fmin(__a, __b);
+}
+inline double min(float const __a, double const __b) {
+ return __builtin_fmin(__a, __b);
+}
+inline double min(double const __a, float const __b) {
+ return __builtin_fmin(__a, __b);
}
-__host__ inline static int max(int __arg1, int __arg2) {
- return __arg1 > __arg2 ? __arg1 : __arg2;
+inline float max(float const __a, float const __b) {
+ return __builtin_fmaxf(__a, __b);
+}
+inline double max(double const __a, double const __b) {
+ return __builtin_fmax(__a, __b);
+}
+inline double max(float const __a, double const __b) {
+ return __builtin_fmax(__a, __b);
}
-#endif // !defined(__HIPCC_RTC__) && !defined(__OPENMP_AMDGCN__)
+inline double max(double const __a, float const __b) {
+ return __builtin_fmax(__a, __b);
+}
+
+#pragma pop_macro("DEFINE_MIN_MAX_FUNCTIONS")
+
+#endif // !defined(__HIPCC_RTC__) && !defined(__OPENMP_AMDGCN__) &&
+ // !defined(__HIP_NO_HOST_MIN_MAX_IN_GLOBAL_NAMESPACE__) && __HIP__
#endif
#pragma pop_macro("__DEVICE__")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HIP] fix host min/max in header (PR #82956)
@@ -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:
I assume these are needed in order to avoid errors about ambiguous overload
resolution when we pass signed/unsigned arguments.
Normally, if we were to use `std::min()` function, the user would have to
explicitly cast arguments or use `std::min()` to resolve the issue.
Implicitly converting int->unsigned under the hood is probably not a good idea
here as we do not know what the user needs/wants and whether it's a WAI or an
error. For min/max converting a negative argument into an unsigned would
probably be an error. I think we do need to force users to use one of the
all-signed or all-unsigned variants here, too, same as with std::min/max.
https://github.com/llvm/llvm-project/pull/82956
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HIP] fix host min/max in header (PR #82956)
@@ -1306,14 +1306,50 @@ float min(float __x, float __y) { return
__builtin_fminf(__x, __y); }
__DEVICE__
double min(double __x, double __y) { return __builtin_fmin(__x, __y); }
+// Define host min/max functions.
+
#if !defined(__HIPCC_RTC__) && !defined(__OPENMP_AMDGCN__)
-__host__ inline static int min(int __arg1, int __arg2) {
- return __arg1 < __arg2 ? __arg1 : __arg2;
-}
-__host__ inline static int max(int __arg1, int __arg2) {
- return __arg1 > __arg2 ? __arg1 : __arg2;
-}
+#pragma push_macro("DEFINE_MIN_MAX_FUNCTIONS")
+#define DEFINE_MIN_MAX_FUNCTIONS(type1, type2) \
+static inline auto min(const type1 __a, const type2 __b) \
+ -> typename std::remove_reference::type { \
+ return (__a < __b) ? __a : __b; \
+} \
+static inline auto max(const type1 __a, const type2 __b) \
+ -> typename std::remove_reference __b ? __a : __b)>::type { \
+ return (__a > __b) ? __a : __b; \
+}
+
+// Define min and max functions for same type comparisons
+DEFINE_MIN_MAX_FUNCTIONS(int, int)
yxsamliu wrote:
it is not just limiting types. template function has lower precedence than
functions in overloading resolution. If users using std::min we will get
different behavior than CUDA
https://github.com/llvm/llvm-project/pull/82956
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HIP] fix host min/max in header (PR #82956)
@@ -1306,14 +1306,50 @@ float min(float __x, float __y) { return
__builtin_fminf(__x, __y); }
__DEVICE__
double min(double __x, double __y) { return __builtin_fmin(__x, __y); }
+// Define host min/max functions.
+
#if !defined(__HIPCC_RTC__) && !defined(__OPENMP_AMDGCN__)
-__host__ inline static int min(int __arg1, int __arg2) {
- return __arg1 < __arg2 ? __arg1 : __arg2;
-}
-__host__ inline static int max(int __arg1, int __arg2) {
- return __arg1 > __arg2 ? __arg1 : __arg2;
-}
+#pragma push_macro("DEFINE_MIN_MAX_FUNCTIONS")
+#define DEFINE_MIN_MAX_FUNCTIONS(type1, type2) \
+static inline auto min(const type1 __a, const type2 __b) \
+ -> typename std::remove_reference::type { \
+ return (__a < __b) ? __a : __b; \
+} \
+static inline auto max(const type1 __a, const type2 __b) \
+ -> typename std::remove_reference __b ? __a : __b)>::type { \
+ return (__a > __b) ? __a : __b; \
+}
+
+// Define min and max functions for same type comparisons
+DEFINE_MIN_MAX_FUNCTIONS(int, int)
jhuber6 wrote:
Could we not do stuff like `static_assert` where we check if both types are the
same modulo `unsigned` or `const`? I'm assuming that would be similar. I'd just
prefer to avoid macro magic if possible, but it we really need it for
compatibility reasons we can use it.
https://github.com/llvm/llvm-project/pull/82956
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HIP] fix host min/max in header (PR #82956)
https://github.com/yxsamliu updated
https://github.com/llvm/llvm-project/pull/82956
>From bd87c56b2d96b834788e8fa449f3ac308faec1f0 Mon Sep 17 00:00:00 2001
From: "Yaxun (Sam) Liu"
Date: Sun, 25 Feb 2024 11:13:40 -0500
Subject: [PATCH] [HIP] fix host min/max in header
CUDA defines min/max functions for host in global namespace.
HIP header needs to define them too to be compatible.
Currently only min/max(int, int) is defined. This causes
wrong result for arguments that are out of range for int.
This patch defines host min/max functions to be compatible
with CUDA.
Also allows users to define
`__HIP_NO_HOST_MIN_MAX_IN_GLOBAL_NAMESPACE__` to disable
host max/min in global namespace.
Fixes: SWDEV-446564
---
clang/lib/Headers/__clang_hip_math.h | 65 +---
1 file changed, 59 insertions(+), 6 deletions(-)
diff --git a/clang/lib/Headers/__clang_hip_math.h
b/clang/lib/Headers/__clang_hip_math.h
index 11e1e7d032586f..8427f872722850 100644
--- a/clang/lib/Headers/__clang_hip_math.h
+++ b/clang/lib/Headers/__clang_hip_math.h
@@ -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)
+
+// Floating-point comparisons using built-in functions
+static inline float min(float const __a, float const __b) {
+ return __builtin_fminf(__a, __b);
+}
+static inline double min(double const __a, double const __b) {
+ return __builtin_fmin(__a, __b);
+}
+static inline double min(float const __a, double const __b) {
+ return __builtin_fmin(__a, __b);
+}
+static inline double min(double const __a, float const __b) {
+ return __builtin_fmin(__a, __b);
}
-__host__ inline static int max(int __arg1, int __arg2) {
- return __arg1 > __arg2 ? __arg1 : __arg2;
+static inline float max(float const __a, float const __b) {
+ return __builtin_fmaxf(__a, __b);
+}
+static inline double max(double const __a, double const __b) {
+ return __builtin_fmax(__a, __b);
+}
+static inline double max(float const __a, double const __b) {
+ return __builtin_fmax(__a, __b);
}
-#endif // !defined(__HIPCC_RTC__) && !defined(__OPENMP_AMDGCN__)
+static inline double max(double const __a, float const __b) {
+ return __builtin_fmax(__a, __b);
+}
+
+#pragma pop_macro("DEFINE_MIN_MAX_FUNCTIONS")
+
+#endif // !defined(__HIPCC_RTC__) && !defined(__OPENMP_AMDGCN__) &&
+ // !defined(__HIP_NO_HOST_MIN_MAX_IN_GLOBAL_NAMESPACE__)
#endif
#pragma pop_macro("__DEVICE__")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HIP] fix host min/max in header (PR #82956)
https://github.com/yxsamliu updated
https://github.com/llvm/llvm-project/pull/82956
>From aa50cadf0baf84ea38379fd3276f306a27164007 Mon Sep 17 00:00:00 2001
From: "Yaxun (Sam) Liu"
Date: Sun, 25 Feb 2024 11:13:40 -0500
Subject: [PATCH] [HIP] fix host min/max in header
CUDA defines min/max functions for host in global namespace.
HIP header needs to define them too to be compatible.
Currently only min/max(int, int) is defined. This causes
wrong result for arguments that are out of range for int.
This patch defines host min/max functions to be compatible
with CUDA.
Fixes: SWDEV-446564
Change-Id: I73363b534db3aa9ac34ee2a136e582b3f71d5dd1
---
clang/lib/Headers/__clang_hip_math.h | 60 ++--
1 file changed, 56 insertions(+), 4 deletions(-)
diff --git a/clang/lib/Headers/__clang_hip_math.h
b/clang/lib/Headers/__clang_hip_math.h
index 11e1e7d032586f..8e048ab0d383aa 100644
--- a/clang/lib/Headers/__clang_hip_math.h
+++ b/clang/lib/Headers/__clang_hip_math.h
@@ -1306,14 +1306,66 @@ float min(float __x, float __y) { return
__builtin_fminf(__x, __y); }
__DEVICE__
double min(double __x, double __y) { return __builtin_fmin(__x, __y); }
+// Define host min/max functions.
+
#if !defined(__HIPCC_RTC__) && !defined(__OPENMP_AMDGCN__)
-__host__ inline static int min(int __arg1, int __arg2) {
- return __arg1 < __arg2 ? __arg1 : __arg2;
+
+#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)
+
+// Floating-point comparisons using built-in functions
+static inline float min(float const __a, float const __b) {
+ return __builtin_fminf(__a, __b);
+}
+static inline double min(double const __a, double const __b) {
+ return __builtin_fmin(__a, __b);
+}
+static inline double min(float const __a, double const __b) {
+ return __builtin_fmin(__a, __b);
+}
+static inline double min(double const __a, float const __b) {
+ return __builtin_fmin(__a, __b);
}
-__host__ inline static int max(int __arg1, int __arg2) {
- return __arg1 > __arg2 ? __arg1 : __arg2;
+static inline float max(float const __a, float const __b) {
+ return __builtin_fmaxf(__a, __b);
+}
+static inline double max(double const __a, double const __b) {
+ return __builtin_fmax(__a, __b);
+}
+static inline double max(float const __a, double const __b) {
+ return __builtin_fmax(__a, __b);
}
+static inline double max(double const __a, float const __b) {
+ return __builtin_fmax(__a, __b);
+}
+
+#pragma pop_macro("DEFINE_MIN_MAX_FUNCTIONS")
+
#endif // !defined(__HIPCC_RTC__) && !defined(__OPENMP_AMDGCN__)
#endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HIP] fix host min/max in header (PR #82956)
@@ -1306,14 +1306,50 @@ float min(float __x, float __y) { return
__builtin_fminf(__x, __y); }
__DEVICE__
double min(double __x, double __y) { return __builtin_fmin(__x, __y); }
+// Define host min/max functions.
+
#if !defined(__HIPCC_RTC__) && !defined(__OPENMP_AMDGCN__)
-__host__ inline static int min(int __arg1, int __arg2) {
- return __arg1 < __arg2 ? __arg1 : __arg2;
-}
-__host__ inline static int max(int __arg1, int __arg2) {
- return __arg1 > __arg2 ? __arg1 : __arg2;
-}
+#pragma push_macro("DEFINE_MIN_MAX_FUNCTIONS")
+#define DEFINE_MIN_MAX_FUNCTIONS(type1, type2) \
+static inline auto min(const type1 __a, const type2 __b) \
+ -> typename std::remove_reference::type { \
+ return (__a < __b) ? __a : __b; \
+} \
+static inline auto max(const type1 __a, const type2 __b) \
+ -> typename std::remove_reference __b ? __a : __b)>::type { \
+ return (__a > __b) ? __a : __b; \
+}
+
+// Define min and max functions for same type comparisons
+DEFINE_MIN_MAX_FUNCTIONS(int, int)
yxsamliu wrote:
sorry I was wrong about the reference.
The issue with the template approach is that it has subtle differences
regarding overloading resolution compared with CUDA, which could lead to
incompatibility.
https://github.com/llvm/llvm-project/pull/82956
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HIP] fix host min/max in header (PR #82956)
@@ -1306,14 +1306,50 @@ float min(float __x, float __y) { return
__builtin_fminf(__x, __y); }
__DEVICE__
double min(double __x, double __y) { return __builtin_fmin(__x, __y); }
+// Define host min/max functions.
+
#if !defined(__HIPCC_RTC__) && !defined(__OPENMP_AMDGCN__)
-__host__ inline static int min(int __arg1, int __arg2) {
- return __arg1 < __arg2 ? __arg1 : __arg2;
-}
-__host__ inline static int max(int __arg1, int __arg2) {
- return __arg1 > __arg2 ? __arg1 : __arg2;
-}
+#pragma push_macro("DEFINE_MIN_MAX_FUNCTIONS")
+#define DEFINE_MIN_MAX_FUNCTIONS(type1, type2) \
+static inline auto min(const type1 __a, const type2 __b) \
+ -> typename std::remove_reference::type { \
+ return (__a < __b) ? __a : __b; \
+} \
+static inline auto max(const type1 __a, const type2 __b) \
+ -> typename std::remove_reference __b ? __a : __b)>::type { \
+ return (__a > __b) ? __a : __b; \
+}
+
+// Define min and max functions for same type comparisons
+DEFINE_MIN_MAX_FUNCTIONS(int, int)
yxsamliu wrote:
it will return reference of `__a` or `__b`. that won't work since it is
reference to stack var
https://github.com/llvm/llvm-project/pull/82956
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HIP] fix host min/max in header (PR #82956)
@@ -1306,14 +1306,50 @@ float min(float __x, float __y) { return
__builtin_fminf(__x, __y); }
__DEVICE__
double min(double __x, double __y) { return __builtin_fmin(__x, __y); }
+// Define host min/max functions.
+
#if !defined(__HIPCC_RTC__) && !defined(__OPENMP_AMDGCN__)
-__host__ inline static int min(int __arg1, int __arg2) {
- return __arg1 < __arg2 ? __arg1 : __arg2;
-}
-__host__ inline static int max(int __arg1, int __arg2) {
- return __arg1 > __arg2 ? __arg1 : __arg2;
-}
+#pragma push_macro("DEFINE_MIN_MAX_FUNCTIONS")
+#define DEFINE_MIN_MAX_FUNCTIONS(type1, type2) \
+static inline auto min(const type1 __a, const type2 __b) \
+ -> typename std::remove_reference::type { \
+ return (__a < __b) ? __a : __b; \
+} \
+static inline auto max(const type1 __a, const type2 __b) \
+ -> typename std::remove_reference __b ? __a : __b)>::type { \
+ return (__a > __b) ? __a : __b; \
+}
+
+// Define min and max functions for same type comparisons
+DEFINE_MIN_MAX_FUNCTIONS(int, int)
jhuber6 wrote:
Could we not do something like this w/ the appropriate static assertion? Or is
there an important restriction on the specific types for this function.
```c
template
static inline auto min(const T &__a, const U &__b) {
return (__a < __b) ? __a : __b;
}
```
https://github.com/llvm/llvm-project/pull/82956
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HIP] fix host min/max in header (PR #82956)
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff 8be39b3901e3326ceebeaf0381f8cc57fdc0d464
e36760486cc794dd3dc1604b0969b83f25bdd757 -- clang/lib/Headers/__clang_hip_math.h
``
View the diff from clang-format here.
``diff
diff --git a/clang/lib/Headers/__clang_hip_math.h
b/clang/lib/Headers/__clang_hip_math.h
index e87070e41b..dc4df40101 100644
--- a/clang/lib/Headers/__clang_hip_math.h
+++ b/clang/lib/Headers/__clang_hip_math.h
@@ -1311,15 +1311,15 @@ double min(double __x, double __y) { return
__builtin_fmin(__x, __y); }
#if !defined(__HIPCC_RTC__) && !defined(__OPENMP_AMDGCN__)
#pragma push_macro("DEFINE_MIN_MAX_FUNCTIONS")
-#define DEFINE_MIN_MAX_FUNCTIONS(type1, type2) \
-static inline auto min(const type1 __a, const type2 __b) \
- -> typename std::remove_reference::type { \
- return (__a < __b) ? __a : __b; \
-} \
-static inline auto max(const type1 __a, const type2 __b) \
- -> typename std::remove_reference __b ? __a : __b)>::type { \
- return (__a > __b) ? __a : __b; \
-}
+#define DEFINE_MIN_MAX_FUNCTIONS(type1, type2)
\
+ static inline auto min(const type1 __a, const type2 __b) ->
\
+ typename std::remove_reference::type {
\
+return (__a < __b) ? __a : __b;
\
+ }
\
+ static inline auto max(const type1 __a, const type2 __b) ->
\
+ typename std::remove_reference __b ? __a : __b)>::type {
\
+return (__a > __b) ? __a : __b;
\
+ }
// Define min and max functions for same type comparisons
DEFINE_MIN_MAX_FUNCTIONS(int, int)
@@ -1338,15 +1338,31 @@ DEFINE_MIN_MAX_FUNCTIONS(long long, unsigned long long)
DEFINE_MIN_MAX_FUNCTIONS(unsigned long long, long long)
// Floating-point comparisons using built-in functions
-static inline float min(float const __a, float const __b) { return
__builtin_fminf(__a, __b); }
-static inline double min(double const __a, double const __b) { return
__builtin_fmin(__a, __b); }
-static inline double min(float const __a, double const __b) { return
__builtin_fmin(__a, __b); }
-static inline double min(double const __a, float const __b) { return
__builtin_fmin(__a, __b); }
-
-static inline float max(float const __a, float const __b) { return
__builtin_fmaxf(__a, __b); }
-static inline double max(double const __a, double const __b) { return
__builtin_fmax(__a, __b); }
-static inline double max(float const __a, double const __b) { return
__builtin_fmax(__a, __b); }
-static inline double max(double const __a, float const __b) { return
__builtin_fmax(__a, __b); }
+static inline float min(float const __a, float const __b) {
+ return __builtin_fminf(__a, __b);
+}
+static inline double min(double const __a, double const __b) {
+ return __builtin_fmin(__a, __b);
+}
+static inline double min(float const __a, double const __b) {
+ return __builtin_fmin(__a, __b);
+}
+static inline double min(double const __a, float const __b) {
+ return __builtin_fmin(__a, __b);
+}
+
+static inline float max(float const __a, float const __b) {
+ return __builtin_fmaxf(__a, __b);
+}
+static inline double max(double const __a, double const __b) {
+ return __builtin_fmax(__a, __b);
+}
+static inline double max(float const __a, double const __b) {
+ return __builtin_fmax(__a, __b);
+}
+static inline double max(double const __a, float const __b) {
+ return __builtin_fmax(__a, __b);
+}
#pragma pop_macro("DEFINE_MIN_MAX_FUNCTIONS")
``
https://github.com/llvm/llvm-project/pull/82956
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HIP] fix host min/max in header (PR #82956)
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Yaxun (Sam) Liu (yxsamliu)
Changes
CUDA defines min/max functions for host in global namespace. HIP header needs
to define them too to be compatible. Currently only min/max(int, int) is
defined. This causes wrong result for arguments that are out of range for int.
This patch defines host min/max functions to be compatible with CUDA.
Fixes: SWDEV-446564
---
Full diff: https://github.com/llvm/llvm-project/pull/82956.diff
1 Files Affected:
- (modified) clang/lib/Headers/__clang_hip_math.h (+42-6)
``diff
diff --git a/clang/lib/Headers/__clang_hip_math.h
b/clang/lib/Headers/__clang_hip_math.h
index 11e1e7d032586f..e87070e41b0ffc 100644
--- a/clang/lib/Headers/__clang_hip_math.h
+++ b/clang/lib/Headers/__clang_hip_math.h
@@ -1306,14 +1306,50 @@ float min(float __x, float __y) { return
__builtin_fminf(__x, __y); }
__DEVICE__
double min(double __x, double __y) { return __builtin_fmin(__x, __y); }
+// Define host min/max functions.
+
#if !defined(__HIPCC_RTC__) && !defined(__OPENMP_AMDGCN__)
-__host__ inline static int min(int __arg1, int __arg2) {
- return __arg1 < __arg2 ? __arg1 : __arg2;
-}
-__host__ inline static int max(int __arg1, int __arg2) {
- return __arg1 > __arg2 ? __arg1 : __arg2;
-}
+#pragma push_macro("DEFINE_MIN_MAX_FUNCTIONS")
+#define DEFINE_MIN_MAX_FUNCTIONS(type1, type2) \
+static inline auto min(const type1 __a, const type2 __b) \
+ -> typename std::remove_reference::type { \
+ return (__a < __b) ? __a : __b; \
+} \
+static inline auto max(const type1 __a, const type2 __b) \
+ -> typename std::remove_reference __b ? __a : __b)>::type { \
+ return (__a > __b) ? __a : __b; \
+}
+
+// Define min and max functions for same type comparisons
+DEFINE_MIN_MAX_FUNCTIONS(int, int)
+DEFINE_MIN_MAX_FUNCTIONS(unsigned int, unsigned int)
+DEFINE_MIN_MAX_FUNCTIONS(long, long)
+DEFINE_MIN_MAX_FUNCTIONS(unsigned long, unsigned long)
+DEFINE_MIN_MAX_FUNCTIONS(long long, long long)
+DEFINE_MIN_MAX_FUNCTIONS(unsigned long long, unsigned long long)
+
+// Define min and max functions for mixed type comparisons
+DEFINE_MIN_MAX_FUNCTIONS(int, unsigned int)
+DEFINE_MIN_MAX_FUNCTIONS(unsigned int, int)
+DEFINE_MIN_MAX_FUNCTIONS(long, unsigned long)
+DEFINE_MIN_MAX_FUNCTIONS(unsigned long, long)
+DEFINE_MIN_MAX_FUNCTIONS(long long, unsigned long long)
+DEFINE_MIN_MAX_FUNCTIONS(unsigned long long, long long)
+
+// Floating-point comparisons using built-in functions
+static inline float min(float const __a, float const __b) { return
__builtin_fminf(__a, __b); }
+static inline double min(double const __a, double const __b) { return
__builtin_fmin(__a, __b); }
+static inline double min(float const __a, double const __b) { return
__builtin_fmin(__a, __b); }
+static inline double min(double const __a, float const __b) { return
__builtin_fmin(__a, __b); }
+
+static inline float max(float const __a, float const __b) { return
__builtin_fmaxf(__a, __b); }
+static inline double max(double const __a, double const __b) { return
__builtin_fmax(__a, __b); }
+static inline double max(float const __a, double const __b) { return
__builtin_fmax(__a, __b); }
+static inline double max(double const __a, float const __b) { return
__builtin_fmax(__a, __b); }
+
+#pragma pop_macro("DEFINE_MIN_MAX_FUNCTIONS")
+
#endif // !defined(__HIPCC_RTC__) && !defined(__OPENMP_AMDGCN__)
#endif
``
https://github.com/llvm/llvm-project/pull/82956
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HIP] fix host min/max in header (PR #82956)
https://github.com/yxsamliu created
https://github.com/llvm/llvm-project/pull/82956
CUDA defines min/max functions for host in global namespace. HIP header needs
to define them too to be compatible. Currently only min/max(int, int) is
defined. This causes wrong result for arguments that are out of range for int.
This patch defines host min/max functions to be compatible with CUDA.
Fixes: SWDEV-446564
>From e36760486cc794dd3dc1604b0969b83f25bdd757 Mon Sep 17 00:00:00 2001
From: "Yaxun (Sam) Liu"
Date: Sun, 25 Feb 2024 11:13:40 -0500
Subject: [PATCH] [HIP] fix host min/max in header
CUDA defines min/max functions for host in global namespace.
HIP header needs to define them too to be compatible.
Currently only min/max(int, int) is defined. This causes
wrong result for arguments that are out of range for int.
This patch defines host min/max functions to be compatible
with CUDA.
Fixes: SWDEV-446564
---
clang/lib/Headers/__clang_hip_math.h | 48
1 file changed, 42 insertions(+), 6 deletions(-)
diff --git a/clang/lib/Headers/__clang_hip_math.h
b/clang/lib/Headers/__clang_hip_math.h
index 11e1e7d032586f..e87070e41b0ffc 100644
--- a/clang/lib/Headers/__clang_hip_math.h
+++ b/clang/lib/Headers/__clang_hip_math.h
@@ -1306,14 +1306,50 @@ float min(float __x, float __y) { return
__builtin_fminf(__x, __y); }
__DEVICE__
double min(double __x, double __y) { return __builtin_fmin(__x, __y); }
+// Define host min/max functions.
+
#if !defined(__HIPCC_RTC__) && !defined(__OPENMP_AMDGCN__)
-__host__ inline static int min(int __arg1, int __arg2) {
- return __arg1 < __arg2 ? __arg1 : __arg2;
-}
-__host__ inline static int max(int __arg1, int __arg2) {
- return __arg1 > __arg2 ? __arg1 : __arg2;
-}
+#pragma push_macro("DEFINE_MIN_MAX_FUNCTIONS")
+#define DEFINE_MIN_MAX_FUNCTIONS(type1, type2) \
+static inline auto min(const type1 __a, const type2 __b) \
+ -> typename std::remove_reference::type { \
+ return (__a < __b) ? __a : __b; \
+} \
+static inline auto max(const type1 __a, const type2 __b) \
+ -> typename std::remove_reference __b ? __a : __b)>::type { \
+ return (__a > __b) ? __a : __b; \
+}
+
+// Define min and max functions for same type comparisons
+DEFINE_MIN_MAX_FUNCTIONS(int, int)
+DEFINE_MIN_MAX_FUNCTIONS(unsigned int, unsigned int)
+DEFINE_MIN_MAX_FUNCTIONS(long, long)
+DEFINE_MIN_MAX_FUNCTIONS(unsigned long, unsigned long)
+DEFINE_MIN_MAX_FUNCTIONS(long long, long long)
+DEFINE_MIN_MAX_FUNCTIONS(unsigned long long, unsigned long long)
+
+// Define min and max functions for mixed type comparisons
+DEFINE_MIN_MAX_FUNCTIONS(int, unsigned int)
+DEFINE_MIN_MAX_FUNCTIONS(unsigned int, int)
+DEFINE_MIN_MAX_FUNCTIONS(long, unsigned long)
+DEFINE_MIN_MAX_FUNCTIONS(unsigned long, long)
+DEFINE_MIN_MAX_FUNCTIONS(long long, unsigned long long)
+DEFINE_MIN_MAX_FUNCTIONS(unsigned long long, long long)
+
+// Floating-point comparisons using built-in functions
+static inline float min(float const __a, float const __b) { return
__builtin_fminf(__a, __b); }
+static inline double min(double const __a, double const __b) { return
__builtin_fmin(__a, __b); }
+static inline double min(float const __a, double const __b) { return
__builtin_fmin(__a, __b); }
+static inline double min(double const __a, float const __b) { return
__builtin_fmin(__a, __b); }
+
+static inline float max(float const __a, float const __b) { return
__builtin_fmaxf(__a, __b); }
+static inline double max(double const __a, double const __b) { return
__builtin_fmax(__a, __b); }
+static inline double max(float const __a, double const __b) { return
__builtin_fmax(__a, __b); }
+static inline double max(double const __a, float const __b) { return
__builtin_fmax(__a, __b); }
+
+#pragma pop_macro("DEFINE_MIN_MAX_FUNCTIONS")
+
#endif // !defined(__HIPCC_RTC__) && !defined(__OPENMP_AMDGCN__)
#endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
