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

[Ben Shi via Phabricator via llvm-commits]

benshi001 marked 3 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
----------------
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.


================
Comment at: llvm/test/CodeGen/RISCV/addimm-mulimm.ll:74
-
-define signext i32 @add_mul_trans_reject_2(i32 %x) {
-; RV32IM:       # %bb.0:
----------------
spatel wrote:
> Why is this test deleted?
This code was also added by me, which is expected to be affected by this patch. But actually not, so I remove them.


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

https://reviews.llvm.org/D83153