Clang: Deprecate float support from __builtin_elementwise_max - #180885
Conversation
Now we have __builtin_elementwise_maxnum __builtin_elementwise_maximum __builtin_elementwise_maximumnum
|
@llvm/pr-subscribers-hlsl @llvm/pr-subscribers-clang-codegen Author: YunQiang Su (wzssyqa) ChangesNow we have Full diff: https://github.com/llvm/llvm-project/pull/180885.diff 5 Files Affected:
diff --git a/clang/docs/LanguageExtensions.rst b/clang/docs/LanguageExtensions.rst
index 29328355c3e6f..745000e79027c 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -839,16 +839,8 @@ of different sizes and signs is forbidden in binary and ternary builtins.
T __builtin_elementwise_copysign(T x, T y) return the magnitude of x with the sign of y. floating point types
T __builtin_elementwise_fmod(T x, T y) return the floating-point remainder of (x/y) whose sign floating point types
matches the sign of x.
- T __builtin_elementwise_max(T x, T y) return x or y, whichever is larger integer and floating point types
- For floating point types, follows semantics of maxNum
- in IEEE 754-2008. See `LangRef
- <http://llvm.org/docs/LangRef.html#i-fminmax-family>`_
- for the comparison.
- T __builtin_elementwise_min(T x, T y) return x or y, whichever is smaller integer and floating point types
- For floating point types, follows semantics of minNum
- in IEEE 754-2008. See `LangRef
- <http://llvm.org/docs/LangRef.html#i-fminmax-family>`_
- for the comparison.
+ T __builtin_elementwise_max(T x, T y) return x or y, whichever is larger integer types
+ T __builtin_elementwise_min(T x, T y) return x or y, whichever is smaller integer types
T __builtin_elementwise_maxnum(T x, T y) return x or y, whichever is larger. Follows IEEE 754-2008 floating point types
semantics (maxNum) with +0.0>-0.0. See `LangRef
<http://llvm.org/docs/LangRef.html#i-fminmax-family>`_
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 0dbea8efc2642..758982d6e6431 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -138,6 +138,9 @@ Non-comprehensive list of changes in this release
Usable in constant expressions. Implicit conversion is supported for
class/struct types with conversion operators.
+- Removed float types support from ``__builtin_elementwise_max`` and
+ ``__builtin_elementwise_min``.
+
New Compiler Flags
------------------
- New option ``-fms-anonymous-structs`` / ``-fno-ms-anonymous-structs`` added
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index cf686581240a5..bb66677fb40c9 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -4066,30 +4066,26 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
Value *Op0 = EmitScalarExpr(E->getArg(0));
Value *Op1 = EmitScalarExpr(E->getArg(1));
Value *Result;
- if (Op0->getType()->isIntOrIntVectorTy()) {
- QualType Ty = E->getArg(0)->getType();
- if (auto *VecTy = Ty->getAs<VectorType>())
- Ty = VecTy->getElementType();
- Result = Builder.CreateBinaryIntrinsic(
- Ty->isSignedIntegerType() ? Intrinsic::smax : Intrinsic::umax, Op0,
- Op1, nullptr, "elt.max");
- } else
- Result = Builder.CreateMaxNum(Op0, Op1, /*FMFSource=*/nullptr, "elt.max");
+ assert(Op0->getType()->isIntOrIntVectorTy());
+ QualType Ty = E->getArg(0)->getType();
+ if (auto *VecTy = Ty->getAs<VectorType>())
+ Ty = VecTy->getElementType();
+ Result = Builder.CreateBinaryIntrinsic(
+ Ty->isSignedIntegerType() ? Intrinsic::smax : Intrinsic::umax, Op0,
+ Op1, nullptr, "elt.max");
return RValue::get(Result);
}
case Builtin::BI__builtin_elementwise_min: {
Value *Op0 = EmitScalarExpr(E->getArg(0));
Value *Op1 = EmitScalarExpr(E->getArg(1));
Value *Result;
- if (Op0->getType()->isIntOrIntVectorTy()) {
- QualType Ty = E->getArg(0)->getType();
- if (auto *VecTy = Ty->getAs<VectorType>())
- Ty = VecTy->getElementType();
- Result = Builder.CreateBinaryIntrinsic(
- Ty->isSignedIntegerType() ? Intrinsic::smin : Intrinsic::umin, Op0,
- Op1, nullptr, "elt.min");
- } else
- Result = Builder.CreateMinNum(Op0, Op1, /*FMFSource=*/nullptr, "elt.min");
+ assert(Op0->getType()->isIntOrIntVectorTy());
+ QualType Ty = E->getArg(0)->getType();
+ if (auto *VecTy = Ty->getAs<VectorType>())
+ Ty = VecTy->getElementType();
+ Result = Builder.CreateBinaryIntrinsic(
+ Ty->isSignedIntegerType() ? Intrinsic::smin : Intrinsic::umin, Op0,
+ Op1, nullptr, "elt.min");
return RValue::get(Result);
}
diff --git a/clang/test/CodeGen/builtins-elementwise-math.c b/clang/test/CodeGen/builtins-elementwise-math.c
index 2df485f0155c3..a201403e8b6b1 100644
--- a/clang/test/CodeGen/builtins-elementwise-math.c
+++ b/clang/test/CodeGen/builtins-elementwise-math.c
@@ -339,32 +339,10 @@ void test_builtin_elementwise_minimum(float f1, float f2, double d1, double d2,
vf1 = __builtin_elementwise_minimum(vf2, cvf1);
}
-void test_builtin_elementwise_max(float f1, float f2, double d1, double d2,
- float4 vf1, float4 vf2, long long int i1,
- long long int i2, si8 vi1, si8 vi2,
+void test_builtin_elementwise_max(long long int i2, si8 vi1, si8 vi2, long long int i1,
unsigned u1, unsigned u2, u4 vu1, u4 vu2,
_BitInt(31) bi1, _BitInt(31) bi2,
unsigned _BitInt(55) bu1, unsigned _BitInt(55) bu2) {
- // CHECK-LABEL: define void @test_builtin_elementwise_max(
- // CHECK: [[F1:%.+]] = load float, ptr %f1.addr, align 4
- // CHECK-NEXT: [[F2:%.+]] = load float, ptr %f2.addr, align 4
- // CHECK-NEXT: call float @llvm.maxnum.f32(float [[F1]], float [[F2]])
- f1 = __builtin_elementwise_max(f1, f2);
-
- // CHECK: [[D1:%.+]] = load double, ptr %d1.addr, align 8
- // CHECK-NEXT: [[D2:%.+]] = load double, ptr %d2.addr, align 8
- // CHECK-NEXT: call double @llvm.maxnum.f64(double [[D1]], double [[D2]])
- d1 = __builtin_elementwise_max(d1, d2);
-
- // CHECK: [[D2:%.+]] = load double, ptr %d2.addr, align 8
- // CHECK-NEXT: call double @llvm.maxnum.f64(double 2.000000e+01, double [[D2]])
- d1 = __builtin_elementwise_max(20.0, d2);
-
- // CHECK: [[VF1:%.+]] = load <4 x float>, ptr %vf1.addr, align 16
- // CHECK-NEXT: [[VF2:%.+]] = load <4 x float>, ptr %vf2.addr, align 16
- // CHECK-NEXT: call <4 x float> @llvm.maxnum.v4f32(<4 x float> [[VF1]], <4 x float> [[VF2]])
- vf1 = __builtin_elementwise_max(vf1, vf2);
-
// CHECK: [[I1:%.+]] = load i64, ptr %i1.addr, align 8
// CHECK-NEXT: [[I2:%.+]] = load i64, ptr %i2.addr, align 8
// CHECK-NEXT: call i64 @llvm.smax.i64(i64 [[I1]], i64 [[I2]])
@@ -403,17 +381,6 @@ void test_builtin_elementwise_max(float f1, float f2, double d1, double d2,
// CHECK-NEXT: call i55 @llvm.umax.i55(i55 [[LOADEDV2]], i55 [[LOADEDV3]])
bu1 = __builtin_elementwise_max(bu1, bu2);
- // CHECK: [[CVF1:%.+]] = load <4 x float>, ptr %cvf1, align 16
- // CHECK-NEXT: [[VF2:%.+]] = load <4 x float>, ptr %vf2.addr, align 16
- // CHECK-NEXT: call <4 x float> @llvm.maxnum.v4f32(<4 x float> [[CVF1]], <4 x float> [[VF2]])
- const float4 cvf1 = vf1;
- vf1 = __builtin_elementwise_max(cvf1, vf2);
-
- // CHECK: [[VF2:%.+]] = load <4 x float>, ptr %vf2.addr, align 16
- // CHECK-NEXT: [[CVF1:%.+]] = load <4 x float>, ptr %cvf1, align 16
- // CHECK-NEXT: call <4 x float> @llvm.maxnum.v4f32(<4 x float> [[VF2]], <4 x float> [[CVF1]])
- vf1 = __builtin_elementwise_max(vf2, cvf1);
-
// CHECK: [[IAS1:%.+]] = load i32, ptr addrspace(1) @int_as_one, align 4
// CHECK-NEXT: [[B:%.+]] = load i32, ptr @b, align 4
// CHECK-NEXT: call i32 @llvm.smax.i32(i32 [[IAS1]], i32 [[B]])
@@ -423,32 +390,10 @@ void test_builtin_elementwise_max(float f1, float f2, double d1, double d2,
i1 = __builtin_elementwise_max(1, 'a');
}
-void test_builtin_elementwise_min(float f1, float f2, double d1, double d2,
- float4 vf1, float4 vf2, long long int i1,
- long long int i2, si8 vi1, si8 vi2,
+void test_builtin_elementwise_min(long long int i2, si8 vi1, si8 vi2, long long int i1,
unsigned u1, unsigned u2, u4 vu1, u4 vu2,
_BitInt(31) bi1, _BitInt(31) bi2,
unsigned _BitInt(55) bu1, unsigned _BitInt(55) bu2) {
- // CHECK-LABEL: define void @test_builtin_elementwise_min(
- // CHECK: [[F1:%.+]] = load float, ptr %f1.addr, align 4
- // CHECK-NEXT: [[F2:%.+]] = load float, ptr %f2.addr, align 4
- // CHECK-NEXT: call float @llvm.minnum.f32(float [[F1]], float [[F2]])
- f1 = __builtin_elementwise_min(f1, f2);
-
- // CHECK: [[D1:%.+]] = load double, ptr %d1.addr, align 8
- // CHECK-NEXT: [[D2:%.+]] = load double, ptr %d2.addr, align 8
- // CHECK-NEXT: call double @llvm.minnum.f64(double [[D1]], double [[D2]])
- d1 = __builtin_elementwise_min(d1, d2);
-
- // CHECK: [[D1:%.+]] = load double, ptr %d1.addr, align 8
- // CHECK-NEXT: call double @llvm.minnum.f64(double [[D1]], double 2.000000e+00)
- d1 = __builtin_elementwise_min(d1, 2.0);
-
- // CHECK: [[VF1:%.+]] = load <4 x float>, ptr %vf1.addr, align 16
- // CHECK-NEXT: [[VF2:%.+]] = load <4 x float>, ptr %vf2.addr, align 16
- // CHECK-NEXT: call <4 x float> @llvm.minnum.v4f32(<4 x float> [[VF1]], <4 x float> [[VF2]])
- vf1 = __builtin_elementwise_min(vf1, vf2);
-
// CHECK: [[I1:%.+]] = load i64, ptr %i1.addr, align 8
// CHECK-NEXT: [[I2:%.+]] = load i64, ptr %i2.addr, align 8
// CHECK-NEXT: call i64 @llvm.smin.i64(i64 [[I1]], i64 [[I2]])
@@ -494,17 +439,6 @@ void test_builtin_elementwise_min(float f1, float f2, double d1, double d2,
// CHECK-NEXT: call i55 @llvm.umin.i55(i55 [[LOADEDV2]], i55 [[LOADEDV3]])
bu1 = __builtin_elementwise_min(bu1, bu2);
- // CHECK: [[CVF1:%.+]] = load <4 x float>, ptr %cvf1, align 16
- // CHECK-NEXT: [[VF2:%.+]] = load <4 x float>, ptr %vf2.addr, align 16
- // CHECK-NEXT: call <4 x float> @llvm.minnum.v4f32(<4 x float> [[CVF1]], <4 x float> [[VF2]])
- const float4 cvf1 = vf1;
- vf1 = __builtin_elementwise_min(cvf1, vf2);
-
- // CHECK: [[VF2:%.+]] = load <4 x float>, ptr %vf2.addr, align 16
- // CHECK-NEXT: [[CVF1:%.+]] = load <4 x float>, ptr %cvf1, align 16
- // CHECK-NEXT: call <4 x float> @llvm.minnum.v4f32(<4 x float> [[VF2]], <4 x float> [[CVF1]])
- vf1 = __builtin_elementwise_min(vf2, cvf1);
-
// CHECK: [[IAS1:%.+]] = load i32, ptr addrspace(1) @int_as_one, align 4
// CHECK-NEXT: [[B:%.+]] = load i32, ptr @b, align 4
// CHECK-NEXT: call i32 @llvm.smin.i32(i32 [[IAS1]], i32 [[B]])
diff --git a/clang/test/CodeGen/strictfp-elementwise-builtins.cpp b/clang/test/CodeGen/strictfp-elementwise-builtins.cpp
index 6453d50f044aa..7de0a396e08f9 100644
--- a/clang/test/CodeGen/strictfp-elementwise-builtins.cpp
+++ b/clang/test/CodeGen/strictfp-elementwise-builtins.cpp
@@ -27,24 +27,24 @@ float4 strict_elementwise_abs(float4 a) {
return __builtin_elementwise_abs(a);
}
-// CHECK-LABEL: define dso_local noundef <4 x float> @_Z22strict_elementwise_maxDv4_fS_
-// CHECK-SAME: (<4 x float> noundef [[A:%.*]], <4 x float> noundef [[B:%.*]]) local_unnamed_addr #[[ATTR0]] {
+// CHECK-LABEL: define dso_local noundef <4 x float> @_Z25strict_elementwise_maxnumDv4_fS_
+// CHECK-SAME: (<4 x float> noundef [[A:%.*]], <4 x float> noundef [[B:%.*]]) local_unnamed_addr #[[ATTR2]] {
// CHECK-NEXT: entry:
-// CHECK-NEXT: [[ELT_MAX:%.*]] = tail call <4 x float> @llvm.experimental.constrained.maxnum.v4f32(<4 x float> [[A]], <4 x float> [[B]], metadata !"fpexcept.strict") #[[ATTR4]]
-// CHECK-NEXT: ret <4 x float> [[ELT_MAX]]
+// CHECK-NEXT: [[ELT_MAXNUM:%.*]] = tail call <4 x float> @llvm.maxnum.v4f32(<4 x float> [[A]], <4 x float> [[B]]) #[[ATTR4]]
+// CHECK-NEXT: ret <4 x float> [[ELT_MAXNUM]]
//
-float4 strict_elementwise_max(float4 a, float4 b) {
- return __builtin_elementwise_max(a, b);
+float4 strict_elementwise_maxnum(float4 a, float4 b) {
+ return __builtin_elementwise_maxnum(a, b);
}
-// CHECK-LABEL: define dso_local noundef <4 x float> @_Z22strict_elementwise_minDv4_fS_
-// CHECK-SAME: (<4 x float> noundef [[A:%.*]], <4 x float> noundef [[B:%.*]]) local_unnamed_addr #[[ATTR0]] {
+// CHECK-LABEL: define dso_local noundef <4 x float> @_Z25strict_elementwise_minnumDv4_fS_
+// CHECK-SAME: (<4 x float> noundef [[A:%.*]], <4 x float> noundef [[B:%.*]]) local_unnamed_addr #[[ATTR2]] {
// CHECK-NEXT: entry:
-// CHECK-NEXT: [[ELT_MIN:%.*]] = tail call <4 x float> @llvm.experimental.constrained.minnum.v4f32(<4 x float> [[A]], <4 x float> [[B]], metadata !"fpexcept.strict") #[[ATTR4]]
-// CHECK-NEXT: ret <4 x float> [[ELT_MIN]]
+// CHECK-NEXT: [[ELT_MINNUM:%.*]] = tail call <4 x float> @llvm.minnum.v4f32(<4 x float> [[A]], <4 x float> [[B]]) #[[ATTR4]]
+// CHECK-NEXT: ret <4 x float> [[ELT_MINNUM]]
//
-float4 strict_elementwise_min(float4 a, float4 b) {
- return __builtin_elementwise_min(a, b);
+float4 strict_elementwise_minnum(float4 a, float4 b) {
+ return __builtin_elementwise_minnum(a, b);
}
// CHECK-LABEL: define dso_local noundef <4 x float> @_Z26strict_elementwise_maximumDv4_fS_
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
🪟 Windows x64 Test Results
✅ The build succeeded and all tests passed. |
|
@AaronBallman Checking on what the policy here is, is this something we can do? |
The PR summary lacks any justification for the change beyond the existence of other builtins, but I don't believe this is a change we'd want to make unless I'm missing something. This will break working code, won't it? |
AaronBallman
left a comment
There was a problem hiding this comment.
Marking as requested changes because it's not clear why these changes are being proposed.
This topic was talked in: #113133: Should we add @nikic suggested that we should remove float support from My suggestion is that we can set |
This builtin was released over five years ago in Clang, so changing the semantics with no notice is pretty user hostile. It doesn't sound like the float support we have is wrong, just that someone wanted to change the semantics around negative zero handling for better optimization opportunities. Is that correct? |
|
The context here is that there are at least 4 notions of what "max" means for floating-point numbers that are in common use, and the used semantics should not hidden behind such an innocuous name. It would probably make more sense to warn if this intrinsic is used with floats rather than not supporting them entirely. |
Because this is a builtin, I agree.
That's how I lean but I don't have a good idea for what the warning conditions would be. I don't think "warn on any float" is viable -- outside of a handful of edge cases, the function does what it says on the tin, right? And the edge cases aren't something we can catch statically in most cases, I believe. |
Deprecation warning |
That seems like a reasonable approach to me; GCC doesn't implement this builtin nor does MSVC, so I don't think we need it for compatibility with other implementations. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/140/builds/39549 Here is the relevant piece of the build log for the reference |
…80885) Now we have __builtin_elementwise_maxnum __builtin_elementwise_maximum __builtin_elementwise_maximumnum
…80885) Now we have __builtin_elementwise_maxnum __builtin_elementwise_maximum __builtin_elementwise_maximumnum
|
@AaronBallman @jcranmer-intel @phoebewang I didn't see this PR when it went through, and I only just noticed that This might not have been noticed because, unlike the Note the difference in generated code when signed zeros are ignored (I had to use |
If you really really want that, that should be a distinct elementwise fmin/fmax. Alternatively we should round out the math pragmas to have nsz |
|
I seldom pay attention to generic intrinsic changes, but I'm not surprise to the change. After #172012, the community consensus is to decouple |
The Clang documentation for |
Now we have
__builtin_elementwise_maxnum
__builtin_elementwise_maximum
__builtin_elementwise_maximumnum