[PATCH] D66477: Simplify main statement flag in ScopBuilder::buildEqivClassBlockStmts

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added a comment.

The change looks good. Thank you for the contribution! Could you change the comments a bit as remarked inline?

Once you updated this diff, I can do the commit with a "Patch by: bin.narwal <bin.narwal at gmail.com>" line if you want.



================
Comment at: lib/Analysis/ScopBuilder.cpp:2144
 
-    // If there is no main instruction, make the first statement the main.
-    bool IsMain;
-    if (MainInst)
-      IsMain = std::find(InstList.begin(), InstList.end(), MainInst) !=
-               InstList.end();
-    else
-      IsMain = (Count == 0);
-    if (IsMain)
-      MainFound = true;
+    // Make the first statement the main if there is no main statement yet.
+    bool IsMain = (MainInst ? MainLeader == Instructions.first : Count == 0);
----------------
"yet" does not fit the context: The decision of which is the main statement has been made before, and will not change during this loop. I suggest to just remove the "yet" (or keep the original comment, I don't see any change in meaning)


================
Comment at: lib/Analysis/ScopBuilder.cpp:2157
   // The epilogue will be removed if no PHIWrite is added to it.
-  std::string EpilogueName = makeStmtName(BB, BBIdx, Count, !MainFound, true);
+  std::string EpilogueName = makeStmtName(BB, BBIdx, Count, Count == 0, true);
   scop->addScopStmt(BB, EpilogueName, L, {});
----------------
Could you add a comment about that `Count == 0` means that there was no other main? Suggestion:
```
// The epilogue becomes the main statement only if there is no other statement that could become main.
```


Repository:
  rPLO Polly

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

https://reviews.llvm.org/D66477