release/20.x: [InstCombine] Fix ninf propagation for fcmp+sel -> minmax (#136433)

[llvmbot]

Backport a0c4876

Requested by: @dtcxzyw

[llvmbot]

@nikic What do you think about merging this PR to the release branch?

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: None (llvmbot)

Changes

Backport a0c4876

Requested by: @dtcxzyw

---

Full diff: https://github.com/llvm/llvm-project/pull/137605.diff

4 Files Affected:

• (modified) llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp (+6-2)
• (modified) llvm/test/Transforms/InstCombine/fcmp-fadd-select.ll (+1-1)
• (modified) llvm/test/Transforms/InstCombine/minmax-fp.ll (+12-2)
• (modified) llvm/test/Transforms/InstCombine/unordered-fcmp-select.ll (+1-1)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
index 29c5cef84ccdb..932628be84846 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
@@ -3898,16 +3898,20 @@ Instruction *InstCombinerImpl::visitSelectInst(SelectInst &SI) {
       if (match(&SI, m_OrdOrUnordFMax(m_Value(X), m_Value(Y)))) {
         Value *BinIntr =
             Builder.CreateBinaryIntrinsic(Intrinsic::maxnum, X, Y, &SI);
-        if (auto *BinIntrInst = dyn_cast<Instruction>(BinIntr))
+        if (auto *BinIntrInst = dyn_cast<Instruction>(BinIntr)) {
           BinIntrInst->setHasNoNaNs(FCmp->hasNoNaNs());
+          BinIntrInst->setHasNoInfs(FCmp->hasNoInfs());
+        }
         return replaceInstUsesWith(SI, BinIntr);
       }
 
       if (match(&SI, m_OrdOrUnordFMin(m_Value(X), m_Value(Y)))) {
         Value *BinIntr =
             Builder.CreateBinaryIntrinsic(Intrinsic::minnum, X, Y, &SI);
-        if (auto *BinIntrInst = dyn_cast<Instruction>(BinIntr))
+        if (auto *BinIntrInst = dyn_cast<Instruction>(BinIntr)) {
           BinIntrInst->setHasNoNaNs(FCmp->hasNoNaNs());
+          BinIntrInst->setHasNoInfs(FCmp->hasNoInfs());
+        }
         return replaceInstUsesWith(SI, BinIntr);
       }
     }
diff --git a/llvm/test/Transforms/InstCombine/fcmp-fadd-select.ll b/llvm/test/Transforms/InstCombine/fcmp-fadd-select.ll
index 15fad55db8df1..e05ef6df1d41b 100644
--- a/llvm/test/Transforms/InstCombine/fcmp-fadd-select.ll
+++ b/llvm/test/Transforms/InstCombine/fcmp-fadd-select.ll
@@ -663,7 +663,7 @@ define float @test_fcmp_ogt_fadd_select_rewrite_flags2(float %in) {
 define float @test_fcmp_ogt_fadd_select_rewrite_and_fastmath(float %in) {
 ; CHECK-LABEL: define float @test_fcmp_ogt_fadd_select_rewrite_and_fastmath(
 ; CHECK-SAME: float [[IN:%.*]]) {
-; CHECK-NEXT:    [[SEL_NEW:%.*]] = call fast float @llvm.maxnum.f32(float [[IN]], float 0.000000e+00)
+; CHECK-NEXT:    [[SEL_NEW:%.*]] = call reassoc nnan nsz arcp contract afn float @llvm.maxnum.f32(float [[IN]], float 0.000000e+00)
 ; CHECK-NEXT:    [[ADD_NEW:%.*]] = fadd fast float [[SEL_NEW]], 1.000000e+00
 ; CHECK-NEXT:    ret float [[ADD_NEW]]
 ;
diff --git a/llvm/test/Transforms/InstCombine/minmax-fp.ll b/llvm/test/Transforms/InstCombine/minmax-fp.ll
index 4fe8cf374344e..a8470a20365e9 100644
--- a/llvm/test/Transforms/InstCombine/minmax-fp.ll
+++ b/llvm/test/Transforms/InstCombine/minmax-fp.ll
@@ -331,7 +331,7 @@ define float @maxnum_ogt_fmf_on_select(float %a, float %b) {
 
 define <2 x float> @maxnum_oge_fmf_on_select(<2 x float> %a, <2 x float> %b) {
 ; CHECK-LABEL: @maxnum_oge_fmf_on_select(
-; CHECK-NEXT:    [[F:%.*]] = call ninf nsz <2 x float> @llvm.maxnum.v2f32(<2 x float> [[A:%.*]], <2 x float> [[B:%.*]])
+; CHECK-NEXT:    [[F:%.*]] = call nsz <2 x float> @llvm.maxnum.v2f32(<2 x float> [[A:%.*]], <2 x float> [[B:%.*]])
 ; CHECK-NEXT:    ret <2 x float> [[F]]
 ;
   %cond = fcmp oge <2 x float> %a, %b
@@ -383,6 +383,16 @@ define float @maxnum_no_nnan(float %a, float %b) {
   ret float %f
 }
 
+define float @minnum_olt_fmf_on_select_both_ninf(float %a, float %b) {
+; CHECK-LABEL: @minnum_olt_fmf_on_select_both_ninf(
+; CHECK-NEXT:    [[F:%.*]] = call ninf nsz float @llvm.minnum.f32(float [[A:%.*]], float [[B:%.*]])
+; CHECK-NEXT:    ret float [[F]]
+;
+  %cond = fcmp ninf olt float %a, %b
+  %f = select nnan ninf nsz i1 %cond, float %a, float %b
+  ret float %f
+}
+
 define float @minnum_olt_fmf_on_select(float %a, float %b) {
 ; CHECK-LABEL: @minnum_olt_fmf_on_select(
 ; CHECK-NEXT:    [[F:%.*]] = call nsz float @llvm.minnum.f32(float [[A:%.*]], float [[B:%.*]])
@@ -395,7 +405,7 @@ define float @minnum_olt_fmf_on_select(float %a, float %b) {
 
 define <2 x float> @minnum_ole_fmf_on_select(<2 x float> %a, <2 x float> %b) {
 ; CHECK-LABEL: @minnum_ole_fmf_on_select(
-; CHECK-NEXT:    [[F:%.*]] = call ninf nsz <2 x float> @llvm.minnum.v2f32(<2 x float> [[A:%.*]], <2 x float> [[B:%.*]])
+; CHECK-NEXT:    [[F:%.*]] = call nsz <2 x float> @llvm.minnum.v2f32(<2 x float> [[A:%.*]], <2 x float> [[B:%.*]])
 ; CHECK-NEXT:    ret <2 x float> [[F]]
 ;
   %cond = fcmp ole <2 x float> %a, %b
diff --git a/llvm/test/Transforms/InstCombine/unordered-fcmp-select.ll b/llvm/test/Transforms/InstCombine/unordered-fcmp-select.ll
index 178795f9f9a83..ab4c997014699 100644
--- a/llvm/test/Transforms/InstCombine/unordered-fcmp-select.ll
+++ b/llvm/test/Transforms/InstCombine/unordered-fcmp-select.ll
@@ -115,7 +115,7 @@ define float @select_max_ugt_2_use_cmp(float %a, float %b) {
 ; CHECK-LABEL: @select_max_ugt_2_use_cmp(
 ; CHECK-NEXT:    [[CMP:%.*]] = fcmp reassoc ugt float [[A:%.*]], [[B:%.*]]
 ; CHECK-NEXT:    call void @foo(i1 [[CMP]])
-; CHECK-NEXT:    [[SEL:%.*]] = call reassoc ninf nsz arcp contract afn float @llvm.maxnum.f32(float [[A]], float [[B]])
+; CHECK-NEXT:    [[SEL:%.*]] = call reassoc nsz arcp contract afn float @llvm.maxnum.f32(float [[A]], float [[B]])
 ; CHECK-NEXT:    ret float [[SEL]]
 ;
   %cmp = fcmp reassoc ugt float %a, %b

[nikic]

I don't think there is a need to backport FMF propagation fixes.

[dtcxzyw]

I don't think there is a need to backport FMF propagation fixes.

Is there a policy to judge whether or not to backport a miscompilation bug fix?
Actually, it is unlikely to trigger this bug in real-world projects. But this fix is simple and safe to be backported.

I am fine with not backporting this if the reason is "it depends on #137131".

[nikic]

I don't think there is a need to backport FMF propagation fixes.

Is there a policy to judge whether or not to backport a miscompilation bug fix? Actually, it is unlikely to trigger this bug in real-world projects. But this fix is simple and safe to be backported.

There is https://llvm.org/docs/HowToReleaseLLVM.html#release-patch-rules, but it's not very useful :)

I don't think there is much value in backporting theoretical miscompilation fixes to the release branch, but I don't particularly care in this case, as the patch itself is simple and unlikely to significantly affect anything.

[arsenm]

Seems fine but doesn't matter