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

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8953
   VPlanTransforms::sinkScalarOperands(*Plan);
-  VPlanTransforms::mergeReplicateRegions(*Plan);
   VPlanTransforms::removeDeadRecipes(*Plan);
+  VPlanTransforms::mergeReplicateRegions(*Plan);
----------------
To be close to current state, dead recipes should be removed as early as possible, but they should also clean up after optimizeInductions() and/or sinkScalarOperands() (but not after mergeReplicateRegions())? Can also run it twice, keeping the late version at the end next to removeRedundantExpandSCEVRecipes(), and issuing it early instead of the preparatory collectTriviallyDeadInstructions().


================
Comment at: llvm/test/Transforms/LoopVectorize/X86/consecutive-ptr-uniforms.ll:101
 ; FORCE-NEXT:    [[TMP7:%.*]] = load i32, i32* [[TMP6]], align 1
-; FORCE-NEXT:    [[TMP8:%.*]] = insertelement <2 x i32> poison, i32 [[TMP7]], i32 0
 ; FORCE-NEXT:    br label [[PRED_LOAD_CONTINUE]]
----------------
Hmm, moving removeDeadRecipes() removed alsoPack or something?

In the case of PR40816, the loads (and phi's they feed) are also redundant and can be discarded, as LV sees through them to figure out the trip count.


================
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
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128408