Re: r374844 - Revert "Dead Virtual Function Elimination"
+1/ping on this - Jorge, could you include some details about the reason for the revert & the revision? On Mon, Oct 14, 2019 at 5:06 PM Richard Smith via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Hi Jorge, > > Please mention the reason for a revert in revert commit messages. Also, > until SVN stops being our system of record, please use SVN revision numbers > to reference prior commits in reverts. > > On Mon, 14 Oct 2019 at 16:22, Jorge Gorbe Moya via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Author: jgorbe >> Date: Mon Oct 14 16:25:25 2019 >> New Revision: 374844 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=374844=rev >> Log: >> Revert "Dead Virtual Function Elimination" >> >> This reverts commit 9f6a873268e1ad9855873d9d8007086c0d01cf4f. >> >> Removed: >> cfe/trunk/test/CodeGenCXX/vcall-visibility-metadata.cpp >> cfe/trunk/test/CodeGenCXX/virtual-function-elimination.cpp >> cfe/trunk/test/Driver/virtual-function-elimination.cpp >> Modified: >> cfe/trunk/include/clang/Basic/CodeGenOptions.def >> cfe/trunk/include/clang/Driver/Options.td >> cfe/trunk/lib/CodeGen/CGClass.cpp >> cfe/trunk/lib/CodeGen/CGVTables.cpp >> cfe/trunk/lib/CodeGen/CodeGenModule.h >> cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp >> cfe/trunk/lib/Driver/ToolChains/Clang.cpp >> cfe/trunk/lib/Frontend/CompilerInvocation.cpp >> >> Modified: cfe/trunk/include/clang/Basic/CodeGenOptions.def >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/CodeGenOptions.def?rev=374844=374843=374844=diff >> >> == >> --- cfe/trunk/include/clang/Basic/CodeGenOptions.def (original) >> +++ cfe/trunk/include/clang/Basic/CodeGenOptions.def Mon Oct 14 16:25:25 >> 2019 >> @@ -278,10 +278,6 @@ CODEGENOPT(EmitLLVMUseLists, 1, 0) ///< >> CODEGENOPT(WholeProgramVTables, 1, 0) ///< Whether to apply whole-program >>/// vtable optimization. >> >> -CODEGENOPT(VirtualFunctionElimination, 1, 0) ///< Whether to apply the >> dead >> - /// virtual function >> elimination >> - /// optimization. >> - >> /// Whether to use public LTO visibility for entities in std and stdext >> /// namespaces. This is enabled by clang-cl's /MT and /MTd flags. >> CODEGENOPT(LTOVisibilityPublicStd, 1, 0) >> >> Modified: cfe/trunk/include/clang/Driver/Options.td >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=374844=374843=374844=diff >> >> == >> --- cfe/trunk/include/clang/Driver/Options.td (original) >> +++ cfe/trunk/include/clang/Driver/Options.td Mon Oct 14 16:25:25 2019 >> @@ -1863,13 +1863,6 @@ def fforce_emit_vtables : Flag<["-"], "f >> HelpText<"Emits more virtual tables to improve devirtualization">; >> def fno_force_emit_vtables : Flag<["-"], "fno-force-emit-vtables">, >> Group, >>Flags<[CoreOption]>; >> - >> -def fvirtual_function_elimination : Flag<["-"], >> "fvirtual-function-elimination">, Group, >> - Flags<[CoreOption, CC1Option]>, >> - HelpText<"Enables dead virtual function elimination optimization. >> Requires -flto=full">; >> -def fno_virtual_function_elimination : Flag<["-"], >> "fno-virtual-function_elimination">, Group, >> - Flags<[CoreOption]>; >> - >> def fwrapv : Flag<["-"], "fwrapv">, Group, Flags<[CC1Option]>, >>HelpText<"Treat signed integer overflow as two's complement">; >> def fwritable_strings : Flag<["-"], "fwritable-strings">, >> Group, Flags<[CC1Option]>, >> >> Modified: cfe/trunk/lib/CodeGen/CGClass.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGClass.cpp?rev=374844=374843=374844=diff >> >> == >> --- cfe/trunk/lib/CodeGen/CGClass.cpp (original) >> +++ cfe/trunk/lib/CodeGen/CGClass.cpp Mon Oct 14 16:25:25 2019 >> @@ -2784,16 +2784,11 @@ void CodeGenFunction::EmitVTablePtrCheck >> >> bool CodeGenFunction::ShouldEmitVTableTypeCheckedLoad(const >> CXXRecordDecl *RD) { >>if (!CGM.getCodeGenOpts().WholeProgramVTables || >> + !SanOpts.has(SanitizerKind::CFIVCall) || >> + !CGM.getCodeGenOpts().SanitizeTrap.has(SanitizerKind::CFIVCall) || >>!CGM.HasHiddenLTOVisibility(RD)) >> return false; >> >> - if (CGM.getCodeGenOpts().VirtualFunctionElimination) >> -return true; >> - >> - if (!SanOpts.has(SanitizerKind::CFIVCall) || >> - !CGM.getCodeGenOpts().SanitizeTrap.has(SanitizerKind::CFIVCall)) >> -return false; >> - >>std::string TypeName = RD->getQualifiedNameAsString(); >>return !getContext().getSanitizerBlacklist().isBlacklistedType( >>SanitizerKind::CFIVCall, TypeName); >> @@ -2816,13 +2811,8 @@ llvm::Value *CodeGenFunction::EmitVTable >> TypeId}); >>llvm::Value
Re: r374844 - Revert "Dead Virtual Function Elimination"
Hi Jorge, Please mention the reason for a revert in revert commit messages. Also, until SVN stops being our system of record, please use SVN revision numbers to reference prior commits in reverts. On Mon, 14 Oct 2019 at 16:22, Jorge Gorbe Moya via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: jgorbe > Date: Mon Oct 14 16:25:25 2019 > New Revision: 374844 > > URL: http://llvm.org/viewvc/llvm-project?rev=374844=rev > Log: > Revert "Dead Virtual Function Elimination" > > This reverts commit 9f6a873268e1ad9855873d9d8007086c0d01cf4f. > > Removed: > cfe/trunk/test/CodeGenCXX/vcall-visibility-metadata.cpp > cfe/trunk/test/CodeGenCXX/virtual-function-elimination.cpp > cfe/trunk/test/Driver/virtual-function-elimination.cpp > Modified: > cfe/trunk/include/clang/Basic/CodeGenOptions.def > cfe/trunk/include/clang/Driver/Options.td > cfe/trunk/lib/CodeGen/CGClass.cpp > cfe/trunk/lib/CodeGen/CGVTables.cpp > cfe/trunk/lib/CodeGen/CodeGenModule.h > cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp > cfe/trunk/lib/Driver/ToolChains/Clang.cpp > cfe/trunk/lib/Frontend/CompilerInvocation.cpp > > Modified: cfe/trunk/include/clang/Basic/CodeGenOptions.def > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/CodeGenOptions.def?rev=374844=374843=374844=diff > > == > --- cfe/trunk/include/clang/Basic/CodeGenOptions.def (original) > +++ cfe/trunk/include/clang/Basic/CodeGenOptions.def Mon Oct 14 16:25:25 > 2019 > @@ -278,10 +278,6 @@ CODEGENOPT(EmitLLVMUseLists, 1, 0) ///< > CODEGENOPT(WholeProgramVTables, 1, 0) ///< Whether to apply whole-program >/// vtable optimization. > > -CODEGENOPT(VirtualFunctionElimination, 1, 0) ///< Whether to apply the > dead > - /// virtual function > elimination > - /// optimization. > - > /// Whether to use public LTO visibility for entities in std and stdext > /// namespaces. This is enabled by clang-cl's /MT and /MTd flags. > CODEGENOPT(LTOVisibilityPublicStd, 1, 0) > > Modified: cfe/trunk/include/clang/Driver/Options.td > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=374844=374843=374844=diff > > == > --- cfe/trunk/include/clang/Driver/Options.td (original) > +++ cfe/trunk/include/clang/Driver/Options.td Mon Oct 14 16:25:25 2019 > @@ -1863,13 +1863,6 @@ def fforce_emit_vtables : Flag<["-"], "f > HelpText<"Emits more virtual tables to improve devirtualization">; > def fno_force_emit_vtables : Flag<["-"], "fno-force-emit-vtables">, > Group, >Flags<[CoreOption]>; > - > -def fvirtual_function_elimination : Flag<["-"], > "fvirtual-function-elimination">, Group, > - Flags<[CoreOption, CC1Option]>, > - HelpText<"Enables dead virtual function elimination optimization. > Requires -flto=full">; > -def fno_virtual_function_elimination : Flag<["-"], > "fno-virtual-function_elimination">, Group, > - Flags<[CoreOption]>; > - > def fwrapv : Flag<["-"], "fwrapv">, Group, Flags<[CC1Option]>, >HelpText<"Treat signed integer overflow as two's complement">; > def fwritable_strings : Flag<["-"], "fwritable-strings">, Group, > Flags<[CC1Option]>, > > Modified: cfe/trunk/lib/CodeGen/CGClass.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGClass.cpp?rev=374844=374843=374844=diff > > == > --- cfe/trunk/lib/CodeGen/CGClass.cpp (original) > +++ cfe/trunk/lib/CodeGen/CGClass.cpp Mon Oct 14 16:25:25 2019 > @@ -2784,16 +2784,11 @@ void CodeGenFunction::EmitVTablePtrCheck > > bool CodeGenFunction::ShouldEmitVTableTypeCheckedLoad(const CXXRecordDecl > *RD) { >if (!CGM.getCodeGenOpts().WholeProgramVTables || > + !SanOpts.has(SanitizerKind::CFIVCall) || > + !CGM.getCodeGenOpts().SanitizeTrap.has(SanitizerKind::CFIVCall) || >!CGM.HasHiddenLTOVisibility(RD)) > return false; > > - if (CGM.getCodeGenOpts().VirtualFunctionElimination) > -return true; > - > - if (!SanOpts.has(SanitizerKind::CFIVCall) || > - !CGM.getCodeGenOpts().SanitizeTrap.has(SanitizerKind::CFIVCall)) > -return false; > - >std::string TypeName = RD->getQualifiedNameAsString(); >return !getContext().getSanitizerBlacklist().isBlacklistedType( >SanitizerKind::CFIVCall, TypeName); > @@ -2816,13 +2811,8 @@ llvm::Value *CodeGenFunction::EmitVTable > TypeId}); >llvm::Value *CheckResult = Builder.CreateExtractValue(CheckedLoad, 1); > > - std::string TypeName = RD->getQualifiedNameAsString(); > - if (SanOpts.has(SanitizerKind::CFIVCall) && > - !getContext().getSanitizerBlacklist().isBlacklistedType( > - SanitizerKind::CFIVCall, TypeName)) { > -
r374844 - Revert "Dead Virtual Function Elimination"
Author: jgorbe Date: Mon Oct 14 16:25:25 2019 New Revision: 374844 URL: http://llvm.org/viewvc/llvm-project?rev=374844=rev Log: Revert "Dead Virtual Function Elimination" This reverts commit 9f6a873268e1ad9855873d9d8007086c0d01cf4f. Removed: cfe/trunk/test/CodeGenCXX/vcall-visibility-metadata.cpp cfe/trunk/test/CodeGenCXX/virtual-function-elimination.cpp cfe/trunk/test/Driver/virtual-function-elimination.cpp Modified: cfe/trunk/include/clang/Basic/CodeGenOptions.def cfe/trunk/include/clang/Driver/Options.td cfe/trunk/lib/CodeGen/CGClass.cpp cfe/trunk/lib/CodeGen/CGVTables.cpp cfe/trunk/lib/CodeGen/CodeGenModule.h cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp cfe/trunk/lib/Driver/ToolChains/Clang.cpp cfe/trunk/lib/Frontend/CompilerInvocation.cpp Modified: cfe/trunk/include/clang/Basic/CodeGenOptions.def URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/CodeGenOptions.def?rev=374844=374843=374844=diff == --- cfe/trunk/include/clang/Basic/CodeGenOptions.def (original) +++ cfe/trunk/include/clang/Basic/CodeGenOptions.def Mon Oct 14 16:25:25 2019 @@ -278,10 +278,6 @@ CODEGENOPT(EmitLLVMUseLists, 1, 0) ///< CODEGENOPT(WholeProgramVTables, 1, 0) ///< Whether to apply whole-program /// vtable optimization. -CODEGENOPT(VirtualFunctionElimination, 1, 0) ///< Whether to apply the dead - /// virtual function elimination - /// optimization. - /// Whether to use public LTO visibility for entities in std and stdext /// namespaces. This is enabled by clang-cl's /MT and /MTd flags. CODEGENOPT(LTOVisibilityPublicStd, 1, 0) Modified: cfe/trunk/include/clang/Driver/Options.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=374844=374843=374844=diff == --- cfe/trunk/include/clang/Driver/Options.td (original) +++ cfe/trunk/include/clang/Driver/Options.td Mon Oct 14 16:25:25 2019 @@ -1863,13 +1863,6 @@ def fforce_emit_vtables : Flag<["-"], "f HelpText<"Emits more virtual tables to improve devirtualization">; def fno_force_emit_vtables : Flag<["-"], "fno-force-emit-vtables">, Group, Flags<[CoreOption]>; - -def fvirtual_function_elimination : Flag<["-"], "fvirtual-function-elimination">, Group, - Flags<[CoreOption, CC1Option]>, - HelpText<"Enables dead virtual function elimination optimization. Requires -flto=full">; -def fno_virtual_function_elimination : Flag<["-"], "fno-virtual-function_elimination">, Group, - Flags<[CoreOption]>; - def fwrapv : Flag<["-"], "fwrapv">, Group, Flags<[CC1Option]>, HelpText<"Treat signed integer overflow as two's complement">; def fwritable_strings : Flag<["-"], "fwritable-strings">, Group, Flags<[CC1Option]>, Modified: cfe/trunk/lib/CodeGen/CGClass.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGClass.cpp?rev=374844=374843=374844=diff == --- cfe/trunk/lib/CodeGen/CGClass.cpp (original) +++ cfe/trunk/lib/CodeGen/CGClass.cpp Mon Oct 14 16:25:25 2019 @@ -2784,16 +2784,11 @@ void CodeGenFunction::EmitVTablePtrCheck bool CodeGenFunction::ShouldEmitVTableTypeCheckedLoad(const CXXRecordDecl *RD) { if (!CGM.getCodeGenOpts().WholeProgramVTables || + !SanOpts.has(SanitizerKind::CFIVCall) || + !CGM.getCodeGenOpts().SanitizeTrap.has(SanitizerKind::CFIVCall) || !CGM.HasHiddenLTOVisibility(RD)) return false; - if (CGM.getCodeGenOpts().VirtualFunctionElimination) -return true; - - if (!SanOpts.has(SanitizerKind::CFIVCall) || - !CGM.getCodeGenOpts().SanitizeTrap.has(SanitizerKind::CFIVCall)) -return false; - std::string TypeName = RD->getQualifiedNameAsString(); return !getContext().getSanitizerBlacklist().isBlacklistedType( SanitizerKind::CFIVCall, TypeName); @@ -2816,13 +2811,8 @@ llvm::Value *CodeGenFunction::EmitVTable TypeId}); llvm::Value *CheckResult = Builder.CreateExtractValue(CheckedLoad, 1); - std::string TypeName = RD->getQualifiedNameAsString(); - if (SanOpts.has(SanitizerKind::CFIVCall) && - !getContext().getSanitizerBlacklist().isBlacklistedType( - SanitizerKind::CFIVCall, TypeName)) { -EmitCheck(std::make_pair(CheckResult, SanitizerKind::CFIVCall), - SanitizerHandler::CFICheckFail, {}, {}); - } + EmitCheck(std::make_pair(CheckResult, SanitizerKind::CFIVCall), +SanitizerHandler::CFICheckFail, nullptr, nullptr); return Builder.CreateBitCast( Builder.CreateExtractValue(CheckedLoad, 0), Modified: cfe/trunk/lib/CodeGen/CGVTables.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVTables.cpp?rev=374844=374843=374844=diff