TargetLowering: Improve handling of (setcc ([sz]ext x) 0, cc) in SimplifySetCC

[• tstellarAMD]

Summary

When SimplifySetCC sees a setcc node that compares the result of a
value extension operation with a constant, it tries to simplify the
setcc node by eliminating the extension and shrinking the constant.

If shrinking the inputs to setcc is deemed not desirable by the target
(e.g. the target does not want a setcc comparing i1 values), then it
is still possible to optimize this sequence in some cases.

This patch adds the following combines to SimplifySetCC when shrinking setcc
inputs is not desirable:

(setcc ([sz]ext (setcc x, y, cc)), 0, setne) -> (setcc (x, y, cc))
(setcc ([sz]ext (setcc x, y, cc)), 0, seteq) -> (setcc (x, Y, !cc))

There are no tests for this yet, but once AMDGPU correctly implements
TargetLowering::isTypeDesirableForOp(), this new combine will be
exercised by the existing CodeGen/AMDGPU/setcc-opt.ll test.

[arsenm]

lib/CodeGen/SelectionDAG/TargetLowering.cpp

1400–1401

What about the opposite compare against the true value case?

[• tstellarAMD]

lib/CodeGen/SelectionDAG/TargetLowering.cpp

1400–1401

I was concerned this case would break things, since targets have different values for boolean true. I think the current transform is safe for all targets.

[arsenm]

lib/CodeGen/SelectionDAG/TargetLowering.cpp

1400–1401

That's easy to check. If it's also just for i1 it doesn't really matter

[• tstellarAMD]

Handle more variants of [sz]ext + setcc

[arsenm]

lib/CodeGen/SelectionDAG/TargetLowering.cpp

1411–1415

I think you should use TLI::isConstTrueVal/isConstFalseVal so this only happens if the correct BooleanContents is used

[• tstellarAMD]

lib/CodeGen/SelectionDAG/TargetLowering.cpp

1411–1415

Do you think this is still necessary since we are only looking at i1 setcc operations?

[jroelofs]

lib/CodeGen/SelectionDAG/TargetLowering.cpp

1411–1415

Yes. I know of at least one out-of-tree target where this matters.

[• tstellarAMD]

Use isConstTrueVal and isConstFalseVal

[arsenm]

lib/CodeGen/SelectionDAG/TargetLowering.cpp

1411–1413

I didn't expect to still have the isOne/isAllOnesValue. I didn't realize that you would still need to check the compare type in addition to making sure the true value matched the extend type. In that case it might be be clearer to check the boolean content along with the value.

In either case this is a lot of parentheses so splitting these into a helper isExtendedTrueVal or something like that would help.

[• tstellarAMD]

Add isExtendedTrueVal() helper.

[arsenm]

LGTM with that fixed

lib/CodeGen/SelectionDAG/TargetLowering.cpp

1248

I think this should be !N->isNull(). If you have a constant true i1, it is -1 for example.

[• tstellarAMD]

lib/CodeGen/SelectionDAG/TargetLowering.cpp

1248

isOne() and isAllOnesValue() both return true for i1 1.

[arsenm]

lib/CodeGen/SelectionDAG/TargetLowering.cpp

1248

OK, then it doesn't really matter

include/llvm/Target/TargetLowering.h

public:
   /// Return if the N is a constant or constant vector equal to the true value
   /// from getBooleanContents().
   bool isConstTrueVal(const SDNode *N) const;
 
   /// Return if the N is a constant or constant vector equal to the false value
   /// from getBooleanContents().
   bool isConstFalseVal(const SDNode *N) const;
 
+  /// Return if \p N is a True value when extended to \p VT.
+  bool isExtendedTrueVal(const ConstantSDNode *N, EVT VT, bool Signed) const;
+
   /// Try to simplify a setcc built with the specified operands and cc. If it is
   /// unable to simplify it, return a null SDValue.
   SDValue SimplifySetCC(EVT VT, SDValue N0, SDValue N1,
                           ISD::CondCode Cond, bool foldBooleans,
                           DAGCombinerInfo &DCI, SDLoc dl) const;
 
   /// Returns true (and the GlobalValue and the offset) if the node is a
   /// GlobalAddress + offset.

lib/CodeGen/SelectionDAG/TargetLowering.cpp

bool TargetLowering::isConstFalseVal(const SDNode *N) const {
   }
 
   if (getBooleanContents(N->getValueType(0)) == UndefinedBooleanContent)
     return !CN->getAPIntValue()[0];
 
   return CN->isNullValue();
 }
 
+bool TargetLowering::isExtendedTrueVal(const ConstantSDNode *N, EVT VT,
+                                       bool SExt) const {
+  if (VT == MVT::i1)
+    return N->isOne();
+
+  TargetLowering::BooleanContent Cnt = getBooleanContents(VT);
+  switch (Cnt) {
+  case TargetLowering::ZeroOrOneBooleanContent:
+    // An extended value of 1 is always true, unless its original type is i1,
+    // in which case it will be sign extended to -1.
+    return (N->isOne() && !SExt) || (SExt && (N->getValueType(0) != MVT::i1));
+  case TargetLowering::UndefinedBooleanContent:
+  case TargetLowering::ZeroOrNegativeOneBooleanContent:
+    return N->isAllOnesValue() && SExt;
+  }
+}
+
 /// Try to simplify a setcc built with the specified operands and cc. If it is
 /// unable to simplify it, return a null SDValue.
 SDValue
 TargetLowering::SimplifySetCC(EVT VT, SDValue N0, SDValue N1,
                               ISD::CondCode Cond, bool foldBooleans,
                               DAGCombinerInfo &DCI, SDLoc dl) const {
   SelectionDAG &DAG = DCI.DAG;
 
if ((Cond == ISD::SETEQ || Cond == ISD::SETNE) &&
           MinBits >= ReqdBits) {
         EVT MinVT = EVT::getIntegerVT(*DAG.getContext(), MinBits);
         if (isTypeDesirableForOp(ISD::SETCC, MinVT)) {
           // Will get folded away.
           SDValue Trunc = DAG.getNode(ISD::TRUNCATE, dl, MinVT, PreExt);
           SDValue C = DAG.getConstant(C1.trunc(MinBits), dl, MinVT);
           return DAG.getSetCC(dl, VT, Trunc, C, Cond);
         }
+
+        // If truncating the setcc operands is not desirable, we can still
+        // simplify the expression in some cases:
+        // setcc ([sz]ext (setcc x, y, cc)), 0, setne) -> setcc (x, y, cc)
+        // setcc ([sz]ext (setcc x, y, cc)), 0, seteq) -> setcc (x, y, inv(cc))
+        // setcc (zext (setcc x, y, cc)), 1, setne) -> setcc (x, y, inv(cc))
+        // setcc (zext (setcc x, y, cc)), 1, seteq) -> setcc (x, y, cc)
+        // setcc (sext (setcc x, y, cc)), -1, setne) -> setcc (x, y, inv(cc))
+        // setcc (sext (setcc x, y, cc)), -1, seteq) -> setcc (x, y, cc)
+        SDValue TopSetCC = N0->getOperand(0);
+        unsigned N0Opc = N0->getOpcode();
+        bool SExt = (N0Opc == ISD::SIGN_EXTEND);
+        if (TopSetCC.getValueType() == MVT::i1 && VT == MVT::i1 &&
+            TopSetCC.getOpcode() == ISD::SETCC &&
+            (N0Opc == ISD::ZERO_EXTEND || N0Opc == ISD::SIGN_EXTEND) &&
+            (isConstFalseVal(N1C) ||
+             isExtendedTrueVal(N1C, N0->getValueType(0), SExt))) {
+
+          bool Inverse = (N1C->isNullValue() && Cond == ISD::SETEQ) ||
+                         (!N1C->isNullValue() && Cond == ISD::SETNE);
+
+          if (!Inverse)
+            return TopSetCC;
+
+          ISD::CondCode InvCond = ISD::getSetCCInverse(
+              cast<CondCodeSDNode>(TopSetCC.getOperand(2))->get(),
+              TopSetCC.getOperand(0).getValueType().isInteger());
+          return DAG.getSetCC(dl, VT, TopSetCC.getOperand(0),
+                                      TopSetCC.getOperand(1),
+                                      InvCond);
+
+        }
       }
     }
 
     // If the LHS is '(and load, const)', the RHS is 0,
     // the test is for equality or unsigned, and all 1 bits of the const are
     // in the same partial word, see if we can shorten the load.
     if (DCI.isBeforeLegalize() &&
         !ISD::isSignedIntSetCC(Cond) &&