[llvm] [InstCombine] Consolidate another fold into select value equivalence (PR #117746)

Nikita Popov via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 26 08:52:19 PST 2024


https://github.com/nikic created https://github.com/llvm/llvm-project/pull/117746

We had a separate fold that handled just the trivial case where we're replacing exactly the argument of the select. Handle this in select value equivalence by relaxing the infinite loop protection to allow a replacement of a non-constant with a constant.

This also fixes https://github.com/llvm/llvm-project/issues/113301, as the separate fold did not handle undef values correctly.

>From 651091ae44a3d175a13db0a426e9ccb480a9837a Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Tue, 26 Nov 2024 17:42:15 +0100
Subject: [PATCH] [InstCombine] Consolidate another fold into select value
 equivalence

We had a separate fold that handled just the trivial case where
we're replacing exactly the argument of the select. Handle this
in select value equivalence by relaxing the infinite loop protection
to allow a replacement of a non-constant with a constant.

This also fixes https://github.com/llvm/llvm-project/issues/113301,
as the separate fold did not handle undef values correctly.
---
 .../Transforms/InstCombine/InstCombineSelect.cpp    | 13 +------------
 .../InstCombine/select-value-equivalence.ll         |  3 +--
 2 files changed, 2 insertions(+), 14 deletions(-)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
index e5525133e5dbb5..245fecb775e8c0 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
@@ -1348,7 +1348,7 @@ Instruction *InstCombinerImpl::foldSelectValueEquivalence(SelectInst &Sel,
     // If we will be able to evaluate f(Y) to a constant, we can allow undef,
     // otherwise Y cannot be undef as we might pick different values for undef
     // in the cmp and in f(Y).
-    if (TrueVal == OldOp)
+    if (TrueVal == OldOp && (isa<Constant>(OldOp) || !isa<Constant>(NewOp)))
       return nullptr;
 
     if (Value *V = simplifyWithOpReplaced(TrueVal, OldOp, NewOp, SQ,
@@ -1925,17 +1925,6 @@ Instruction *InstCombinerImpl::foldSelectInstWithICmp(SelectInst &SI,
   ICmpInst::Predicate Pred = ICI->getPredicate();
   Value *CmpLHS = ICI->getOperand(0);
   Value *CmpRHS = ICI->getOperand(1);
-  if (CmpRHS != CmpLHS && isa<Constant>(CmpRHS) && !isa<Constant>(CmpLHS)) {
-    if (CmpLHS == TrueVal && Pred == ICmpInst::ICMP_EQ) {
-      // Transform (X == C) ? X : Y -> (X == C) ? C : Y
-      replaceOperand(SI, 1, CmpRHS);
-      Changed = true;
-    } else if (CmpLHS == FalseVal && Pred == ICmpInst::ICMP_NE) {
-      // Transform (X != C) ? Y : X -> (X != C) ? Y : C
-      replaceOperand(SI, 2, CmpRHS);
-      Changed = true;
-    }
-  }
 
   if (Instruction *NewSel = foldSelectICmpEq(SI, ICI, *this))
     return NewSel;
diff --git a/llvm/test/Transforms/InstCombine/select-value-equivalence.ll b/llvm/test/Transforms/InstCombine/select-value-equivalence.ll
index d55766ddf40405..812b4d8f6be793 100644
--- a/llvm/test/Transforms/InstCombine/select-value-equivalence.ll
+++ b/llvm/test/Transforms/InstCombine/select-value-equivalence.ll
@@ -322,12 +322,11 @@ define <2 x i8> @select_vec_op_const_no_undef(<2 x i8> %x) {
   ret <2 x i8> %xr
 }
 
-; FIXME: This is a miscompile.
 define <2 x i8> @select_vec_op_const_undef(<2 x i8> %x) {
 ; CHECK-LABEL: define <2 x i8> @select_vec_op_const_undef(
 ; CHECK-SAME: <2 x i8> [[X:%.*]]) {
 ; CHECK-NEXT:    [[XZ:%.*]] = icmp eq <2 x i8> [[X]], <i8 1, i8 undef>
-; CHECK-NEXT:    [[XR:%.*]] = select <2 x i1> [[XZ]], <2 x i8> <i8 1, i8 undef>, <2 x i8> <i8 4, i8 3>
+; CHECK-NEXT:    [[XR:%.*]] = select <2 x i1> [[XZ]], <2 x i8> [[X]], <2 x i8> <i8 4, i8 3>
 ; CHECK-NEXT:    ret <2 x i8> [[XR]]
 ;
   %xz = icmp eq <2 x i8> %x, <i8 1, i8 undef>



More information about the llvm-commits mailing list