[PATCH] D85848: [InlineCost] Skip scalable vectors in visitAlloca
Eli Friedman via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 12 14:39:25 PDT 2020
efriedma added inline comments.
================
Comment at: llvm/lib/Analysis/InlineCost.cpp:857
+ if (isa<ScalableVectorType>(I.getAllocatedType()))
+ return false;
+
----------------
c-rhodes wrote:
> efriedma wrote:
> > This will refuse to inline any function with a scalable vector variable; that seems so conservative that it's likely to cause practical issues. Please take the time to fix the code properly.
> > This will refuse to inline any function with a scalable vector variable; that seems so conservative that it's likely to cause practical issues. Please take the time to fix the code properly.
>
> Is that not better than crashing? As far as I can tell either `getFixedSize` will be called causing a crash or it's a dynamic alloca that wont be inlined anyway.
>
> I know we want to support scalable vectors here but unless I'm missing something this seems like a worthwhile improvement for the time being.
Reading the code, I think you could just replace the uses of getFixedSize() with getKnownMinSize() and get roughly the right behavior.
The reason I don't want to put this off is that inlining is very important for optimizations; if we add a hack now, we're inevitably going to have to look at it again very soon, so I'd rather just get it done now.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85848/new/
https://reviews.llvm.org/D85848
More information about the llvm-commits
mailing list