[PATCH] D116720: [RISCV] Use shift for zero extension when Zbb and Zbp are not enabled

[Craig Topper via Phabricator via llvm-commits]

craig.topper added a comment.

In D116720#3224515 <https://reviews.llvm.org/D116720#3224515>, @Luhaocong wrote:

> In D116720#3224271 <https://reviews.llvm.org/D116720#3224271>, @craig.topper wrote:
>
>> The AND could be better for loops if LICM can move the constant materialization out of the loop.
>>
>> I’ve wondered about doing this as a machine IR peephole after LICM has its chance. But I haven’t spent any time on it.
>
> Would it be better as follow？
>
> 1. Always Select SLLI + SRLI when input is all ones value exceeding simm12.
> 2. in PeepholeOptimzer or other pass，revert SLLI + SRLI to LUI + ADDI + AND if they are in loop.

PeepholeOptimizer is after LICM so that would be too late. It would need to be before LICM.

Another thing I just realized. MachineIR LICM doesn't visit every loop, just the outermost loop with a preheader. So LICM would probably move the LUI+ADDI as far out as it can. And rematerialization in register allocation isn't powerful enough to bring it back in when necessary to avoid a spill since it is two instructions.



================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.td:414
+// least significant bit with the remainder zero and exceeds simm12.
+def LSBOneMask : ImmLeaf<XLenVT, [{
+  if (Subtarget->is64Bit())
----------------
Make this a PatLeaf like `AddiPair` and move the one use check in here to get rid of and_const_oneuse.

I think you can then do
```
return `isInt<12>(N->getSExtValue()) && isMask_64(N->getZExtValue())
```

No need for Subtarget check.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.td:1061
+// AND with mask exceeding simm12.
+let Predicates = [NotHasStdExtZbbOrZbp] in
+def : Pat<(XLenVT (and_const_oneuse GPR:$rs, LSBOneMask:$mask)),
----------------
Now that it's generalized, I don't think you want this predicate check here. The zext.h patterns should get priority since it is look for a specific immediate. We would still want this to fire for other masks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116720