[PATCH] D45076: Prevent data races in concurrent ThinLTO processes

[Steven Wu via Phabricator via llvm-commits]

steven_wu accepted this revision.
steven_wu added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks!



================
Comment at: lib/LTO/ThinLTOCodeGenerator.cpp:1035
             auto ReloadedBufferOrErr = CacheEntry.tryLoadingBuffer();
-            if (auto EC = ReloadedBufferOrErr.getError()) {
-              // On error, keeping the preexisting buffer and printing a
-              // diagnostic is more friendly than just crashing.
-              errs() << "error: can't reload cached file '" << CacheEntryPath
-                     << "': " << EC.message() << "\n";
-            } else {
+            if (!ReloadedBufferOrErr.getError()) {
               OutputBuffer = std::move(*ReloadedBufferOrErr);
----------------
kromanova wrote:
> steven_wu wrote:
> > You also don't need {} here
> > 
> > Is there any specific reason why the error message is removed here? We should not hit that if we fixed all the races, but it can still happen from user errors. 
> I restored the code the way it was before my change, except I modified the comment slightly. I don't understand what crashing the comment is referring to, so I removed it.
> This error message is just a diagnostic printed on the stderr, it won't stop the compilation, right? We want the compilation to continue even if the error occurred.
Looks correct. This is simply printing to stderr. 


Repository:
  rL LLVM

https://reviews.llvm.org/D45076