[PATCH] D46179: [X86] Lowering adds/addus/subs/subus intrinsics to native IR (LLVM part)

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:32275
+      std::swap(CondLHS, CondRHS);
+      CC = ISD::getSetCCInverse(CC, true);
+    }
----------------
Shouldn't this be getSetCCSwappedOperands?


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:32290
+    if (Other.getNode() && Other->getNumOperands() == 2 &&
+        (DAG.isEqualTo(Other->getOperand(0), CondLHS) ||
+         DAG.isEqualTo(Other->getOperand(1), CondLHS))) {
----------------
isEqualTo is overkill here. You can just compare Other->getOperand(0) == CondLHS

And it should be "Other." instead of "Other->" in most of this code.  Technically they are equivalent, but we tend to use the . on SDValue.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:32301
+      if ((CC == ISD::SETULE) &&
+          Other->getOpcode() == ISD::ADD && DAG.isEqualTo(Other, CondRHS))
+        return SplitOpsAndApply(DAG, Subtarget, DL, VT, { OpLHS, OpRHS },
----------------
Again you don't need isEqualTo.


Repository:
  rL LLVM

https://reviews.llvm.org/D46179