[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:
> 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. 
If an immediate have Num (which is less than 32) consecutive zeros across Hi32 and Lo32 we will return the pos, but if all of these consecutive zeros are within Lo32/Hi32, we will return 0. I assumed this is not a correct result. Well, after reconsidering I think it's OK to have no assertion. I will remove it in the next update. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92089