[PATCH] D113291: [WIP] Add LowerTableBasedCTZ and anable it for AARCH64 in -O3

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LowerTableBasedCTZ.cpp:234
+                 m_ZExtOrSelf(m_Mul(
+                     m_And(m_Sub(m_SpecificInt(0), m_Value(X1)), m_Value(X2)),
+                     m_ConstantInt(MulConst))),
----------------
Use m_Neg instead of m_Sub?

And is Commutable so maybe m_c_And?


================
Comment at: llvm/lib/Transforms/Scalar/LowerTableBasedCTZ.cpp:248
+
+  Argument *ArgX = dyn_cast_or_null<Argument>(X1);
+  if (!ArgX)
----------------
Why does it need to be an Argument?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113291