[PATCH] D152741: [WPD] implement -funknown-vtable-visibility-filepaths

modimo added a comment. In D152741#4445807 , @wenlei wrote: >> For concrete data are you talking about between the different solutions e.g. >> --lto-whole-program-visibility, -funknown-vtable-visibility-filepaths, RTTI >> based, FatLTO based etc or

[PATCH] D152741: [WPD] implement -funknown-vtable-visibility-filepaths

modimo updated this revision to Diff 534134. modimo added a comment. Feedback, add documentation for flag and unknown visibility. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152741/new/ https://reviews.llvm.org/D152741 Files:

[PATCH] D152741: [WPD] implement -funknown-vtable-visibility-filepaths

modimo marked 4 inline comments as done. modimo added a comment. In D152741#4445112 , @wenlei wrote: >> The big advantage of doing this in the FE is that we know which types are >> actually coming from the native headers. Blocking all types in the TU is

[PATCH] D152741: [WPD] implement -fskip-vtable-filepaths

modimo updated this revision to Diff 533326. modimo added a comment. Address feedback. Allow empty string to unset the flag Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152741/new/ https://reviews.llvm.org/D152741 Files:

[PATCH] D152741: [WPD] implement -fskip-vtable-filepaths

modimo added a comment. In D152741#4438007 , @tejohnson wrote: > Ok what I missed is that you don't want to apply this to entire TUs, but > rather just some paths that are header files, which may be included in many > source files. So in your above

[PATCH] D152741: [WPD] implement -fskip-vtable-filepaths

modimo updated this revision to Diff 533048. modimo added a comment. Remove leftover code from original implementation Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152741/new/ https://reviews.llvm.org/D152741 Files:

[PATCH] D152741: [WPD] implement -fskip-vtable-filepaths

modimo added a comment. In D152741#4421067 , @tejohnson wrote: > In D152741#4419366 , @modimo wrote: > >> In D152741#4419324 , @tejohnson >> wrote: >> >>> In

[PATCH] D152741: [WPD] implement -fskip-vtable-filepaths

modimo updated this revision to Diff 531151. modimo added a comment. Remove unrelated change, fix typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152741/new/ https://reviews.llvm.org/D152741 Files: clang/include/clang/Basic/CodeGenOptions.h

[PATCH] D152741: [WPD] implement -fskip-vtable-filepaths

modimo updated this revision to Diff 531149. modimo added a comment. Herald added subscribers: llvm-commits, ormris, steven_wu, hiraditya. Herald added a project: LLVM. Implement using VCallVisibilityUnknown Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D152741: [WPD] implement -fskip-vtable-filepaths

modimo added a comment. In D152741#4419324 , @tejohnson wrote: > In D152741#4419265 , @modimo wrote: > >> In D152741#4418831 , @tejohnson >> wrote: >> >>> I think I

[PATCH] D152741: [WPD] implement -fskip-vtable-filepaths

modimo added a comment. In D152741#4418831 , @tejohnson wrote: > I think I understand the motivation, but not sure I agree this is the right > approach - can you simply not pass -flto-unit and -fwhole-program-vtables for > these files? For our

[PATCH] D152741: -fskip-vtable-filepaths

modimo created this revision. Herald added subscribers: hoy, wenlei. Herald added a project: All. modimo requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D152741

[PATCH] D132186: Clang: Add a new flag Wmisnoinline for printing hot noinline functions

modimo added a comment. In D132186#3802989 , @paulkirth wrote: > @iamarchit123 I think the standard advice is to start w/ the llvm-test-suite > and then explore other benchmarks as needed. Also, Clang itself is often a > very good starting point. > >

[PATCH] D132186: Clang: Add a new flag Wmisnoinline for printing hot noinline functions

modimo added a comment. Thanks for taking a look! In D132186#3752150 , @paulkirth wrote: > In D132186#3751985 , @tejohnson > wrote: > >> I have seen a few cases where noinline was used for performance, in

[PATCH] D124563: Drop '* text=auto' from .gitattributes and normalize

modimo added a comment. In D124563#3478978 , @aaronpuchert wrote: > In D124563#3478968 , @modimo wrote: > >> I used `arc patch` and also saw the same thing. > > The patch does actually change the files to LF

[PATCH] D124563: Drop '* text=auto' from .gitattributes and normalize

modimo added a comment. In D124563#3478951 , @aaronpuchert wrote: > In D124563#3478937 , @modimo wrote: > >> Checking locally I'm seeing LF as the line ending in crlf.cpp in the working >> directory. Can you

[PATCH] D124563: Drop '* text=auto' from .gitattributes and normalize

modimo added a comment. In D124563#3478937 , @modimo wrote: > Checking locally I'm seeing LF as the line ending in crlf.cpp in the working > directory. Can you double check that everything matches up? Ah it was some strange setup on my end. Confirmed

[PATCH] D124563: Drop '* text=auto' from .gitattributes and normalize

modimo requested changes to this revision. modimo added a comment. This revision now requires changes to proceed. Checking locally I'm seeing LF as the line ending in crlf.cpp in the working directory. Can you double check that everything matches up? Repository: rG LLVM Github Monorepo

[PATCH] D124563: Drop '* text=auto' from .gitattributes and normalize

modimo added a comment. In D124563#3478915 , @aaronpuchert wrote: > Drop `* text=auto`, so that we renormalize only the files that need it. Makes sense to me, thanks for putting it up. If you want to commandeer I can accept the change or you can

[PATCH] D124563: Drop '* text=auto' from .gitattributes and normalize

modimo added a comment. In D124563#3478781 , @aaronpuchert wrote: > In D124563#3478653 , @modimo wrote: > >> I think the way to go is to revert ac5f7be6a868 >>

[PATCH] D124563: Renormalize line endings after ac5f7be6a8688955a282becf00eebc542238a86b

modimo added a comment. In D124563#3478627 , @smeenai wrote: > If I check out this commit and then check out the previous commit, > `clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp` and >

[PATCH] D124563: Renormalize line endings after ac5f7be6a8688955a282becf00eebc542238a86b

modimo added a comment. In D124563#3478561 , @smeenai wrote: > The following files have their line endings (when checked out on disk) > changed from CRLF to LF by this patch. Seems harmless, but I just wanted to > confirm that it was expected: > > >

[PATCH] D124563: renormalize

modimo created this revision. Herald added subscribers: hoy, wenlei. Herald added a project: All. modimo requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D124563

[PATCH] D113523: Add toggling for -fnew-infallible/-fno-new-infallible

This revision was automatically updated to reflect the committed changes. Closed by commit rG47f230ba2c8f: Add toggling for -fnew-infallible/-fno-new-infallible (authored by modimo). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113523/new/

[PATCH] D114130: [Clang] Add option to disable -mconstructor-aliases with -mno-constructor-aliases

This revision was automatically updated to reflect the committed changes. Closed by commit rG9b704d31b54a: [Clang] Add option to disable -mconstructor-aliases with -mno-constructor… (authored by modimo). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D114130: [Clang] Add option to disable -mconstructor-aliases with -mno-constructor-aliases

modimo updated this revision to Diff 388307. modimo added a comment. Condense lines Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114130/new/ https://reviews.llvm.org/D114130 Files: clang/include/clang/Driver/Options.td

[PATCH] D114130: [Clang] Add option to disable -mconstructor-aliases with -mno-constructor-aliases

modimo created this revision. Herald added subscribers: jeroen.dobbelaere, hoy, wenlei, lxfind, dang. modimo requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D114130 Files:

[PATCH] D113523: Add toggling for -fnew-infallible/-fno-new-infallible

modimo added a comment. Gentle ping @bruno Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113523/new/ https://reviews.llvm.org/D113523 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D113523: Add toggling for -fnew-infallible/-fno-new-infallible

modimo updated this revision to Diff 386059. modimo added a comment. Add driver test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113523/new/ https://reviews.llvm.org/D113523 Files: clang/docs/ClangCommandLineReference.rst

[PATCH] D113523: Add toggling for -fnew-infallible/-fno-new-infallible

modimo updated this revision to Diff 385996. modimo added a comment. Remove whitespace change Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113523/new/ https://reviews.llvm.org/D113523 Files: clang/docs/ClangCommandLineReference.rst

[PATCH] D113523: Add toggling for -fnew-infallible/-fno-new-infallible

modimo created this revision. Herald added subscribers: hoy, wenlei, lxfind, dang. modimo requested review of this revision. Herald added a reviewer: jdoerfert. Herald added subscribers: cfe-commits, sstefan1. Herald added a project: clang. Repository: rG LLVM Github Monorepo

[PATCH] D36850: [ThinLTO] Add noRecurse and noUnwind thinlink function attribute propagation

modimo added a comment. Thanks for the thorough review @tejohnson! I'll do additional validation on FB code that uses exceptions and if that all looks good I'll send up a change to turn this default on with the findings. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D36850: [ThinLTO] Add noRecurse and noUnwind thinlink function attribute propagation

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG20faf789199d: [ThinLTO] Add noRecurse and noUnwind thinlink function attribute propagation (authored by modimo). Changed prior to commit:

[PATCH] D36850: [ThinLTO] Add noRecurse and noUnwind thinlink function attribute propagation

modimo added inline comments. Comment at: clang/test/CodeGen/thinlto-funcattr-prop.ll:17 -; CHECK: ^2 = gv: (guid: 13959900437860518209, summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 1, dsoLocal: 1,

[PATCH] D36850: [ThinLTO] Add noRecurse and noUnwind thinlink function attribute propagation

modimo updated this revision to Diff 374934. modimo marked an inline comment as done. modimo added a comment. Complete explanation in thinlto-funcattr-prop.ll, also fix up diff to contain all changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D36850: [ThinLTO] Add noRecurse and noUnwind thinlink function attribute propagation

modimo added inline comments. Comment at: clang/test/CodeGen/thinlto-funcattr-prop.ll:16 + +; CHECK: ^2 = gv: (guid: 13959900437860518209, summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 1, dsoLocal: 1,

[PATCH] D36850: [ThinLTO] Add noRecurse and noUnwind thinlink function attribute propagation

modimo updated this revision to Diff 374723. modimo marked 3 inline comments as done. modimo added a comment. Address follow-ups Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D36850/new/ https://reviews.llvm.org/D36850 Files:

[PATCH] D36850: [ThinLTO] Add norecurse function attribute propagation

modimo added inline comments. Comment at: clang/test/CodeGen/thinlto-funcattr-prop.ll:14 + +; RUN: llvm-dis %t/b.bc -o - | FileCheck %s + tejohnson wrote: > This is checking the summary generated by opt, not the result of the > llvm-lto2 run. Fixed.

[PATCH] D36850: [ThinLTO] Add norecurse function attribute propagation

modimo updated this revision to Diff 374431. modimo marked 23 inline comments as done. modimo added a comment. Address feedback, rename funcattrs-prop-indirect.ll to funcattrs-prop-unknown.ll Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D36850/new/

[PATCH] D36850: [ThinLTO] Add norecurse function attribute propagation

modimo added a comment. In D36850#3005254 , @tejohnson wrote: > Ok thanks. I need to go through the propagation code and tests again more > closely now, but one question/suggestion below in the meantime. Thanks! Comment at:

[PATCH] D36850: [ThinLTO] Add norecurse function attribute propagation

modimo updated this revision to Diff 372398. modimo added a comment. Add mayThrow flag Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D36850/new/ https://reviews.llvm.org/D36850 Files: clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll

[PATCH] D36850: [ThinLTO] Add norecurse function attribute propagation

modimo added a comment. In D36850#2990907 , @tejohnson wrote: > In D36850#2990847 , @modimo wrote: > >> In D36850#2990771 , @tejohnson >> wrote: >> >>> In D36850#2968536

[PATCH] D36850: [ThinLTO] Add norecurse function attribute propagation

modimo added a comment. In D36850#2990771 , @tejohnson wrote: > In D36850#2968536 , @modimo wrote: > >> In D36850#2964293 , @tejohnson >> wrote: >> >>> Good point on

[PATCH] D36850: [ThinLTO] Add norecurse function attribute propagation

modimo added a comment. gentle ping @tejohnson Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D36850/new/ https://reviews.llvm.org/D36850 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D108905: [ItaniumCXXABI] Make __cxa_end_catch calls unconditionally nounwind

modimo added a comment. In D108905#2973099 , @rjmccall wrote: > Yeah, I think this is the most natural interpretation of the current > standard. But that would be a very unfortunate rule, because people who > write `catch (...) {}` do reasonably

[PATCH] D108905: [ItaniumCXXABI] Make __cxa_end_catch calls unconditionally nounwind

modimo added a comment. In D108905#2971861 , @rjmccall wrote: > I'm not really sure what the standard expects to happen if an exception > destructor throws. The standard tells us when the destruction happens — the > latest point it can — but that

[PATCH] D36850: [ThinLTO] Add norecurse function attribute propagation

modimo updated this revision to Diff 369158. modimo added a comment. Check for CachedAttributes count in map rather than value so conservative results are not re-calculated Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D36850/new/

[PATCH] D36850: [ThinLTO] Add norecurse function attribute propagation

modimo updated this revision to Diff 369024. modimo marked 2 inline comments as done. modimo added a comment. Use prevailing for linkonce/weak. Add hasUnknownCall to model virtual and indirect calls Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D36850: [ThinLTO] Add norecurse function attribute propagation

modimo marked 3 inline comments as done. modimo added a comment. In D36850#2964293 , @tejohnson wrote: > Good point on indirect calls. Rather than add a bit to the summary, can the > flags just be set conservatively in any function containing an indirect

[PATCH] D36850: [ThinLTO] Add norecurse function attribute propagation

modimo added a comment. Checking build timing in release mode Clang self-build looking at purely thinlink timing: | Mode | Time (s) | | base | 2.254| | base + propagation | 2.556| | noinline | 8.870| | noinline + propagation |

[PATCH] D36850: [ThinLTO] Add norecurse function attribute propagation

modimo updated this revision to Diff 368178. modimo added a comment. Remove llvm/test/ThinLTO/X86/weak_externals.ll from diffs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D36850/new/ https://reviews.llvm.org/D36850 Files:

[PATCH] D36850: [ThinLTO] Add norecurse function attribute propagation

modimo updated this revision to Diff 368174. modimo added a comment. Minor test fixups Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D36850/new/ https://reviews.llvm.org/D36850 Files: clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll

[PATCH] D36850: [ThinLTO] Add norecurse function attribute propagation

modimo added a comment. @tejohnson Indirect calls are not captured in FunctionSummaries in CallGraph or in a flag form saying they exist. Also looks like speculative candidates for ICP do make

[PATCH] D36850: [ThinLTO] Add norecurse function attribute propagation

modimo updated this revision to Diff 367937. modimo marked 3 inline comments as done. modimo edited the summary of this revision. modimo added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. Adding more test cases and changed logic around weak linkages

[PATCH] D105225: [clang] Add support for optional flag -fnew-infallible to restrict exception propagation

modimo added a comment. In D105225#2921471 , @rsmith wrote: > Sorry I'm late to the party here... is there any work ongoing to add this to > GCC too? If not, would it make sense to send a quick note to the GCC > development list pointing this out to

[PATCH] D105225: [clang] Add support for optional flag -fnew-infallible to restrict exception propagation

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb40a2a533a9d: [clang] Add support for optional flag -fnew-infallible to restrict exception… (authored by modimo). Herald added a project: clang.

[PATCH] D102820: [Clang] Check for returns_nonnull when deciding to add allocation null checks

This revision was automatically updated to reflect the committed changes. Closed by commit rG42b99e094c4f: [Clang] Check for returns_nonnull when deciding to add allocation null checks (authored by modimo). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D94333: [Inliner] Change inline remark format and update ReplayInlineAdvisor to use it

modimo added a comment. In D94333#2829233 , @sylvestre.ledru wrote: >> I think this change broke apt.llvm.org > > Confirmed: reverting this change fixed the link issue What exact commit/download package and build command repros this? M68kSubtarget.cpp

[PATCH] D102820: [Clang] Check for returns_nonnull when deciding to add allocation null checks

modimo added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102820/new/ https://reviews.llvm.org/D102820 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102820: [Clang] Check for returns_nonnull when deciding to add allocation null checks

modimo added a comment. @rsmith @lebedev.ri thoughts on adding this directly to FE generation? As mentioned this isn't strictly needed and the BE can elide the check but we can also not emit it to save on AST/IR processing cost. CHANGES SINCE LAST ACTION

[PATCH] D102820: [Clang] Check for returns_nonnull when deciding to add allocation null checks

modimo added a comment. In D102820#2772184 , @bruno wrote: > Sounds reasonable to me! Can you double check whether this attribute gets > correctly serialized/deserialized in face of `CXXNewExpr`? An example of how > to test that would be in

[PATCH] D102820: [Clang] Check for returns_nonnull when deciding to add allocation null checks

modimo added a subscriber: urnathan. modimo added a comment. Discussing with @urnathan this makes more sense for the BE to handle when optimizing by eliding the generated null check. Confirmed this is indeed the case so removing the generation in the FE isn't really needed. CHANGES SINCE LAST

[PATCH] D102820: [Clang] Check for returns_nonnull when deciding to add allocation null checks

modimo updated this revision to Diff 346610. modimo added a comment. Go back to single statement return, fix up ATTR_BUILTIN_NOTHROW_NEW test label that pointed to nothing to correct ATTR_NOBUILTIN_NOUNWIND_ALLOCSIZE label. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102820/new/

[PATCH] D102820: [Clang] Check for returns_nonnull when deciding to add allocation null checks

modimo created this revision. modimo added reviewers: bruno, lebedev.ri, rsmith. Herald added subscribers: hoy, wenlei, lxfind. modimo requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Non-throwing allocators currently will always get

[PATCH] D94333: [Inliner] Change inline remark format and update ReplayInlineAdvisor to use it

modimo added inline comments. Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:412 + + Remark << ";"; } wenlei wrote: > modimo wrote: > > wenlei wrote: > > > nit: any special reason for adding this? doesn't seem consistent with > > > other remarks we have. > >

[PATCH] D94333: [Inliner] Change inline remark format and update ReplayInlineAdvisor to use it

modimo added inline comments. Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:412 + + Remark << ";"; } wenlei wrote: > nit: any special reason for adding this? doesn't seem consistent with other > remarks we have. If you grab the remark outputs via

[PATCH] D94333: [Inliner] Change inline remark format and update ReplayInlineAdvisor to use it

This revision was automatically updated to reflect the committed changes. Closed by commit rG2a49b7c64a33: [Inliner] Change inline remark format and update ReplayInlineAdvisor to use it (authored by modimo). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG

[PATCH] D86156: [BFI] Make BFI information available through loop passes inside LoopStandardAnalysisResults

modimo added a comment. Thanks @asbirlea! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86156/new/ https://reviews.llvm.org/D86156 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D86156: [BFI] Make BFI information available through loop passes inside LoopStandardAnalysisResults

modimo updated this revision to Diff 292044. modimo added a comment. Rebase #2 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86156/new/ https://reviews.llvm.org/D86156 Files: llvm/include/llvm/Analysis/LoopAnalysisManager.h llvm/include/llvm/Transforms/Scalar/LoopPassManager.h

[PATCH] D86156: [BFI] Make BFI information available through loop passes inside LoopStandardAnalysisResults

modimo updated this revision to Diff 291353. modimo added a comment. Rebase CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86156/new/ https://reviews.llvm.org/D86156 Files: llvm/include/llvm/Analysis/LoopAnalysisManager.h llvm/include/llvm/Transforms/Scalar/LoopPassManager.h

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

modimo added a comment. In D86156#2245103 , @nikic wrote: > I have no familiarity with BFI, so possibly stupid question: There is already > some similar handling as part of BFIImpl here: >

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

modimo updated this revision to Diff 288719. modimo added a comment. Remove redundant VH callback as @nikic helpfully pointed out! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86156/new/ https://reviews.llvm.org/D86156 Files: llvm/include/llvm/Analysis/LoopAnalysisManager.h

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

modimo added inline comments. Comment at: llvm/include/llvm/Transforms/Scalar/LoopPassManager.h:273 : nullptr; +BlockFrequencyInfo *BFI = UseBlockFrequencyInfo + ? ((F)) asbirlea wrote: > Add `&&

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

modimo updated this revision to Diff 288477. modimo added a comment. Condition usage of BFI to PGO in newPM as well CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86156/new/ https://reviews.llvm.org/D86156 Files: llvm/include/llvm/Analysis/BlockFrequencyInfo.h

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

modimo added a comment. In D86156#2242872 , @asbirlea wrote: > As a general note, it may make sense to include BFI in the set of loop passes > always preserved (`getLoopPassPreservedAnalyses()`), if its nature is to > always be preserved (with some

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

modimo updated this revision to Diff 288445. modimo added a comment. only use BFI when profile is enabled, have LICM mark BFI as preserved CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86156/new/ https://reviews.llvm.org/D86156 Files: clang/test/CodeGen/thinlto-distributed-newpm.ll

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

modimo added inline comments. Comment at: llvm/lib/Passes/PassBuilder.cpp:522 FPM.addPass(createFunctionToLoopPassAdaptor( - std::move(LPM2), /*UseMemorySSA=*/false, DebugLogging)); + std::move(LPM2), /*UseMemorySSA=*/false, /*UseBlockFrequencyInfo=*/true, +

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

modimo updated this revision to Diff 288172. modimo added a comment. Remove usage need for BFI in LPM2 and set unswitching to preserve lazy BPI/BFI so it can remain in the same loop pass as LICM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86156/new/ https://reviews.llvm.org/D86156

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

modimo added a comment. In D86156#2231710 , @nikic wrote: > This change adds three PDT calculations to the standard pipeline. Please try > to avoid the PDT calculations if PGO is not used, possibly by using LazyBPI. Good catch, changed to use LazyBFI

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

modimo updated this revision to Diff 288125. modimo added a comment. Change to LazyBFI for legacy pass manager to prevent rebuilding the post-dominator tree CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86156/new/ https://reviews.llvm.org/D86156 Files:

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

modimo added inline comments. Comment at: llvm/include/llvm/Transforms/Scalar/LoopPassManager.h:408 FunctionToLoopPassAdaptor createFunctionToLoopPassAdaptor(LoopPassT Pass, bool UseMemorySSA = false, +bool UseBlockFrequencyInfo = false,

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

modimo updated this revision to Diff 287152. modimo added a comment. Herald added subscribers: lxfind, nikic. @asbirlea Thanks for taking a look! I updated BFI to resemble MSSA as recommended which removed the BFI calculation unless LICM is invoked. Also removed the spurious diffs from

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

modimo updated this revision to Diff 286392. modimo added a comment. Commit my changes (crazy I know) so that the diff is actually updated for linting CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86156/new/ https://reviews.llvm.org/D86156 Files:

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

modimo updated this revision to Diff 286390. modimo added a comment. Linting CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86156/new/ https://reviews.llvm.org/D86156 Files: clang/test/CodeGen/thinlto-distributed-newpm.ll llvm/include/llvm/Analysis/BlockFrequencyInfo.h

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

modimo created this revision. modimo added reviewers: wenlei, asbirlea, vsk. modimo added a project: LLVM. Herald added subscribers: llvm-commits, cfe-commits, dexonsmith, steven_wu, hiraditya. Herald added a project: clang. modimo requested review of this revision. D65060