[PATCH] D35192: [ARM] Use ADDCARRY / SUBCARRY

[Eli Friedman via Phabricator via llvm-commits]

efriedma accepted this revision.
efriedma added a comment.

LGTM



================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:9891-9894
+  // If HiAdd is a predecessor of ADDC, the replacement below will create a
+  // cycle.
+  if (AddcNode->isPredecessorOf(HiAdd->getNode()))
+    return SDValue();
----------------
rogfer01 wrote:
> This fixes the problem in PR34045.
> 
> I wonder if `AddeNode` + `LowAdd` might have the same problem but I don't think it can happen.
> 
> The rest of boringssl has built correctly using `-mthumb`, though.
> I wonder if AddeNode + LowAdd might have the same problem but I don't think it can happen.

loAdd is a predecessor of AddcNode, which is a predecessor of AddeNode, so I don't think you can run into problems there.   (It's possible for loAdd to use the result of the UMUL_LOHI, but you don't end up with a cycle in that case, just a redundant multiply.)

I guess this issue only shows up with your patch because it's impossible to create a DAG like that without ADDCARRY transforms?


https://reviews.llvm.org/D35192