[PATCH] D92089: [PowerPC] Materialize i64 constants by enumerated patterns.

[EsmeYi via Phabricator via llvm-commits]

Esme added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:805
+static int findContiguousZerosAtLeast(uint64_t Imm, unsigned Num) {
+  assert((Num > 32 && Num < 64) && "Unexpected number.");
+  unsigned HiTZ = countTrailingZeros<uint32_t>(Hi_32(Imm));
----------------
steven.zhang wrote:
> So, why do we need this assertion ?
> So, why do we need this assertion ?

This function only works for 32 < Num < 64. As for other Nums, we will get an incorrect result. Perhaps we should change the assertion to a `return 0`?


================
Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:867
+                                    getI32Imm((Imm >> TZ) & 0xffff));
+    return CurDAG->getMachineNode(PPC::RLDIC, dl, MVT::i64, SDValue(Result, 0),
+                                  getI32Imm(TZ), getI32Imm(LZ));
----------------
steven.zhang wrote:
> You can use SLDI if the TZ and LZ meet some requirement.  A better idea is to fix it in InstAlias.
> You can use SLDI if the TZ and LZ meet some requirement.  A better idea is to fix it in InstAlias.

RLDIC was chosen over RLDICR because RLDIC can cover more patterns than RLDICR, even though RLDICR can be transformed to SLDI in InstAlias. I prefer to add InstAlias for RLDIC as a follow-up work.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:903
+      findContiguousZeros(~Imm, 49, Shift)) {
+    uint64_t RotImm = (Imm >> Shift) | (Imm << (64 - Shift));
+    Result = CurDAG->getMachineNode(PPC::LI8, dl, MVT::i64,
----------------
steven.zhang wrote:
> There is rotate routines for you to do this.
> There is rotate routines for you to do this.

I know we have `APInt APInt::rotr(unsigned rotateAmt)` for rotation right, but it's not convenient enough since I have to transform `uint64_t Imm` to `APInt`. Is there any other rotation routines?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92089