[llvm] [SelectionDAG] Fold (icmp eq/ne (shift X, C), 0) -> (icmp eq/ne X, 0) (PR #88801)

[Björn Pettersson via llvm-commits]

================
@@ -4516,6 +4516,35 @@ SDValue TargetLowering::SimplifySetCC(EVT VT, SDValue N0, SDValue N1,
         }
       }
     }
+
+    // Optimize
+    //    (setcc (shift N00, N01C), 0, eq/ne) -> (setcc N00, 0, eq/ne)
+    // If all shifted out bits are known to be zero, then the zero'd ness
+    // doesn't change and we can omit the shift.
+    // If all shifted out bits are equal to at least one bit that isn't
+    // shifted out, then the zero'd ness doesn't change and we can omit the
+    // shift.
+    if ((Cond == ISD::SETEQ || Cond == ISD::SETNE) && C1.isZero() &&
+        N0.hasOneUse() &&
+        (N0.getOpcode() == ISD::SHL || N0.getOpcode() == ISD::SRL ||
+         N0.getOpcode() == ISD::SRA)) {
+      bool IsRightShift = N0.getOpcode() != ISD::SHL;
+      SDValue N00 = N0.getOperand(0);
+      // Quick checks based on exact/nuw/nsw flags.
+      if (IsRightShift ? N0->getFlags().hasExact()
+                       : (N0->getFlags().hasNoUnsignedWrap() ||
+                          N0->getFlags().hasNoSignedWrap()))
+        return DAG.getSetCC(dl, VT, N00, N1, Cond);
+      // More expensive checks based on known bits.
+      if (const APInt *ShAmt = DAG.getValidMaximumShiftAmountConstant(N0)) {
+        KnownBits Known = DAG.computeKnownBits(N00);
+        if (IsRightShift)
+          Known = Known.reverseBits();
+        if (ShAmt->ule(Known.countMinLeadingZeros()) ||
+            ShAmt->ult(Known.countMinSignBits()))
----------------
bjope wrote:

Are you saying that SelectionDAG/DAGCombiner is deriving those flags somewhere? It is unclear to me if that actually is done.
Or are you assuming that the input to SelectionDAG already has these flags (and that lowering/legalization always are setting the flags when introducing shifts)?

Anyway, there is at least one special case here that isn't covered by the flags (although no idea how important it is). But imagine a right shift where the five least significant bits are known to be either one or zero. If shifting 4 steps the shift isn't exact, but we still know that the zero:edness doesn't change, as the shifted out bits are equal to at least one of the remaining bits.

But I think that in general poison generating flags mainly should be used when needed to imply something that isn't known otherwise (such as UB on signed overflow). When used for a condition that can be derived by value tracking those flags are just caching some information. But there aren't really any rules that say that it isn't OK to drop the flags, or that a transform must derive that information, right(?).
We are for example dropping poison generating flags when moving freeze instruction through the DAG. So they may disappear during DAG combine, while I think that the transform here still would be legal. Therefore it seems weird to base the transform on the existence of the flags.

https://github.com/llvm/llvm-project/pull/88801