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

Johannes Doerfert via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 14 10:13:26 PDT 2019


jdoerfert added a comment.

Reverted the comments/examples in the comments back to their original form. Added the second more complex one.
The comment states that this might not be what you get (as it isn't now) but they explain the idea rather nicely.

I also added a printer pass and the tests from the example so we can "see" the progress.



================
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.
+///
----------------
hfinkel wrote:
> 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).
That was me changing the comment from the original to the less powerful striped down version even if that was actually not necessary. Will make it consistent again (see below).


================
Comment at: llvm/include/llvm/Analysis/MustExecute.h:250
+/// Start Instruction   | Visit Set
+/// A                   | A, B,
+///    B,               |    B,
----------------
hfinkel wrote:
> Why does this not also include E and F?
In this patch it wouldn't derive this, though, maybe I should make the comment include them again (the original comment, and others I removed did contain E and F.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65186





More information about the llvm-commits mailing list