[PATCH] D53774: [InstCombine] Combine nested min/max intrinsics with constants

[Volkan Keles via Phabricator via llvm-commits]

volkan added a comment.

In https://reviews.llvm.org/D53774#1281306, @spatel wrote:

> Is it worth adjusting/adding some tests with NaN constants?


I don't think it's necessary as there are already some tests in `unittests/ADT/APFloatTest.cpp`.



================
Comment at: test/Transforms/InstCombine/maxnum.ll:177
 ; CHECK-LABEL: @maxnum_f32_p0_maxnum_val_n0(
-; CHECK-NEXT: [[Y:%.*]] = call float @llvm.maxnum.f32(float %x, float -0.000000e+00)
-; CHECK-NEXT: [[RES:%.*]] = call float @llvm.maxnum.f32(float [[Y]], float 0.000000e+00)
+; CHECK-NEXT: [[RES:%.*]] = call float @llvm.maxnum.f32(float %x, float 0.000000e+00)
 ; CHECK-NEXT: ret float [[RES]]
----------------
spatel wrote:
> maxnum(0.0, -0.0) is unspecified, but consistent within LLVM, right? Ie, we should not get random test failures on this test and the similar minnum test.
Yes, it's consistent since we use the functions defined in APFloat to get the result. See: `maxnum` in  include/llvm/ADT/APFloat.h.


https://reviews.llvm.org/D53774