Skip to content

Commit 7486f9d

Browse files
handle indirect callee and indirect ref vtable consistently
1 parent eff8695 commit 7486f9d

File tree

1 file changed

+26
-25
lines changed

1 file changed

+26
-25
lines changed

llvm/lib/Transforms/IPO/FunctionImport.cpp

Lines changed: 26 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -178,12 +178,11 @@ static cl::opt<bool> ImportAssumeUniqueLocal(
178178
"import-assume-unique-local", cl::init(false),
179179
cl::desc(
180180
"By default, a local-linkage global variable won't be imported in the "
181-
"edge mod1:func -> mod2:func_bar -> mod2:local-var since compiler "
182-
"cannot assume mod2 is compiled with full path or local-var has a "
183-
"unique GUID. "
184-
"Set this option to true will help cross-module import of such "
185-
"variables. But it is only safe if the compiler user specify the full "
186-
"module path."),
181+
"edge mod1:func -> mod2:local-var (from value profiles) since compiler "
182+
"cannot assume mod2 is compiled with full path which gives local-var a "
183+
"program-wide unique GUID. Set this option to true will help cross "
184+
"module import of such variables. This is only safe if the compiler "
185+
"user specify the full module path."),
187186
cl::Hidden);
188187

189188
namespace llvm {
@@ -208,6 +207,23 @@ static std::unique_ptr<Module> loadFile(const std::string &FileName,
208207
return Result;
209208
}
210209

210+
static shouldSkipLocalInAnotherModule(const GlobalVarSummary *RefSummary,
211+
size_t NumRefs,
212+
StringRef ImporterModule) {
213+
// We can import a local from another module if all inputs are compiled
214+
// with full paths or when there is one entry in the list.
215+
if (ImportAssumeUniqueLocal || NumRefs == 1)
216+
return false;
217+
// If this is a local variable, make sure we import the copy
218+
// in the caller's module. The only time a local variable can
219+
// share an entry in the index is if there is a local with the same name
220+
// in another module that had the same source file name (in a different
221+
// directory), where each was compiled in their own directory so there
222+
// was not distinguishing path.
223+
return GlobalValue::isLocalLinkage(RefSummary->linkage()) &&
224+
RefSummary->modulePath() != ImporterModule;
225+
}
226+
211227
/// Given a list of possible callee implementation for a call site, qualify the
212228
/// legality of importing each. The return is a range of pairs. Each pair
213229
/// corresponds to a candidate. The first value is the ImportFailureReason for
@@ -250,9 +266,8 @@ static auto qualifyCalleeCandidates(
250266
// entry in the list - in that case this must be a reference due
251267
// to indirect call profile data, since a function pointer can point to
252268
// a local in another module.
253-
if (GlobalValue::isLocalLinkage(Summary->linkage()) &&
254-
CalleeSummaryList.size() > 1 &&
255-
Summary->modulePath() != CallerModulePath)
269+
if (shouldSkipLocalInAnotherModule(Summary, CalleeSummaryList.size(),
270+
CallerModulePath))
256271
return {
257272
FunctionImporter::ImportFailureReason::LocalLinkageNotInModule,
258273
GVSummary};
@@ -371,21 +386,6 @@ class GlobalsImporter final {
371386

372387
LLVM_DEBUG(dbgs() << " ref -> " << VI << "\n");
373388

374-
// If this is a local variable, make sure we import the copy
375-
// in the caller's module. The only time a local variable can
376-
// share an entry in the index is if there is a local with the same name
377-
// in another module that had the same source file name (in a different
378-
// directory), where each was compiled in their own directory so there
379-
// was not distinguishing path.
380-
auto LocalNotInModule =
381-
[&](const GlobalValueSummary *RefSummary) -> bool {
382-
if (ImportAssumeUniqueLocal)
383-
return false;
384-
385-
return GlobalValue::isLocalLinkage(RefSummary->linkage()) &&
386-
RefSummary->modulePath() != Summary.modulePath();
387-
};
388-
389389
for (const auto &RefSummary : VI.getSummaryList()) {
390390
const auto *GVS = dyn_cast<GlobalVarSummary>(RefSummary.get());
391391
// Functions could be referenced by global vars - e.g. a vtable; but we
@@ -394,7 +394,8 @@ class GlobalsImporter final {
394394
// based on profile information). Should we decide to handle them here,
395395
// we can refactor accordingly at that time.
396396
if (!GVS || !Index.canImportGlobalVar(GVS, /* AnalyzeRefs */ true) ||
397-
LocalNotInModule(GVS))
397+
shouldSkipLocalInAnotherModule(GVS, VI.getSummaryList().size(),
398+
Summary.modulePath()))
398399
continue;
399400

400401
// If there isn't an entry for GUID, insert <GUID, Definition> pair.

0 commit comments

Comments
 (0)