[PATCH] D44524: Add an analysis printer for must execute reasoning

[Philip Reames via Phabricator via llvm-commits]

reames added inline comments.


================
Comment at: lib/Analysis/MustExecute.cpp:87
+    OS << "\t(mustexec in ";
+    bool first = true;
+    for (const Loop *L : MustExec.lookup(V)) {
----------------
anna wrote:
> reames wrote:
> > anna wrote:
> > > Perhaps rename this to exactlyOneLoop and  define that as  `MustExec[V]->size()  == 1`  
> > Er, what?  I don't understand your comment.  The "first" variable is simply being used to join a set of strings with a comma in between.  
> What I meant is we can do something like this instead of using the first variable:
> ```
> const bool exactlyOneLoop = MustExec[V]->size()  == 1;
> for (const Loop *L : MustExec.lookup(V)) {
>       OS << L->getHeader()->getName();
>       if (!ExactlyOneLoop)
>           OS << ", ";
> }
> ```
> 
Ah, I see where you're going.  And actually you're pointing out a bug in the code.  What I'd actually wanted was: "name, name, name".  What I'd wrote produced "name name, name, "  Your suggestion is a cleanup of the code as written, but doesn't work once I fix the formatting bug. 


Repository:
  rL LLVM

https://reviews.llvm.org/D44524