[PATCH] D46199: [SLPVectorizer] Debug intrinsics shouldn't affect spill cost

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:2559
+      if ((isa<CallInst>(&*PrevInstIt) &&
+           !isa<DbgInfoIntrinsic>(&*PrevInstIt)) &&
+          &*PrevInstIt != PrevInst) {
----------------
davide wrote:
> hsaito wrote:
> > fhahn wrote:
> > > vsk wrote:
> > > > hsaito wrote:
> > > > > If LLVM doesn't have a generic "skip for cost" utility already, I think it's time to start having one. It's very likely that this goes beyond just DbgInfoIntrinsic and functionality would be common to many optimizers. If there is one, we should use that instead.
> > > > > 
> > > > I think you're right. Recently an instructionsWithoutDebug() helper method was introduced in BasicBlock, which is a step in the right direction. It would be nice to use that utility here, although that might be better as a follow-up, since the change looks invasive.
> > > Yep I think having such a utility would be great. When adding instructionsWithoutDebug(), I was also prototyping a more generic skipInstructions function, which takes a range iterator and instruction types to skip and returns an iterator that skips over the ignored instructions. I can put it somewhere on phabricator again.
> > Sounds like a plan. I don't intend to slow down this patch waiting for that. Please follow up towards generic utility direction in the future.
> That would be great, Florian. I think it's going to be of great use now that we're trying to squash down these debuginfo bugs.
Unfortunately the iterators here seem incompatible with range iterators.

But would a helper function like `isFreeInstruction` be useful, that returns true for instructions that won't result in any instructions in the generated code, e.g. dbg intrinsics, lifetime markers, assume. Here (and probably in plenty of more places), we should also ignore those.


Repository:
  rL LLVM

https://reviews.llvm.org/D46199