Skip to content

Commit

Permalink
[LTO] Ignore unreachable virtual functions in WPD in hybrid LTO.
Browse files Browse the repository at this point in the history
Differential Revision: https://reviews.llvm.org/D115492
  • Loading branch information
minglotus-6 committed Dec 14, 2021
1 parent 1695dd1 commit 09a704c
Show file tree
Hide file tree
Showing 21 changed files with 452 additions and 40 deletions.
2 changes: 1 addition & 1 deletion clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
Expand Up @@ -36,7 +36,7 @@
; Round trip it through llvm-as
; RUN: llvm-dis %t.o.thinlto.bc -o - | llvm-as -o - | llvm-dis -o - | FileCheck %s --check-prefix=CHECK-DIS
; CHECK-DIS: ^0 = module: (path: "{{.*}}thinlto-distributed-cfi-devirt.ll.tmp.o", hash: ({{.*}}, {{.*}}, {{.*}}, {{.*}}, {{.*}}))
; CHECK-DIS: ^1 = gv: (guid: 8346051122425466633, summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 1, dsoLocal: 0, canAutoHide: 0), insts: 18, funcFlags: (readNone: 0, readOnly: 0, noRecurse: 0, returnDoesNotAlias: 0, noInline: 0, alwaysInline: 0, noUnwind: 0, mayThrow: 0, hasUnknownCall: 1), typeIdInfo: (typeTests: (^2), typeCheckedLoadVCalls: (vFuncId: (^2, offset: 8), vFuncId: (^2, offset: 0))))))
; CHECK-DIS: ^1 = gv: (guid: 8346051122425466633, summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 1, dsoLocal: 0, canAutoHide: 0), insts: 18, funcFlags: (readNone: 0, readOnly: 0, noRecurse: 0, returnDoesNotAlias: 0, noInline: 0, alwaysInline: 0, noUnwind: 0, mayThrow: 0, hasUnknownCall: 1, mustBeUnreachable: 0), typeIdInfo: (typeTests: (^2), typeCheckedLoadVCalls: (vFuncId: (^2, offset: 8), vFuncId: (^2, offset: 0))))))
; CHECK-DIS: ^2 = typeid: (name: "_ZTS1A", summary: (typeTestRes: (kind: allOnes, sizeM1BitWidth: 7), wpdResolutions: ((offset: 0, wpdRes: (kind: branchFunnel)), (offset: 8, wpdRes: (kind: singleImpl, singleImplName: "_ZN1A1nEi"))))) ; guid = 7004155349499253778

; RUN: %clang_cc1 -triple x86_64-grtev4-linux-gnu \
Expand Down
2 changes: 1 addition & 1 deletion clang/test/CodeGen/thinlto-distributed-cfi.ll
Expand Up @@ -24,7 +24,7 @@
; Round trip it through llvm-as
; RUN: llvm-dis %t.o.thinlto.bc -o - | llvm-as -o - | llvm-dis -o - | FileCheck %s --check-prefix=CHECK-DIS
; CHECK-DIS: ^0 = module: (path: "{{.*}}thinlto-distributed-cfi.ll.tmp.o", hash: ({{.*}}, {{.*}}, {{.*}}, {{.*}}, {{.*}}))
; CHECK-DIS: ^1 = gv: (guid: 8346051122425466633, summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 1, dsoLocal: 0, canAutoHide: 0), insts: 7, funcFlags: (readNone: 0, readOnly: 0, noRecurse: 1, returnDoesNotAlias: 0, noInline: 0, alwaysInline: 0, noUnwind: 1, mayThrow: 0, hasUnknownCall: 0), typeIdInfo: (typeTests: (^2)))))
; CHECK-DIS: ^1 = gv: (guid: 8346051122425466633, summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 1, dsoLocal: 0, canAutoHide: 0), insts: 7, funcFlags: (readNone: 0, readOnly: 0, noRecurse: 1, returnDoesNotAlias: 0, noInline: 0, alwaysInline: 0, noUnwind: 1, mayThrow: 0, hasUnknownCall: 0, mustBeUnreachable: 0), typeIdInfo: (typeTests: (^2)))))
; CHECK-DIS: ^2 = typeid: (name: "_ZTS1A", summary: (typeTestRes: (kind: single, sizeM1BitWidth: 0))) ; guid = 7004155349499253778

; RUN: %clang_cc1 -triple x86_64-grtev4-linux-gnu \
Expand Down
2 changes: 1 addition & 1 deletion clang/test/CodeGen/thinlto-funcattr-prop.ll
Expand Up @@ -17,7 +17,7 @@
;; Summary for call_extern. Note that llvm-lto2 writes out the index before propagation occurs so call_extern doesn't have its flags updated.
; CHECK-INDEX: ^2 = gv: (guid: 13959900437860518209, summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 1, dsoLocal: 1, canAutoHide: 0), insts: 2, calls: ((callee: ^3)))))
;; Summary for extern
; CHECK-INDEX: ^3 = gv: (guid: 14959766916849974397, summaries: (function: (module: ^1, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 1, dsoLocal: 0, canAutoHide: 0), insts: 1, funcFlags: (readNone: 0, readOnly: 0, noRecurse: 1, returnDoesNotAlias: 0, noInline: 0, alwaysInline: 0, noUnwind: 1, mayThrow: 0, hasUnknownCall: 0))))
; CHECK-INDEX: ^3 = gv: (guid: 14959766916849974397, summaries: (function: (module: ^1, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 1, dsoLocal: 0, canAutoHide: 0), insts: 1, funcFlags: (readNone: 0, readOnly: 0, noRecurse: 1, returnDoesNotAlias: 0, noInline: 0, alwaysInline: 0, noUnwind: 1, mayThrow: 0, hasUnknownCall: 0, mustBeUnreachable: 0))))

;--- a.ll
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
Expand Down
1 change: 1 addition & 0 deletions llvm/include/llvm/AsmParser/LLToken.h
Expand Up @@ -407,6 +407,7 @@ enum Kind {
kw_noUnwind,
kw_mayThrow,
kw_hasUnknownCall,
kw_mustBeUnreachable,
kw_calls,
kw_callee,
kw_params,
Expand Down
12 changes: 11 additions & 1 deletion llvm/include/llvm/IR/ModuleSummaryIndex.h
Expand Up @@ -581,6 +581,13 @@ class FunctionSummary : public GlobalValueSummary {
// If there are calls to unknown targets (e.g. indirect)
unsigned HasUnknownCall : 1;

// Indicate if a function must be an unreachable function.
//
// This bit is sufficient but not necessary;
// if this bit is on, the function must be regarded as unreachable;
// if this bit is off, the function might be reachable or unreachable.
unsigned MustBeUnreachable : 1;

FFlags &operator&=(const FFlags &RHS) {
this->ReadNone &= RHS.ReadNone;
this->ReadOnly &= RHS.ReadOnly;
Expand All @@ -591,13 +598,15 @@ class FunctionSummary : public GlobalValueSummary {
this->NoUnwind &= RHS.NoUnwind;
this->MayThrow &= RHS.MayThrow;
this->HasUnknownCall &= RHS.HasUnknownCall;
this->MustBeUnreachable &= RHS.MustBeUnreachable;
return *this;
}

bool anyFlagSet() {
return this->ReadNone | this->ReadOnly | this->NoRecurse |
this->ReturnDoesNotAlias | this->NoInline | this->AlwaysInline |
this->NoUnwind | this->MayThrow | this->HasUnknownCall;
this->NoUnwind | this->MayThrow | this->HasUnknownCall |
this->MustBeUnreachable;
}

operator std::string() {
Expand All @@ -613,6 +622,7 @@ class FunctionSummary : public GlobalValueSummary {
OS << ", noUnwind: " << this->NoUnwind;
OS << ", mayThrow: " << this->MayThrow;
OS << ", hasUnknownCall: " << this->HasUnknownCall;
OS << ", mustBeUnreachable: " << this->MustBeUnreachable;
OS << ")";
return OS.str();
}
Expand Down
26 changes: 24 additions & 2 deletions llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
Expand Up @@ -234,6 +234,26 @@ static bool isNonVolatileStore(const Instruction *I) {
return false;
}

// Returns true if `F` must be an unreachable function.
//
// Note if this helper function returns true, `F` is guaranteed
// to be unreachable; if it returns false, `F` might still
// be unreachable but not covered by this helper function.
static bool mustBeUnreachableFunction(const Function &F) {
if (!F.empty()) {
const BasicBlock &entryBlock = F.getEntryBlock();
// A function must be unreachable if its entry block
// ends with an 'unreachable'.
if (!entryBlock.empty()) {
const Instruction *inst = &(*entryBlock.rbegin());
if (inst->getOpcode() == Instruction::Unreachable) {
return true;
}
}
}
return false;
}

static void computeFunctionSummary(
ModuleSummaryIndex &Index, const Module &M, const Function &F,
BlockFrequencyInfo *BFI, ProfileSummaryInfo *PSI, DominatorTree &DT,
Expand Down Expand Up @@ -488,7 +508,8 @@ static void computeFunctionSummary(
// Don't try to import functions with noinline attribute.
F.getAttributes().hasFnAttr(Attribute::NoInline),
F.hasFnAttribute(Attribute::AlwaysInline),
F.hasFnAttribute(Attribute::NoUnwind), MayThrow, HasUnknownCall};
F.hasFnAttribute(Attribute::NoUnwind), MayThrow, HasUnknownCall,
mustBeUnreachableFunction(F)};
std::vector<FunctionSummary::ParamAccess> ParamAccesses;
if (auto *SSI = GetSSICallback(F))
ParamAccesses = SSI->getParamAccesses(Index);
Expand Down Expand Up @@ -737,7 +758,8 @@ ModuleSummaryIndex llvm::buildModuleSummaryIndex(
F->hasFnAttribute(Attribute::AlwaysInline),
F->hasFnAttribute(Attribute::NoUnwind),
/* MayThrow */ true,
/* HasUnknownCall */ true},
/* HasUnknownCall */ true,
/* MustBeUnreachable */ false},
/*EntryCount=*/0, ArrayRef<ValueInfo>{},
ArrayRef<FunctionSummary::EdgeTy>{},
ArrayRef<GlobalValue::GUID>{},
Expand Down
1 change: 1 addition & 0 deletions llvm/lib/AsmParser/LLLexer.cpp
Expand Up @@ -773,6 +773,7 @@ lltok::Kind LLLexer::LexIdentifier() {
KEYWORD(noUnwind);
KEYWORD(mayThrow);
KEYWORD(hasUnknownCall);
KEYWORD(mustBeUnreachable);
KEYWORD(calls);
KEYWORD(callee);
KEYWORD(params);
Expand Down
7 changes: 7 additions & 0 deletions llvm/lib/AsmParser/LLParser.cpp
Expand Up @@ -8534,6 +8534,7 @@ bool LLParser::parseFlag(unsigned &Val) {
/// [',' 'noUnwind' ':' Flag]? ')'
/// [',' 'mayThrow' ':' Flag]? ')'
/// [',' 'hasUnknownCall' ':' Flag]? ')'
/// [',' 'mustBeUnreachable' ':' Flag]? ')'

bool LLParser::parseOptionalFFlags(FunctionSummary::FFlags &FFlags) {
assert(Lex.getKind() == lltok::kw_funcFlags);
Expand Down Expand Up @@ -8600,6 +8601,12 @@ bool LLParser::parseOptionalFFlags(FunctionSummary::FFlags &FFlags) {
return true;
FFlags.HasUnknownCall = Val;
break;
case lltok::kw_mustBeUnreachable:
Lex.Lex();
if (parseToken(lltok::colon, "expected ':'") || parseFlag(Val))
return true;
FFlags.MustBeUnreachable = Val;
break;
default:
return error(Lex.getLoc(), "expected function flag type");
}
Expand Down
1 change: 1 addition & 0 deletions llvm/lib/Bitcode/Reader/BitcodeReader.cpp
Expand Up @@ -932,6 +932,7 @@ static FunctionSummary::FFlags getDecodedFFlags(uint64_t RawFlags) {
Flags.NoUnwind = (RawFlags >> 6) & 0x1;
Flags.MayThrow = (RawFlags >> 7) & 0x1;
Flags.HasUnknownCall = (RawFlags >> 8) & 0x1;
Flags.MustBeUnreachable = (RawFlags >> 9) & 0x1;
return Flags;
}

Expand Down
1 change: 1 addition & 0 deletions llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
Expand Up @@ -1066,6 +1066,7 @@ static uint64_t getEncodedFFlags(FunctionSummary::FFlags Flags) {
RawFlags |= (Flags.NoUnwind << 6);
RawFlags |= (Flags.MayThrow << 7);
RawFlags |= (Flags.HasUnknownCall << 8);
RawFlags |= (Flags.MustBeUnreachable << 9);
return RawFlags;
}

Expand Down
16 changes: 11 additions & 5 deletions llvm/lib/IR/ModuleSummaryIndex.cpp
Expand Up @@ -447,11 +447,17 @@ static std::string linkageToString(GlobalValue::LinkageTypes LT) {

static std::string fflagsToString(FunctionSummary::FFlags F) {
auto FlagValue = [](unsigned V) { return V ? '1' : '0'; };
char FlagRep[] = {FlagValue(F.ReadNone), FlagValue(F.ReadOnly),
FlagValue(F.NoRecurse), FlagValue(F.ReturnDoesNotAlias),
FlagValue(F.NoInline), FlagValue(F.AlwaysInline),
FlagValue(F.NoUnwind), FlagValue(F.MayThrow),
FlagValue(F.HasUnknownCall), 0};
char FlagRep[] = {FlagValue(F.ReadNone),
FlagValue(F.ReadOnly),
FlagValue(F.NoRecurse),
FlagValue(F.ReturnDoesNotAlias),
FlagValue(F.NoInline),
FlagValue(F.AlwaysInline),
FlagValue(F.NoUnwind),
FlagValue(F.MayThrow),
FlagValue(F.HasUnknownCall),
FlagValue(F.MustBeUnreachable),
0};

return FlagRep;
}
Expand Down
76 changes: 73 additions & 3 deletions llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
Expand Up @@ -359,6 +359,36 @@ template <> struct DenseMapInfo<VTableSlotSummary> {

namespace {

// Returns true if the function must be unreachable based on ValueInfo.
//
// In particular, identifies a function as unreachable in the following
// conditions
// 1) All summaries are live.
// 2) All function summaries indicate it's unreachable
bool mustBeUnreachableFunction(ValueInfo TheFnVI) {
if ((!TheFnVI) || TheFnVI.getSummaryList().empty()) {
// Returns false if ValueInfo is absent, or the summary list is empty
// (e.g., function declarations).
return false;
}

for (auto &Summary : TheFnVI.getSummaryList()) {
// Conservatively returns false if any non-live functions are seen.
// In general either all summaries should be live or all should be dead.
if (!Summary->isLive())
return false;
if (auto *FS = dyn_cast<FunctionSummary>(Summary.get())) {
if (!FS->fflags().MustBeUnreachable)
return false;
}
// Do nothing if a non-function has the same GUID (which is rare).
// This is correct since non-function summaries are not relevant.
}
// All function summaries are live and all of them agree that the function is
// unreachble.
return true;
}

// A virtual call site. VTable is the loaded virtual table pointer, and CS is
// the indirect virtual call.
struct VirtualCallSite {
Expand Down Expand Up @@ -562,10 +592,12 @@ struct DevirtModule {
void buildTypeIdentifierMap(
std::vector<VTableBits> &Bits,
DenseMap<Metadata *, std::set<TypeMemberInfo>> &TypeIdMap);

bool
tryFindVirtualCallTargets(std::vector<VirtualCallTarget> &TargetsForSlot,
const std::set<TypeMemberInfo> &TypeMemberInfos,
uint64_t ByteOffset);
uint64_t ByteOffset,
ModuleSummaryIndex *ExportSummary);

void applySingleImplDevirt(VTableSlotInfo &SlotInfo, Constant *TheFn,
bool &IsExported);
Expand Down Expand Up @@ -640,6 +672,12 @@ struct DevirtModule {

bool run();

// Look up the corresponding ValueInfo entry of `TheFn` in `ExportSummary`.
//
// Caller guarantees that `ExportSummary` is not nullptr.
static ValueInfo lookUpFunctionValueInfo(Function *TheFn,
ModuleSummaryIndex *ExportSummary);

// Lower the module using the action and summary passed as command line
// arguments. For testing purposes only.
static bool
Expand Down Expand Up @@ -969,7 +1007,8 @@ void DevirtModule::buildTypeIdentifierMap(

bool DevirtModule::tryFindVirtualCallTargets(
std::vector<VirtualCallTarget> &TargetsForSlot,
const std::set<TypeMemberInfo> &TypeMemberInfos, uint64_t ByteOffset) {
const std::set<TypeMemberInfo> &TypeMemberInfos, uint64_t ByteOffset,
ModuleSummaryIndex *ExportSummary) {
for (const TypeMemberInfo &TM : TypeMemberInfos) {
if (!TM.Bits->GV->isConstant())
return false;
Expand Down Expand Up @@ -997,6 +1036,13 @@ bool DevirtModule::tryFindVirtualCallTargets(
if (Fn->getName() == "__cxa_pure_virtual")
continue;

// We can disregard unreachable functions as possible call targets, as
// unreachable functions shouldn't be called.
if (ExportSummary && (mustBeUnreachableFunction(
lookUpFunctionValueInfo(Fn, ExportSummary)))) {
continue;
}

TargetsForSlot.push_back({Fn, &TM});
}

Expand Down Expand Up @@ -2014,6 +2060,30 @@ void DevirtModule::removeRedundantTypeTests() {
}
}

ValueInfo
DevirtModule::lookUpFunctionValueInfo(Function *TheFn,
ModuleSummaryIndex *ExportSummary) {
assert((ExportSummary != nullptr) &&
"Caller guarantees ExportSummary is not nullptr");

const auto TheFnGUID = TheFn->getGUID();
const auto TheFnGUIDWithExportedName = GlobalValue::getGUID(TheFn->getName());
// Look up ValueInfo with the GUID in the current linkage.
ValueInfo TheFnVI = ExportSummary->getValueInfo(TheFnGUID);
// If no entry is found and GUID is different from GUID computed using
// exported name, look up ValueInfo with the exported name unconditionally.
// This is a fallback.
//
// The reason to have a fallback:
// 1. LTO could enable global value internalization via
// `enable-lto-internalization`.
// 2. The GUID in ExportedSummary is computed using exported name.
if ((!TheFnVI) && (TheFnGUID != TheFnGUIDWithExportedName)) {
TheFnVI = ExportSummary->getValueInfo(TheFnGUIDWithExportedName);
}
return TheFnVI;
}

bool DevirtModule::run() {
// If only some of the modules were split, we cannot correctly perform
// this transformation. We already checked for the presense of type tests
Expand Down Expand Up @@ -2137,7 +2207,7 @@ bool DevirtModule::run() {
cast<MDString>(S.first.TypeID)->getString())
.WPDRes[S.first.ByteOffset];
if (tryFindVirtualCallTargets(TargetsForSlot, TypeMemberInfos,
S.first.ByteOffset)) {
S.first.ByteOffset, ExportSummary)) {

if (!trySingleImplDevirt(ExportSummary, TargetsForSlot, S.second, Res)) {
DidVirtualConstProp |=
Expand Down

0 comments on commit 09a704c

Please sign in to comment.