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

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:1802
+        else
+          for (BasicBlock *R : PDT.getRoots()) {
+            if (!DT.isReachableFromEntry(R))
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78932