[PATCH] D132237: [AArch64] expand is-power-of-2 pattern that uses popcount

Mon Aug 22 04:34:56 PDT 2022

davezarzycki added inline comments.

================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:18593-18594

+  // (ctpop x) == 1 --> (x != 0) && ((x & x-1) == 0)
+  // (ctpop x) != 1 --> (x == 0) || ((x & x-1) != 0)
+  if (LHS.getOpcode() == ISD::CTPOP && LHS.hasOneUse() && isOneConstant(RHS) &&
----------------
spatel wrote:
> davezarzycki wrote:
> > For your consideration:
> > 
> > (ctpop x) <= 1 --> (x & x-1) == 0
> > (ctpop x) > 1 --> (x & x-1) != 0
> Ah, I forgot that we added a popcount target hook with D89952. Do you think we should use that for this pattern too? Or did you decide it wasn't worth pursuing (D91093)?
> 
> I'm looking back at the comments in:
> https://bugs.llvm.org/show_bug.cgi?id=47825
> ...and we cited AArch64 scalar custom lowering/combining as a missing piece. But we could adapt the TLI hook if that seems better.
I think the D91093 work is good, but it stopped being a good use of my time to pursue. (Sorry)

Nevertheless the target hook is there. This seems like a reasonable use of it, no?

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

https://reviews.llvm.org/D132237