[PATCH] [opt] Replace the recursive walk for GC with a worklist algorithm.

[Chandler Carruth]

I'll confirm with you that the worklist logic below is OK, and then land. Thanks!


================
Comment at: COFF/Symbols.h:130
@@ +129,3 @@
+// Symbols defined via a COFF object file.
+class ObjectFileDefined : public Defined {
+  friend SymbolBody;
----------------
ruiu wrote:
> I still want you to rename this class for consistency.
Sorry, this should not have been part of this patch. It'll be in D10792, and I'll update that review thread shortly.

================
Comment at: COFF/Writer.cpp:135
@@ +134,3 @@
+    SectionChunk *SC = Worklist.pop_back_val();
+    assert(SC->isLive() && "We mark as live when pushing onto the worklist!");
+
----------------
ruiu wrote:
> So you can write SC->markLive() here only once, no? Then you can remove markLive() from all the other places in this function.
No, that is a different algorithm.

While we look for the symbols referenced by this section, we may see a symbol more than once. In that case, before we put it into the worklist, we first test that it is not-yet-live, mark it live, and then add it to the worklist. If I make the change you are suggesting, then we would potentially add the same not-yet-live symbol to the worklist many times.

This would either require adding a set to the worklist, or make the worklist grow much larger and require an early exit from the processing loop. Neither seem good. Given that we have a bit to track the liveness embedded in the chunk, it seems much better to use that to add them to the worklist exactly once.

http://reviews.llvm.org/D10790

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/