[PATCH] D115750: [SLP]Further improvement of the cost model for scalars used in buildvectors.

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added a comment.

In D115750#3499441 <https://reviews.llvm.org/D115750#3499441>, @mstorsjo wrote:

> This caused (or exposed?) failing asserts:
>
>   $ cat sqrtf.c
>   float *a;
>   float c;
>   float sqrtf(float);
>   void b() {
>     float d, e, f, g;
>     d = c * (0 + 2 * sqrtf(c) * 0);
>     e = c * (0 - (c + 1) * 0);
>     f = c * (0 - 2 * sqrtf(c) * 0);
>     g = 2 * (0 + (c + 1) * 0);
>     a[0] = d / 0;
>     a[1] = e / 0;
>     a[2] = f / 0;
>     a[3] = g / 0;
>   }
>   $ clang -target x86_64-linux-gnu -c -O2 -fno-math-errno sqrtf.c
>   clang: ../lib/Transforms/Vectorize/SLPVectorizer.cpp:6592: llvm::InstructionCost llvm::slpvectorizer::BoUpSLP::getTreeCost(llvm::ArrayRef<llvm::Value*>): Assertion `Mask[InIdx] == UndefMaskElem && "InsertElementInstruction used already."' failed.

Hi, thanks for the report, feel free to revert, I'll fix it tomorrow.



================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:6583
+                             "insertelement users.\n";
+                   if (TEs.front()) TEs.front()->dump(); TEs.back()->dump();
+                   dbgs() << "SLP: Current total cost = " << Cost << "\n");
----------------
RKSimon wrote:
> ABataev wrote:
> > RKSimon wrote:
> > > ABataev wrote:
> > > > ABataev wrote:
> > > > > RKSimon wrote:
> > > > > > Is this correct - not sure if you want braces here or just a newline
> > > > > Want to dump TreeEntries here, if possible, helps to get more info about the graph, rather than just an instruction.
> > > > Ah, did not understand the question. Everything is ok here, just the first element might be a `nullptr`
> > > OK - but maybe put TEs.back()->dump(); on a separate line so it doesn't look like it should be:
> > > ```
> > > if (TEs.front()) { TEs.front()->dump(); TEs.back()->dump(); }
> > > ```
> > > I'm very surprised clang-format didn't catch this
> > Will enclose first dump into braces explicitly, clang-format wants to make them in one line.
> That's scary - I guess because this is all inside the LLVM_DEBUG macro...
Yes, I believe so.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115750