[PATCH] D128408: [LV] Remove collectTriviallyDeadInstructions, already handled by VP DCE.

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked an inline comment as done.
fhahn added inline comments.


================
Comment at: llvm/test/Transforms/LoopVectorize/pointer-induction.ll:39
 ; CHECK:       pred.store.if:
-; CHECK-NEXT:    [[TMP11:%.*]] = getelementptr inbounds i8, i8* [[NEXT_GEP]], i64 -1
-; CHECK-NEXT:    store i8 95, i8* [[TMP11]], align 1
----------------
Ayal wrote:
> fhahn wrote:
> > Ayal wrote:
> > > fhahn wrote:
> > > > Ayal wrote:
> > > > > Avoid sinking scalar GEP into first triangle?
> > > > > 
> > > > > Perhaps worth separating the movement of removeDeadRecipes() from the removal of collectTriviallyDeadInstructions() into separate patches, to clarify the origin of these changes.
> > > > I think this change is combination of moving removeDeadRecipes and removing collectTriviallyDeadInstructions, through which we end up avoiding to sink
> > > So does it make sense to first only hoist removeDeadRecipes() above mergeReplicateRegions(), and then remove collectTriviallyDeadInstructions in a separate patch, isolating any such effects?
> > I can hoist `removeDeadRecipes` up separately, but moving it alone won't show any changes in the existing tests unfortunately. 
> > I can hoist removeDeadRecipes up separately, but moving it alone won't show any changes in the existing tests unfortunately.
> 
> So be it. Is the hoist important?
The movement is important to avoid some regressions in some of the tests, due to dead instructions blocking region merging. I managed to construct a test case and split off the movement of `removeDeadRecipes` to D128831


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128408