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

[Qing Shan Zhang via Phabricator via llvm-commits]

steven.zhang 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));
----------------
Esme wrote:
> 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`?
The semantics for this function is to get the pos of the Num at least consecutive zeros across Hi32 and Lo32. Otherwise, return zero. Is it right ? So, I am confused about the incorrect result you mean. 


================
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,
----------------
Esme wrote:
> 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?
In fact, you can declare the Imm as APInt and it will make the bit operation more easy. But it is up to you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92089