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

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked 3 inline comments as done.
fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8953
   VPlanTransforms::sinkScalarOperands(*Plan);
-  VPlanTransforms::mergeReplicateRegions(*Plan);
   VPlanTransforms::removeDeadRecipes(*Plan);
+  VPlanTransforms::mergeReplicateRegions(*Plan);
----------------
Ayal wrote:
> 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().
I am not sure we can move it up much, as the interleave group code still accesses values via IR references. In theory we could remove a load that's never used and crash.




================
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]]
----------------
Ayal wrote:
> 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.
I think we remove some unnecessary users earlier, hence no need to pack.

We can also remove the unused phis, will do in a late patch. 


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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128408