[PATCH] Refactor: Simplify boolean conditional return statements in llvm/lib/CodeGen/SelectionDAG

Richard legalize at xmission.com
Mon May 25 06:35:11 PDT 2015


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:9806
@@ -9807,6 +9805,3 @@
     EVT TruncateType = Inst->getValueType(0);
-    if (TruncateType != SliceType &&
-        !TLI.isOperationLegal(ISD::ZERO_EXTEND, TruncateType))
-      return false;
-
-    return true;
+    return !(TruncateType != SliceType &&
+        !TLI.isOperationLegal(ISD::ZERO_EXTEND, TruncateType));
----------------
craig.topper wrote:
> Push the negate through and use an OR
Fixed.

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:9910
@@ -9914,6 +9909,3 @@
     // 3. Check that we do not have a zext in the way.
-    if (Inst->getValueType(0) != getLoadedType())
-      return false;
-
-    return true;
+    return !(Inst->getValueType(0) != getLoadedType());
   }
----------------
craig.topper wrote:
> Use ==
Fixed.

What's interesting here is that the tool already pushes through `!(e1 != e2)` to `e1 == e2` when it is a simple `==` comparison.  So I dug into this one to find out why it hadn't done that and it's because this is an `operator==` applied to an abstract data type.  Because it is possible that `operator!=` has been overridden, but `operator==` has not been overridden, I didn't want to make the tool blindly push through on this kind of change.  I'll need to figure out how to determine if `operator==` has been appropriately overridden before pushing through the simplification safely.

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:2317
@@ -2316,5 +2316,3 @@
   bool isGNU = Triple(TM.getTargetTriple()).getEnvironment() == Triple::GNU;
-  if (isGNU && !TM.Options.UnsafeFPMath)
-    return false;
-  return true;
+  return !(isGNU && !TM.Options.UnsafeFPMath);
 }
----------------
craig.topper wrote:
> Push the negate through
Fixed.

What's a good guideline for the tool here?

I was thinking that it should apply DeMorgan's Theorem for the cases `!(!e1 && !e2 && ... && !en)` and `!(!e1 || !e2 || ... || !en)`.  In that situation it seems like a "no brainer" because it's the classic double negative on every term in the expression.

Here though, we have one of the inner terms negated and one not negated, so there's no clear simplification.

Is it always desirable to push through the outer negate whenever **any** of the inner terms also has a negation?  I can see this being a matter of preference and therefore perhaps tweakable through a configuration option for the tool.

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAG.cpp:2702
@@ -2703,3 +2701,3 @@
 
-  if (Op.getOpcode() == ISD::OR &&
+  return !(Op.getOpcode() == ISD::OR &&
       !MaskedValueIsZero(Op.getOperand(0),
----------------
craig.topper wrote:
> Push the negate through
Fixed.

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:1365
@@ -1364,3 +1364,3 @@
   // physical register.
-  if (!OPI2->isReg() ||
+  return !(!OPI2->isReg() ||
       (!TargetRegisterInfo::isPhysicalRegister(OPI->getReg()) &&
----------------
craig.topper wrote:
> Push the negate through
Fixed.

http://reviews.llvm.org/D9971

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list