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

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

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

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

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

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.

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

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

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

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

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

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

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

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/

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

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

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

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

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

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

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

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

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

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

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

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,

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

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

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

/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

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 < revi...@reviews.llvm.org> wrote: > aganea added a comment. > > There seems to be

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

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

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

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

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

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: >

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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: >

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

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

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

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, ///<

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

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

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

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

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

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

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

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

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

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.