[PATCH] D24020: [LTO] Added flag to generate assembly file with after LTO passes

[Teresa Johnson via llvm-commits]

tejohnson added inline comments.

================
Comment at: include/llvm/LTO/Config.h:150
@@ -145,1 +149,3 @@
 
+  /// This module hook is called before code generation. It generates
+  /// assembly file by running all the codegen passes on module
----------------
mehdi_amini wrote:
> Comment says "before", name is "after".
> 
> Also you shouldn't describe what a hook is actually doing, this is up to the client.
Part of the issue is that there is already a PreCodeGenModuleHook. That can't be used here though because of the need for passing the TM. So either name it something specific to asm generation like PreCodeGenAsmHook, or specific to taking the TM like PreCodeGenTMHook. Mehdi - WDYT?

================
Comment at: include/llvm/LTO/Config.h:248
@@ -247,3 +256,2 @@
 };
-
 }
----------------
Remove the whitespace change.

================
Comment at: lib/LTO/LTOBackend.cpp:91
@@ +90,3 @@
+      std::string PathPrefix;
+      // If this is the combined module (not a ThinLTO backend compile) or the
+      // user hasn't requested using the input module's path, emit to a file
----------------
The block below here that computes the name and opens the file can be refactored out into a named lambda or helper function.

================
Comment at: lib/LTO/LTOBackend.cpp:123
@@ -79,1 +122,3 @@
+  };
+
   setHook("0.preopt", PreOptModuleHook);
----------------
mehdi_amini wrote:
> This will be "costly", we don't want this by default with "addSaveTemps".
> And since it hasn't been needed till now, it is likely a "rare" need. Most of the time (i.e. always till now), using `clang -O2 -disable-llvm-optzns  output.lto.bc -S` is enough to reproduce the codegen and produce an assembly file out of the save-temps bitcode if needed.
Right, I had suggested keeping this optional in my original suggestion about moving this to a hook. I think it should go back under the option from the original patch.

================
Comment at: lib/LTO/LTOBackend.cpp:230
@@ -229,3 +280,2 @@
 }
-
 }
----------------
Remove whitespace change


https://reviews.llvm.org/D24020