[PATCH] D83153: [DAGCombiner] Prevent regression in isMulAddWithConstProfitable

Wed Jul 22 19:30:15 PDT 2020

benshi001 marked 2 inline comments as done.
benshi001 added inline comments.

================
Comment at: llvm/test/CodeGen/AArch64/urem-seteq-nonzero.ll:7-10
+; CHECK-NEXT:    mov w9, #43691
+; CHECK-NEXT:    sub w8, w0, #1 // =1
+; CHECK-NEXT:    movk w9, #43690, lsl #16
+; CHECK-NEXT:    mul w8, w8, w9
----------------
benshi001 wrote:
> efriedma wrote:
> > spatel wrote:
> > > I think we need to hear from someone with more AArch knowledge if this is an improvement or acceptable. cc @dmgreen @efriedma @fhahn 
> > The new code is worse; madd is essentially the same cost as mul, so the new code has one more arithmetic instruction.  Better to spend an extra instruction materializing a constant, particularly if the code is in a loop.
> But there are also other cases instruction count is reduced, see line 156-162 and line 176-180, I think it is optimized in total, though worse in specific cases.
There are many aarch64 cases affected, some became better while some became worse, but they are better in total, what's your opinion?

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

https://reviews.llvm.org/D83153