From 987b4b6f61678a58e344fbed4e5c363ab734815c Mon Sep 17 00:00:00 2001
From: Hal Finkel <hfinkel@anl.gov>
Date: Sun, 6 Sep 2015 05:42:13 +0000
Subject: [PATCH] [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).
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@246938 91177308-0d34-0410-b5e6-96231b3b80d8
---
lib/CodeGen/SelectionDAG/SelectionDAG.cpp | 13 +++++++------
test/CodeGen/ARM/2012-11-14-subs_carry.ll | 10 +++++++---
2 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index 7ccaaf72c37..3acf749bf3e 100644
--- a/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ b/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -3348,6 +3348,13 @@ SDValue SelectionDAG::getNode(unsigned Opcode, SDLoc DL, EVT VT, SDValue N1,
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 Opcode, SDLoc DL, EVT VT, SDValue N1,
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);
diff --git a/test/CodeGen/ARM/2012-11-14-subs_carry.ll b/test/CodeGen/ARM/2012-11-14-subs_carry.ll
index 33083303a3d..f7abac06919 100644
--- a/test/CodeGen/ARM/2012-11-14-subs_carry.ll
+++ b/test/CodeGen/ARM/2012-11-14-subs_carry.ll
@@ -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 {
--
2.34.1