[PATCH] D71907: [WPD/VFE] Always emit vcall_visibility metadata for -fwhole-program-vtables

[Teresa Johnson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG458676db6e41: [WPD/VFE] Always emit vcall_visibility 
metadata for -fwhole-program-vtables (authored by tejohnson).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71907/new/

https://reviews.llvm.org/D71907

Files:
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGenCXX/vcall-visibility-metadata.cpp
  llvm/include/llvm/IR/GlobalObject.h
  llvm/lib/IR/Metadata.cpp
  llvm/lib/Transforms/IPO/GlobalDCE.cpp
  llvm/lib/Transforms/IPO/GlobalSplit.cpp
  llvm/test/Transforms/GlobalDCE/virtual-functions-base-call.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-base-pointer-call.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-derived-call.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-derived-pointer-call.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-novfe.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-post-lto.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-pre-lto.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions.ll
  llvm/test/Transforms/GlobalDCE/vtable-rtti.ll
  llvm/test/Transforms/GlobalSplit/basic.ll

Index: llvm/test/Transforms/GlobalSplit/basic.ll
===
--- llvm/test/Transforms/GlobalSplit/basic.ll
+++ llvm/test/Transforms/GlobalSplit/basic.ll
@@ -12,13 +12,13 @@
 ]
 
 ; CHECK-NOT: @global =
-; CHECK: @global.0 = private constant [2 x i8* ()*] [i8* ()* @f1, i8* ()* @f2], !type [[T1:![0-9]+]], !type [[T2:![0-9]+]], !type [[T3:![0-9]+$]]
-; CHECK: @global.1 = private constant [1 x i8* ()*] [i8* ()* @f3], !type [[T4:![0-9]+]], !type [[T5:![0-9]+$]]
+; CHECK: @global.0 = private constant [2 x i8* ()*] [i8* ()* @f1, i8* ()* @f2], !type [[T1:![0-9]+]], !type [[T2:![0-9]+]], !type [[T3:![0-9]+]], !vcall_visibility [[VIS:![0-9]+$]]
+; CHECK: @global.1 = private constant [1 x i8* ()*] [i8* ()* @f3], !type [[T4:![0-9]+]], !type [[T5:![0-9]+]], !vcall_visibility [[VIS$]]
 ; CHECK-NOT: @global =
 @global = internal constant { [2 x i8* ()*], [1 x i8* ()*] } {
   [2 x i8* ()*] [i8* ()* @f1, i8* ()* @f2],
   [1 x i8* ()*] [i8* ()* @f3]
-}, !type !0, !type !1, !type !2, !type !3, !type !4
+}, !type !0, !type !1, !type !2, !type !3, !type !4, !vcall_visibility !5
 
 ; CHECK: define i8* @f1()
 define i8* @f1() {
@@ -54,6 +54,7 @@
 ; CHECK: [[T1]] = !{i32 0, !"foo"}
 ; CHECK: [[T2]] = !{i32 15, !"bar"}
 ; CHECK: [[T3]] = !{i32 16, !"a"}
+; CHECK: [[VIS]] = !{i64 2}
 ; CHECK: [[T4]] = !{i32 1, !"b"}
 ; CHECK: [[T5]] = !{i32 8, !"c"}
 !0 = !{i32 0, !"foo"}
@@ -61,3 +62,4 @@
 !2 = !{i32 16, !"a"}
 !3 = !{i32 17, !"b"}
 !4 = !{i32 24, !"c"}
+!5 = !{i64 2}
Index: llvm/test/Transforms/GlobalDCE/vtable-rtti.ll
===
--- llvm/test/Transforms/GlobalDCE/vtable-rtti.ll
+++ llvm/test/Transforms/GlobalDCE/vtable-rtti.ll
@@ -39,9 +39,10 @@
 declare dso_local noalias nonnull i8* @_Znwm(i64)
 @_ZTVN10__cxxabiv117__class_type_infoE = external dso_local global i8*
 
-!llvm.module.flags = !{!3}
+!llvm.module.flags = !{!3, !4}
 
 !0 = !{i64 16, !"_ZTS1A"}
 !1 = !{i64 16, !"_ZTSM1AFvvE.virtual"}
 !2 = !{i64 2} ; translation-unit vcall visibility
 !3 = !{i32 1, !"LTOPostLink", i32 1}
+!4 = !{i32 1, !"Virtual Function Elim", i32 1}
Index: llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-pre-lto.ll
===
--- llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-pre-lto.ll
+++ llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-pre-lto.ll
@@ -85,10 +85,11 @@
 
 declare dso_local noalias nonnull i8* @_Znwm(i64)
 
-!llvm.module.flags = !{}
+!llvm.module.flags = !{!5}
 
 !0 = !{i64 16, !"_ZTS1A"}
 !1 = !{i64 16, !"_ZTSM1AFvvE.virtual"}
 !2 = !{i64 0} ; public vcall visibility
 !3 = !{i64 1} ; linkage-unit vcall visibility
 !4 = !{i64 2} ; translation-unit vcall visibility
+!5 = !{i32 1, !"Virtual Function Elim", i32 1}
Index: llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-post-lto.ll
===
--- llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-post-lto.ll
+++ llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-post-lto.ll
@@ -85,7 +85,7 @@
 
 declare dso_local noalias nonnull i8* @_Znwm(i64)
 
-!llvm.module.flags = !{!5}
+!llvm.module.flags = !{!5, !6}
 
 !0 = !{i64 16, !"_ZTS1A"}
 !1 = !{i64 16, !"_ZTSM1AFvvE.virtual"}
@@ -93,3 +93,4 @@
 !3 = !{i64 1} ; linkage-unit vcall visibility
 !4 = !{i64 2} ; translation-unit vcall visibility
 !5 = !{i32 1, !"LTOPostLink", i32 1}
+!6 = !{i32 1, !"Virtual Function Elim", i32 1}
Index: llvm/test/Transforms/GlobalDCE/virtual-functions.ll
===
--- llvm/test/Transforms/GlobalDCE/virtual-functions.ll
+++ 

[PATCH] D71907: [WPD/VFE] Always emit vcall_visibility metadata for -fwhole-program-vtables

[Eugene Leviant via Phabricator via cfe-commits]

evgeny777 accepted this revision.
evgeny777 added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71907/new/

https://reviews.llvm.org/D71907



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71907: [WPD/VFE] Always emit vcall_visibility metadata for -fwhole-program-vtables

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson updated this revision to Diff 239888.
tejohnson marked an inline comment as done.
tejohnson added a comment.

Test GlobalSplit handling of vcall_visibility metadata


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71907/new/

https://reviews.llvm.org/D71907

Files:
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGenCXX/vcall-visibility-metadata.cpp
  llvm/include/llvm/IR/GlobalObject.h
  llvm/lib/IR/Metadata.cpp
  llvm/lib/Transforms/IPO/GlobalDCE.cpp
  llvm/lib/Transforms/IPO/GlobalSplit.cpp
  llvm/test/Transforms/GlobalDCE/virtual-functions-base-call.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-base-pointer-call.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-derived-call.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-derived-pointer-call.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-novfe.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-post-lto.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-pre-lto.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions.ll
  llvm/test/Transforms/GlobalDCE/vtable-rtti.ll
  llvm/test/Transforms/GlobalSplit/basic.ll

Index: llvm/test/Transforms/GlobalSplit/basic.ll
===
--- llvm/test/Transforms/GlobalSplit/basic.ll
+++ llvm/test/Transforms/GlobalSplit/basic.ll
@@ -12,13 +12,13 @@
 ]
 
 ; CHECK-NOT: @global =
-; CHECK: @global.0 = private constant [2 x i8* ()*] [i8* ()* @f1, i8* ()* @f2], !type [[T1:![0-9]+]], !type [[T2:![0-9]+]], !type [[T3:![0-9]+$]]
-; CHECK: @global.1 = private constant [1 x i8* ()*] [i8* ()* @f3], !type [[T4:![0-9]+]], !type [[T5:![0-9]+$]]
+; CHECK: @global.0 = private constant [2 x i8* ()*] [i8* ()* @f1, i8* ()* @f2], !type [[T1:![0-9]+]], !type [[T2:![0-9]+]], !type [[T3:![0-9]+]], !vcall_visibility [[VIS:![0-9]+$]]
+; CHECK: @global.1 = private constant [1 x i8* ()*] [i8* ()* @f3], !type [[T4:![0-9]+]], !type [[T5:![0-9]+]], !vcall_visibility [[VIS$]]
 ; CHECK-NOT: @global =
 @global = internal constant { [2 x i8* ()*], [1 x i8* ()*] } {
   [2 x i8* ()*] [i8* ()* @f1, i8* ()* @f2],
   [1 x i8* ()*] [i8* ()* @f3]
-}, !type !0, !type !1, !type !2, !type !3, !type !4
+}, !type !0, !type !1, !type !2, !type !3, !type !4, !vcall_visibility !5
 
 ; CHECK: define i8* @f1()
 define i8* @f1() {
@@ -54,6 +54,7 @@
 ; CHECK: [[T1]] = !{i32 0, !"foo"}
 ; CHECK: [[T2]] = !{i32 15, !"bar"}
 ; CHECK: [[T3]] = !{i32 16, !"a"}
+; CHECK: [[VIS]] = !{i64 2}
 ; CHECK: [[T4]] = !{i32 1, !"b"}
 ; CHECK: [[T5]] = !{i32 8, !"c"}
 !0 = !{i32 0, !"foo"}
@@ -61,3 +62,4 @@
 !2 = !{i32 16, !"a"}
 !3 = !{i32 17, !"b"}
 !4 = !{i32 24, !"c"}
+!5 = !{i64 2}
Index: llvm/test/Transforms/GlobalDCE/vtable-rtti.ll
===
--- llvm/test/Transforms/GlobalDCE/vtable-rtti.ll
+++ llvm/test/Transforms/GlobalDCE/vtable-rtti.ll
@@ -39,9 +39,10 @@
 declare dso_local noalias nonnull i8* @_Znwm(i64)
 @_ZTVN10__cxxabiv117__class_type_infoE = external dso_local global i8*
 
-!llvm.module.flags = !{!3}
+!llvm.module.flags = !{!3, !4}
 
 !0 = !{i64 16, !"_ZTS1A"}
 !1 = !{i64 16, !"_ZTSM1AFvvE.virtual"}
 !2 = !{i64 2} ; translation-unit vcall visibility
 !3 = !{i32 1, !"LTOPostLink", i32 1}
+!4 = !{i32 1, !"Virtual Function Elim", i32 1}
Index: llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-pre-lto.ll
===
--- llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-pre-lto.ll
+++ llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-pre-lto.ll
@@ -85,10 +85,11 @@
 
 declare dso_local noalias nonnull i8* @_Znwm(i64)
 
-!llvm.module.flags = !{}
+!llvm.module.flags = !{!5}
 
 !0 = !{i64 16, !"_ZTS1A"}
 !1 = !{i64 16, !"_ZTSM1AFvvE.virtual"}
 !2 = !{i64 0} ; public vcall visibility
 !3 = !{i64 1} ; linkage-unit vcall visibility
 !4 = !{i64 2} ; translation-unit vcall visibility
+!5 = !{i32 1, !"Virtual Function Elim", i32 1}
Index: llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-post-lto.ll
===
--- llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-post-lto.ll
+++ llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-post-lto.ll
@@ -85,7 +85,7 @@
 
 declare dso_local noalias nonnull i8* @_Znwm(i64)
 
-!llvm.module.flags = !{!5}
+!llvm.module.flags = !{!5, !6}
 
 !0 = !{i64 16, !"_ZTS1A"}
 !1 = !{i64 16, !"_ZTSM1AFvvE.virtual"}
@@ -93,3 +93,4 @@
 !3 = !{i64 1} ; linkage-unit vcall visibility
 !4 = !{i64 2} ; translation-unit vcall visibility
 !5 = !{i32 1, !"LTOPostLink", i32 1}
+!6 = !{i32 1, !"Virtual Function Elim", i32 1}
Index: llvm/test/Transforms/GlobalDCE/virtual-functions.ll
===
--- llvm/test/Transforms/GlobalDCE/virtual-functions.ll
+++ 

[PATCH] D71907: [WPD/VFE] Always emit vcall_visibility metadata for -fwhole-program-vtables

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson marked 2 inline comments as done.
tejohnson added inline comments.



Comment at: llvm/lib/Transforms/IPO/GlobalSplit.cpp:116
+if (GV.hasMetadata(LLVMContext::MD_vcall_visibility))
+  SplitGV->setVCallVisibilityMetadata(GV.getVCallVisibility());
   }

evgeny777 wrote:
> I think this needs a test. Removal of this code doesn't break anything
Adding some checking in a GlobalSplit test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71907/new/

https://reviews.llvm.org/D71907



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71907: [WPD/VFE] Always emit vcall_visibility metadata for -fwhole-program-vtables

[Eugene Leviant via Phabricator via cfe-commits]

evgeny777 added inline comments.



Comment at: llvm/lib/Transforms/IPO/GlobalSplit.cpp:116
+if (GV.hasMetadata(LLVMContext::MD_vcall_visibility))
+  SplitGV->setVCallVisibilityMetadata(GV.getVCallVisibility());
   }

I think this needs a test. Removal of this code doesn't break anything


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71907/new/

https://reviews.llvm.org/D71907



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71907: [WPD/VFE] Always emit vcall_visibility metadata for -fwhole-program-vtables

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson updated this revision to Diff 239406.
tejohnson added a comment.

Remove stale change that didn't belong here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71907/new/

https://reviews.llvm.org/D71907

Files:
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGenCXX/vcall-visibility-metadata.cpp
  llvm/include/llvm/IR/GlobalObject.h
  llvm/lib/IR/Metadata.cpp
  llvm/lib/Transforms/IPO/GlobalDCE.cpp
  llvm/lib/Transforms/IPO/GlobalSplit.cpp
  llvm/test/Transforms/GlobalDCE/virtual-functions-base-call.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-base-pointer-call.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-derived-call.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-derived-pointer-call.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-novfe.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-post-lto.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-pre-lto.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions.ll
  llvm/test/Transforms/GlobalDCE/vtable-rtti.ll

Index: llvm/test/Transforms/GlobalDCE/vtable-rtti.ll
===
--- llvm/test/Transforms/GlobalDCE/vtable-rtti.ll
+++ llvm/test/Transforms/GlobalDCE/vtable-rtti.ll
@@ -39,9 +39,10 @@
 declare dso_local noalias nonnull i8* @_Znwm(i64)
 @_ZTVN10__cxxabiv117__class_type_infoE = external dso_local global i8*
 
-!llvm.module.flags = !{!3}
+!llvm.module.flags = !{!3, !4}
 
 !0 = !{i64 16, !"_ZTS1A"}
 !1 = !{i64 16, !"_ZTSM1AFvvE.virtual"}
 !2 = !{i64 2} ; translation-unit vcall visibility
 !3 = !{i32 1, !"LTOPostLink", i32 1}
+!4 = !{i32 1, !"Virtual Function Elim", i32 1}
Index: llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-pre-lto.ll
===
--- llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-pre-lto.ll
+++ llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-pre-lto.ll
@@ -85,10 +85,11 @@
 
 declare dso_local noalias nonnull i8* @_Znwm(i64)
 
-!llvm.module.flags = !{}
+!llvm.module.flags = !{!5}
 
 !0 = !{i64 16, !"_ZTS1A"}
 !1 = !{i64 16, !"_ZTSM1AFvvE.virtual"}
 !2 = !{i64 0} ; public vcall visibility
 !3 = !{i64 1} ; linkage-unit vcall visibility
 !4 = !{i64 2} ; translation-unit vcall visibility
+!5 = !{i32 1, !"Virtual Function Elim", i32 1}
Index: llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-post-lto.ll
===
--- llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-post-lto.ll
+++ llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-post-lto.ll
@@ -85,7 +85,7 @@
 
 declare dso_local noalias nonnull i8* @_Znwm(i64)
 
-!llvm.module.flags = !{!5}
+!llvm.module.flags = !{!5, !6}
 
 !0 = !{i64 16, !"_ZTS1A"}
 !1 = !{i64 16, !"_ZTSM1AFvvE.virtual"}
@@ -93,3 +93,4 @@
 !3 = !{i64 1} ; linkage-unit vcall visibility
 !4 = !{i64 2} ; translation-unit vcall visibility
 !5 = !{i32 1, !"LTOPostLink", i32 1}
+!6 = !{i32 1, !"Virtual Function Elim", i32 1}
Index: llvm/test/Transforms/GlobalDCE/virtual-functions.ll
===
--- llvm/test/Transforms/GlobalDCE/virtual-functions.ll
+++ llvm/test/Transforms/GlobalDCE/virtual-functions.ll
@@ -48,8 +48,11 @@
   ret i32 %call1
 }
 
+!llvm.module.flags = !{!4}
+
 !0 = !{i64 16, !"_ZTS1A"}
 !1 = !{i64 16, !"_ZTSM1AFivE.virtual"}
 !2 = !{i64 24, !"_ZTSM1AFivE.virtual"}
 !3 = !{i64 2}
+!4 = !{i32 1, !"Virtual Function Elim", i32 1}
 !9 = !{}
Index: llvm/test/Transforms/GlobalDCE/virtual-functions-novfe.ll
===
--- llvm/test/Transforms/GlobalDCE/virtual-functions-novfe.ll
+++ llvm/test/Transforms/GlobalDCE/virtual-functions-novfe.ll
@@ -1,3 +1,5 @@
+; Tests that VFE is not performed when the Virtual Function Elim metadata set
+; to 0. This is the same as virtual-functions.ll otherwise.
 ; RUN: opt < %s -globaldce -S | FileCheck %s
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
@@ -11,14 +13,12 @@
 ; intrinsic. Function test_A makes a call to A::foo, but there is no call to
 ; A::bar anywhere, so A::bar can be deleted, and its vtable slot replaced with
 ; null.
+; However, with the metadata set to 0 we should not perform this VFE.
 
 %struct.A = type { i32 (...)** }
 
-; The pointer to A::bar in the vtable can be removed, because it will never be
-; loaded. We replace it with null to keep the layout the same. Because it is at
-; the end of the vtable we could potentially shrink the vtable, but don't
-; currently do that.
-; CHECK: @_ZTV1A = internal unnamed_addr constant { [4 x i8*] } { [4 x i8*] [i8* null, i8* null, i8* bitcast (i32 (%struct.A*)* @_ZN1A3fooEv to i8*), i8* null] }
+; We should retain @_ZN1A3barEv in the vtable.
+; CHECK: @_ZTV1A = internal unnamed_addr constant { [4 x i8*] } { [4 x i8*] 

[PATCH] D71907: [WPD/VFE] Always emit vcall_visibility metadata for -fwhole-program-vtables

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson marked an inline comment as done.
tejohnson added inline comments.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:676
+  bool ShouldEmitWPDInfo = CGM.getCodeGenOpts().WholeProgramVTables &&
+   CGM.HasHiddenLTOVisibility(RD);
   llvm::Value *VirtualFn = nullptr;

evgeny777 wrote:
> Why are we checking for hidden visibility here?
This one is leftover from an earlier attempt at doing something more clever 
during GlobalOpt to decide whether we should be doing VFE or not, and I 
eventually abandoned that as it didn't work well, and went with the module flag 
here. You're right that we should be emitting the type test here under 
WholeProgramVtables, and not just with hidden visibility, but that belongs in 
D71913 (where I make the other change to insert type tests without hidden 
visibility). I've removed this change from this patch and am going to add it 
along with a test to that later patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71907/new/

https://reviews.llvm.org/D71907



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71907: [WPD/VFE] Always emit vcall_visibility metadata for -fwhole-program-vtables

[Eugene Leviant via Phabricator via cfe-commits]

evgeny777 added inline comments.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:676
+  bool ShouldEmitWPDInfo = CGM.getCodeGenOpts().WholeProgramVTables &&
+   CGM.HasHiddenLTOVisibility(RD);
   llvm::Value *VirtualFn = nullptr;

Why are we checking for hidden visibility here?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71907/new/

https://reviews.llvm.org/D71907



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71907: [WPD/VFE] Always emit vcall_visibility metadata for -fwhole-program-vtables

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson updated this revision to Diff 238561.
tejohnson added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71907/new/

https://reviews.llvm.org/D71907

Files:
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/vcall-visibility-metadata.cpp
  llvm/include/llvm/IR/GlobalObject.h
  llvm/lib/IR/Metadata.cpp
  llvm/lib/Transforms/IPO/GlobalDCE.cpp
  llvm/lib/Transforms/IPO/GlobalSplit.cpp
  llvm/test/Transforms/GlobalDCE/virtual-functions-base-call.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-base-pointer-call.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-derived-call.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-derived-pointer-call.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-novfe.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-post-lto.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-pre-lto.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions.ll
  llvm/test/Transforms/GlobalDCE/vtable-rtti.ll

Index: llvm/test/Transforms/GlobalDCE/vtable-rtti.ll
===
--- llvm/test/Transforms/GlobalDCE/vtable-rtti.ll
+++ llvm/test/Transforms/GlobalDCE/vtable-rtti.ll
@@ -39,9 +39,10 @@
 declare dso_local noalias nonnull i8* @_Znwm(i64)
 @_ZTVN10__cxxabiv117__class_type_infoE = external dso_local global i8*
 
-!llvm.module.flags = !{!3}
+!llvm.module.flags = !{!3, !4}
 
 !0 = !{i64 16, !"_ZTS1A"}
 !1 = !{i64 16, !"_ZTSM1AFvvE.virtual"}
 !2 = !{i64 2} ; translation-unit vcall visibility
 !3 = !{i32 1, !"LTOPostLink", i32 1}
+!4 = !{i32 1, !"Virtual Function Elim", i32 1}
Index: llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-pre-lto.ll
===
--- llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-pre-lto.ll
+++ llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-pre-lto.ll
@@ -85,10 +85,11 @@
 
 declare dso_local noalias nonnull i8* @_Znwm(i64)
 
-!llvm.module.flags = !{}
+!llvm.module.flags = !{!5}
 
 !0 = !{i64 16, !"_ZTS1A"}
 !1 = !{i64 16, !"_ZTSM1AFvvE.virtual"}
 !2 = !{i64 0} ; public vcall visibility
 !3 = !{i64 1} ; linkage-unit vcall visibility
 !4 = !{i64 2} ; translation-unit vcall visibility
+!5 = !{i32 1, !"Virtual Function Elim", i32 1}
Index: llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-post-lto.ll
===
--- llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-post-lto.ll
+++ llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-post-lto.ll
@@ -85,7 +85,7 @@
 
 declare dso_local noalias nonnull i8* @_Znwm(i64)
 
-!llvm.module.flags = !{!5}
+!llvm.module.flags = !{!5, !6}
 
 !0 = !{i64 16, !"_ZTS1A"}
 !1 = !{i64 16, !"_ZTSM1AFvvE.virtual"}
@@ -93,3 +93,4 @@
 !3 = !{i64 1} ; linkage-unit vcall visibility
 !4 = !{i64 2} ; translation-unit vcall visibility
 !5 = !{i32 1, !"LTOPostLink", i32 1}
+!6 = !{i32 1, !"Virtual Function Elim", i32 1}
Index: llvm/test/Transforms/GlobalDCE/virtual-functions.ll
===
--- llvm/test/Transforms/GlobalDCE/virtual-functions.ll
+++ llvm/test/Transforms/GlobalDCE/virtual-functions.ll
@@ -48,8 +48,11 @@
   ret i32 %call1
 }
 
+!llvm.module.flags = !{!4}
+
 !0 = !{i64 16, !"_ZTS1A"}
 !1 = !{i64 16, !"_ZTSM1AFivE.virtual"}
 !2 = !{i64 24, !"_ZTSM1AFivE.virtual"}
 !3 = !{i64 2}
+!4 = !{i32 1, !"Virtual Function Elim", i32 1}
 !9 = !{}
Index: llvm/test/Transforms/GlobalDCE/virtual-functions-novfe.ll
===
--- llvm/test/Transforms/GlobalDCE/virtual-functions-novfe.ll
+++ llvm/test/Transforms/GlobalDCE/virtual-functions-novfe.ll
@@ -1,3 +1,5 @@
+; Tests that VFE is not performed when the Virtual Function Elim metadata set
+; to 0. This is the same as virtual-functions.ll otherwise.
 ; RUN: opt < %s -globaldce -S | FileCheck %s
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
@@ -11,14 +13,12 @@
 ; intrinsic. Function test_A makes a call to A::foo, but there is no call to
 ; A::bar anywhere, so A::bar can be deleted, and its vtable slot replaced with
 ; null.
+; However, with the metadata set to 0 we should not perform this VFE.
 
 %struct.A = type { i32 (...)** }
 
-; The pointer to A::bar in the vtable can be removed, because it will never be
-; loaded. We replace it with null to keep the layout the same. Because it is at
-; the end of the vtable we could potentially shrink the vtable, but don't
-; currently do that.
-; CHECK: @_ZTV1A = internal unnamed_addr constant { [4 x i8*] } { [4 x i8*] [i8* null, i8* null, i8* bitcast (i32 (%struct.A*)* @_ZN1A3fooEv to i8*), i8* null] }
+; We should retain @_ZN1A3barEv in the vtable.
+; CHECK: @_ZTV1A = internal unnamed_addr constant { [4 x i8*] } { [4 x i8*] 

[PATCH] D71907: [WPD/VFE] Always emit vcall_visibility metadata for -fwhole-program-vtables

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

In D71907#1797218 , @evgeny777 wrote:

> Looks good at first sight. Do you have linker patch for me to try this out?


The linker changes are in D71913 . This one 
should be a noop by itself (other than just getting the additional metadata). 
You need all three (this one, D71911  and 
D71913 ) to implement the RFC.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71907/new/

https://reviews.llvm.org/D71907



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71907: [WPD/VFE] Always emit vcall_visibility metadata for -fwhole-program-vtables

[Eugene Leviant via Phabricator via cfe-commits]

evgeny777 added a comment.

Looks good at first sight. Do you have linker patch for me to try this out?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71907/new/

https://reviews.llvm.org/D71907



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71907: [WPD/VFE] Always emit vcall_visibility metadata for -fwhole-program-vtables

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson marked an inline comment as done.
tejohnson added inline comments.



Comment at: llvm/lib/IR/Metadata.cpp:1505
+  // updating.
+  eraseMetadata(LLVMContext::MD_vcall_visibility);
   addMetadata(LLVMContext::MD_vcall_visibility,

The erasing of old metadata is needed to enable upgrading the visibility to 
hidden (linkage unit) in a subsequent patch. Made that change here as well as 
the associated rename of the method to try to contain the main vcall visibility 
metadata changes in a single patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71907/new/

https://reviews.llvm.org/D71907



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71907: [WPD/VFE] Always emit vcall_visibility metadata for -fwhole-program-vtables

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson created this revision.
tejohnson added reviewers: pcc, ostannard, evgeny777, steven_wu.
Herald added subscribers: dexonsmith, hiraditya, Prazek, mehdi_amini.
Herald added projects: clang, LLVM.

First patch to support Safe Whole Program Devirtualization Enablement,
see RFC here: http://lists.llvm.org/pipermail/llvm-dev/2019-December/137543.html

Always emit !vcall_visibility metadata under -fwhole-program-vtables,
and not just for -fvirtual-function-elimination. The vcall visibility
metadata will (in a subsequent patch) be used to communicate to WPD
which vtables are safe to devirtualize, and we will optionally convert
the metadata to hidden visibility at link time. Subsequent follow on
patches will help enable this by adding vcall_visibility metadata to the
ThinLTO summaries, and always emit type test intrinsics under
-fwhole-program-vtables (and not just for vtables with hidden
visibility).

In order to do this safely with VFE, since for VFE all vtable loads must
be type checked loads which will no longer be the case, this patch adds
a new "Virtual Function Elim" module flag to communicate to GlobalDCE
whether to perform VFE using the vcall_visibility metadata.

One additional advantage of using the vcall_visibility metadata to drive
more WPD at LTO link time is that we can use the same mechanism to
enable more aggressive VFE at LTO link time as well. The link time
option proposed in the RFC will convert vcall_visibility metadata to
hidden (aka linkage unit visibility), which combined with
-fvirtual-function-elimination will allow it to be done more
aggressively at LTO link time under the same conditions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71907

Files:
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/vcall-visibility-metadata.cpp
  llvm/include/llvm/IR/GlobalObject.h
  llvm/lib/IR/Metadata.cpp
  llvm/lib/Transforms/IPO/GlobalDCE.cpp
  llvm/lib/Transforms/IPO/GlobalSplit.cpp
  llvm/test/Transforms/GlobalDCE/virtual-functions-base-call.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-base-pointer-call.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-derived-call.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-derived-pointer-call.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-novfe.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-post-lto.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-pre-lto.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions.ll
  llvm/test/Transforms/GlobalDCE/vtable-rtti.ll

Index: llvm/test/Transforms/GlobalDCE/vtable-rtti.ll
===
--- llvm/test/Transforms/GlobalDCE/vtable-rtti.ll
+++ llvm/test/Transforms/GlobalDCE/vtable-rtti.ll
@@ -39,9 +39,10 @@
 declare dso_local noalias nonnull i8* @_Znwm(i64)
 @_ZTVN10__cxxabiv117__class_type_infoE = external dso_local global i8*
 
-!llvm.module.flags = !{!3}
+!llvm.module.flags = !{!3, !4}
 
 !0 = !{i64 16, !"_ZTS1A"}
 !1 = !{i64 16, !"_ZTSM1AFvvE.virtual"}
 !2 = !{i64 2} ; translation-unit vcall visibility
 !3 = !{i32 1, !"LTOPostLink", i32 1}
+!4 = !{i32 1, !"Virtual Function Elim", i32 1}
Index: llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-pre-lto.ll
===
--- llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-pre-lto.ll
+++ llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-pre-lto.ll
@@ -85,10 +85,11 @@
 
 declare dso_local noalias nonnull i8* @_Znwm(i64)
 
-!llvm.module.flags = !{}
+!llvm.module.flags = !{!5}
 
 !0 = !{i64 16, !"_ZTS1A"}
 !1 = !{i64 16, !"_ZTSM1AFvvE.virtual"}
 !2 = !{i64 0} ; public vcall visibility
 !3 = !{i64 1} ; linkage-unit vcall visibility
 !4 = !{i64 2} ; translation-unit vcall visibility
+!5 = !{i32 1, !"Virtual Function Elim", i32 1}
Index: llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-post-lto.ll
===
--- llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-post-lto.ll
+++ llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-post-lto.ll
@@ -85,7 +85,7 @@
 
 declare dso_local noalias nonnull i8* @_Znwm(i64)
 
-!llvm.module.flags = !{!5}
+!llvm.module.flags = !{!5, !6}
 
 !0 = !{i64 16, !"_ZTS1A"}
 !1 = !{i64 16, !"_ZTSM1AFvvE.virtual"}
@@ -93,3 +93,4 @@
 !3 = !{i64 1} ; linkage-unit vcall visibility
 !4 = !{i64 2} ; translation-unit vcall visibility
 !5 = !{i32 1, !"LTOPostLink", i32 1}
+!6 = !{i32 1, !"Virtual Function Elim", i32 1}
Index: llvm/test/Transforms/GlobalDCE/virtual-functions.ll
===
--- llvm/test/Transforms/GlobalDCE/virtual-functions.ll
+++ llvm/test/Transforms/GlobalDCE/virtual-functions.ll
@@ -48,8 +48,11 @@
   ret i32 %call1
 }
 
+!llvm.module.flags = !{!4}
+
 !0 = !{i64 16, !"_ZTS1A"}
 !1 = !{i64 16,