[PATCH] D34515: [ARM] Materialise some boolean values to avoid a branch

[Sam Parker via Phabricator via llvm-commits]

samparker added inline comments.


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:12106
+  // Materialize a boolean comparison for integers so we can avoid branching.
+  if (VT.isInteger()) {
+    if (isNullConstant(FalseVal)) {
----------------
early exit when not integer instead?


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:12136
+                          DAG.getConstant(1, dl, MVT::i32), Neg.getValue(1));
+          Res = DAG.getNode(ISD::ADDCARRY, dl, VTs, Sub, Neg, Carry);
+        }
----------------
I don't follow how this sequence better? What makes this more efficient than a cmp and conditional move? On a Thumb1Only target, even a cmp and branch will only be 3 cycles, I think.


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:12185
+      if (ShiftAmount)
+        Res = DAG.getNode(ISD::SHL, dl, VT, Res,
+                          DAG.getConstant(ShiftAmount, dl, MVT::i32));
----------------
Same as above, I guess I'm missing something... could you explain why this is better please?


https://reviews.llvm.org/D34515