[DAG] Peek through freeze when deciding whether we should convert setcc to math or not.

[deadalnix]

Summary

Because later tranform push the freeze through the setcc, we end up having to reconstruct the setcc, which can cause infinite loops.

[RKSimon]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

12251

Is this fold the underlying problem? Every other freeze fold we have tries to push the freeze upward through the DAG, but this pulling it down?

[deadalnix]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

12251

I'm not sure it is the underlying problem, but it is indeed part of the loop.

The underlying problem is that we want to combine setcc to math, unless the setcc is used in a brcond. The math is turned back into a setcc by the brcond in that case, which created an infinite loop and this is why PreferSetCC was introduced in the first place.

As to pushing the freeze up or down, I don't really have an opinion on the matter.

[deadalnix]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

12251

Reworded: the loop problem existed in the past and was fixed, but this transform defeated the previous fix in the presence of a freeze.

I do indeed think that there needs to be some thought put into how we want to handle freeze, as this doesn't look very solid to me.

[nikic]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

12251

Possible alternative: https://reviews.llvm.org/D152544

[RKSimon]

Please can you abandon this and rebase/parent D127115 to D152544

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

SDValue DAGCombiner::visitSELECT_CC(SDNode *N) {
   // fold select_cc into other things, such as min/max/abs
   return SimplifySelectCC(SDLoc(N), N0, N1, N2, N3, CC);
 }
 
 SDValue DAGCombiner::visitSETCC(SDNode *N) {
   // setcc is very commonly used as an argument to brcond. This pattern
   // also lend itself to numerous combines and, as a result, it is desired
   // we keep the argument to a brcond as a setcc as much as possible.
-  bool PreferSetCC =
-      N->hasOneUse() && N->use_begin()->getOpcode() == ISD::BRCOND;
+  bool PreferSetCC = [&]() {
+    if (!N->hasOneUse()) {
+      return false;
+    }
+
+    SDNode *U = *N->use_begin();
+    if (U->getOpcode() == ISD::FREEZE && U->hasOneUse()) {
+      U = *U->use_begin();
+    }
+
+    return U->getOpcode() == ISD::BRCOND;
+  }();
 
   ISD::CondCode Cond = cast<CondCodeSDNode>(N->getOperand(2))->get();
   EVT VT = N->getValueType(0);
 
   //   SETCC(FREEZE(X), CONST, Cond)
   // =>
   //   FREEZE(SETCC(X, CONST, Cond))
   // This is correct if FREEZE(X) has one use and SETCC(FREEZE(X), CONST, Cond)
   // isn't equivalent to true or false.
   // For example, SETCC(FREEZE(X), -128, SETULT) cannot be folded to
   // FREEZE(SETCC(X, -128, SETULT)) because X can be poison.
   //
   // This transformation is beneficial because visitBRCOND can fold
   // BRCOND(FREEZE(X)) to BRCOND(X).