[PATCH] D60089: [ARM] Don't replicate instructions in Ifcvt at minsize

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:1939
+    if (TBB.pred_size() != 1 || FBB.pred_size() != 1)
+      return false;
+  }
----------------
Maybe explain why this increases codesize in a comment.  I guess the issue here is that if a block has multiple predecessors, you essentially have to clone it, then if-convert the clone?  (Theoretically, if we if-convert all the predecessors, it could actually save codesize, but I guess that's unlikely to come up in practice.)


================
Comment at: llvm/test/CodeGen/Thumb2/ifcvt-minsize.ll:17
+; CHECK-NEXT:    pop {r4, pc}
+; CHECK-NEXT:  .LBB0_2: @ %if.end
 ; CHECK-NEXT:    bl fn
----------------
The testcases don't really seem to demonstrate the point of this... yes, you're saving two bytes, but only because we can use cbz here.


================
Comment at: llvm/test/CodeGen/Thumb2/ifcvt-minsize.ll:48
+; CHECK-NEXT:  .LBB1_2: @ %f
 ; CHECK-NEXT:    bx lr
 entry:
----------------
I guess this is sort of orthogonal to your patch, but why aren't we generating "pop {r7, pc}" here?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60089/new/

https://reviews.llvm.org/D60089