[PATCH] D52961: [RISCV] Introduce the RISCVMatInt::generateInstSeq helper

[Alex Bradbury via Phabricator via llvm-commits]

asb added inline comments.


================
Comment at: lib/Target/RISCV/Utils/RISCVMatInt.cpp:66
+  // performed when the recursion returns.
+
+  int64_t Lo12 = SignExtend64<12>(Val);
----------------
lewis-revill wrote:
> I noticed MathExtras.h has a function `isMask_64`, which essentially tells you whether a value is a non-zero set of 1s from the least significant bit up to a high bit. From this it's quite simple to implement the ADDI -1 + SRLI as described in D52962. Inserting the following code at this point would mean that the sequence it replaces will be at least 3 instructions as Val > 32 bits and Lo12 would be full of 1s:
> 
> ```
>   // Find the special case of the value being a mask containing all ones from
>   // bit position 0 up to bit position N. In other words, a right shift of the
>   // constant -1. Uses only two instructions, ADDI+SRLI.
>   if (isMask_64((uint64_t)Val)) {
>     Res.push_back(Inst(RISCV::ADDI, -1));
> 
>     int ShiftAmount = findLastSet((uint64_t)Val);
>     Res.push_back(Inst(RISCV::SRLI, ShiftAmount));
> 
>     return;
>   }
> ```
I agree it's easy to add, but I wanted to perform this refactoring as a no-op with improvements going in further patches.

Plus handling this case is one of the tasks I walk people through in my RISC-V LLVM Coding Lab this afternoon :)


https://reviews.llvm.org/D52961