modimo updated this revision to Diff 533326.
modimo added a comment.
Address feedback. Allow empty string to unset the flag
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D152741/new/
https://reviews.llvm.org/D152741
Files:
clang/include/clang/Basic/CodeGenOptions.h
clang/include/clang/Driver/Options.td
clang/lib/CodeGen/CGVTables.cpp
clang/lib/CodeGen/CodeGenModule.h
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/CodeGenCXX/type-metadata-unknown-vtable-visibility-filepaths.cpp
clang/test/Driver/funknown-vtable-visibility-filepaths.c
llvm/include/llvm/IR/GlobalObject.h
llvm/lib/IR/Metadata.cpp
llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
llvm/test/ThinLTO/X86/devirt.ll
Index: llvm/test/ThinLTO/X86/devirt.ll
===================================================================
--- llvm/test/ThinLTO/X86/devirt.ll
+++ llvm/test/ThinLTO/X86/devirt.ll
@@ -42,9 +42,11 @@
; RUN: -r=%t2.o,_ZN1B1fEi,p \
; RUN: -r=%t2.o,_ZN1C1fEi,p \
; RUN: -r=%t2.o,_ZN1D1mEi,p \
+; RUN: -r=%t2.o,_ZN1E1nEi,p \
; RUN: -r=%t2.o,_ZTV1B,px \
; RUN: -r=%t2.o,_ZTV1C,px \
-; RUN: -r=%t2.o,_ZTV1D,px 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: -r=%t2.o,_ZTV1D,px \
+; RUN: -r=%t2.o,_ZTV1F,px 2>&1 | FileCheck %s --check-prefix=REMARK
; RUN: llvm-dis %t3.1.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR
; Check that we're able to prevent specific function from being
@@ -58,9 +60,11 @@
; RUN: -r=%t2.o,_ZN1B1fEi,p \
; RUN: -r=%t2.o,_ZN1C1fEi,p \
; RUN: -r=%t2.o,_ZN1D1mEi,p \
+; RUN: -r=%t2.o,_ZN1E1nEi,p \
; RUN: -r=%t2.o,_ZTV1B,px \
; RUN: -r=%t2.o,_ZTV1C,px \
-; RUN: -r=%t2.o,_ZTV1D,px 2>&1 | FileCheck %s --check-prefix=SKIP
+; RUN: -r=%t2.o,_ZTV1D,px \
+; RUN: -r=%t2.o,_ZTV1F,px 2>&1 | FileCheck %s --check-prefix=SKIP
; RUN: llvm-lto2 run %t.o -save-temps -pass-remarks=. \
; RUN: -whole-program-visibility \
@@ -70,16 +74,20 @@
; RUN: -r=%t.o,_ZN1B1fEi,p \
; RUN: -r=%t.o,_ZN1C1fEi,p \
; RUN: -r=%t.o,_ZN1D1mEi,p \
+; RUN: -r=%t.o,_ZN1E1nEi,p \
; RUN: -r=%t.o,_ZTV1B, \
; RUN: -r=%t.o,_ZTV1C, \
; RUN: -r=%t.o,_ZTV1D, \
+; RUN: -r=%t.o,_ZTV1F, \
; RUN: -r=%t.o,_ZN1A1nEi, \
; RUN: -r=%t.o,_ZN1B1fEi, \
; RUN: -r=%t.o,_ZN1C1fEi, \
; RUN: -r=%t.o,_ZN1D1mEi, \
+; RUN: -r=%t.o,_ZN1E1nEi, \
; RUN: -r=%t.o,_ZTV1B,px \
; RUN: -r=%t.o,_ZTV1C,px \
-; RUN: -r=%t.o,_ZTV1D,px 2>&1 | FileCheck %s --check-prefix=REMARK --dump-input=fail
+; RUN: -r=%t.o,_ZTV1D,px \
+; RUN: -r=%t.o,_ZTV1F,px 2>&1 | FileCheck %s --check-prefix=REMARK --dump-input=fail
; RUN: llvm-dis %t3.1.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR
; REMARK-DAG: single-impl: devirtualized a call to _ZN1A1nEi
@@ -95,13 +103,18 @@
%struct.C = type { %struct.A }
%struct.D = type { ptr }
+%struct.E = type { ptr }
+%struct.F = type { %struct.E }
+
@_ZTV1B = constant { [4 x ptr] } { [4 x ptr] [ptr null, ptr undef, ptr @_ZN1B1fEi, ptr @_ZN1A1nEi] }, !type !0, !type !1
@_ZTV1C = constant { [4 x ptr] } { [4 x ptr] [ptr null, ptr undef, ptr @_ZN1C1fEi, ptr @_ZN1A1nEi] }, !type !0, !type !2
@_ZTV1D = constant { [3 x ptr] } { [3 x ptr] [ptr null, ptr undef, ptr @_ZN1D1mEi] }, !type !3
+@_ZTV1F = constant { [3 x ptr] } { [3 x ptr] [ptr null, ptr undef, ptr @_ZN1E1nEi] }, !type !5, !vcall_visibility !6
+
; CHECK-IR-LABEL: define i32 @test
-define i32 @test(ptr %obj, ptr %obj2, i32 %a) {
+define i32 @test(ptr %obj, ptr %obj2, ptr %obj3, i32 %a) {
entry:
%vtable = load ptr, ptr %obj
%p = call i1 @llvm.type.test(ptr %vtable, metadata !"_ZTS1A")
@@ -114,7 +127,7 @@
; Ensure !prof and !callees metadata for indirect call promotion removed.
; CHECK-IR-NOT: prof
; CHECK-IR-NOT: callees
- %call = tail call i32 %fptr1(ptr nonnull %obj, i32 %a), !prof !5, !callees !6
+ %call = tail call i32 %fptr1(ptr nonnull %obj, i32 %a), !prof !7, !callees !8
%fptr22 = load ptr, ptr %vtable, align 8
@@ -131,7 +144,18 @@
; Check that the call was devirtualized.
; CHECK-IR: %call4 = tail call i32 @_ZN1D1mEi
%call4 = tail call i32 %fptr33(ptr nonnull %obj2, i32 %call3)
- ret i32 %call4
+
+ %vtable1 = load ptr, ptr %obj3
+ %p3 = call i1 @llvm.type.test(ptr %vtable1, metadata !"_ZTS1E")
+ call void @llvm.assume(i1 %p3)
+ %fptrptr1 = getelementptr ptr, ptr %vtable1, i32 0
+ %fptr44 = load ptr, ptr %fptrptr1, align 8
+
+ ; Check that the call was not devirtualized because of "unknown"
+ ; vcall_visibility.
+ ; CHECK-IR: %call5 = tail call i32 %fptr44
+ %call5 = tail call i32 %fptr44(ptr nonnull %obj, i32 %call4)
+ ret i32 %call5
}
; CHECK-IR-LABEL: ret i32
; CHECK-IR-LABEL: }
@@ -155,6 +179,10 @@
ret i32 0;
}
+define i32 @_ZN1E1nEi(ptr %this, i32 %a) #0 {
+ ret i32 0;
+}
+
; Make sure we don't inline or otherwise optimize out the direct calls.
attributes #0 = { noinline optnone }
@@ -163,5 +191,7 @@
!2 = !{i64 16, !"_ZTS1C"}
!3 = !{i64 16, !4}
!4 = distinct !{}
-!5 = !{!"VP", i32 0, i64 1, i64 1621563287929432257, i64 1}
-!6 = !{ptr @_ZN1A1nEi}
+!5 = !{i64 16, !"_ZTS1E"}
+!6 = !{i64 3}
+!7 = !{!"VP", i32 0, i64 1, i64 1621563287929432257, i64 1}
+!8 = !{ptr @_ZN1A1nEi}
Index: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
===================================================================
--- llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
+++ llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
@@ -996,9 +996,11 @@
return false;
// We cannot perform whole program devirtualization analysis on a vtable
- // with public LTO visibility.
+ // with public or unknown LTO visibility.
if (TM.Bits->GV->getVCallVisibility() ==
- GlobalObject::VCallVisibilityPublic)
+ GlobalObject::VCallVisibilityPublic ||
+ TM.Bits->GV->getVCallVisibility() ==
+ GlobalObject::VCallVisibilityUnknown)
return false;
Constant *Ptr = getPointerAtOffset(TM.Bits->GV->getInitializer(),
@@ -1075,7 +1077,8 @@
VS = CurVS;
// We cannot perform whole program devirtualization analysis on a vtable
// with public LTO visibility.
- if (VS->getVCallVisibility() == GlobalObject::VCallVisibilityPublic)
+ if (VS->getVCallVisibility() == GlobalObject::VCallVisibilityPublic ||
+ VS->getVCallVisibility() == GlobalObject::VCallVisibilityUnknown)
return false;
}
}
Index: llvm/lib/IR/Metadata.cpp
===================================================================
--- llvm/lib/IR/Metadata.cpp
+++ llvm/lib/IR/Metadata.cpp
@@ -1712,7 +1712,7 @@
uint64_t Val = cast<ConstantInt>(
cast<ConstantAsMetadata>(MD->getOperand(0))->getValue())
->getZExtValue();
- assert(Val <= 2 && "unknown vcall visibility!");
+ assert(Val <= 3 && "unknown vcall visibility!");
return (VCallVisibility)Val;
}
return VCallVisibility::VCallVisibilityPublic;
Index: llvm/include/llvm/IR/GlobalObject.h
===================================================================
--- llvm/include/llvm/IR/GlobalObject.h
+++ llvm/include/llvm/IR/GlobalObject.h
@@ -37,6 +37,8 @@
VCallVisibilityLinkageUnit = 1,
// Type is only visible to code in the current Module.
VCallVisibilityTranslationUnit = 2,
+ // Type is unknown and should always be treated conservatively.
+ VCallVisibilityUnknown = 3,
};
protected:
Index: clang/test/Driver/funknown-vtable-visibility-filepaths.c
===================================================================
--- /dev/null
+++ clang/test/Driver/funknown-vtable-visibility-filepaths.c
@@ -0,0 +1,9 @@
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin 2>&1 | FileCheck --check-prefix=NOSKIP %s
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -fwhole-program-vtables -funknown-vtable-visibility-filepaths=abc 2>&1 | FileCheck --check-prefix=SKIP %s
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -fwhole-program-vtables -funknown-vtable-visibility-filepaths=abc -funknown-vtable-visibility-filepaths= 2>&1 | FileCheck --check-prefix=NOSKIP %s
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -funknown-vtable-visibility-filepaths=abc 2>&1 | FileCheck --check-prefix=ERROR1 %s
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -funknown-vtable-visibility-filepaths=abc -funknown-vtable-visibility-filepaths= 2>&1 | FileCheck --check-prefix=NOSKIP %s
+
+// SKIP: "-funknown-vtable-visibility-filepaths=abc"
+// NOSKIP-NOT: "-funknown-vtable-visibility-filepaths=abc"
+// ERROR1: error: invalid argument '-funknown-vtable-visibility-filepaths' only allowed with '-fwhole-program-vtables'
Index: clang/test/CodeGenCXX/type-metadata-unknown-vtable-visibility-filepaths.cpp
===================================================================
--- /dev/null
+++ clang/test/CodeGenCXX/type-metadata-unknown-vtable-visibility-filepaths.cpp
@@ -0,0 +1,35 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -flto=thin -flto-unit -fwhole-program-vtables -triple x86_64-unknown-linux -fvisibility=hidden -emit-llvm -o - a.cpp | FileCheck %s
+// RUN: %clang_cc1 -flto=thin -flto-unit -fwhole-program-vtables -triple x86_64-unknown-linux -funknown-vtable-visibility-filepaths=h$ -fvisibility=hidden -emit-llvm -o - a.cpp | FileCheck -check-prefix=SKIP-PATH %s
+
+// CHECK-DAG: gv: (name: "_ZTV1A", {{.*}} vcall_visibility: 1
+// CHECK-DAG: gv: (name: "_ZTV1B", {{.*}} vcall_visibility: 1
+
+// Check that unknown visibility only applies to type from header file.
+// SKIP-PATH-DAG: gv: (name: "_ZTV1A", {{.*}} vcall_visibility: 3
+// SKIP-PATH-DAG: gv: (name: "_ZTV1B", {{.*}} vcall_visibility: 1
+
+//--- a.h
+struct A {
+ virtual int f() { return 1; }
+};
+
+//--- a.cpp
+#include "a.h"
+
+struct B {
+ virtual int f() { return 1; }
+};
+
+int f() {
+ auto a = new A();
+ return a->f();
+}
+
+int f1() {
+ auto b = new B();
+ return b->f();
+}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===================================================================
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1578,6 +1578,10 @@
GenerateOptimizationRemark(Args, SA, OPT_Rpass_analysis_EQ, "pass-analysis",
Opts.OptimizationRemarkAnalysis);
+ if (Opts.SkipVtableFilepaths.hasValidPattern())
+ GenerateArg(Args, OPT_funknown_vtable_visibility_filepaths_EQ,
+ Opts.SkipVtableFilepaths.Pattern, SA);
+
GenerateArg(Args, OPT_fdiagnostics_hotness_threshold_EQ,
Opts.DiagnosticsHotnessThreshold
? Twine(*Opts.DiagnosticsHotnessThreshold)
@@ -1989,6 +1993,19 @@
Opts.OptimizationRemarkMissed.hasValidPattern() ||
Opts.OptimizationRemarkAnalysis.hasValidPattern();
+ if (Arg *A = Args.getLastArg(OPT_funknown_vtable_visibility_filepaths_EQ)) {
+ StringRef Val = A->getValue();
+ std::string RegexError;
+ std::shared_ptr<llvm::Regex> Pattern = std::make_shared<llvm::Regex>(Val);
+ if (!Pattern->isValid(RegexError)) {
+ Diags.Report(diag::err_drv_optimization_remark_pattern)
+ << RegexError << A->getAsString(Args);
+ Pattern.reset();
+ }
+ Opts.SkipVtableFilepaths =
+ CodeGenOptions::RegexWithPattern(std::string(Val), Pattern);
+ }
+
bool UsingSampleProfile = !Opts.SampleProfileFile.empty();
bool UsingProfile =
UsingSampleProfile || !Opts.ProfileInstrumentUsePath.empty();
Index: clang/lib/Driver/ToolChains/Clang.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7245,6 +7245,20 @@
if (SplitLTOUnit)
CmdArgs.push_back("-fsplit-lto-unit");
+ if (const Arg *A =
+ Args.getLastArg(options::OPT_funknown_vtable_visibility_filepaths_EQ)) {
+ StringRef Path = A->getValue();
+ if (!Path.empty()) {
+ if (!WholeProgramVTables)
+ D.Diag(diag::err_drv_argument_only_allowed_with)
+ << "-funknown-vtable-visibility-filepaths"
+ << "-fwhole-program-vtables";
+ CmdArgs.push_back(
+ Args.MakeArgString("-funknown-vtable-visibility-filepaths=" + Path));
+ }
+ A->claim();
+ }
+
if (Arg *A = Args.getLastArg(options::OPT_fglobal_isel,
options::OPT_fno_global_isel)) {
CmdArgs.push_back("-mllvm");
Index: clang/lib/CodeGen/CodeGenModule.h
===================================================================
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -1431,6 +1431,11 @@
/// optimization.
bool HasHiddenLTOVisibility(const CXXRecordDecl *RD);
+ /// Returns whether the given record has unknown LTO visibility and therefore
+ /// should never participate in (single-module) CFI and whole-program vtable
+ /// optimization.
+ bool HasUnknownLTOVisibility(const CXXRecordDecl *RD);
+
/// Returns whether the given record has public LTO visibility (regardless of
/// -lto-whole-program-visibility) and therefore may not participate in
/// (single-module) CFI and whole-program vtable optimization.
Index: clang/lib/CodeGen/CGVTables.cpp
===================================================================
--- clang/lib/CodeGen/CGVTables.cpp
+++ clang/lib/CodeGen/CGVTables.cpp
@@ -1247,6 +1247,29 @@
return !AlwaysHasLTOVisibilityPublic(RD);
}
+bool CodeGenModule::HasUnknownLTOVisibility(const CXXRecordDecl *RD) {
+ if (CodeGenOpts.SkipVtableFilepaths.hasValidPattern()) {
+ const PresumedLoc &PLoc =
+ Context.getFullLoc(RD->getBeginLoc()).getPresumedLoc(true);
+ assert(PLoc.isValid() && "Source location is expected to be always valid.");
+ std::string FilePath = PLoc.getFilename();
+
+ if (CodeGenOpts.SkipVtableFilepaths.patternMatches(FilePath))
+ return true;
+ }
+ return false;
+}
+
+static llvm::GlobalObject::VCallVisibility
+getMinVCallVisibility(llvm::GlobalObject::VCallVisibility TypeVisA,
+ llvm::GlobalObject::VCallVisibility TypeVisB) {
+ if (TypeVisA == llvm::GlobalObject::VCallVisibilityUnknown ||
+ TypeVisB == llvm::GlobalObject::VCallVisibilityUnknown)
+ return llvm::GlobalObject::VCallVisibilityUnknown;
+
+ return std::min(TypeVisA, TypeVisB);
+}
+
llvm::GlobalObject::VCallVisibility CodeGenModule::GetVCallVisibilityLevel(
const CXXRecordDecl *RD, llvm::DenseSet<const CXXRecordDecl *> &Visited) {
// If we have already visited this RD (which means this is a recursive call
@@ -1259,7 +1282,9 @@
LinkageInfo LV = RD->getLinkageAndVisibility();
llvm::GlobalObject::VCallVisibility TypeVis;
- if (!isExternallyVisible(LV.getLinkage()))
+ if (HasUnknownLTOVisibility(RD))
+ TypeVis = llvm::GlobalObject::VCallVisibilityUnknown;
+ else if (!isExternallyVisible(LV.getLinkage()))
TypeVis = llvm::GlobalObject::VCallVisibilityTranslationUnit;
else if (HasHiddenLTOVisibility(RD))
TypeVis = llvm::GlobalObject::VCallVisibilityLinkageUnit;
@@ -1268,13 +1293,13 @@
for (const auto &B : RD->bases())
if (B.getType()->getAsCXXRecordDecl()->isDynamicClass())
- TypeVis = std::min(
+ TypeVis = getMinVCallVisibility(
TypeVis,
GetVCallVisibilityLevel(B.getType()->getAsCXXRecordDecl(), Visited));
for (const auto &B : RD->vbases())
if (B.getType()->getAsCXXRecordDecl()->isDynamicClass())
- TypeVis = std::min(
+ TypeVis = getMinVCallVisibility(
TypeVis,
GetVCallVisibilityLevel(B.getType()->getAsCXXRecordDecl(), Visited));
Index: clang/include/clang/Driver/Options.td
===================================================================
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -3157,6 +3157,9 @@
CodeGenOpts<"WholeProgramVTables">, DefaultFalse,
PosFlag<SetTrue, [CC1Option], "Enables whole-program vtable optimization. Requires -flto">,
NegFlag<SetFalse>, BothFlags<[CoreOption]>>;
+def funknown_vtable_visibility_filepaths_EQ : Joined<["-"], "funknown-vtable-visibility-filepaths=">, Group<f_Group>,
+ HelpText<"Skip types from participating in whole-program vtable optimization defined under these filepaths. Pass empty string to unset">,
+ Flags<[CC1Option]>;
defm split_lto_unit : BoolFOption<"split-lto-unit",
CodeGenOpts<"EnableSplitLTOUnit">, DefaultFalse,
PosFlag<SetTrue, [CC1Option], "Enables splitting of the LTO unit">,
Index: clang/include/clang/Basic/CodeGenOptions.h
===================================================================
--- clang/include/clang/Basic/CodeGenOptions.h
+++ clang/include/clang/Basic/CodeGenOptions.h
@@ -186,6 +186,29 @@
/// Regexes separated by a semi-colon to filter the files to not instrument.
std::string ProfileExcludeFiles;
+ /// Regular expression with its original pattern string
+ struct RegexWithPattern {
+ std::string Pattern;
+ std::shared_ptr<llvm::Regex> Regex;
+
+ RegexWithPattern() = default;
+ RegexWithPattern(std::string Pattern, std::shared_ptr<llvm::Regex> Regex)
+ : Pattern(Pattern), Regex(Regex) {}
+
+ /// Returns true iff the optimization remark holds a valid regular
+ /// expression.
+ bool hasValidPattern() const { return Regex != nullptr; }
+
+ /// Matches the given string against the regex, if there is some.
+ bool patternMatches(StringRef String) const {
+ return hasValidPattern() && Regex->match(String);
+ }
+ };
+
+ /// Skip types defined in these paths when performing whole-program vtable
+ /// optimization.
+ RegexWithPattern SkipVtableFilepaths;
+
/// The version string to put into coverage files.
char CoverageVersion[4];
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
