[PATCH] D71829: [PowerPC] Exploit the rlwinm instructions for "and" with constant.

[ChenZheng via Phabricator via llvm-commits]

shchenz added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:4457
+    if (isRunOfOnes64(Imm64, MB, ME)) {
+      if (MB >= 32 && MB <= ME) {
+        //                MB  ME
----------------
steven.zhang wrote:
> steven.zhang wrote:
> > shchenz wrote:
> > > for ` and reg, 0xffffffffffff00ffULL`, it should be replaced by one single rlwinm instruction, seems it is not handled here?
> > Good point. I will add it together.
> Hmm, after double check, we cannot do it with single rlwinm instructions as it is a wrapping, and the content [0, 31] has been filled with the content of [32, 63].  We need another instructions to clear it. That is the case of 'else' clause.
Sorry, I made a mistake here. `and reg, 0xffffffffffff00ffULL` I think this case is not a good candidate for one single rlwinm instruction. We get a wrap mask and the result is only related to the low 32 bit of `and` input. But from the mask 0xffffffffffff00ffULL, the result should be also related to high 32 bit of `and` input. But I think it is a good candidate for your patch https://reviews.llvm.org/D71831.


================
Comment at: llvm/test/CodeGen/PowerPC/opt-cmp-inst-cr0-live.ll:10
   %4 = ashr exact i64 %3, 48
-; CHECK: ANDI8o killed {{[^,]+}}, 65520, implicit-def dead $cr0
+; CHECK: RLWINM8 killed %3:g8rc, 0, 16, 27
 ; CHECK: CMPLDI
----------------
The case change seems strange here, the left uses a record form ANDI8, so we can eliminate the CMPLDI with 0 after peephole. But with this change,  ANDI8 is converted to RLWINM8, which is not record form. but it surprises me we still can delete the CMPLDI with 0 and replace RLWINM8 back with ANDI8. I think here we should use record form RLWINM8o. But I think it is not this patch's scope.


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

https://reviews.llvm.org/D71829