[PATCH] D35635: Optimize {s,u}{add,sub}.with.overflow on ARM

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

> Good idea. However, the ARMISD::BRCOND isn't combined until after saddo is lowered.

That shouldn't completely block all transforms; you could pattern-match the ARMISD nodes.  But I guess the patterns become a lot more complicated, so maybe not worth doing.



================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:4582
+    assert((CC == ISD::SETEQ || CC == ISD::SETNE) &&
+           "Unexpected condition code.");
+    // Only lower legal XALUO ops.
----------------
Is this assert actually guaranteed somehow?  I mean, it should be possible to transform any relevant condition to an equivalent SETEQ or SETNE, but I don't see any code to actually ensure this.


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:4601
+    if (isOneConstant(RHS))
+      ARMcc = InvertCondCode(ARMcc);
+    SDValue CCR = DAG.getRegister(ARM::CPSR, MVT::i32);
----------------
I would probably write "if ((CC == ISD::SETNE) != isOneConstant(RHS))".


https://reviews.llvm.org/D35635