[PATCH] D74817: [MustExecute] Add backward exploration for must-be-executed-context

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

Thank you for picking this up!

I inlined some minor comments but you can address them before commiting this.

LGTM.



================
Comment at: llvm/include/llvm/Analysis/MustExecute.h:410
+      bool ExploreInterBlock, bool ExploreCFGForward = true,
+      bool ExploreCFGBackward = true,
       GetterTy<const LoopInfo> LIGetter =
----------------
I think we should not set default values but force the user to decide.


================
Comment at: llvm/lib/Analysis/MustExecute.cpp:362
   SmallVector<DominatorTree *, 8> DTs;
   SmallVector<LoopInfo *, 8> LIs;
 
----------------
"Unrelated": We have to make these unique_ptr so we can remove the delete at the end.


================
Comment at: llvm/test/Analysis/MustExecute/must_be_executed_context.ll:27
 ;
 ; FIXME: We miss the B -> E and backward exploration.
 ;
----------------
We have backward exploration now.


================
Comment at: llvm/test/Analysis/MustExecute/must_be_executed_context.ll:232
 ; ME-NEXT: br
-; FIXME: Missing A, B, and D (backward), as well as F
 ; MBEC:      -- Explore context of:   call void @E()
----------------
Still missing F


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

https://reviews.llvm.org/D74817