[CSSPGO] Warn instead of error out for modules that are not probed.

[hoy]

Summary

Modules that are not compiled with pseudo probe enabled can still be compiled with a sample profile input, such as in LTO postlink where other modules are probed. Since the profile is unrelated to the current modules, we should warn instead of error out the compilation.

[wenlei]

Modules that are not compiled with pseudo probe enabled can reference functions that are probed, cause those functions imported into the non-probed module.

If module is not compiled with probe, we would run into problem even if there's no importing at all, right? The problem seems to be that we're loading csspgo profile (have a valid reader) for that module without probe.

The purpose of the original change it to make sure csspgo is always 100% effective, and if not, we should fix the build. With this change, how do we make sure that is still the case? i.e. we can now pass csspgo profile to a build without adding pseudo-probe flag, and the build will still succeed, right?

[hoy]

In D115642#3189421, @wenlei wrote:

Modules that are not compiled with pseudo probe enabled can reference functions that are probed, cause those functions imported into the non-probed module.

If module is not compiled with probe, we would run into problem even if there's no importing at all, right? The problem seems to be that we're loading csspgo profile (have a valid reader) for that module without probe.

Modules not compiled with probe should not have its functions show up in the profile, thus the profile file will not be loaded and the error should not be triggered. The case I was seeing that triggered the error is due to imported modules and it only failed in thinlto postlink. This can be fixed by always importing probe desc for importing modules. However, we've decided not to do that to favor build time.

The purpose of the original change it to make sure csspgo is always 100% effective, and if not, we should fix the build. With this change, how do we make sure that is still the case? i.e. we can now pass csspgo profile to a build without adding pseudo-probe flag, and the build will still succeed, right?

Yes, in this case the build will still succeed with the warnings. The build may not be easy to fix when the modules not probes are not compiled with clang, say Rust. If we want the build to be blocked, what we could do is to add a fancy check here against non-imported functions for now. What do you think?

[wenlei]

Modules not compiled with probe should not have its functions show up in the profile, thus the profile file will not be loaded and the error should not be triggered.

My point is that modules not compiled with probe should not have fprofile-sample-use at all. If fprofile-sample-use is specified and csspgo profile is used, then we expect probe flag to be on too. Is that assumption not true? That is the underlying problem I'm getting at.

[hoy]

In D115642#3191214, @wenlei wrote:

Modules not compiled with probe should not have its functions show up in the profile, thus the profile file will not be loaded and the error should not be triggered.

My point is that modules not compiled with probe should not have fprofile-sample-use at all. If fprofile-sample-use is specified and csspgo profile is used, then we expect probe flag to be on too. Is that assumption not true? That is the underlying problem I'm getting at.

Right, they currently do not have the profile passed in during clang compilation, ie. LTO prelink, as far as I see. What failed for me was LTO postlink, where the profile was shared by all modules. So in your case, the failure happened in prelink?

[wenlei]

In D115642#3191292, @hoy wrote:

In D115642#3191214, @wenlei wrote:

Modules not compiled with probe should not have its functions show up in the profile, thus the profile file will not be loaded and the error should not be triggered.

My point is that modules not compiled with probe should not have fprofile-sample-use at all. If fprofile-sample-use is specified and csspgo profile is used, then we expect probe flag to be on too. Is that assumption not true? That is the underlying problem I'm getting at.

Right, they currently do not have the profile passed in during clang compilation, ie. LTO prelink, as far as I see. What failed for me was LTO postlink, where the profile was shared by all modules. So in your case, the failure happened in prelink?

Ok, can you update the description then? It doesn't seem related to importing. It's an error with some modules for the same LTO build not having probe flag, but still using the same csspgo profile at post link.

llvm/lib/Transforms/IPO/SampleProfile.cpp

bool SampleProfileLoader::doInitialization(Module &M,
   }
 
   // Load pseudo probe descriptors for probe-based function samples.
   if (Reader->profileIsProbeBased()) {
     ProbeManager = std::make_unique<PseudoProbeManager>(M);
     if (!ProbeManager->moduleIsProbed(M)) {
       const char *Msg =
           "Pseudo-probe-based profile requires SampleProfileProbePass";
-      Ctx.diagnose(DiagnosticInfoSampleProfile(Filename, Msg));
+      Ctx.diagnose(DiagnosticInfoSampleProfile(M.getModuleIdentifier(), Msg,
+                                               DS_Warning));
       return false;
     }
   }
 
   return true;
 }
 
 ModulePass *llvm::createSampleProfileLoaderPass() {