[LTO][WPD] Ignore unreachable function by analyzing IR.

[mingmingl]

Summary

In regular LTO, analyze IR and discard unreachable functions when finding virtual call targets.

[mingmingl]

Combine if into &&.

[mingmingl]

Resolve a compile error.

[tejohnson]

llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp

1057

Since you are adding the IR check above, thinking that to optimize for compile time this lookup should be ignored if we have the def of Fn available (avoids an extra hash table lookup and analysis).

[mingmingl]

Save one index lookup if function def is present for IR analysis. Also simplify test case.

[mingmingl]

thanks for review!

I simplify the test case here, and could do the same for the previous two test cases either here or in a separate patch.

llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp

1057

Done by "first analyze function def in IR if that's present, and use export summary as a fallback".

I took the liberty to add a comment like [1]; I'm wondering what's the idiomatic way to provide pointer a line of code w/ commit for reference?

[1]

// FWIW, the writer of `ExportSummary` guarantees that function declarations
  // won't have an entry in `ExportSummary`;
  // see llvm/lib/Analysis/ModuleSummaryAnalysis.cpp#L785 at commit cf9e61a for
  // a reference.

[mingmingl]

remove No newline at end of file by using a different editor

[tejohnson]

lgtm

llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp

1057

That's a good question on code reference. Usually I see references to the file or function granularity, not this specific. However, I don't think it is really needed here. Note on that below.

2117

I don't think this comment is really relevant here. Note you are coming into this part of the handling specifically when we *do* have a function declaration in this module, so it is a little confusing to explain why we don't create a summary for them. The reason why we may have a summary for what is a function decl in this combined regular LTO module is that we have the combined summaries of all of the ThinLTO modules, and the def is in one of those. So I would just keep the first sentence of this comment block ("Learn unreachability from ...").

[mingmingl]

Remove comments on function declarations, as a combined index may contain entries for func declarations.

[mingmingl]

thanks!

llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp

1057

Got it, thanks for the inputs!

2117

The reason why we may have a summary for what is a function decl in this combined regular LTO module is that we have the combined summaries of all of the ThinLTO modules, and the def is in one of those.

I will keep this in mind, thanks for pointing it out!

Removed the comment for function declaration.

llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp

for (auto &Summary : TheFnVI.getSummaryList()) {
     // 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;
 }
 
+// Returns true if the function definition must be unreachable.
+//
+// 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.
+bool mustBeUnreachableFunction(Function *const F) {
+  // A function must be unreachable if its entry block ends with an
+  // 'unreachable'.
+  if (F->isDeclaration())
+    return false;
+  return isa<UnreachableInst>(F->getEntryBlock().getTerminator());
+}
+
 // A virtual call site. VTable is the loaded virtual table pointer, and CS is
 // the indirect virtual call.
 struct VirtualCallSite {
   Value *VTable = nullptr;
   CallBase &CB;
 
   // If non-null, this field points to the associated unsafe use count stored in
   // the DevirtModule::NumUnsafeUsesForTypeTest map below. See the description
for (const TypeMemberInfo &TM : TypeMemberInfos) {
 
     // We can disregard __cxa_pure_virtual as a possible call target, as
     // calls to pure virtuals are UB.
     if (Fn->getName() == "__cxa_pure_virtual")
       continue;
 
     // We can disregard unreachable functions as possible call targets, as
     // unreachable functions shouldn't be called.
+    if (mustBeUnreachableFunction(Fn))
+      continue;
+
     if (ExportSummary && (mustBeUnreachableFunction(
                              lookUpFunctionValueInfo(Fn, ExportSummary)))) {
       continue;
     }
 
     TargetsForSlot.push_back({Fn, &TM});
   }
 
   // Give up if we couldn't find any targets.
bool DevirtModule::run() {
   // with partially split modules during the thin link, and would have emitted
   // an error if any were found, so here we can simply return.
   if ((ExportSummary && ExportSummary->partiallySplitLTOUnits()) ||
       (ImportSummary && ImportSummary->partiallySplitLTOUnits()))
     return false;
 
   Function *TypeTestFunc =
       M.getFunction(Intrinsic::getName(Intrinsic::type_test));
   Function *TypeCheckedLoadFunc =
       M.getFunction(Intrinsic::getName(Intrinsic::type_checked_load));
   Function *AssumeFunc = M.getFunction(Intrinsic::getName(Intrinsic::assume));
 
   // Normally if there are no users of the devirtualization intrinsics in the
   // module, this pass has nothing to do. But if we are exporting, we also need
   // to handle any users that appear only in the function summaries.
   if (!ExportSummary &&
       (!TypeTestFunc || TypeTestFunc->use_empty() || !AssumeFunc ||

llvm/test/Transforms/WholeProgramDevirt/devirt_single_after_filtering_unreachable_function.ll

+; Test that regular LTO will analyze IR, detect unreachable functions and discard unreachable functions
+; when finding virtual call targets.
+; In this test case, the unreachable function is the virtual deleting destructor of an abstract class.
+
+; RUN: opt -S -passes=wholeprogramdevirt -whole-program-visibility -pass-remarks=wholeprogramdevirt %s 2>&1 | FileCheck %s
+
+; CHECK: remark: tmp.cc:21:3: single-impl: devirtualized a call to _ZN7DerivedD0Ev
+; CHECK: remark: <unknown>:0:0: devirtualized _ZN7DerivedD0Ev
+
+source_filename = "tmp.cc"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+%class.Derived = type { %class.Base }
+%class.Base = type { i32 (...)** }
+
+@_ZTV7Derived = constant { [4 x i8*] } { [4 x i8*] [ i8* bitcast ({ i8*, i8*, i8* }* @_ZTI7Derived to i8*), i8* bitcast (void (%class.Derived*)* @_ZN7DerivedD2Ev to i8*), i8* bitcast (void (%class.Derived*)* @_ZN7DerivedD0Ev to i8*), i8* bitcast (void (%class.Derived*)* @_ZN7Derived1xEv to i8*)] }, !type !0, !type !1, !type !2, !type !3, !vcall_visibility !4
+@_ZTVN10__cxxabiv120__si_class_type_infoE = external global i8*
+@_ZTS7Derived = constant [9 x i8] c"7Derived\00"
+@_ZTVN10__cxxabiv117__class_type_infoE = external global i8*
+@_ZTS4Base = constant [6 x i8] c"4Base\00"
+@_ZTI4Base = constant { i8*, i8* } { i8* bitcast (i8** getelementptr inbounds (i8*, i8** @_ZTVN10__cxxabiv117__class_type_infoE, i64 2) to i8*), i8* getelementptr inbounds ([6 x i8], [6 x i8]* @_ZTS4Base, i32 0, i32 0) }
+@_ZTI7Derived = constant { i8*, i8*, i8* } { i8* bitcast (i8** getelementptr inbounds (i8*, i8** @_ZTVN10__cxxabiv120__si_class_type_infoE, i64 2) to i8*), i8* getelementptr inbounds ([9 x i8], [9 x i8]* @_ZTS7Derived, i32 0, i32 0), i8* bitcast ({ i8*, i8* }* @_ZTI4Base to i8*) }
+@_ZTV4Base = constant { [4 x i8*] } { [4 x i8*] [ i8* bitcast ({ i8*, i8* }* @_ZTI4Base to i8*), i8* bitcast (void (%class.Base*)* @_ZN4BaseD2Ev to i8*), i8* bitcast (void (%class.Base*)* @_ZN4BaseD0Ev to i8*), i8* bitcast (void ()* @__cxa_pure_virtual to i8*)] }, !type !0, !type !1, !vcall_visibility !4
+
+define void @_ZN7Derived1xEv(%class.Derived* %this) {
+entry:
+  ret void
+}
+
+declare i1 @llvm.type.test(i8*, metadata)
+
+declare void @llvm.assume(i1)
+
+define i32 @func(%class.Base* %b) {
+entry:
+  %0 = bitcast %class.Base* %b to void (%class.Base*)***, !dbg !11
+  %vtable = load void (%class.Base*)**, void (%class.Base*)*** %0, !dbg !11
+  %1 = bitcast void (%class.Base*)** %vtable to i8*, !dbg !11
+  %2 = tail call i1 @llvm.type.test(i8* %1, metadata !"_ZTS4Base"), !dbg !11
+  tail call void @llvm.assume(i1 %2), !dbg !11
+  %vfn = getelementptr inbounds void (%class.Base*)*, void (%class.Base*)** %vtable, i64 0, !dbg !11
+  %3 = load void (%class.Base*)*, void (%class.Base*)** %vfn, !dbg !11
+  tail call void %3(%class.Base* %b), !dbg !11
+  ret i32 0
+}
+
+define void @_ZN7DerivedD2Ev(%class.Derived* %this) {
+entry:
+  ret void
+}
+
+define void @_ZN7DerivedD0Ev(%class.Derived* %this) {
+entry:
+  ret void
+}
+
+define void @_ZN4BaseD2Ev(%class.Base* %this) {
+entry:
+  ret void
+}
+
+define void @_ZN4BaseD0Ev(%class.Base* %this) {
+entry:
+  tail call void @llvm.trap()
+  unreachable
+}
+
+declare void @__cxa_pure_virtual() unnamed_addr
+
+declare void @llvm.trap()
+
+!llvm.dbg.cu = !{!5}
+!llvm.module.flags = !{!7}
+
+!0 = !{i64 16, !"_ZTS4Base"}
+!1 = !{i64 32, !"_ZTSM4BaseFvvE.virtual"}
+!2 = !{i64 16, !"_ZTS7Derived"}
+!3 = !{i64 32, !"_ZTSM7DerivedFvvE.virtual"}
+!4 = !{i64 1}
+!5 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !6, isOptimized: true, runtimeVersion: 0, emissionKind: NoDebug, splitDebugInlining: false, nameTableKind: None)
+!6 = !DIFile(filename: "tmp.cc", directory: "")
+!7 = !{i32 2, !"Debug Info Version", i32 3}
+!8 = !DIFile(filename: "tmp.cc", directory: "")
+!9 = !DISubroutineType(types: !{})
+!10= distinct !DISubprogram(name: "func", scope: !8, file: !8, line: 20, type: !9, scopeLine: 20, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !5, retainedNodes: !{})
+!11 = !DILocation(line: 21, column: 3, scope: !10)
 No newline at end of file