[PATCH] D33151: ThinLTO: Verify bitcode before lauching the ThinLTOCodeGenerator.

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added inline comments.


================
Comment at: include/llvm/LTO/legacy/ThinLTOCodeGenerator.h:335
+  bool VerifiedInput = false;
+
   /// IR Optimization Level [0-3].
----------------
This assumes that there exists a main module, which is a foreign concept to ThinLTOCodegenerator till now AFAIK. I'm not sure what you're trying to express here?
Especially, this seems used only in the `codegen`, which is only use in a very specific mode (i.e. split model where you store optimized bitcode and reprocess to perform only the codegen). In such case either you want to verify all the modules that you're about to codegen or none. I have the impression that you'll check only the first one. Not sure if I missed something.


================
Comment at: lib/LTO/ThinLTOCodeGenerator.cpp:190
   }
+  verifyLoadedModule(*ModuleOrErr.get());
   return std::move(ModuleOrErr.get());
----------------
I'm still not sure how this plays with importing: what happens here with a lazy-loaded module?

I currently think we need to verify here only non-lazy-loaded modules (i.e. main modules). And implement another verification after materialization, but before importing in FunctionImporter.cpp


================
Comment at: lib/LTO/ThinLTOCodeGenerator.cpp:214
+  // Verify the module again to catch any cross-imported illegal metadata.
+  verifyLoadedModule(TheModule);
 }
----------------
That shouldn't be necessary if we checked every module at load time.


https://reviews.llvm.org/D33151