Pass -fprofile-sample-use to lto backends.

[danielcdh]

Summary

LTO backend will not invoke SampleProfileLoader pass even if -fprofile-sample-use is specified. This patch passes the flag down so that pass manager can add the SampleProfileLoader pass correctly.

[tejohnson]

LGTM

[danielcdh]

The breaks some buildbots thus I reverted the patch:

http://lab.llvm.org:8011/builders/clang-x86-windows-msvc2015/builds/1889
http://lab.llvm.org:8080/green/job/clang-stage1-cmake-RA-incremental_check/32242/

Unfortunately I could not reproduce the error locally.

Any quick insights why this test change would break?

Thanks,
Dehao

[tejohnson]

In D28588#644467, @danielcdh wrote:

The breaks some buildbots thus I reverted the patch:

http://lab.llvm.org:8011/builders/clang-x86-windows-msvc2015/builds/1889
http://lab.llvm.org:8080/green/job/clang-stage1-cmake-RA-incremental_check/32242/

Unfortunately I could not reproduce the error locally.

Any quick insights why this test change would break?

Looks like there was no output (at least on the buildbot that I looked at). Most likely because this is using debug output and perhaps those are build NDEBUG? I wonder if there is a better way to test this. Otherwise I think "; REQUIRES: asserts" might do the trick?

Thanks,
Dehao

[tejohnson]

In D28588#644472, @tejohnson wrote:

Otherwise I think "; REQUIRES: asserts" might do the trick?

Looks like it - I looked at another test that used -debug output and it requires asserts.

[danielcdh]

Thanks for the prompt response.

But looks like several other tests also has "-mllvm -debug-pass=Structure" in their tests:

tools/clang/test/CodeGen/pgo-instrumentation.c
test/CodeGen/Generic/llc-start-stop.ll

I just verified that if I cmake with -DCMAKE_BUILD_TYPE=Release, it still passes locally.

Thanks,
Dehao

[tejohnson]

In D28588#644489, @danielcdh wrote:

Thanks for the prompt response.

But looks like several other tests also has "-mllvm -debug-pass=Structure" in their tests:

tools/clang/test/CodeGen/pgo-instrumentation.c
test/CodeGen/Generic/llc-start-stop.ll

I just verified that if I cmake with -DCMAKE_BUILD_TYPE=Release, it still passes locally.

Hmm, maybe that flavor of -debug output is enabled with NDEBUG then. I'm not really sure unfortunately.

I don't even see an error message for the second one, do you? Just an error code.

Thanks,
Dehao

[danielcdh]

Updates the unittests to clang_cc1 to see if it fixes the buildbot failure.

[danielcdh]

Looks like this is still breaking these buildbots:

http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/3216/steps/ninja%20check%201/logs/FAIL%3A%20Clang%3A%3Athinlto_backend.ll

I reverted the test change for now, and am thinking of how to reproduce/fix the problem...

[tejohnson]

In D28588#644637, @danielcdh wrote:

Looks like this is still breaking these buildbots:

http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/3216/steps/ninja%20check%201/logs/FAIL%3A%20Clang%3A%3Athinlto_backend.ll

I reverted the test change for now, and am thinking of how to reproduce/fix the problem...

Aha - the test is missing "-target x86_64-unknown-linux-gnu". Probably because it was cloned from the one just above or below it, which are testing for other failures hit early on and didn't need it (they should probably have it as well to avoid confusion like this in the future though).

lib/CodeGen/BackendUtil.cpp

void EmitAssemblyHelper::EmitAssemblyWithNewPassManager(
   // Now if needed, run the legacy PM for codegen.
   if (NeedCodeGen) {
     PrettyStackTraceString CrashInfo("Code generation");
     CodeGenPasses.run(*TheModule);
   }
 }
 
 static void runThinLTOBackend(ModuleSummaryIndex *CombinedIndex, Module *M,
-                              std::unique_ptr<raw_pwrite_stream> OS) {
+                              std::unique_ptr<raw_pwrite_stream> OS,
+                              std::string SampleProfile) {
   StringMap<std::map<GlobalValue::GUID, GlobalValueSummary *>>
       ModuleToDefinedGVSummaries;
   CombinedIndex->collectDefinedGVSummariesPerModule(ModuleToDefinedGVSummaries);
 
   // We can simply import the values mentioned in the combined index, since
   // we should only invoke this using the individual indexes written out
   // via a WriteIndexesThinBackend.
   FunctionImporter::ImportMapTy ImportList;
for (auto &I : ImportList) {
     }
 
     OwnedImports.push_back(std::move(*MBOrErr));
   }
   auto AddStream = [&](size_t Task) {
     return llvm::make_unique<lto::NativeObjectStream>(std::move(OS));
   };
   lto::Config Conf;
+  Conf.SampleProfile = SampleProfile;
   if (Error E = thinBackend(
           Conf, 0, AddStream, *M, *CombinedIndex, ImportList,
           ModuleToDefinedGVSummaries[M->getModuleIdentifier()], ModuleMap)) {
     handleAllErrors(std::move(E), [&](ErrorInfoBase &EIB) {
       errs() << "Error running ThinLTO backend: " << EIB.message() << '\n';
     });
   }
 }
if (!IndexOrErr) {
       return;
     }
     std::unique_ptr<ModuleSummaryIndex> CombinedIndex = std::move(*IndexOrErr);
     // A null CombinedIndex means we should skip ThinLTO compilation
     // (LLVM will optionally ignore empty index files, returning null instead
     // of an error).
     bool DoThinLTOBackend = CombinedIndex != nullptr;
     if (DoThinLTOBackend) {
-      runThinLTOBackend(CombinedIndex.get(), M, std::move(OS));
+      runThinLTOBackend(CombinedIndex.get(), M, std::move(OS),
+                        CGOpts.SampleProfileFile);
       return;
     }
   }
 
   EmitAssemblyHelper AsmHelper(Diags, HeaderOpts, CGOpts, TOpts, LOpts, M);
 
   if (CGOpts.ExperimentalNewPassManager)
     AsmHelper.EmitAssemblyWithNewPassManager(Action, std::move(OS));

test/CodeGen/thinlto_backend.ll

 ; REQUIRES: x86-registered-target
 
 ; RUN: opt -module-summary -o %t1.o %s
 ; RUN: opt -module-summary -o %t2.o %S/Inputs/thinlto_backend.ll
 ; RUN: llvm-lto -thinlto -o %t %t1.o %t2.o
 
 ; Ensure clang -cc1 give expected error for incorrect input type
 ; RUN: not %clang_cc1 -O2 -o %t1.o -x c %s -c -fthinlto-index=%t.thinlto.bc 2>&1 | FileCheck %s -check-prefix=CHECK-WARNING
 ; CHECK-WARNING: error: invalid argument '-fthinlto-index={{.*}}' only allowed with '-x ir'
 
+; Ensure sample profile pass are passed to backend
+; RUN: %clang -O2 -o %t4.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc -fprofile-sample-use=%S/Inputs/pgo-sample.prof -mllvm -debug-pass=Structure 2>&1 | FileCheck %s -check-prefix=CHECK-SAMPLEPGO
+; CHECK-SAMPLEPGO: Sample profile pass
+
 ; Ensure we get expected error for missing index file
 ; RUN: %clang -O2 -o %t4.o -x ir %t1.o -c -fthinlto-index=bad.thinlto.bc 2>&1 | FileCheck %s -check-prefix=CHECK-ERROR1
 ; CHECK-ERROR1: Error loading index file 'bad.thinlto.bc'
 
 ; Ensure we ignore empty index file under -ignore-empty-index-file, and run
 ; non-ThinLTO compilation which would not import f2
 ; RUN: touch %t4.thinlto.bc
 ; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t4.o -x ir %t1.o -c -fthinlto-index=%t4.thinlto.bc -mllvm -ignore-empty-index-file