[PATCH] D54546: [ARM] Don't expand sdiv when optimising for minsize

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added inline comments.


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:7806
+  const ARMSubtarget& ST = static_cast<const ARMSubtarget&>(DAG.getSubtarget());
+  const bool hasDivide = ST.isThumb() ? ST.hasDivideInThumbMode()
+                                      : ST.hasDivideInARMMode();
----------------
efriedma wrote:
> ARM mode?  It looks like the rest of the function doesn't handle it correctly, and you don't have any tests.
Oops, thanks for catching this! I completely focused on Thumb, and forgot about ARM. But yes, will add ARM too.


================
Comment at: test/CodeGen/ARM/sdiv-opt-size.ll:11
+; V6M: .file {{.*}}
+; V6M-NOT:  sdiv
+
----------------
efriedma wrote:
> Could you also check that we don't generate any libcalls for v6m?
Yep, will do.


================
Comment at: test/CodeGen/ARM/sdiv-opt-size.ll:18
+; CHECK-NEXT:  sdiv    r0, r0, r1
+; CHECK-NEXT:  sxth    r0, r0
+; CHECK-NEXT:  bx      lr
----------------
efriedma wrote:
> This sxth is redundant?  Probably need to implement ComputeNumSignBits for SDIV, or something like that.
Interesting! Hadn't looked at that, but yes, that's redundant. Will address that in a follow-up patch.


https://reviews.llvm.org/D54546