[PATCH] D106850: [InstCombine] Simplify llvm.vscale when vscale_range attribute exists

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:5787
+        return nullptr;
+      unsigned MinScalarVectorSize, MaxScalarVectorSize;
+      std::tie(MinScalarVectorSize, MaxScalarVectorSize) =
----------------
frasercrmck wrote:
> david-arm wrote:
> > nit: I think 'Scalar' here is a bit confusing as that's kind of the opposite of 'Vector'. Perhaps 'Scalable' is better?
> Yeah. Or `VScaleMin` and `VScaleMax`?
Or just `Min` and `Max`?


================
Comment at: llvm/test/Transforms/InstSimplify/fold-vscale.ll:4
+
+define i64 @vscale_i64() #1 {
+; CHECK-LABEL: @vscale_i64(
----------------
nit: it would be useful to add `_range_<min>_<max>` to the name as well, since the attributes are at the bottom, making it slightly easier to read, e.g.`@ vscale_i64_range_1_1()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106850