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

[Ayal Zaks via Phabricator via llvm-commits]

Ayal 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
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128408