[PATCH] D103153: [InstCombine] Add fold for extracting known elements from a stepvector

Caroline via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 3 02:50:10 PDT 2021


CarolineConcatto added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:355
+        break;
+      case Intrinsic::experimental_stepvector:
+        // Early break when Index is greater than the scalable vector minimum
----------------
sdesmalen wrote:
> It seems a bit silly to have a switch statement to handle a single case, if you can write:
>   if (IID == Intrinsic::experimental_stepvector && IndexC->getValue().ult(NumElts)) {
>     ...
>   }
Yes, for now, it may seem silly, but if we decide to add another intrinsic check this will come in handy.
But I've changed to follow to police:  don't write code you don't need now, despite the fact the I still like my implementation.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:359
+        // be created at run-time.
+        if (!IndexC->getValue().ult(NumElts))
+          break;
----------------
sdesmalen wrote:
> nit: `s/!IndexC->getValue().ult/IndexC->getValue().uge/`
This is not needed anymore.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103153



More information about the llvm-commits mailing list