[PATCH] D78932: [DSE,MSSA] Relax post-dom restriction for objs visible after return.

[Florian Hahn via Phabricator via llvm-commits]

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

ping :)



================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:1802
+        else
+          for (BasicBlock *R : PDT.getRoots()) {
+            if (!DT.isReachableFromEntry(R))
----------------
fhahn wrote:
> george.burgess.iv wrote:
> > fhahn wrote:
> > > george.burgess.iv wrote:
> > > > i'm unsure how we reach this case. AFAICT, `PDT.dominates(nullptr, AnythingElse) == false`, since the PDT should fail to lookup the node for `nullptr`, no?
> > > We can hit this scenario when there's no single exit block that post-dominates all killing blocks, like in the example below. Instead the special 'nullptr'
> > > block is used, but for starting the traversal we need to start with all exit blocks I think.
> > > 
> > > ```
> > > define void @test12(i32* %P) {
> > >   store i32 0, i32* %P
> > >   br i1 true, label %bb1, label %bb2
> > > 
> > > bb1:                                              ; preds = %0
> > >   store i32 1, i32* %P
> > >   br label %bb3
> > > 
> > > bb2:                                              ; preds = %0
> > >   store i32 1, i32* %P
> > >   ret void
> > > 
> > > bb3:                                              ; preds = %bb1
> > >   ret void
> > > }
> > > ```
> > Sorry, I should've been a bit more clear. Specifically, this code is guarded by `PDT.dominates(CommonPred, DomAccess->getBlock())`. `DomAccess->getBlock()` should never be `nullptr` and `PDT.dominates(nullptr, not_nullptr)` should always be `false` AFAIK, so I think we should never hit this `else` if `!CommonPred`? Or maybe we do for `DomAccess`es which are in unreachable code (but in that case, do we care to do extra work?)
> > PDT.dominates(nullptr, not_nullptr) should always be false
> 
> Hm, IIUC nullptr stands for the (virtual) root node in the post dominator tree. Unless I am missing something, I think ` PDT.dominates(nullptr, not_nullptr)` should always be `true`, i.e. each node in the function is post-dominated by the (virtual) root.
@george.burgess.iv does the above make sense?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78932