[DAGCombine] Do not try to deduplicate commutative operations if both operand are the same.

[deadalnix]

Summary

It is creating useless work as the commuted nodes is the same as the node we are working on in that case.

[RKSimon]

Do you have any stats?

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

1615–1616

Why you're here, please can you change this to SDISel to make it clearer.

[deadalnix]

make comment clearer.

[RKSimon]

In D33840#771551, @RKSimon wrote:

Do you have any stats?

Any compile time stats? Compiling clang before/after this patch should be good enough for an initial comparison.

[deadalnix]

Hi,

I got sidetracked into some other very urgent project during June. I'll do my best to provide that number as soon as I can. I don't expect the gain, as this, to be that great, but who knows ? What I'm after here is to reduce the impact of D33587 on performances.

[deadalnix]

OK, benchmarks. Compiling clang from a bc containing clang in its entierety. With the patch:

real    9m45.457s
user    9m44.085s
sys     0m1.384s

Without

real    9m45.521s
user    9m44.016s
sys     0m1.517s

The impact on perfs is not very significant. It's within the noise.

[deadalnix]

rebase

[nemanjai]

I think that as expected, this has a minimal impact on compile time. But of course, it can't hurt I suppose. I don't see why anyone would be opposed to this.

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

case ISD::ANY_EXTEND:
       break;
     case ISD::LOAD:
       if (PromoteLoad(SDValue(N, 0)))
         RV = SDValue(N, 0);
       break;
     }
   }
 
-  // If N is a commutative binary node, try commuting it to enable more
-  // sdisel CSE.
+  // If N is a commutative binary node, try eliminate it if the commuted
+  // version is already present in the DAG.
   if (!RV.getNode() && TLI.isCommutativeBinOp(N->getOpcode()) &&
       N->getNumValues() == 1) {
     SDValue N0 = N->getOperand(0);
     SDValue N1 = N->getOperand(1);
 
     // Constant operands are canonicalized to RHS.
-    if (isa<ConstantSDNode>(N0) || !isa<ConstantSDNode>(N1)) {
+    if (N0 != N1 && (isa<ConstantSDNode>(N0) || !isa<ConstantSDNode>(N1))) {
       SDValue Ops[] = {N1, N0};
       SDNode *CSENode = DAG.getNodeIfExists(N->getOpcode(), N->getVTList(), Ops,
                                             N->getFlags());
       if (CSENode)
         return SDValue(CSENode, 0);
     }
   }