[PATCH] D62166: [mips] Always check that `shift and add` optimization is efficient

Simon Dardis via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 21 16:53:26 PDT 2019


sdardis added a comment.

I've raised some points inline, I think (a) is most relevant to this patch but shouldn't stop it going forward but should be addressed quickly, (b) and (c) could be noted as TODO:s.



================
Comment at: llvm/lib/Target/Mips/MipsSEISelLowering.cpp:724-737
+  //
+  // If we have taken more than 12[1] / 8[2] steps to attempt the
+  // optimization for a native sized value, it is more than likely that this
+  // optimization will make things worse.
+  //
+  // [1] MIPS64 requires 6 instructions at most to materialize any constant,
+  //     multiplication requires at least 4 cycles, but another cycle (or two)
----------------
Looking at this and the original version, some things come to mind:

a) MaxSteps needs to consider the VT of the constant for the target. I.E. for a 32 bit multiplication on a N32/N64 target the maximum number of steps is incorrect. I believe the calculation of the maximum number of steps needs to consider the case of <=i32 and >i32 && <=i64 cases for natively supported types.

I would suggest looking at starting with the maximum number of steps as equal to the number of cycles that it takes to perform a constant materialization sequence worst case, then applying a "legalization penalty" to the number of steps if the type of the operands is not natively supported.

b) This optimization can occur before type legalization, it may be worth considering restricting this optimization to after type legalization so that there is no fudge on the legalization penalty of an illegal type for some constant value (lines 771-780).

c) This optimization should be account for -Os, -Oz, as the optimization can increase code size.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62166





More information about the llvm-commits mailing list