[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-07-14 Thread Teresa Johnson via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6014c46c80ca: Restore [WPD/LowerTypeTests] Delay 
lowering/removal of type tests until after… (authored by tejohnson).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73242

Files:
  llvm/include/llvm/IR/ModuleSummaryIndex.h
  llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Transforms/IPO/LowerTypeTests.cpp
  llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
  llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
  llvm/test/Bitcode/summary_version.ll
  llvm/test/Other/new-pm-lto-defaults.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
  llvm/test/ThinLTO/X86/Inputs/cfi-unsat.ll
  llvm/test/ThinLTO/X86/cfi-unsat.ll
  llvm/test/ThinLTO/X86/nodevirt-nonpromoted-typeid.ll
  llvm/test/ThinLTO/X86/type_test_noindircall.ll
  llvm/test/Transforms/WholeProgramDevirt/branch-funnel.ll
  llvm/test/Transforms/WholeProgramDevirt/devirt-single-impl2.ll
  llvm/test/Transforms/WholeProgramDevirt/export-single-impl.ll
  llvm/test/Transforms/WholeProgramDevirt/export-uniform-ret-val.ll
  llvm/test/Transforms/WholeProgramDevirt/export-unique-ret-val.ll
  llvm/test/Transforms/WholeProgramDevirt/export-vcp.ll
  llvm/test/Transforms/WholeProgramDevirt/import-indir.ll
  llvm/test/Transforms/WholeProgramDevirt/uniform-retval.ll

Index: llvm/test/Transforms/WholeProgramDevirt/uniform-retval.ll
===
--- llvm/test/Transforms/WholeProgramDevirt/uniform-retval.ll
+++ llvm/test/Transforms/WholeProgramDevirt/uniform-retval.ll
@@ -25,7 +25,7 @@
   %fptr = load i8*, i8** %fptrptr
   %fptr_casted = bitcast i8* %fptr to i32 (i8*)*
   %result = call i32 %fptr_casted(i8* %obj)
-  ; CHECK-NOT: call
+  ; CHECK-NOT: call i32 %
   ; CHECK: ret i32 123
   ret i32 %result
 }
Index: llvm/test/Transforms/WholeProgramDevirt/import-indir.ll
===
--- llvm/test/Transforms/WholeProgramDevirt/import-indir.ll
+++ llvm/test/Transforms/WholeProgramDevirt/import-indir.ll
@@ -32,7 +32,7 @@
 ; SUMMARY-NEXT: TypeIdMap:
 ; SUMMARY-NEXT:   typeid1:
 ; SUMMARY-NEXT: TTRes:
-; SUMMARY-NEXT:   Kind:Unsat
+; SUMMARY-NEXT:   Kind:Unknown
 ; SUMMARY-NEXT:   SizeM1BitWidth:  0
 ; SUMMARY-NEXT:   AlignLog2:   0
 ; SUMMARY-NEXT:   SizeM1:  0
Index: llvm/test/Transforms/WholeProgramDevirt/export-vcp.ll
===
--- llvm/test/Transforms/WholeProgramDevirt/export-vcp.ll
+++ llvm/test/Transforms/WholeProgramDevirt/export-vcp.ll
@@ -9,7 +9,7 @@
 ; SUMMARY:  TypeIdMap:
 ; SUMMARY-NEXT:   typeid3:
 ; SUMMARY-NEXT: TTRes:
-; SUMMARY-NEXT:   Kind:Unsat
+; SUMMARY-NEXT:   Kind:Unknown
 ; SUMMARY-NEXT:   SizeM1BitWidth:  0
 ; SUMMARY-NEXT:   AlignLog2:   0
 ; SUMMARY-NEXT:   SizeM1:  0
@@ -29,7 +29,7 @@
 ; SUMMARY-ARM-NEXT: Bit: 1
 ; SUMMARY-NEXT:   typeid4:
 ; SUMMARY-NEXT: TTRes:
-; SUMMARY-NEXT:   Kind:Unsat
+; SUMMARY-NEXT:   Kind:Unknown
 ; SUMMARY-NEXT:   SizeM1BitWidth:  0
 ; SUMMARY-NEXT:   AlignLog2:   0
 ; SUMMARY-NEXT:   SizeM1:  0
Index: llvm/test/Transforms/WholeProgramDevirt/export-unique-ret-val.ll
===
--- llvm/test/Transforms/WholeProgramDevirt/export-unique-ret-val.ll
+++ llvm/test/Transforms/WholeProgramDevirt/export-unique-ret-val.ll
@@ -6,7 +6,7 @@
 ; SUMMARY:  TypeIdMap:
 ; SUMMARY-NEXT:   typeid3:
 ; SUMMARY-NEXT: TTRes:
-; SUMMARY-NEXT:   Kind:Unsat
+; SUMMARY-NEXT:   Kind:Unknown
 ; SUMMARY-NEXT:   SizeM1BitWidth:  0
 ; SUMMARY-NEXT:   AlignLog2:   0
 ; SUMMARY-NEXT:   SizeM1:  0
@@ -24,7 +24,7 @@
 ; SUMMARY-NEXT: Bit: 0
 ; SUMMARY-NEXT:   typeid4:
 ; SUMMARY-NEXT: TTRes:
-; SUMMARY-NEXT:   Kind:Unsat
+; SUMMARY-NEXT:   Kind:Unknown
 ; SUMMARY-NEXT:   SizeM1BitWidth:  0
 ; SUMMARY-NEXT:   AlignLog2:   0
 ; SUMMARY-NEXT:   SizeM1:  0
Index: llvm/test/Transforms/WholeProgramDevirt/export-uniform-ret-val.ll
===
--- llvm/test/Transforms/WholeProgramDevirt/export-uniform-ret-val.ll
+++ llvm/test/Transforms/WholeProgramDevirt/export-uniform-ret-val.ll
@@ -6,7 +6,7 @@
 ; SUMMARY:  TypeIdMap:
 ; SUMMARY-NEXT:   typeid4:
 ; SUMMARY-NEXT: TTRes:
-; SUMMARY-NEXT:   Kind:Unsat
+; SUMMARY-NEXT:   Kind:

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-07-08 Thread Eugene Leviant via Phabricator via cfe-commits 
evgeny777 accepted this revision.
evgeny777 added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73242



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

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-06-26 Thread Teresa Johnson via Phabricator via cfe-commits 
tejohnson marked an inline comment as done.
tejohnson added a comment.

In D73242#1963451 , @evgeny777 wrote:

> This needs to be rebased


Sorry for the delay, I've been side tracked on other work. Rebased and beefed 
up the test as requested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73242



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

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-06-26 Thread Teresa Johnson via Phabricator via cfe-commits 
tejohnson updated this revision to Diff 273730.
tejohnson added a comment.

Rebase and beef up test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73242

Files:
  llvm/include/llvm/IR/ModuleSummaryIndex.h
  llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Transforms/IPO/LowerTypeTests.cpp
  llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
  llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
  llvm/test/Bitcode/summary_version.ll
  llvm/test/Other/new-pm-lto-defaults.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
  llvm/test/ThinLTO/X86/Inputs/cfi-unsat.ll
  llvm/test/ThinLTO/X86/cfi-unsat.ll
  llvm/test/ThinLTO/X86/nodevirt-nonpromoted-typeid.ll
  llvm/test/ThinLTO/X86/type_test_noindircall.ll
  llvm/test/Transforms/WholeProgramDevirt/branch-funnel.ll
  llvm/test/Transforms/WholeProgramDevirt/devirt-single-impl2.ll
  llvm/test/Transforms/WholeProgramDevirt/export-single-impl.ll
  llvm/test/Transforms/WholeProgramDevirt/export-uniform-ret-val.ll
  llvm/test/Transforms/WholeProgramDevirt/export-unique-ret-val.ll
  llvm/test/Transforms/WholeProgramDevirt/export-vcp.ll
  llvm/test/Transforms/WholeProgramDevirt/import-indir.ll
  llvm/test/Transforms/WholeProgramDevirt/uniform-retval.ll

Index: llvm/test/Transforms/WholeProgramDevirt/uniform-retval.ll
===
--- llvm/test/Transforms/WholeProgramDevirt/uniform-retval.ll
+++ llvm/test/Transforms/WholeProgramDevirt/uniform-retval.ll
@@ -25,7 +25,7 @@
   %fptr = load i8*, i8** %fptrptr
   %fptr_casted = bitcast i8* %fptr to i32 (i8*)*
   %result = call i32 %fptr_casted(i8* %obj)
-  ; CHECK-NOT: call
+  ; CHECK-NOT: call i32 %
   ; CHECK: ret i32 123
   ret i32 %result
 }
Index: llvm/test/Transforms/WholeProgramDevirt/import-indir.ll
===
--- llvm/test/Transforms/WholeProgramDevirt/import-indir.ll
+++ llvm/test/Transforms/WholeProgramDevirt/import-indir.ll
@@ -32,7 +32,7 @@
 ; SUMMARY-NEXT: TypeIdMap:
 ; SUMMARY-NEXT:   typeid1:
 ; SUMMARY-NEXT: TTRes:
-; SUMMARY-NEXT:   Kind:Unsat
+; SUMMARY-NEXT:   Kind:Unknown
 ; SUMMARY-NEXT:   SizeM1BitWidth:  0
 ; SUMMARY-NEXT:   AlignLog2:   0
 ; SUMMARY-NEXT:   SizeM1:  0
Index: llvm/test/Transforms/WholeProgramDevirt/export-vcp.ll
===
--- llvm/test/Transforms/WholeProgramDevirt/export-vcp.ll
+++ llvm/test/Transforms/WholeProgramDevirt/export-vcp.ll
@@ -9,7 +9,7 @@
 ; SUMMARY:  TypeIdMap:
 ; SUMMARY-NEXT:   typeid3:
 ; SUMMARY-NEXT: TTRes:
-; SUMMARY-NEXT:   Kind:Unsat
+; SUMMARY-NEXT:   Kind:Unknown
 ; SUMMARY-NEXT:   SizeM1BitWidth:  0
 ; SUMMARY-NEXT:   AlignLog2:   0
 ; SUMMARY-NEXT:   SizeM1:  0
@@ -29,7 +29,7 @@
 ; SUMMARY-ARM-NEXT: Bit: 1
 ; SUMMARY-NEXT:   typeid4:
 ; SUMMARY-NEXT: TTRes:
-; SUMMARY-NEXT:   Kind:Unsat
+; SUMMARY-NEXT:   Kind:Unknown
 ; SUMMARY-NEXT:   SizeM1BitWidth:  0
 ; SUMMARY-NEXT:   AlignLog2:   0
 ; SUMMARY-NEXT:   SizeM1:  0
Index: llvm/test/Transforms/WholeProgramDevirt/export-unique-ret-val.ll
===
--- llvm/test/Transforms/WholeProgramDevirt/export-unique-ret-val.ll
+++ llvm/test/Transforms/WholeProgramDevirt/export-unique-ret-val.ll
@@ -6,7 +6,7 @@
 ; SUMMARY:  TypeIdMap:
 ; SUMMARY-NEXT:   typeid3:
 ; SUMMARY-NEXT: TTRes:
-; SUMMARY-NEXT:   Kind:Unsat
+; SUMMARY-NEXT:   Kind:Unknown
 ; SUMMARY-NEXT:   SizeM1BitWidth:  0
 ; SUMMARY-NEXT:   AlignLog2:   0
 ; SUMMARY-NEXT:   SizeM1:  0
@@ -24,7 +24,7 @@
 ; SUMMARY-NEXT: Bit: 0
 ; SUMMARY-NEXT:   typeid4:
 ; SUMMARY-NEXT: TTRes:
-; SUMMARY-NEXT:   Kind:Unsat
+; SUMMARY-NEXT:   Kind:Unknown
 ; SUMMARY-NEXT:   SizeM1BitWidth:  0
 ; SUMMARY-NEXT:   AlignLog2:   0
 ; SUMMARY-NEXT:   SizeM1:  0
Index: llvm/test/Transforms/WholeProgramDevirt/export-uniform-ret-val.ll
===
--- llvm/test/Transforms/WholeProgramDevirt/export-uniform-ret-val.ll
+++ llvm/test/Transforms/WholeProgramDevirt/export-uniform-ret-val.ll
@@ -6,7 +6,7 @@
 ; SUMMARY:  TypeIdMap:
 ; SUMMARY-NEXT:   typeid4:
 ; SUMMARY-NEXT: TTRes:
-; SUMMARY-NEXT:   Kind:Unsat
+; SUMMARY-NEXT:   Kind:Unknown
 ; SUMMARY-NEXT:   SizeM1BitWidth:  0
 ; SUMMARY-NEXT:   AlignLog2:   0
 ;

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-04-06 Thread Eugene Leviant via Phabricator via cfe-commits 
evgeny777 added a comment.

This needs to be rebased




Comment at: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp:1784
+  ImportSummary->getTypeIdSummary(cast(TypeId)->getString());
+  if (!TidSummary)
+RemoveTypeTestAssumes();

tejohnson wrote:
> Here is the fix for the issue with the multi-stage clang bootstrap test 
> failures described in D75201. I also restructured this code to try to make it 
> clearer (rather than an early continue when we don't need to remove type test 
> assumes, make it an explicit removal when needed).
> 
> Added new test llvm/test/ThinLTO/X86/type_test_noindircall.ll for this.
Can you please also test the case when LTO unit is split?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73242



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

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-03-22 Thread Teresa Johnson via Phabricator via cfe-commits 
tejohnson marked 2 inline comments as done.
tejohnson added a comment.

PTAL




Comment at: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp:1999
+WholeProgramDevirtResolution *Res = nullptr;
+if (ExportSummary && isa(S.first.TypeID))
+  // Create the type id summary resolution regardlness of whether we can

tejohnson wrote:
> The fix needed for the Chromium issue was to guard the TypeIdSummary creation 
> here by whether the TypeID exists in the TypeIdMap (which makes it match the 
> comment in fact), as we don't want to create a summary if the type id is not 
> used on a global (in which case it should in fact be Unsat). The equivalent 
> code in the index-only WPD is essentially already guarded by that condition, 
> because of the way the CallSlots are created (and in fact there is an assert 
> in that code that we have a use on a vtable, i.e. that a 
> TypeIdCompatibleVtableSummary is found for the TypeID).
Improved the fix I had made here for Chromium as it wasn't robust enough to 
handle all cases. Specifically, if we came through this code multiple times 
with the same type id, the TypeIdMap entry would no longer be missing since we 
unconditionally created it in the call to tryFindVirtualCallTargets below (via 
the operator[]). Changed it to check for a non-empty set (after more eagerly 
calling operator[]). I beefed up the test I had for this issue 
(llvm/test/ThinLTO/X86/cfi-unsat.ll) to cover this additional case.



Comment at: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp:1784
+  ImportSummary->getTypeIdSummary(cast(TypeId)->getString());
+  if (!TidSummary)
+RemoveTypeTestAssumes();

Here is the fix for the issue with the multi-stage clang bootstrap test 
failures described in D75201. I also restructured this code to try to make it 
clearer (rather than an early continue when we don't need to remove type test 
assumes, make it an explicit removal when needed).

Added new test llvm/test/ThinLTO/X86/type_test_noindircall.ll for this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73242



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

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-03-22 Thread Teresa Johnson via Phabricator via cfe-commits 
tejohnson updated this revision to Diff 251893.
tejohnson added a comment.

Includes fixe for 2-stage clang bootstrap test failures and an expanded
fix for Chromium issue, plus new tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73242

Files:
  llvm/include/llvm/IR/ModuleSummaryIndex.h
  llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Transforms/IPO/LowerTypeTests.cpp
  llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
  llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
  llvm/test/Bitcode/summary_version.ll
  llvm/test/Other/new-pm-lto-defaults.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
  llvm/test/ThinLTO/X86/Inputs/cfi-unsat.ll
  llvm/test/ThinLTO/X86/cfi-unsat.ll
  llvm/test/ThinLTO/X86/nodevirt-nonpromoted-typeid.ll
  llvm/test/ThinLTO/X86/type_test_noindircall.ll
  llvm/test/Transforms/WholeProgramDevirt/branch-funnel.ll
  llvm/test/Transforms/WholeProgramDevirt/devirt-single-impl2.ll
  llvm/test/Transforms/WholeProgramDevirt/export-single-impl.ll
  llvm/test/Transforms/WholeProgramDevirt/export-uniform-ret-val.ll
  llvm/test/Transforms/WholeProgramDevirt/export-unique-ret-val.ll
  llvm/test/Transforms/WholeProgramDevirt/export-vcp.ll
  llvm/test/Transforms/WholeProgramDevirt/import-indir.ll
  llvm/test/Transforms/WholeProgramDevirt/uniform-retval.ll

Index: llvm/test/Transforms/WholeProgramDevirt/uniform-retval.ll
===
--- llvm/test/Transforms/WholeProgramDevirt/uniform-retval.ll
+++ llvm/test/Transforms/WholeProgramDevirt/uniform-retval.ll
@@ -25,7 +25,7 @@
   %fptr = load i8*, i8** %fptrptr
   %fptr_casted = bitcast i8* %fptr to i32 (i8*)*
   %result = call i32 %fptr_casted(i8* %obj)
-  ; CHECK-NOT: call
+  ; CHECK-NOT: call i32 %
   ; CHECK: ret i32 123
   ret i32 %result
 }
Index: llvm/test/Transforms/WholeProgramDevirt/import-indir.ll
===
--- llvm/test/Transforms/WholeProgramDevirt/import-indir.ll
+++ llvm/test/Transforms/WholeProgramDevirt/import-indir.ll
@@ -32,7 +32,7 @@
 ; SUMMARY-NEXT: TypeIdMap:
 ; SUMMARY-NEXT:   typeid1:
 ; SUMMARY-NEXT: TTRes:
-; SUMMARY-NEXT:   Kind:Unsat
+; SUMMARY-NEXT:   Kind:Unknown
 ; SUMMARY-NEXT:   SizeM1BitWidth:  0
 ; SUMMARY-NEXT:   AlignLog2:   0
 ; SUMMARY-NEXT:   SizeM1:  0
Index: llvm/test/Transforms/WholeProgramDevirt/export-vcp.ll
===
--- llvm/test/Transforms/WholeProgramDevirt/export-vcp.ll
+++ llvm/test/Transforms/WholeProgramDevirt/export-vcp.ll
@@ -9,7 +9,7 @@
 ; SUMMARY:  TypeIdMap:
 ; SUMMARY-NEXT:   typeid3:
 ; SUMMARY-NEXT: TTRes:
-; SUMMARY-NEXT:   Kind:Unsat
+; SUMMARY-NEXT:   Kind:Unknown
 ; SUMMARY-NEXT:   SizeM1BitWidth:  0
 ; SUMMARY-NEXT:   AlignLog2:   0
 ; SUMMARY-NEXT:   SizeM1:  0
@@ -29,7 +29,7 @@
 ; SUMMARY-ARM-NEXT: Bit: 1
 ; SUMMARY-NEXT:   typeid4:
 ; SUMMARY-NEXT: TTRes:
-; SUMMARY-NEXT:   Kind:Unsat
+; SUMMARY-NEXT:   Kind:Unknown
 ; SUMMARY-NEXT:   SizeM1BitWidth:  0
 ; SUMMARY-NEXT:   AlignLog2:   0
 ; SUMMARY-NEXT:   SizeM1:  0
Index: llvm/test/Transforms/WholeProgramDevirt/export-unique-ret-val.ll
===
--- llvm/test/Transforms/WholeProgramDevirt/export-unique-ret-val.ll
+++ llvm/test/Transforms/WholeProgramDevirt/export-unique-ret-val.ll
@@ -6,7 +6,7 @@
 ; SUMMARY:  TypeIdMap:
 ; SUMMARY-NEXT:   typeid3:
 ; SUMMARY-NEXT: TTRes:
-; SUMMARY-NEXT:   Kind:Unsat
+; SUMMARY-NEXT:   Kind:Unknown
 ; SUMMARY-NEXT:   SizeM1BitWidth:  0
 ; SUMMARY-NEXT:   AlignLog2:   0
 ; SUMMARY-NEXT:   SizeM1:  0
@@ -24,7 +24,7 @@
 ; SUMMARY-NEXT: Bit: 0
 ; SUMMARY-NEXT:   typeid4:
 ; SUMMARY-NEXT: TTRes:
-; SUMMARY-NEXT:   Kind:Unsat
+; SUMMARY-NEXT:   Kind:Unknown
 ; SUMMARY-NEXT:   SizeM1BitWidth:  0
 ; SUMMARY-NEXT:   AlignLog2:   0
 ; SUMMARY-NEXT:   SizeM1:  0
Index: llvm/test/Transforms/WholeProgramDevirt/export-uniform-ret-val.ll
===
--- llvm/test/Transforms/WholeProgramDevirt/export-uniform-ret-val.ll
+++ llvm/test/Transforms/WholeProgramDevirt/export-uniform-ret-val.ll
@@ -6,7 +6,7 @@
 ; SUMMARY:  TypeIdMap:
 ; SUMMARY-NEXT:   typeid4:
 ; SUMMARY-NEXT: TTRes:
-; SUMMARY-NEXT:   Kind:Unsat
+; SUMMARY-NEXT:   Kind:Unknown
 ;

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-03-22 Thread Teresa Johnson via Phabricator via cfe-commits 
tejohnson reopened this revision.
tejohnson added a comment.
This revision is now accepted and ready to land.

Updating with fixes since this was reverted in 
80bf137fa132ea33204e98bbefa924afe9258a4e 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73242



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

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-03-02 Thread Teresa Johnson via Phabricator via cfe-commits 
tejohnson added a comment.

Reverted in 80bf137fa132ea33204e98bbefa924afe9258a4e 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73242



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

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-02-26 Thread Teresa Johnson via Phabricator via cfe-commits 
tejohnson added a comment.

In D73242#1894146 , @tejohnson wrote:

> Sent fix for review in D75201 .
>
> If the new problem showed up later and not when this one first went in, then 
> it seems likely it is different than this issue.  I'll see if I can reproduce 
> and take a look.


I am not seeing this, although my client is from Friday. So it is likely 
something committed subsequently that caused it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73242



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

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-02-26 Thread Teresa Johnson via Phabricator via cfe-commits 
tejohnson added a comment.

Sent fix for review in D75201 .

If the new problem showed up later and not when this one first went in, then it 
seems likely it is different than this issue.  I'll see if I can reproduce and 
take a look.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73242



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

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-02-26 Thread Alexandre Ganea via Phabricator via cfe-commits 
aganea added a comment.

In D73242#1893506 , @tejohnson wrote:

> In D73242#1888295 , @tejohnson wrote:
>
> > FYI I have reproduced the failure, and am starting to debug.
>
>
> I see what is going on and have a simple fix, just need to create a test 
> case. Expect to send a patch to fix this today, hopefully this morning.


Thanks! Let me know, I'll test right away.

I'm seeing another (new?) bug with latest trunk, I'm not sure if it's related 
or not. The repro is the same, just sync the latest, and do the two-stage 
build, then `ninja check-lld`, all the LTO tests fail with this stack:

   #0 0x7ff63bd7e891 `anonymous namespace'::RealFile::`scalar deleting 
dtor'(unsigned int) 
(d:\llvm-project\buildninjarelmimalloc\bin\ld.lld.exe+0x1e891)
   #1 0x7ff63c47d4ac llvm::FPPassManager::~FPPassManager 
D:\llvm-project\llvm\include\llvm\IR\LegacyPassManagers.h:461:0
   #2 0x7ff63c47c98c `anonymous namespace'::MPPassManager::~MPPassManager 
D:\llvm-project\llvm\lib\IR\LegacyPassManager.cpp:407:0
   #3 0x7ff63c475fac llvm::PMTopLevelManager::~PMTopLevelManager(void) 
D:\llvm-project\llvm\lib\IR\LegacyPassManager.cpp:855:0
   #4 0x7ff63c47c3bc llvm::legacy::FunctionPassManagerImpl::`scalar 
deleting dtor'(unsigned int) 
D:\llvm-project\llvm\lib\IR\LegacyPassManager.cpp:324:0
   #5 0x7ff63c39aa41 `anonymous namespace'::codegen 
D:\llvm-project\llvm\lib\LTO\LTOBackend.cpp:373:0
   #6 0x7ff63c39732c llvm::lto::backend(struct llvm::lto::Config const &, 
class std::function<(unsigned int)>, unsigned int, class std::unique_ptr>, class 
llvm::ModuleSummaryIndex &) D:\llvm-project\llvm\lib\LTO\LTOBackend.cpp:466:0
   #7 0x7ff63be8a49a llvm::lto::LTO::run(class std::function<(unsigned 
int)>, class std::function<(unsigned int, class llvm::StringRef)>) 
D:\llvm-project\llvm\lib\LTO\LTO.cpp:942:0
   #8 0x7ff63c153eb5 lld::elf::BitcodeCompiler::compile(void) 
D:\llvm-project\lld\ELF\LTO.cpp:261:0
   #9 0x7ff63bdc0b01 lld::elf::LinkerDriver::main 
D:\llvm-project\lld\ELF\Driver.cpp:522:0
  #10 0x7ff63bdba6dc lld::elf::link(class llvm::ArrayRef, 
bool, class llvm::raw_ostream &, class llvm::raw_ostream &) 
D:\llvm-project\lld\ELF\Driver.cpp:117:0
  #11 0x7ff63bd61807 main D:\llvm-project\lld\tools\lld\lld.cpp:166:0
  #12 0x7ff63e9cf914 __scrt_common_main_seh 
d:\agent\_work\5\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288:0
  #13 0x7ffa95737bd4 (C:\WINDOWS\System32\KERNEL32.DLL+0x17bd4)
  #14 0x7ffa972eced1 (C:\WINDOWS\SYSTEM32\ntdll.dll+0x6ced1)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73242



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

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-02-26 Thread Teresa Johnson via Phabricator via cfe-commits 
tejohnson added a comment.

In D73242#1888295 , @tejohnson wrote:

> FYI I have reproduced the failure, and am starting to debug.


I see what is going on and have a simple fix, just need to create a test case. 
Expect to send a patch to fix this today, hopefully this morning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73242



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

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-02-23 Thread Teresa Johnson via Phabricator via cfe-commits 
tejohnson added a comment.

FYI I have reproduced the failure, and am starting to debug.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73242



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

RE: [PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-02-20 Thread Alexandre Ganea via cfe-commits 
No need to revert, I can wait.
Thanks!

De : Teresa Johnson 
Envoyé : February 20, 2020 5:07 PM
À : reviews+d73242+public+5822844df8563...@reviews.llvm.org
Cc : Peter Collingbourne ; Evgeny Leviant 
; Alexandre Ganea ; 
joker@gmail.com; piotr.padlew...@gmail.com; hiradi...@msn.com; 
steve...@apple.com; dexonsm...@apple.com; arpha...@gmail.com; 
davi...@google.com; cfe-commits@lists.llvm.org; llvm-comm...@lists.llvm.org; 
mlek...@skidmore.edu; blitzrak...@gmail.com; shen...@google.com; 
ju...@samsung.com; l...@inglorion.net; yuanfang.c...@sony.com
Objet : Re: [PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type 
tests until after ICP

I'm currently traveling but will take a look tomorrow. If necessary go ahead 
and revert, I will not be able to do so myself until tomorrow. Teresa

On Tue, Feb 18, 2020, 8:48 PM Alexandre Ganea via Phabricator 
mailto:revi...@reviews.llvm.org>> wrote:
aganea added a comment.

There seems to be still an issue with this patch, when linking the LLVM unit 
tests on a two-stage build, it ends with:

  FAILED: tools/clang/unittests/StaticAnalyzer/StaticAnalysisTests.exe
  cmd.exe /C "cd . && "C:\Program Files\CMake\bin\cmake.exe" -E vs_link_exe 
--intdir=tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir
 --rc=C:\PROGRA~2\WI3CF2~1\10\bin\100183~1.0\x64\rc.exe 
--mt=C:\PROGRA~2\WI3CF2~1\10\bin\100183~1.0\x64\mt.exe --manifests  -- 
D:\llvm-project\buildninjaRel\bin\lld-link.exe /nologo 
tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir\AnalyzerOptionsTest.cpp.obj
 
tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir\CallDescriptionTest.cpp.obj
 
tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir\StoreTest.cpp.obj
 
tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir\RegisterCustomCheckersTest.cpp.obj
 
tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir\SymbolReaperTest.cpp.obj
 
tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir\D_\llvm-project\llvm\resources\windows_version_resource.rc.res
  /out:tools\clang\unittests\StaticAnalyzer\StaticAnalysisTests.exe 
/implib:tools\clang\unittests\StaticAnalyzer\StaticAnalysisTests.lib 
/pdb:tools\clang\unittests\StaticAnalyzer\StaticAnalysisTests.pdb /version:0.0  
/machine:x64 -fuse-ld=lld /STACK:1000 /DEBUG /OPT:REF /OPT:ICF 
/lldltocache:D:/llvm-project/buildninjaRelMiMalloc/lto.cache /INCREMENTAL:NO 
/subsystem:console  lib\LLVMSupport.lib  lib\LLVMSupport.lib  
lib\gtest_main.lib  lib\gtest.lib  lib\clangBasic.lib  lib\clangAnalysis.lib  
lib\clangAST.lib  lib\clangASTMatchers.lib  lib\clangCrossTU.lib  
lib\clangFrontend.lib  lib\clangSerialization.lib  
lib\clangStaticAnalyzerCore.lib  lib\clangStaticAnalyzerFrontend.lib  
lib\clangTooling.lib  lib\clangStaticAnalyzerCheckers.lib  
lib\clangStaticAnalyzerCore.lib  lib\clangCrossTU.lib  lib\clangIndex.lib  
lib\clangFrontend.lib  lib\clangParse.lib  lib\clangSerialization.lib  
lib\clangSema.lib  lib\clangAnalysis.lib  lib\clangASTMatchers.lib  
lib\clangEdit.lib  lib\clangDriver.lib  version.lib  lib\LLVMOption.lib  
lib\clangFormat.lib  lib\clangToolingInclusions.lib  lib\clangToolingCore.lib  
lib\clangAST.lib  lib\LLVMFrontendOpenMP.lib  lib\LLVMTransformUtils.lib  
lib\LLVMAnalysis.lib  lib\LLVMProfileData.lib  lib\LLVMObject.lib  
lib\LLVMBitReader.lib  lib\LLVMMCParser.lib  lib\LLVMTextAPI.lib  
lib\clangRewrite.lib  lib\clangLex.lib  lib\clangBasic.lib  lib\LLVMCore.lib  
lib\LLVMRemarks.lib  lib\LLVMBitstreamReader.lib  lib\LLVMMC.lib  
lib\LLVMBinaryFormat.lib  lib\LLVMDebugInfoCodeView.lib  
lib\LLVMDebugInfoMSF.lib  lib\LLVMSupport.lib  psapi.lib  shell32.lib  
ole32.lib  uuid.lib  advapi32.lib  delayimp.lib  -delayload:shell32.dll  
-delayload:ole32.dll  lib\LLVMDemangle.lib  kernel32.lib user32.lib gdi32.lib 
winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib 
advapi32.lib && cd ."
  LINK: command "D:\llvm-project\buildninjaRel\bin\lld-link.exe /nologo 
tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir\AnalyzerOptionsTest.cpp.obj
 
tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir\CallDescriptionTest.cpp.obj
 
tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir\StoreTest.cpp.obj
 
tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir\RegisterCustomCheckersTest.cpp.obj
 
tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir\SymbolReaperTest.cpp.obj
 
tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir\D_\llvm-project\llvm\resources\windows_version_resource.rc.res
 /out:tools\clang\unittests\StaticAnalyzer\StaticAnalysisTests.exe 
/implib:tools\clang\unittests\StaticAnalyzer\StaticAnalysisTests.lib 
/pdb:tools\clang\unittests\StaticAnalyzer\StaticAnalysisTests.pdb /version:0.0 
/machine:x64 -fuse-ld=lld /STACK:1000

Re: [PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-02-20 Thread Teresa Johnson via cfe-commits 
I'm currently traveling but will take a look tomorrow. If necessary go
ahead and revert, I will not be able to do so myself until tomorrow. Teresa

On Tue, Feb 18, 2020, 8:48 PM Alexandre Ganea via Phabricator <
revi...@reviews.llvm.org> wrote:

> aganea added a comment.
>
> There seems to be still an issue with this patch, when linking the LLVM
> unit tests on a two-stage build, it ends with:
>
>   FAILED: tools/clang/unittests/StaticAnalyzer/StaticAnalysisTests.exe
>   cmd.exe /C "cd . && "C:\Program Files\CMake\bin\cmake.exe" -E
> vs_link_exe
> --intdir=tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir
> --rc=C:\PROGRA~2\WI3CF2~1\10\bin\100183~1.0\x64\rc.exe
> --mt=C:\PROGRA~2\WI3CF2~1\10\bin\100183~1.0\x64\mt.exe --manifests  --
> D:\llvm-project\buildninjaRel\bin\lld-link.exe /nologo
> tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir\AnalyzerOptionsTest.cpp.obj
> tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir\CallDescriptionTest.cpp.obj
> tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir\StoreTest.cpp.obj
> tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir\RegisterCustomCheckersTest.cpp.obj
> tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir\SymbolReaperTest.cpp.obj
> tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir\D_\llvm-project\llvm\resources\windows_version_resource.rc.res
> /out:tools\clang\unittests\StaticAnalyzer\StaticAnalysisTests.exe
> /implib:tools\clang\unittests\StaticAnalyzer\StaticAnalysisTests.lib
> /pdb:tools\clang\unittests\StaticAnalyzer\StaticAnalysisTests.pdb
> /version:0.0  /machine:x64 -fuse-ld=lld /STACK:1000 /DEBUG /OPT:REF
> /OPT:ICF /lldltocache:D:/llvm-project/buildninjaRelMiMalloc/lto.cache
> /INCREMENTAL:NO /subsystem:console  lib\LLVMSupport.lib
> lib\LLVMSupport.lib  lib\gtest_main.lib  lib\gtest.lib  lib\clangBasic.lib
> lib\clangAnalysis.lib  lib\clangAST.lib  lib\clangASTMatchers.lib
> lib\clangCrossTU.lib  lib\clangFrontend.lib  lib\clangSerialization.lib
> lib\clangStaticAnalyzerCore.lib  lib\clangStaticAnalyzerFrontend.lib
> lib\clangTooling.lib  lib\clangStaticAnalyzerCheckers.lib
> lib\clangStaticAnalyzerCore.lib  lib\clangCrossTU.lib  lib\clangIndex.lib
> lib\clangFrontend.lib  lib\clangParse.lib  lib\clangSerialization.lib
> lib\clangSema.lib  lib\clangAnalysis.lib  lib\clangASTMatchers.lib
> lib\clangEdit.lib  lib\clangDriver.lib  version.lib  lib\LLVMOption.lib
> lib\clangFormat.lib  lib\clangToolingInclusions.lib
> lib\clangToolingCore.lib  lib\clangAST.lib  lib\LLVMFrontendOpenMP.lib
> lib\LLVMTransformUtils.lib  lib\LLVMAnalysis.lib  lib\LLVMProfileData.lib
> lib\LLVMObject.lib  lib\LLVMBitReader.lib  lib\LLVMMCParser.lib
> lib\LLVMTextAPI.lib  lib\clangRewrite.lib  lib\clangLex.lib
> lib\clangBasic.lib  lib\LLVMCore.lib  lib\LLVMRemarks.lib
> lib\LLVMBitstreamReader.lib  lib\LLVMMC.lib  lib\LLVMBinaryFormat.lib
> lib\LLVMDebugInfoCodeView.lib  lib\LLVMDebugInfoMSF.lib
> lib\LLVMSupport.lib  psapi.lib  shell32.lib  ole32.lib  uuid.lib
> advapi32.lib  delayimp.lib  -delayload:shell32.dll  -delayload:ole32.dll
> lib\LLVMDemangle.lib  kernel32.lib user32.lib gdi32.lib winspool.lib
> shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib && cd
> ."
>   LINK: command "D:\llvm-project\buildninjaRel\bin\lld-link.exe /nologo
> tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir\AnalyzerOptionsTest.cpp.obj
> tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir\CallDescriptionTest.cpp.obj
> tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir\StoreTest.cpp.obj
> tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir\RegisterCustomCheckersTest.cpp.obj
> tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir\SymbolReaperTest.cpp.obj
> tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir\D_\llvm-project\llvm\resources\windows_version_resource.rc.res
> /out:tools\clang\unittests\StaticAnalyzer\StaticAnalysisTests.exe
> /implib:tools\clang\unittests\StaticAnalyzer\StaticAnalysisTests.lib
> /pdb:tools\clang\unittests\StaticAnalyzer\StaticAnalysisTests.pdb
> /version:0.0 /machine:x64 -fuse-ld=lld /STACK:1000 /DEBUG /OPT:REF
> /OPT:ICF /lldltocache:D:/llvm-project/buildninjaRelMiMalloc/lto.cache
> /INCREMENTAL:NO /subsystem:console lib\LLVMSupport.lib lib\LLVMSupport.lib
> lib\gtest_main.lib lib\gtest.lib lib\clangBasic.lib lib\clangAnalysis.lib
> lib\clangAST.lib lib\clangASTMatchers.lib lib\clangCrossTU.lib
> lib\clangFrontend.lib lib\clangSerialization.lib
> lib\clangStaticAnalyzerCore.lib lib\clangStaticAnalyzerFrontend.lib
> lib\clangTooling.lib lib\clangStaticAnalyzerCheckers.lib
> lib\clangStaticAnalyzerCore.lib lib\clangCrossTU.lib lib\clangIndex.lib
> lib\clangFrontend.lib lib\clangParse.lib lib\clangSerialization.lib
> lib\clangSema.lib lib\clangAnalysis.lib

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-02-18 Thread Alexandre Ganea via Phabricator via cfe-commits 
aganea added a comment.

There seems to be still an issue with this patch, when linking the LLVM unit 
tests on a two-stage build, it ends with:

  FAILED: tools/clang/unittests/StaticAnalyzer/StaticAnalysisTests.exe
  cmd.exe /C "cd . && "C:\Program Files\CMake\bin\cmake.exe" -E vs_link_exe 
--intdir=tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir
 --rc=C:\PROGRA~2\WI3CF2~1\10\bin\100183~1.0\x64\rc.exe 
--mt=C:\PROGRA~2\WI3CF2~1\10\bin\100183~1.0\x64\mt.exe --manifests  -- 
D:\llvm-project\buildninjaRel\bin\lld-link.exe /nologo 
tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir\AnalyzerOptionsTest.cpp.obj
 
tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir\CallDescriptionTest.cpp.obj
 
tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir\StoreTest.cpp.obj
 
tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir\RegisterCustomCheckersTest.cpp.obj
 
tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir\SymbolReaperTest.cpp.obj
 
tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir\D_\llvm-project\llvm\resources\windows_version_resource.rc.res
  /out:tools\clang\unittests\StaticAnalyzer\StaticAnalysisTests.exe 
/implib:tools\clang\unittests\StaticAnalyzer\StaticAnalysisTests.lib 
/pdb:tools\clang\unittests\StaticAnalyzer\StaticAnalysisTests.pdb /version:0.0  
/machine:x64 -fuse-ld=lld /STACK:1000 /DEBUG /OPT:REF /OPT:ICF 
/lldltocache:D:/llvm-project/buildninjaRelMiMalloc/lto.cache /INCREMENTAL:NO 
/subsystem:console  lib\LLVMSupport.lib  lib\LLVMSupport.lib  
lib\gtest_main.lib  lib\gtest.lib  lib\clangBasic.lib  lib\clangAnalysis.lib  
lib\clangAST.lib  lib\clangASTMatchers.lib  lib\clangCrossTU.lib  
lib\clangFrontend.lib  lib\clangSerialization.lib  
lib\clangStaticAnalyzerCore.lib  lib\clangStaticAnalyzerFrontend.lib  
lib\clangTooling.lib  lib\clangStaticAnalyzerCheckers.lib  
lib\clangStaticAnalyzerCore.lib  lib\clangCrossTU.lib  lib\clangIndex.lib  
lib\clangFrontend.lib  lib\clangParse.lib  lib\clangSerialization.lib  
lib\clangSema.lib  lib\clangAnalysis.lib  lib\clangASTMatchers.lib  
lib\clangEdit.lib  lib\clangDriver.lib  version.lib  lib\LLVMOption.lib  
lib\clangFormat.lib  lib\clangToolingInclusions.lib  lib\clangToolingCore.lib  
lib\clangAST.lib  lib\LLVMFrontendOpenMP.lib  lib\LLVMTransformUtils.lib  
lib\LLVMAnalysis.lib  lib\LLVMProfileData.lib  lib\LLVMObject.lib  
lib\LLVMBitReader.lib  lib\LLVMMCParser.lib  lib\LLVMTextAPI.lib  
lib\clangRewrite.lib  lib\clangLex.lib  lib\clangBasic.lib  lib\LLVMCore.lib  
lib\LLVMRemarks.lib  lib\LLVMBitstreamReader.lib  lib\LLVMMC.lib  
lib\LLVMBinaryFormat.lib  lib\LLVMDebugInfoCodeView.lib  
lib\LLVMDebugInfoMSF.lib  lib\LLVMSupport.lib  psapi.lib  shell32.lib  
ole32.lib  uuid.lib  advapi32.lib  delayimp.lib  -delayload:shell32.dll  
-delayload:ole32.dll  lib\LLVMDemangle.lib  kernel32.lib user32.lib gdi32.lib 
winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib 
advapi32.lib && cd ."
  LINK: command "D:\llvm-project\buildninjaRel\bin\lld-link.exe /nologo 
tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir\AnalyzerOptionsTest.cpp.obj
 
tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir\CallDescriptionTest.cpp.obj
 
tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir\StoreTest.cpp.obj
 
tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir\RegisterCustomCheckersTest.cpp.obj
 
tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir\SymbolReaperTest.cpp.obj
 
tools\clang\unittests\StaticAnalyzer\CMakeFiles\StaticAnalysisTests.dir\D_\llvm-project\llvm\resources\windows_version_resource.rc.res
 /out:tools\clang\unittests\StaticAnalyzer\StaticAnalysisTests.exe 
/implib:tools\clang\unittests\StaticAnalyzer\StaticAnalysisTests.lib 
/pdb:tools\clang\unittests\StaticAnalyzer\StaticAnalysisTests.pdb /version:0.0 
/machine:x64 -fuse-ld=lld /STACK:1000 /DEBUG /OPT:REF /OPT:ICF 
/lldltocache:D:/llvm-project/buildninjaRelMiMalloc/lto.cache /INCREMENTAL:NO 
/subsystem:console lib\LLVMSupport.lib lib\LLVMSupport.lib lib\gtest_main.lib 
lib\gtest.lib lib\clangBasic.lib lib\clangAnalysis.lib lib\clangAST.lib 
lib\clangASTMatchers.lib lib\clangCrossTU.lib lib\clangFrontend.lib 
lib\clangSerialization.lib lib\clangStaticAnalyzerCore.lib 
lib\clangStaticAnalyzerFrontend.lib lib\clangTooling.lib 
lib\clangStaticAnalyzerCheckers.lib lib\clangStaticAnalyzerCore.lib 
lib\clangCrossTU.lib lib\clangIndex.lib lib\clangFrontend.lib 
lib\clangParse.lib lib\clangSerialization.lib lib\clangSema.lib 
lib\clangAnalysis.lib lib\clangASTMatchers.lib lib\clangEdit.lib 
lib\clangDriver.lib version.lib lib\LLVMOption.lib lib\clangFormat.lib 
lib\clangToolingInclusions.lib lib\clangToolingCore.lib lib\clangAST.lib 
lib\LLVMFrontendOpenMP.lib lib\LLVMTransformUtils.lib lib\LLVMAnalysis.lib 
lib\LLVMProfileData.lib

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-02-11 Thread Teresa Johnson via Phabricator via cfe-commits 
tejohnson marked an inline comment as done.
tejohnson added a comment.

In D73242#1861125 , @tejohnson wrote:

> In D73242#186 , @thakis wrote:
>
> > This makes lld crash when linking chromium's base_unittests and probably 
> > does the same for most other binaries that use both thinlto and cfi: 
> > https://bugs.chromium.org/p/chromium/issues/detail?id=1049434
>
>
> Reverted at 25aa2eef993e17708889abf56ed1ffad5074a9f4 
> . Will 
> investigate using repro @thakis sent off patch.


Recommitted with fix and additional test case at 
80d0a137a5aba6998fadb764f1e11cb901aae233 
.




Comment at: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp:1999
+WholeProgramDevirtResolution *Res = nullptr;
+if (ExportSummary && isa(S.first.TypeID))
+  // Create the type id summary resolution regardlness of whether we can

The fix needed for the Chromium issue was to guard the TypeIdSummary creation 
here by whether the TypeID exists in the TypeIdMap (which makes it match the 
comment in fact), as we don't want to create a summary if the type id is not 
used on a global (in which case it should in fact be Unsat). The equivalent 
code in the index-only WPD is essentially already guarded by that condition, 
because of the way the CallSlots are created (and in fact there is an assert in 
that code that we have a use on a vtable, i.e. that a 
TypeIdCompatibleVtableSummary is found for the TypeID).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73242



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

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-02-05 Thread Teresa Johnson via Phabricator via cfe-commits 
tejohnson added a comment.

In D73242#186 , @thakis wrote:

> This makes lld crash when linking chromium's base_unittests and probably does 
> the same for most other binaries that use both thinlto and cfi: 
> https://bugs.chromium.org/p/chromium/issues/detail?id=1049434


Reverted at 25aa2eef993e17708889abf56ed1ffad5074a9f4 
. Will 
investigate using repro @thakis sent off patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73242



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

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-02-05 Thread Nico Weber via Phabricator via cfe-commits 
thakis added a comment.

This makes lld crash when linking chromium's base_unittests and probably does 
the same for most other binaries that use both thinlto and cfi: 
https://bugs.chromium.org/p/chromium/issues/detail?id=1049434


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73242



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

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-02-05 Thread Teresa Johnson via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG748bb5a0f196: [WPD/LowerTypeTests] Delay lowering/removal of 
type tests until after ICP (authored by tejohnson).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73242

Files:
  llvm/include/llvm/IR/ModuleSummaryIndex.h
  llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Transforms/IPO/LowerTypeTests.cpp
  llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
  llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
  llvm/test/Bitcode/summary_version.ll
  llvm/test/Other/new-pm-lto-defaults.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
  llvm/test/Transforms/WholeProgramDevirt/branch-funnel.ll
  llvm/test/Transforms/WholeProgramDevirt/devirt-single-impl2.ll
  llvm/test/Transforms/WholeProgramDevirt/export-single-impl.ll
  llvm/test/Transforms/WholeProgramDevirt/export-uniform-ret-val.ll
  llvm/test/Transforms/WholeProgramDevirt/export-unique-ret-val.ll
  llvm/test/Transforms/WholeProgramDevirt/export-vcp.ll
  llvm/test/Transforms/WholeProgramDevirt/import-indir.ll
  llvm/test/Transforms/WholeProgramDevirt/uniform-retval.ll

Index: llvm/test/Transforms/WholeProgramDevirt/uniform-retval.ll
===
--- llvm/test/Transforms/WholeProgramDevirt/uniform-retval.ll
+++ llvm/test/Transforms/WholeProgramDevirt/uniform-retval.ll
@@ -25,7 +25,7 @@
   %fptr = load i8*, i8** %fptrptr
   %fptr_casted = bitcast i8* %fptr to i32 (i8*)*
   %result = call i32 %fptr_casted(i8* %obj)
-  ; CHECK-NOT: call
+  ; CHECK-NOT: call i32 %
   ; CHECK: ret i32 123
   ret i32 %result
 }
Index: llvm/test/Transforms/WholeProgramDevirt/import-indir.ll
===
--- llvm/test/Transforms/WholeProgramDevirt/import-indir.ll
+++ llvm/test/Transforms/WholeProgramDevirt/import-indir.ll
@@ -32,7 +32,7 @@
 ; SUMMARY-NEXT: TypeIdMap:
 ; SUMMARY-NEXT:   typeid1:
 ; SUMMARY-NEXT: TTRes:
-; SUMMARY-NEXT:   Kind:Unsat
+; SUMMARY-NEXT:   Kind:Unknown
 ; SUMMARY-NEXT:   SizeM1BitWidth:  0
 ; SUMMARY-NEXT:   AlignLog2:   0
 ; SUMMARY-NEXT:   SizeM1:  0
Index: llvm/test/Transforms/WholeProgramDevirt/export-vcp.ll
===
--- llvm/test/Transforms/WholeProgramDevirt/export-vcp.ll
+++ llvm/test/Transforms/WholeProgramDevirt/export-vcp.ll
@@ -9,7 +9,7 @@
 ; SUMMARY:  TypeIdMap:
 ; SUMMARY-NEXT:   typeid3:
 ; SUMMARY-NEXT: TTRes:
-; SUMMARY-NEXT:   Kind:Unsat
+; SUMMARY-NEXT:   Kind:Unknown
 ; SUMMARY-NEXT:   SizeM1BitWidth:  0
 ; SUMMARY-NEXT:   AlignLog2:   0
 ; SUMMARY-NEXT:   SizeM1:  0
@@ -29,7 +29,7 @@
 ; SUMMARY-ARM-NEXT: Bit: 1
 ; SUMMARY-NEXT:   typeid4:
 ; SUMMARY-NEXT: TTRes:
-; SUMMARY-NEXT:   Kind:Unsat
+; SUMMARY-NEXT:   Kind:Unknown
 ; SUMMARY-NEXT:   SizeM1BitWidth:  0
 ; SUMMARY-NEXT:   AlignLog2:   0
 ; SUMMARY-NEXT:   SizeM1:  0
Index: llvm/test/Transforms/WholeProgramDevirt/export-unique-ret-val.ll
===
--- llvm/test/Transforms/WholeProgramDevirt/export-unique-ret-val.ll
+++ llvm/test/Transforms/WholeProgramDevirt/export-unique-ret-val.ll
@@ -6,7 +6,7 @@
 ; SUMMARY:  TypeIdMap:
 ; SUMMARY-NEXT:   typeid3:
 ; SUMMARY-NEXT: TTRes:
-; SUMMARY-NEXT:   Kind:Unsat
+; SUMMARY-NEXT:   Kind:Unknown
 ; SUMMARY-NEXT:   SizeM1BitWidth:  0
 ; SUMMARY-NEXT:   AlignLog2:   0
 ; SUMMARY-NEXT:   SizeM1:  0
@@ -24,7 +24,7 @@
 ; SUMMARY-NEXT: Bit: 0
 ; SUMMARY-NEXT:   typeid4:
 ; SUMMARY-NEXT: TTRes:
-; SUMMARY-NEXT:   Kind:Unsat
+; SUMMARY-NEXT:   Kind:Unknown
 ; SUMMARY-NEXT:   SizeM1BitWidth:  0
 ; SUMMARY-NEXT:   AlignLog2:   0
 ; SUMMARY-NEXT:   SizeM1:  0
Index: llvm/test/Transforms/WholeProgramDevirt/export-uniform-ret-val.ll
===
--- llvm/test/Transforms/WholeProgramDevirt/export-uniform-ret-val.ll
+++ llvm/test/Transforms/WholeProgramDevirt/export-uniform-ret-val.ll
@@ -6,7 +6,7 @@
 ; SUMMARY:  TypeIdMap:
 ; SUMMARY-NEXT:   typeid4:
 ; SUMMARY-NEXT: TTRes:
-; SUMMARY-NEXT:   Kind:Unsat
+; SUMMARY-NEXT:   Kind:Unknown
 ; SUMMARY-NEXT:   SizeM1BitWidth:  0
 ; SUMMARY-NEXT:   AlignLog2:   0
 ; SUMMARY-NEXT:   SizeM1:  0
Index:

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-02-05 Thread 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/D73242/new/

https://reviews.llvm.org/D73242



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

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-02-05 Thread Teresa Johnson via Phabricator via cfe-commits 
tejohnson marked an inline comment as done.
tejohnson added inline comments.



Comment at: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp:1678
+// breaks any uses on assumes.
+if (TypeIdMap.count(TypeId))
+  continue;

evgeny777 wrote:
> tejohnson wrote:
> > evgeny777 wrote:
> > > tejohnson wrote:
> > > > evgeny777 wrote:
> > > > > tejohnson wrote:
> > > > > > evgeny777 wrote:
> > > > > > > I don't think, I understand this.
> > > > > > > It looks like that if (a) we have type.test in the module and (b) 
> > > > > > > we don't have vtable definition with corresponding type metadata 
> > > > > > > in the same module then we'll remove type.test/assume sequence 
> > > > > > > before even getting to ICP. This seems strange in the context of 
> > > > > > > previous discussion because virtual function may be called in 
> > > > > > > different module from one where vtable is defined, like so:
> > > > > > > ```
> > > > > > > struct A { virtual int foo() { return 42; } };
> > > > > > > 
> > > > > > > // calls pA->foo
> > > > > > > extern int callFoo(A *pA);
> > > > > > > int main() { A a; return callFoo(); }
> > > > > > > ```
> > > > > > > 
> > > > > > We will only be able to do ICP this way if we have the target 
> > > > > > vtable in the module (similar to how we currently can only do ICP 
> > > > > > if the target function is in the module). I anticipate doing 
> > > > > > something similar for vtable defs as to what we do for virtual 
> > > > > > functions, where a fake call edge is added to the summary based on 
> > > > > > the value profile results to ensure they are imported before the 
> > > > > > LTO backend ICP.
> > > > > > We will only be able to do ICP this way if we have the target 
> > > > > > vtable in the module (similar to how we currently can only do ICP 
> > > > > > if the target function is in the module).
> > > > > 
> > > > > I was thinking of slightly different approach: it seems you have 
> > > > > required type information in combined index together with type name 
> > > > > in the module, so you probably can add external declarations of 
> > > > > required vtables (this may require promotion) to the module in the 
> > > > > ICP pass and run ICP over them. Do you think this can work?
> > > > Possibly, but we'd still need to have type metadata on those vtable 
> > > > declarations, because the type metadata provides the address point 
> > > > offset which is needed in the comparison sequence.
> > > > Possibly, but we'd still need to have type metadata on those vtable 
> > > > declarations, because the type metadata provides the address point 
> > > > offset which is needed in the comparison sequence.
> > > 
> > > I think it's stored in the index in VFuncId structures. Isn't it?
> > > I think it's stored in the index in VFuncId structures. Isn't it?
> > 
> > No, that holds the offset from the address point to the virtual function 
> > within the vtable def, not the address point offset itself, which is what 
> > we need from the type metadata. But actually, we need both offsets:
> > 
> > Consider the following example:
> > 
> > ```
> > class A {
> >virtual void foo();
> > }
> > 
> > void bar(A *a) {
> >a->foo();
> > }
> > ```
> > 
> > The indirect call sequence will look like (not showing the type test for 
> > simplicity):
> > 
> > ```
> >  %0 = bitcast %class.A* %a to i32 (%class.A*)***
> >   %vtable = load i32 (%class.A*)**, i32 (%class.A*)*** %0
> >   %1 = load i32 (%class.A*)*, i32 (%class.A*)** %vtable
> >   %call = tail call i32 %1(%class.A* %a)
> > ```
> > 
> > With type profiling we will profile the value of %vtable, and figure out 
> > that it is associated with the symbol for A's vtable (the profiling support 
> > uses symbol table info for this), which is:
> > 
> > ```
> > @_ZTV1A = dso_local unnamed_addr constant { [3 x i8*] } { [3 x i8*] [i8* 
> > null, i8* bitcast ({ i8*, i8* }* @_ZTI1A to i8*), i8* bitcast (i32 
> > (%class.A*)* @_ZN1A3fooEv to i8*)] }, align 8, !type !0
> > 
> > !0 = !{i64 16, !"_ZTS1A"}
> > ```
> > 
> > When we promote the call using the vtable profile results, we need to add 
> > the address point offset (16) from the type metadata to the profiled result 
> > which will be the symbol @_ZTV1A, before comparing against %vtable.
> > 
> > We then need to look at the IR and compute the offset to the function 
> > address used there (0 in the above IR), and add it to the address point 
> > offset, resulting in 16 here, looking in the vtable def to see what 
> > function is at the combined offset (_ZN1A3fooEv here), so that we insert a 
> > direct call to that function.
> > 
> > For the address point offset, we only have it in the summary for index-only 
> > WPD (TypeIdCompatibleVtableMap). However, I don't want to restrict the ICP 
> > improvements to that build mode.
> > 
> > For the latter, the VFuncId does summarize the offsets, but we can compute 
> > it from the IR as I described above (it's the amount

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-02-05 Thread Eugene Leviant via Phabricator via cfe-commits 
evgeny777 added inline comments.



Comment at: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp:1678
+// breaks any uses on assumes.
+if (TypeIdMap.count(TypeId))
+  continue;

tejohnson wrote:
> evgeny777 wrote:
> > tejohnson wrote:
> > > evgeny777 wrote:
> > > > tejohnson wrote:
> > > > > evgeny777 wrote:
> > > > > > I don't think, I understand this.
> > > > > > It looks like that if (a) we have type.test in the module and (b) 
> > > > > > we don't have vtable definition with corresponding type metadata in 
> > > > > > the same module then we'll remove type.test/assume sequence before 
> > > > > > even getting to ICP. This seems strange in the context of previous 
> > > > > > discussion because virtual function may be called in different 
> > > > > > module from one where vtable is defined, like so:
> > > > > > ```
> > > > > > struct A { virtual int foo() { return 42; } };
> > > > > > 
> > > > > > // calls pA->foo
> > > > > > extern int callFoo(A *pA);
> > > > > > int main() { A a; return callFoo(); }
> > > > > > ```
> > > > > > 
> > > > > We will only be able to do ICP this way if we have the target vtable 
> > > > > in the module (similar to how we currently can only do ICP if the 
> > > > > target function is in the module). I anticipate doing something 
> > > > > similar for vtable defs as to what we do for virtual functions, where 
> > > > > a fake call edge is added to the summary based on the value profile 
> > > > > results to ensure they are imported before the LTO backend ICP.
> > > > > We will only be able to do ICP this way if we have the target vtable 
> > > > > in the module (similar to how we currently can only do ICP if the 
> > > > > target function is in the module).
> > > > 
> > > > I was thinking of slightly different approach: it seems you have 
> > > > required type information in combined index together with type name in 
> > > > the module, so you probably can add external declarations of required 
> > > > vtables (this may require promotion) to the module in the ICP pass and 
> > > > run ICP over them. Do you think this can work?
> > > Possibly, but we'd still need to have type metadata on those vtable 
> > > declarations, because the type metadata provides the address point offset 
> > > which is needed in the comparison sequence.
> > > Possibly, but we'd still need to have type metadata on those vtable 
> > > declarations, because the type metadata provides the address point offset 
> > > which is needed in the comparison sequence.
> > 
> > I think it's stored in the index in VFuncId structures. Isn't it?
> > I think it's stored in the index in VFuncId structures. Isn't it?
> 
> No, that holds the offset from the address point to the virtual function 
> within the vtable def, not the address point offset itself, which is what we 
> need from the type metadata. But actually, we need both offsets:
> 
> Consider the following example:
> 
> ```
> class A {
>virtual void foo();
> }
> 
> void bar(A *a) {
>a->foo();
> }
> ```
> 
> The indirect call sequence will look like (not showing the type test for 
> simplicity):
> 
> ```
>  %0 = bitcast %class.A* %a to i32 (%class.A*)***
>   %vtable = load i32 (%class.A*)**, i32 (%class.A*)*** %0
>   %1 = load i32 (%class.A*)*, i32 (%class.A*)** %vtable
>   %call = tail call i32 %1(%class.A* %a)
> ```
> 
> With type profiling we will profile the value of %vtable, and figure out that 
> it is associated with the symbol for A's vtable (the profiling support uses 
> symbol table info for this), which is:
> 
> ```
> @_ZTV1A = dso_local unnamed_addr constant { [3 x i8*] } { [3 x i8*] [i8* 
> null, i8* bitcast ({ i8*, i8* }* @_ZTI1A to i8*), i8* bitcast (i32 
> (%class.A*)* @_ZN1A3fooEv to i8*)] }, align 8, !type !0
> 
> !0 = !{i64 16, !"_ZTS1A"}
> ```
> 
> When we promote the call using the vtable profile results, we need to add the 
> address point offset (16) from the type metadata to the profiled result which 
> will be the symbol @_ZTV1A, before comparing against %vtable.
> 
> We then need to look at the IR and compute the offset to the function address 
> used there (0 in the above IR), and add it to the address point offset, 
> resulting in 16 here, looking in the vtable def to see what function is at 
> the combined offset (_ZN1A3fooEv here), so that we insert a direct call to 
> that function.
> 
> For the address point offset, we only have it in the summary for index-only 
> WPD (TypeIdCompatibleVtableMap). However, I don't want to restrict the ICP 
> improvements to that build mode.
> 
> For the latter, the VFuncId does summarize the offsets, but we can compute it 
> from the IR as I described above (it's the amount added to %vtable before 
> loading the virtual function pointer) and don't need the summary. And in any 
> case the VFuncId doesn't tell us which function is at the aggregate offset, 
> we need the vtable def to tell us that (or the vTableFuncs array saved on the 
>

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-02-04 Thread Teresa Johnson via Phabricator via cfe-commits 
tejohnson updated this revision to Diff 242493.
tejohnson added a comment.

Rebase and implement suggestion (move Unknown to end and bump index version)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73242

Files:
  llvm/include/llvm/IR/ModuleSummaryIndex.h
  llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Transforms/IPO/LowerTypeTests.cpp
  llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
  llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
  llvm/test/Bitcode/summary_version.ll
  llvm/test/Other/new-pm-lto-defaults.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
  llvm/test/Transforms/WholeProgramDevirt/branch-funnel.ll
  llvm/test/Transforms/WholeProgramDevirt/devirt-single-impl2.ll
  llvm/test/Transforms/WholeProgramDevirt/export-single-impl.ll
  llvm/test/Transforms/WholeProgramDevirt/export-uniform-ret-val.ll
  llvm/test/Transforms/WholeProgramDevirt/export-unique-ret-val.ll
  llvm/test/Transforms/WholeProgramDevirt/export-vcp.ll
  llvm/test/Transforms/WholeProgramDevirt/import-indir.ll
  llvm/test/Transforms/WholeProgramDevirt/uniform-retval.ll

Index: llvm/test/Transforms/WholeProgramDevirt/uniform-retval.ll
===
--- llvm/test/Transforms/WholeProgramDevirt/uniform-retval.ll
+++ llvm/test/Transforms/WholeProgramDevirt/uniform-retval.ll
@@ -25,7 +25,7 @@
   %fptr = load i8*, i8** %fptrptr
   %fptr_casted = bitcast i8* %fptr to i32 (i8*)*
   %result = call i32 %fptr_casted(i8* %obj)
-  ; CHECK-NOT: call
+  ; CHECK-NOT: call i32 %
   ; CHECK: ret i32 123
   ret i32 %result
 }
Index: llvm/test/Transforms/WholeProgramDevirt/import-indir.ll
===
--- llvm/test/Transforms/WholeProgramDevirt/import-indir.ll
+++ llvm/test/Transforms/WholeProgramDevirt/import-indir.ll
@@ -32,7 +32,7 @@
 ; SUMMARY-NEXT: TypeIdMap:
 ; SUMMARY-NEXT:   typeid1:
 ; SUMMARY-NEXT: TTRes:
-; SUMMARY-NEXT:   Kind:Unsat
+; SUMMARY-NEXT:   Kind:Unknown
 ; SUMMARY-NEXT:   SizeM1BitWidth:  0
 ; SUMMARY-NEXT:   AlignLog2:   0
 ; SUMMARY-NEXT:   SizeM1:  0
Index: llvm/test/Transforms/WholeProgramDevirt/export-vcp.ll
===
--- llvm/test/Transforms/WholeProgramDevirt/export-vcp.ll
+++ llvm/test/Transforms/WholeProgramDevirt/export-vcp.ll
@@ -9,7 +9,7 @@
 ; SUMMARY:  TypeIdMap:
 ; SUMMARY-NEXT:   typeid3:
 ; SUMMARY-NEXT: TTRes:
-; SUMMARY-NEXT:   Kind:Unsat
+; SUMMARY-NEXT:   Kind:Unknown
 ; SUMMARY-NEXT:   SizeM1BitWidth:  0
 ; SUMMARY-NEXT:   AlignLog2:   0
 ; SUMMARY-NEXT:   SizeM1:  0
@@ -29,7 +29,7 @@
 ; SUMMARY-ARM-NEXT: Bit: 1
 ; SUMMARY-NEXT:   typeid4:
 ; SUMMARY-NEXT: TTRes:
-; SUMMARY-NEXT:   Kind:Unsat
+; SUMMARY-NEXT:   Kind:Unknown
 ; SUMMARY-NEXT:   SizeM1BitWidth:  0
 ; SUMMARY-NEXT:   AlignLog2:   0
 ; SUMMARY-NEXT:   SizeM1:  0
Index: llvm/test/Transforms/WholeProgramDevirt/export-unique-ret-val.ll
===
--- llvm/test/Transforms/WholeProgramDevirt/export-unique-ret-val.ll
+++ llvm/test/Transforms/WholeProgramDevirt/export-unique-ret-val.ll
@@ -6,7 +6,7 @@
 ; SUMMARY:  TypeIdMap:
 ; SUMMARY-NEXT:   typeid3:
 ; SUMMARY-NEXT: TTRes:
-; SUMMARY-NEXT:   Kind:Unsat
+; SUMMARY-NEXT:   Kind:Unknown
 ; SUMMARY-NEXT:   SizeM1BitWidth:  0
 ; SUMMARY-NEXT:   AlignLog2:   0
 ; SUMMARY-NEXT:   SizeM1:  0
@@ -24,7 +24,7 @@
 ; SUMMARY-NEXT: Bit: 0
 ; SUMMARY-NEXT:   typeid4:
 ; SUMMARY-NEXT: TTRes:
-; SUMMARY-NEXT:   Kind:Unsat
+; SUMMARY-NEXT:   Kind:Unknown
 ; SUMMARY-NEXT:   SizeM1BitWidth:  0
 ; SUMMARY-NEXT:   AlignLog2:   0
 ; SUMMARY-NEXT:   SizeM1:  0
Index: llvm/test/Transforms/WholeProgramDevirt/export-uniform-ret-val.ll
===
--- llvm/test/Transforms/WholeProgramDevirt/export-uniform-ret-val.ll
+++ llvm/test/Transforms/WholeProgramDevirt/export-uniform-ret-val.ll
@@ -6,7 +6,7 @@
 ; SUMMARY:  TypeIdMap:
 ; SUMMARY-NEXT:   typeid4:
 ; SUMMARY-NEXT: TTRes:
-; SUMMARY-NEXT:   Kind:Unsat
+; SUMMARY-NEXT:   Kind:Unknown
 ; SUMMARY-NEXT:   SizeM1BitWidth:  0
 ; SUMMARY-NEXT:   AlignLog2:   0
 ; SUMMARY-NEXT:   SizeM1:  0
Index: llvm/test/Transforms/WholeProgramDevirt/export-single-impl.ll

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-02-04 Thread Teresa Johnson via Phabricator via cfe-commits 
tejohnson marked an inline comment as done.
tejohnson added inline comments.



Comment at: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp:1678
+// breaks any uses on assumes.
+if (TypeIdMap.count(TypeId))
+  continue;

evgeny777 wrote:
> tejohnson wrote:
> > evgeny777 wrote:
> > > tejohnson wrote:
> > > > evgeny777 wrote:
> > > > > I don't think, I understand this.
> > > > > It looks like that if (a) we have type.test in the module and (b) we 
> > > > > don't have vtable definition with corresponding type metadata in the 
> > > > > same module then we'll remove type.test/assume sequence before even 
> > > > > getting to ICP. This seems strange in the context of previous 
> > > > > discussion because virtual function may be called in different module 
> > > > > from one where vtable is defined, like so:
> > > > > ```
> > > > > struct A { virtual int foo() { return 42; } };
> > > > > 
> > > > > // calls pA->foo
> > > > > extern int callFoo(A *pA);
> > > > > int main() { A a; return callFoo(); }
> > > > > ```
> > > > > 
> > > > We will only be able to do ICP this way if we have the target vtable in 
> > > > the module (similar to how we currently can only do ICP if the target 
> > > > function is in the module). I anticipate doing something similar for 
> > > > vtable defs as to what we do for virtual functions, where a fake call 
> > > > edge is added to the summary based on the value profile results to 
> > > > ensure they are imported before the LTO backend ICP.
> > > > We will only be able to do ICP this way if we have the target vtable in 
> > > > the module (similar to how we currently can only do ICP if the target 
> > > > function is in the module).
> > > 
> > > I was thinking of slightly different approach: it seems you have required 
> > > type information in combined index together with type name in the module, 
> > > so you probably can add external declarations of required vtables (this 
> > > may require promotion) to the module in the ICP pass and run ICP over 
> > > them. Do you think this can work?
> > Possibly, but we'd still need to have type metadata on those vtable 
> > declarations, because the type metadata provides the address point offset 
> > which is needed in the comparison sequence.
> > Possibly, but we'd still need to have type metadata on those vtable 
> > declarations, because the type metadata provides the address point offset 
> > which is needed in the comparison sequence.
> 
> I think it's stored in the index in VFuncId structures. Isn't it?
> I think it's stored in the index in VFuncId structures. Isn't it?

No, that holds the offset from the address point to the virtual function within 
the vtable def, not the address point offset itself, which is what we need from 
the type metadata. But actually, we need both offsets:

Consider the following example:

```
class A {
   virtual void foo();
}

void bar(A *a) {
   a->foo();
}
```

The indirect call sequence will look like (not showing the type test for 
simplicity):

```
 %0 = bitcast %class.A* %a to i32 (%class.A*)***
  %vtable = load i32 (%class.A*)**, i32 (%class.A*)*** %0
  %1 = load i32 (%class.A*)*, i32 (%class.A*)** %vtable
  %call = tail call i32 %1(%class.A* %a)
```

With type profiling we will profile the value of %vtable, and figure out that 
it is associated with the symbol for A's vtable (the profiling support uses 
symbol table info for this), which is:

```
@_ZTV1A = dso_local unnamed_addr constant { [3 x i8*] } { [3 x i8*] [i8* null, 
i8* bitcast ({ i8*, i8* }* @_ZTI1A to i8*), i8* bitcast (i32 (%class.A*)* 
@_ZN1A3fooEv to i8*)] }, align 8, !type !0

!0 = !{i64 16, !"_ZTS1A"}
```

When we promote the call using the vtable profile results, we need to add the 
address point offset (16) from the type metadata to the profiled result which 
will be the symbol @_ZTV1A, before comparing against %vtable.

We then need to look at the IR and compute the offset to the function address 
used there (0 in the above IR), and add it to the address point offset, 
resulting in 16 here, looking in the vtable def to see what function is at the 
combined offset (_ZN1A3fooEv here), so that we insert a direct call to that 
function.

For the address point offset, we only have it in the summary for index-only WPD 
(TypeIdCompatibleVtableMap). However, I don't want to restrict the ICP 
improvements to that build mode.

For the latter, the VFuncId does summarize the offsets, but we can compute it 
from the IR as I described above (it's the amount added to %vtable before 
loading the virtual function pointer) and don't need the summary. And in any 
case the VFuncId doesn't tell us which function is at the aggregate offset, we 
need the vtable def to tell us that (or the vTableFuncs array saved on the 
GlobalVarSummary, again only in index-only WPD mode).

Also, I believe we can support this type profiling based ICP in non-ThinLTO 
mode as well - with my recent changes to make WPD rely on

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-02-04 Thread Eugene Leviant via Phabricator via cfe-commits 
evgeny777 added inline comments.



Comment at: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp:1678
+// breaks any uses on assumes.
+if (TypeIdMap.count(TypeId))
+  continue;

tejohnson wrote:
> evgeny777 wrote:
> > tejohnson wrote:
> > > evgeny777 wrote:
> > > > I don't think, I understand this.
> > > > It looks like that if (a) we have type.test in the module and (b) we 
> > > > don't have vtable definition with corresponding type metadata in the 
> > > > same module then we'll remove type.test/assume sequence before even 
> > > > getting to ICP. This seems strange in the context of previous 
> > > > discussion because virtual function may be called in different module 
> > > > from one where vtable is defined, like so:
> > > > ```
> > > > struct A { virtual int foo() { return 42; } };
> > > > 
> > > > // calls pA->foo
> > > > extern int callFoo(A *pA);
> > > > int main() { A a; return callFoo(); }
> > > > ```
> > > > 
> > > We will only be able to do ICP this way if we have the target vtable in 
> > > the module (similar to how we currently can only do ICP if the target 
> > > function is in the module). I anticipate doing something similar for 
> > > vtable defs as to what we do for virtual functions, where a fake call 
> > > edge is added to the summary based on the value profile results to ensure 
> > > they are imported before the LTO backend ICP.
> > > We will only be able to do ICP this way if we have the target vtable in 
> > > the module (similar to how we currently can only do ICP if the target 
> > > function is in the module).
> > 
> > I was thinking of slightly different approach: it seems you have required 
> > type information in combined index together with type name in the module, 
> > so you probably can add external declarations of required vtables (this may 
> > require promotion) to the module in the ICP pass and run ICP over them. Do 
> > you think this can work?
> Possibly, but we'd still need to have type metadata on those vtable 
> declarations, because the type metadata provides the address point offset 
> which is needed in the comparison sequence.
> Possibly, but we'd still need to have type metadata on those vtable 
> declarations, because the type metadata provides the address point offset 
> which is needed in the comparison sequence.

I think it's stored in the index in VFuncId structures. Isn't it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73242



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

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-02-04 Thread Teresa Johnson via Phabricator via cfe-commits 
tejohnson marked an inline comment as done.
tejohnson added inline comments.



Comment at: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp:1678
+// breaks any uses on assumes.
+if (TypeIdMap.count(TypeId))
+  continue;

evgeny777 wrote:
> tejohnson wrote:
> > evgeny777 wrote:
> > > I don't think, I understand this.
> > > It looks like that if (a) we have type.test in the module and (b) we 
> > > don't have vtable definition with corresponding type metadata in the same 
> > > module then we'll remove type.test/assume sequence before even getting to 
> > > ICP. This seems strange in the context of previous discussion because 
> > > virtual function may be called in different module from one where vtable 
> > > is defined, like so:
> > > ```
> > > struct A { virtual int foo() { return 42; } };
> > > 
> > > // calls pA->foo
> > > extern int callFoo(A *pA);
> > > int main() { A a; return callFoo(); }
> > > ```
> > > 
> > We will only be able to do ICP this way if we have the target vtable in the 
> > module (similar to how we currently can only do ICP if the target function 
> > is in the module). I anticipate doing something similar for vtable defs as 
> > to what we do for virtual functions, where a fake call edge is added to the 
> > summary based on the value profile results to ensure they are imported 
> > before the LTO backend ICP.
> > We will only be able to do ICP this way if we have the target vtable in the 
> > module (similar to how we currently can only do ICP if the target function 
> > is in the module).
> 
> I was thinking of slightly different approach: it seems you have required 
> type information in combined index together with type name in the module, so 
> you probably can add external declarations of required vtables (this may 
> require promotion) to the module in the ICP pass and run ICP over them. Do 
> you think this can work?
Possibly, but we'd still need to have type metadata on those vtable 
declarations, because the type metadata provides the address point offset which 
is needed in the comparison sequence.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73242



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

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-02-04 Thread Eugene Leviant via Phabricator via cfe-commits 
evgeny777 added inline comments.



Comment at: llvm/lib/Passes/PassBuilder.cpp:1380
+  // in ICP (which is performed earlier than this in the regular LTO pipeline).
+  MPM.addPass(LowerTypeTestsPass(nullptr, nullptr, true));
 

tejohnson wrote:
> evgeny777 wrote:
> > Can we get rid of two identical passes here (and in other places also)? For 
> > instance 
> > ```
> > MPM.addPass(LowerTypeTestsPass(ExportSummary, nullptr, true));
> > ```
> > can both lower type metadata and do final cleanup.
> The problem is that currently under DropTypeTests=true, LTT will drop the 
> type tests up front and return without doing any other lowering. Which is 
> what we want in the already existing callers of LTT with DropTypeTests=true. 
> Here, we want LTT to perform its usual lowering, and only afterwards do the 
> dropping. We want to do this only during regular LTO, but unfortunately we 
> can't just key off of the ExportSummary (e.g. drop at the start if 
> ExportSummary is nullptr, and drop at the end if ExportSummary is 
> non-nullptr), since in some cases here ExportSummary may be nullptr (i.e. 
> regular LTO invoked via the old LTO API). So I would need to introduce 
> another option to essentially say "drop them after lowering". I can certainly 
> do this if you think it would be better (I was thinking about that before, 
> but thought it might be more confusing). WDYT?
> So I would need to introduce another option to essentially say "drop them 
> after lowering". I can certainly do this if you think it would be better (I 
> was thinking about that before, but thought it might be more confusing). WDYT?

Ok, I see. I don't think having this another option is significantly better. So 
let's leave it as is for now



Comment at: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp:1678
+// breaks any uses on assumes.
+if (TypeIdMap.count(TypeId))
+  continue;

tejohnson wrote:
> evgeny777 wrote:
> > I don't think, I understand this.
> > It looks like that if (a) we have type.test in the module and (b) we don't 
> > have vtable definition with corresponding type metadata in the same module 
> > then we'll remove type.test/assume sequence before even getting to ICP. 
> > This seems strange in the context of previous discussion because virtual 
> > function may be called in different module from one where vtable is 
> > defined, like so:
> > ```
> > struct A { virtual int foo() { return 42; } };
> > 
> > // calls pA->foo
> > extern int callFoo(A *pA);
> > int main() { A a; return callFoo(); }
> > ```
> > 
> We will only be able to do ICP this way if we have the target vtable in the 
> module (similar to how we currently can only do ICP if the target function is 
> in the module). I anticipate doing something similar for vtable defs as to 
> what we do for virtual functions, where a fake call edge is added to the 
> summary based on the value profile results to ensure they are imported before 
> the LTO backend ICP.
> We will only be able to do ICP this way if we have the target vtable in the 
> module (similar to how we currently can only do ICP if the target function is 
> in the module).

I was thinking of slightly different approach: it seems you have required type 
information in combined index together with type name in the module, so you 
probably can add external declarations of required vtables (this may require 
promotion) to the module in the ICP pass and run ICP over them. Do you think 
this can work?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73242



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

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-02-03 Thread Teresa Johnson via Phabricator via cfe-commits 
tejohnson marked 3 inline comments as done.
tejohnson added inline comments.



Comment at: llvm/include/llvm/IR/ModuleSummaryIndex.h:830
   enum Kind {
+Unknown,   ///< Unknown (analysis not performed, don't lower)
 Unsat, ///< Unsatisfiable type (i.e. no global has this type metadata)

evgeny777 wrote:
> I don't think such implicit conversion of Unsat to Unknown is good idea. I 
> guess problem will arise for legacy index files: for those ByteArray 
> resolution will become Unsat and so on. I suggest moving Unknown the end of 
> the list and bumping index version respectively (parseTypeIdSummaryRecord 
> doesn't verify TheKind).
Sounds like a good idea, will do.



Comment at: llvm/lib/Passes/PassBuilder.cpp:1380
+  // in ICP (which is performed earlier than this in the regular LTO pipeline).
+  MPM.addPass(LowerTypeTestsPass(nullptr, nullptr, true));
 

evgeny777 wrote:
> Can we get rid of two identical passes here (and in other places also)? For 
> instance 
> ```
> MPM.addPass(LowerTypeTestsPass(ExportSummary, nullptr, true));
> ```
> can both lower type metadata and do final cleanup.
The problem is that currently under DropTypeTests=true, LTT will drop the type 
tests up front and return without doing any other lowering. Which is what we 
want in the already existing callers of LTT with DropTypeTests=true. Here, we 
want LTT to perform its usual lowering, and only afterwards do the dropping. We 
want to do this only during regular LTO, but unfortunately we can't just key 
off of the ExportSummary (e.g. drop at the start if ExportSummary is nullptr, 
and drop at the end if ExportSummary is non-nullptr), since in some cases here 
ExportSummary may be nullptr (i.e. regular LTO invoked via the old LTO API). So 
I would need to introduce another option to essentially say "drop them after 
lowering". I can certainly do this if you think it would be better (I was 
thinking about that before, but thought it might be more confusing). WDYT?



Comment at: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp:1678
+// breaks any uses on assumes.
+if (TypeIdMap.count(TypeId))
+  continue;

evgeny777 wrote:
> I don't think, I understand this.
> It looks like that if (a) we have type.test in the module and (b) we don't 
> have vtable definition with corresponding type metadata in the same module 
> then we'll remove type.test/assume sequence before even getting to ICP. This 
> seems strange in the context of previous discussion because virtual function 
> may be called in different module from one where vtable is defined, like so:
> ```
> struct A { virtual int foo() { return 42; } };
> 
> // calls pA->foo
> extern int callFoo(A *pA);
> int main() { A a; return callFoo(); }
> ```
> 
We will only be able to do ICP this way if we have the target vtable in the 
module (similar to how we currently can only do ICP if the target function is 
in the module). I anticipate doing something similar for vtable defs as to what 
we do for virtual functions, where a fake call edge is added to the summary 
based on the value profile results to ensure they are imported before the LTO 
backend ICP.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73242



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

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-01-31 Thread Eugene Leviant via Phabricator via cfe-commits 
evgeny777 added a comment.

This patch is to be rebased against D73094 




Comment at: llvm/include/llvm/IR/ModuleSummaryIndex.h:830
   enum Kind {
+Unknown,   ///< Unknown (analysis not performed, don't lower)
 Unsat, ///< Unsatisfiable type (i.e. no global has this type metadata)

I don't think such implicit conversion of Unsat to Unknown is good idea. I 
guess problem will arise for legacy index files: for those ByteArray resolution 
will become Unsat and so on. I suggest moving Unknown the end of the list and 
bumping index version respectively (parseTypeIdSummaryRecord doesn't verify 
TheKind).



Comment at: llvm/lib/Passes/PassBuilder.cpp:1380
+  // in ICP (which is performed earlier than this in the regular LTO pipeline).
+  MPM.addPass(LowerTypeTestsPass(nullptr, nullptr, true));
 

Can we get rid of two identical passes here (and in other places also)? For 
instance 
```
MPM.addPass(LowerTypeTestsPass(ExportSummary, nullptr, true));
```
can both lower type metadata and do final cleanup.



Comment at: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp:1678
+// breaks any uses on assumes.
+if (TypeIdMap.count(TypeId))
+  continue;

I don't think, I understand this.
It looks like that if (a) we have type.test in the module and (b) we don't have 
vtable definition with corresponding type metadata in the same module then 
we'll remove type.test/assume sequence before even getting to ICP. This seems 
strange in the context of previous discussion because virtual function may be 
called in different module from one where vtable is defined, like so:
```
struct A { virtual int foo() { return 42; } };

// calls pA->foo
extern int callFoo(A *pA);
int main() { A a; return callFoo(); }
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73242



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

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-01-30 Thread Teresa Johnson via Phabricator via cfe-commits 
tejohnson added a comment.

In D73242#1849484 , @tejohnson wrote:

> Both of these approaches need the type tests to determine the correct address 
> point offset (the offset in the type test) to use in the compare sequence.


I typed this too fast - the offset is in the type metadata, but we need the 
type tests to correlate them with the indirect call sites.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73242



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

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-01-30 Thread Teresa Johnson via Phabricator via cfe-commits 
tejohnson marked an inline comment as done.
tejohnson added a comment.

In D73242#1847051 , @evgeny777 wrote:

> > This is an enabler for upcoming enhancements to indirect call promotion, 
> > for example streamlined promotion guard sequences that compare against 
> > vtable address instead of the target function
>
> Can you please describe the whole approach in more detail? At the moment ICP 
> is capable to do (a sort of) devirtualization is replacing indirect vtbl call 
> with sequence of function address comparisons and direct calls.
>  Are you going to speedup this by means of comparing vtable pointers instead 
> of function pointers (thus eliminating a single load per each vtbl call) or 
> there is also something else in mind?


That's exactly what we want to do here. We found a relatively significant 
number of cycles are being spent on virtual function pointer loads in these 
sequences, and by doing a vtable comparison instead, that is moved off the 
critical path. I had prototyped something like this in ICP awhile back and 
found a speedup in an important app.

> If that's true, what's the next
>  step? Make ICP pass analyze type test intrinsics?

There are a few ways to do the alternate ICP compare sequences, one is using 
statically available info from the vtable definitions in the module that 
utilize the profiled target. This relies on ThinLTO to import all the relevant 
vtable definitions. The other is to profile vtable addresses with FDO (not just 
the target function pointer) - I've got the type profiling implemented, but it 
needs some cleanup before I send for review. Both of these approaches need the 
type tests to determine the correct address point offset (the offset in the 
type test) to use in the compare sequence. And in both cases you want to trade 
off the number of comparisons needed for the two approaches to determine 
whether a vtable compare or a target function compare is better. I.e. if there 
are a lot of vtable definitions that utilize a hot target, it is likely better 
to do a single target function comparison.




Comment at: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp:1660
+cast(CI->getArgOperand(1))->getMetadata();
 // If we found any, add them to CallSlots.
 if (!Assumes.empty()) {

evgeny777 wrote:
> This change seems to be unrelated
It is needed to have the TypeId available outside this if statement (see the 
map check below).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73242



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

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-01-29 Thread Eugene Leviant via Phabricator via cfe-commits 
evgeny777 added a comment.

> This is an enabler for upcoming enhancements to indirect call promotion, for 
> example streamlined promotion guard sequences that compare against vtable 
> address instead of the target function

Can you please describe the whole approach in more detail? At the moment ICP is 
capable to do (a sort of) devirtualization is replacing indirect vtbl call with 
sequence of function address comparisons and direct calls.
Are you going to speedup this by means of comparing vtable pointers instead of 
function pointers (thus eliminating a single load per each vtbl call) or there 
is also something else in mind? If that's true, what's the next
step? Make ICP pass analyze type test intrinsics?




Comment at: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp:1660
+cast(CI->getArgOperand(1))->getMetadata();
 // If we found any, add them to CallSlots.
 if (!Assumes.empty()) {

This change seems to be unrelated


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73242



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

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-01-22 Thread Teresa Johnson via Phabricator via cfe-commits 
tejohnson marked an inline comment as done.
tejohnson added inline comments.



Comment at: llvm/test/Transforms/WholeProgramDevirt/uniform-retval.ll:28
   %result = call i32 %fptr_casted(i8* %obj)
-  ; CHECK-NOT: call
+  ; CHECK-NOT: call i32 %
   ; CHECK: ret i32 123

This change is to distinguish the indirect call here that should have been 
removed from the @llvm.type.test and @llvm.assume calls that are no longer 
removed at this point.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73242



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

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-01-22 Thread Teresa Johnson via Phabricator via cfe-commits 
tejohnson created this revision.
tejohnson added reviewers: pcc, evgeny777.
Herald added subscribers: arphaman, dexonsmith, steven_wu, hiraditya, Prazek, 
mehdi_amini.
Herald added projects: clang, LLVM.

Currently type test assume sequences inserted for devirtualization are
removed during WPD. This patch delays their removal until later in the
optimization pipeline. This is an enabler for upcoming enhancements to
indirect call promotion, for example streamlined promotion guard
sequences that compare against vtable address instead of the target
function, when there are small number of possible vtables (either
determined via WPD or by in-progress type profiling). We need the type
tests to correlate the callsites with the address point offset needed in
the compare sequence, and optionally to associated type summary info
computed during WPD.

This depends on work in D71913  to enable 
invocation of LowerTypeTests to
drop type test assume sequences, which will now be invoked following ICP
in the ThinLTO post-LTO link pipelines, and also after the existing
export phase LowerTypeTests invocation in regular LTO (which is already
after ICP). We cannot simply move the existing import phase
LowerTypeTests pass later in the ThinLTO post link pipelines, as the
comment in PassBuilder.cpp notes (it must run early because when
performing CFI other passes may disturb the sequences it looks for).

This necessitated adding a new type test resolution "Unknown" that we
can use on the type test assume sequences previously removed by WPD,
that we now want LTT to ignore.

Depends on D71913 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73242

Files:
  clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
  clang/test/CodeGen/thinlto-distributed-cfi.ll
  llvm/include/llvm/IR/ModuleSummaryIndex.h
  llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Transforms/IPO/LowerTypeTests.cpp
  llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
  llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
  llvm/test/Other/new-pm-lto-defaults.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
  llvm/test/ThinLTO/X86/cfi-icall-only-defuse.ll
  llvm/test/ThinLTO/X86/cfi-icall.ll
  llvm/test/Transforms/WholeProgramDevirt/branch-funnel.ll
  llvm/test/Transforms/WholeProgramDevirt/export-single-impl.ll
  llvm/test/Transforms/WholeProgramDevirt/export-uniform-ret-val.ll
  llvm/test/Transforms/WholeProgramDevirt/export-unique-ret-val.ll
  llvm/test/Transforms/WholeProgramDevirt/export-vcp.ll
  llvm/test/Transforms/WholeProgramDevirt/import-indir.ll
  llvm/test/Transforms/WholeProgramDevirt/uniform-retval.ll

Index: llvm/test/Transforms/WholeProgramDevirt/uniform-retval.ll
===
--- llvm/test/Transforms/WholeProgramDevirt/uniform-retval.ll
+++ llvm/test/Transforms/WholeProgramDevirt/uniform-retval.ll
@@ -25,7 +25,7 @@
   %fptr = load i8*, i8** %fptrptr
   %fptr_casted = bitcast i8* %fptr to i32 (i8*)*
   %result = call i32 %fptr_casted(i8* %obj)
-  ; CHECK-NOT: call
+  ; CHECK-NOT: call i32 %
   ; CHECK: ret i32 123
   ret i32 %result
 }
Index: llvm/test/Transforms/WholeProgramDevirt/import-indir.ll
===
--- llvm/test/Transforms/WholeProgramDevirt/import-indir.ll
+++ llvm/test/Transforms/WholeProgramDevirt/import-indir.ll
@@ -32,7 +32,7 @@
 ; SUMMARY-NEXT: TypeIdMap:
 ; SUMMARY-NEXT:   typeid1:
 ; SUMMARY-NEXT: TTRes:
-; SUMMARY-NEXT:   Kind:Unsat
+; SUMMARY-NEXT:   Kind:Unknown
 ; SUMMARY-NEXT:   SizeM1BitWidth:  0
 ; SUMMARY-NEXT:   AlignLog2:   0
 ; SUMMARY-NEXT:   SizeM1:  0
Index: llvm/test/Transforms/WholeProgramDevirt/export-vcp.ll
===
--- llvm/test/Transforms/WholeProgramDevirt/export-vcp.ll
+++ llvm/test/Transforms/WholeProgramDevirt/export-vcp.ll
@@ -9,7 +9,7 @@
 ; SUMMARY:  TypeIdMap:
 ; SUMMARY-NEXT:   typeid3:
 ; SUMMARY-NEXT: TTRes:
-; SUMMARY-NEXT:   Kind:Unsat
+; SUMMARY-NEXT:   Kind:Unknown
 ; SUMMARY-NEXT:   SizeM1BitWidth:  0
 ; SUMMARY-NEXT:   AlignLog2:   0
 ; SUMMARY-NEXT:   SizeM1:  0
@@ -29,7 +29,7 @@
 ; SUMMARY-ARM-NEXT: Bit: 1
 ; SUMMARY-NEXT:   typeid4:
 ; SUMMARY-NEXT: TTRes:
-; SUMMARY-NEXT:   Kind:Unsat
+; SUMMARY-NEXT:   Kind:Unknown
 ; SUMMARY-NEXT:   SizeM1BitWidth:  0
 ; SUMMARY-NEXT:   AlignLog2:   0
 ; SUMMARY-NEXT:   SizeM1:  0
Index: llvm/test/Transforms/WholeProgramDevirt/export-unique-ret-val.ll