Re: r374844 - Revert "Dead Virtual Function Elimination"

2019-10-28 Thread David Blaikie via cfe-commits
+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"

2019-10-14 Thread Richard Smith via cfe-commits
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"

2019-10-14 Thread Jorge Gorbe Moya via cfe-commits
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