[PATCH] D65186: [MustExec] Add a generic "must-be-executed-context" explorer

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added a comment.

I think that this looks fine, but it should have some direct tests. Either make an analysis that prints out things and then some lit tests, or, some unit tests.



================
Comment at: llvm/include/llvm/Analysis/MustExecute.h:232
+/// outcomes. If we start the iterator at A, B, or E, we will visit only A, B,
+/// (and E). If we start at C or D, we will visit all instructions A-E.
+///
----------------
This comment, that all instructions A-E will be visited, seems contradicted by the table below (where starting at A visits {A, B} and starting at B visits {B} and so on).


================
Comment at: llvm/include/llvm/Analysis/MustExecute.h:250
+/// Start Instruction   | Visit Set
+/// A                   | A, B,
+///    B,               |    B,
----------------
Why does this not also include E and F?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65186