[DAG][PowerPC] Fix dropped `nsw` flag in `SimplifySetCC` by adding `doesNodeExist` helper

[lkail]

Summary

SimplifySetCC invokes getNodeIfExists without passing Flags argument and getNodeIfExists uses a default SDNodeFlags to intersect the original flags, as a consequence, flags like nsw is dropped. Added a new helper function doesNodeExist to check if a node exists without modifying its flags.

[qiucf]

This patch can fix the issue reflected by your case. However, if you call getNodeIfExists somewhere else with argument Flags, I guess you will still meet the problem that flags of the unrelated existing node are modified.

The optional argument Flags looks confusing sometimes: does it mean (1) 'I want the node with the same opcodes, operands, and flags' or (2) 'I want the node with the same opcodes and operands, apply flags to it if found'?

If (1), the node id doesn't put flags into consideration (also to getNode). If (2), why use intersectFlags and modifying the original node still looks not reasonable.

[efriedma]

I think getNodeIfExists() should have the same semantics with respect to flags as getNode().

Maybe we can add a new helper "doesNodeExist()" which just returns a boolean, and doesn't mess with the flags?

[lkail]

I think what @efriedma suggests is viable and eases the confusion mentioned by @qiucf .

[lkail]

Ping.

[lkail]

Gentle ping.

[nemanjai]

LGTM. This seems like a reasonable solution.

[lkail]

Reformat code.

llvm/include/llvm/CodeGen/SelectionDAG.h

SDValue getTargetInsertSubreg(int SRIdx, const SDLoc &DL, EVT VT,
                                 SDValue Operand, SDValue Subreg);
 
   /// Get the specified node if it's already available, or else return NULL.
   SDNode *getNodeIfExists(unsigned Opcode, SDVTList VTList,
                           ArrayRef<SDValue> Ops, const SDNodeFlags Flags);
   SDNode *getNodeIfExists(unsigned Opcode, SDVTList VTList,
                           ArrayRef<SDValue> Ops);
 
+  /// Check if a node exists without modifying its flags.
+  SDNode *doesNodeExist(unsigned Opcode, SDVTList VTList,
+                        ArrayRef<SDValue> Ops);
+
   /// Creates a SDDbgValue node.
   SDDbgValue *getDbgValue(DIVariable *Var, DIExpression *Expr, SDNode *N,
                           unsigned R, bool IsIndirect, const DebugLoc &DL,
                           unsigned O);
 
   /// Creates a constant SDDbgValue node.
   SDDbgValue *getConstantDbgValue(DIVariable *Var, DIExpression *Expr,
                                   const Value *C, const DebugLoc &DL,

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp

if (VTList.VTs[VTList.NumVTs - 1] != MVT::Glue) {
     if (SDNode *E = FindNodeOrInsertPos(ID, SDLoc(), IP)) {
       E->intersectFlagsWith(Flags);
       return E;
     }
   }
   return nullptr;
 }
 
+/// doesNodeExist - Check if a node exists without modifying its flags.
+SDNode *SelectionDAG::doesNodeExist(unsigned Opcode, SDVTList VTList,
+                                    ArrayRef<SDValue> Ops) {
+  if (VTList.VTs[VTList.NumVTs - 1] != MVT::Glue) {
+    FoldingSetNodeID ID;
+    AddNodeIDNode(ID, Opcode, VTList, Ops);
+    void *IP = nullptr;
+    if (SDNode *E = FindNodeOrInsertPos(ID, SDLoc(), IP))
+      return E;
+  }
+  return nullptr;
+}
+
 /// getDbgValue - Creates a SDDbgValue node.
 ///
 /// SDNode
 SDDbgValue *SelectionDAG::getDbgValue(DIVariable *Var, DIExpression *Expr,
                                       SDNode *N, unsigned R, bool IsIndirect,
                                       const DebugLoc &DL, unsigned O) {
   assert(cast<DILocalVariable>(Var)->isValidLocationForIntrinsic(DL) &&
          "Expected inlined-at fields to agree");

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

SDValue TargetLowering::SimplifySetCC(EVT VT, SDValue N0, SDValue N1,
 
   // If we have a subtract with the same 2 non-constant operands as this setcc
   // -- but in reverse order -- then try to commute the operands of this setcc
   // to match. A matching pair of setcc (cmp) and sub may be combined into 1
   // instruction on some targets.
   if (!isConstOrConstSplat(N0) && !isConstOrConstSplat(N1) &&
       (DCI.isBeforeLegalizeOps() ||
        isCondCodeLegal(SwappedCC, N0.getSimpleValueType())) &&
-      DAG.getNodeIfExists(ISD::SUB, DAG.getVTList(OpVT), { N1, N0 } ) &&
-      !DAG.getNodeIfExists(ISD::SUB, DAG.getVTList(OpVT), { N0, N1 } ))
+      DAG.doesNodeExist(ISD::SUB, DAG.getVTList(OpVT), {N1, N0}) &&
+      !DAG.doesNodeExist(ISD::SUB, DAG.getVTList(OpVT), {N0, N1}))
     return DAG.getSetCC(dl, VT, N1, N0, SwappedCC);
 
   if (auto *N1C = isConstOrConstSplat(N1)) {
     const APInt &C1 = N1C->getAPIntValue();
 
     // Optimize some CTPOP cases.
     if (SDValue V = simplifySetCCWithCTPOP(*this, VT, N0, C1, Cond, dl, DAG))
       return V;

llvm/test/CodeGen/PowerPC/setcc-sub-flag.ll

 ; RUN: llc -mtriple=powerpc64le -simplify-mir -verify-machineinstrs \
 ; RUN:   -stop-after=finalize-isel < %s | FileCheck %s
 
 declare void @foo(i64)
 declare void @bar(i1)
 
 define void @f(i64 %a, i64 %b) {
   ; CHECK-LABEL: name: f
   ; CHECK: bb.0 (%ir-block.0):
   ; CHECK:   liveins: $x3, $x4
   ; CHECK:   [[COPY:%[0-9]+]]:g8rc = COPY $x4
   ; CHECK:   [[COPY1:%[0-9]+]]:g8rc = COPY $x3
-  ; CHECK:   [[SUBF8_:%[0-9]+]]:g8rc = SUBF8 [[COPY1]], [[COPY]]
+  ; CHECK:   [[SUBF8_:%[0-9]+]]:g8rc = nsw SUBF8 [[COPY1]], [[COPY]]
   %c = sub nsw i64 %b, %a
   call void @foo(i64 %c)
   %d = icmp slt i64 %a, %b
   call void @bar(i1 %d)
   ret void
 }
 
 define void @g(i64 %a, i64 %b) {