[llvm] r246938 - [SelectionDAG] Swap commutative binops before constant-based folding

[Hal Finkel via llvm-commits]

Author: hfinkel
Date: Sun Sep  6 00:42:13 2015
New Revision: 246938

URL: http://llvm.org/viewvc/llvm-project?rev=246938&view=rev
Log:
[SelectionDAG] Swap commutative binops before constant-based folding

In searching for a fix for the underlying code-quality bug highlighted by
r246937 (that SDAG simplification can lead to us generating an ISD::OR node
with a constant zero LHS), I ran across this:

We generically canonicalize commutative binary-operation nodes in SDAG getNode
so that, if only one operand is a constant, it will be on the RHS.  However, we
were doing this only after a bunch of constant-based simplification checks that
all assume this canonical form (that any constant will be on the RHS). Moving
the operand-swapping canonicalization prior to these checks seems like the
right thing to do (and, as it turns out, causes SDAG to completely fold away the
computation in test/CodeGen/ARM/2012-11-14-subs_carry.ll, just like InstCombine
would do).

Modified:
    llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
    llvm/trunk/test/CodeGen/ARM/2012-11-14-subs_carry.ll

Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp?rev=246938&r1=246937&r2=246938&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp (original)
+++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp Sun Sep  6 00:42:13 2015
@@ -3348,6 +3348,13 @@ SDValue SelectionDAG::getNode(unsigned O
                               SDValue N2, const SDNodeFlags *Flags) {
   ConstantSDNode *N1C = dyn_cast<ConstantSDNode>(N1);
   ConstantSDNode *N2C = dyn_cast<ConstantSDNode>(N2);
+
+  // Canonicalize constant to RHS if commutative.
+  if (N1C && !N2C && isCommutativeBinOp(Opcode)) {
+    std::swap(N1C, N2C);
+    std::swap(N1, N2);
+  }
+
   switch (Opcode) {
   default: break;
   case ISD::TokenFactor:
@@ -3692,12 +3699,6 @@ SDValue SelectionDAG::getNode(unsigned O
           FoldConstantArithmetic(Opcode, DL, VT, N1.getNode(), N2.getNode()))
     return SV;
 
-  // Canonicalize constant to RHS if commutative.
-  if (N1C && !N2C && isCommutativeBinOp(Opcode)) {
-    std::swap(N1C, N2C);
-    std::swap(N1, N2);
-  }
-
   // Constant fold FP operations.
   bool HasFPExceptions = TLI->hasFloatingPointExceptions();
   ConstantFPSDNode *N1CFP = dyn_cast<ConstantFPSDNode>(N1);

Modified: llvm/trunk/test/CodeGen/ARM/2012-11-14-subs_carry.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/2012-11-14-subs_carry.ll?rev=246938&r1=246937&r2=246938&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/ARM/2012-11-14-subs_carry.ll (original)
+++ llvm/trunk/test/CodeGen/ARM/2012-11-14-subs_carry.ll Sun Sep  6 00:42:13 2015
@@ -1,10 +1,14 @@
 ; RUN: llc < %s -mtriple=thumbv7-apple-ios -arm-atomic-cfg-tidy=0 | FileCheck %s
 
 ;CHECK-LABEL: foo:
-;CHECK: adds
-;CHECK-NEXT: adc
-;CHECK-NEXT: bx
+;CHECK: movs r0, #0
+;CHECK-NEXT: bx lr
 
+; Note: This test case originally checked, per r167963, for:
+;       adds
+;       adc
+;       bx
+; But SDAG now, like InstCombine, can fold everything away.
 ;rdar://12028498
 
 define i32 @foo() nounwind ssp {