[PATCH] D62857: [RISCV] Prevent re-ordering some adds after shifts

[Sam Elliott via Phabricator via llvm-commits]

lenary marked 4 inline comments as done.
lenary added inline comments.


================
Comment at: llvm/lib/Target/RISCV/Utils/RISCVMatInt.cpp:81
+int getIntMatCost(const APInt &Val, bool Is64Bit) {
+  unsigned BitSize = Val.getMinSignedBits();
+  int PlatRegSize = Is64Bit ? 64 : 32;
----------------
asb wrote:
> I think this isn't going to give the desired result if Val is e.g. -1. In that case, getMinSignedBits is going to return 1, meaning the loop is only executed once regardless of the original type width. If the original type was e.g. an i64 on RV32, then two instructions are required to materialise the constant, yet the logic in this function will only return 1.
> 
> I think the solutation is to more closely mirror the AArch64TTIImpl::getIntImmCost function I mentioned before, and add a Type parameter (and maybe also adopt similar logic for sign-extending constants to be a multiple of the PlatRegSize).
I think I now cover chunking correctly. As in my message above, I don't need to sign extend Val before chunking, because each chunk is extended to be PlatRegSize within the loop.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62857