[PATCH] D104852: [AArch64][SVEIntrinsicOpts] Convect cntb/h/w/d to vscale intrinsic or constant.

[David Sherwood via Phabricator via cfe-commits]

david-arm added a comment.

Hi @junparser, the patch looks sensible to me! I just had a couple of minor comments if that's ok.



================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:658
+    return IC.replaceInstUsesWith(II, VScale);
+  } else if (Pattern >= AArch64SVEPredPattern::vl1 &&
+             Pattern <= AArch64SVEPredPattern::vl8 && NumElts >= Pattern) {
----------------
nit: Do you need to fix the clang-tidy warning here?


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:662
+    return IC.replaceInstUsesWith(II, StepVal);
+  } else if (Pattern == AArch64SVEPredPattern::vl16 && NumElts == 16) {
+    Constant *StepVal = ConstantInt::get(II.getType(), NumElts);
----------------
Could you potentially fold these two cases into one somehow? Maybe with a switch-case statement? I'm just imagining a situation where we might want other patterns too like vl32, vl64, etc.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104852