[PATCH] D37317: [X86] Don't pull carry through X86ISD::ADD carryin, -1 if we can't guranteed we're really using the carry flag from the add.

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

Some inline comments, but I haven't stepped through the actual problem yet...from the recent bug report comments, it seems more will be needed:
https://bugs.llvm.org/show_bug.cgi?id=34381



================
Comment at: lib/Target/X86/X86ISelLowering.cpp:30972-30974
+  // When legalizing carry, we create carries via add X, -1
+  // If that comes from an actual carry, via setcc, we use the
+  // carry directly.
----------------
We have this explanatory comment above the helper, so no need to repeat it here and below.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:34858-34859
 
+
+
   return SDValue();
----------------
Stray space diff?


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:35024
 
-// Optimize RES, EFLAGS = X86ISD::ADD LHS, RHS
-static SDValue combineX86ADD(SDNode *N, SelectionDAG &DAG,
-                             X86TargetLowering::DAGCombinerInfo &DCI) {
+static SDValue combineX86SBB(SDNode *N, SelectionDAG &DAG) {
   // When legalizing carry, we create carries via add X, -1
----------------
Call this 'combineSBB' for symmetry with 'combineADC'. The x86-ness is implied by the mnemonics.


================
Comment at: test/CodeGen/X86/pr34381.ll:11
+; Function Attrs: noinline nounwind optnone uwtable
+define void @_Z3foov() {
+; CHECK-LABEL: _Z3foov:
----------------
Something's not right with this test - it passes on trunk already.


https://reviews.llvm.org/D37317