[PATCH] D101918: [clang][Driver] Add -fintegrate-as to debug-pass-structure test

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/D101918/new/ https://reviews.llvm.org/D101918

[PATCH] D99599: [NewPM] Add an option to dump pass structure

evgeny777 added a comment. > Could you explain why -debug-pass-manager doesn't fit your use case? I wanted to have something similar to -debug-pass=Structure, because the above is too verbose and lacks identation. What's the problem with this one, anyway? Repository: rG LLVM Github Monorepo

[PATCH] D99599: [NewPM] Add an option to dump pass structure

evgeny777 added a comment. > I've already run into having to update these two golden file tests twice I think you can just reduce the tests. No need to revert the entire change, unless there is a replacement for it (D101797 is not) Repository: rG LLVM

[PATCH] D99599: [NewPM] Add an option to dump pass structure

evgeny777 added a comment. Should be fixed with `c81ec19fba27ec` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99599/new/ https://reviews.llvm.org/D99599 ___ cfe-commits mailing list

[PATCH] D99599: [NewPM] Add an option to dump pass structure

evgeny777 added a comment. Hm ... I see BarrierNoop pass being added before `annotation-remarks`. Why's that? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99599/new/ https://reviews.llvm.org/D99599

[PATCH] D99599: [NewPM] Add an option to dump pass structure

evgeny777 added a comment. @haowei What are LLVM configuration options? Also please send output from /opt/s/w/ir/x/w/staging/llvm_build/bin/clang -flegacy-pass-manager -fdebug-pass-structure -O0 -S -emit-llvm /opt/s/w/ir/x/w/llvm-project/clang/test/Driver/debug-pass-structure.c -o /dev/null

[PATCH] D99599: [NewPM] Add an option to dump pass structure

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6a0283d0d23c: [NewPM] Add an option to dump pass structure (authored by evgeny777). Herald added a project: clang. Herald added a subscriber:

[PATCH] D91812: [ThinLTO/WPD] Enable -wholeprogramdevirt-skip in ThinLTO backends

evgeny777 accepted this revision. evgeny777 added a comment. This revision is now accepted and ready to land. Thanks, LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91812/new/ https://reviews.llvm.org/D91812

[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

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] D75655: [Docs] Document -lto-whole-program-visibility

evgeny777 added a comment. @pcc > That case seems somewhat questionable to me. If the symbols are being > exported, it is presumably for the purpose of allowing the symbols to be used > outside of the defining DSO. But LTO visibility based optimizations could > make any such use of the

[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

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

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

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

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

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] D73418: [WPD] Emit vcall_visibility metadata for MicrosoftCXXABI

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/D73418/new/ https://reviews.llvm.org/D73418

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

evgeny777 accepted this revision. evgeny777 added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71907/new/ https://reviews.llvm.org/D71907

[PATCH] D71911: [ThinLTO] Summarize vcall_visibility metadata

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/D71911/new/ https://reviews.llvm.org/D71911

[PATCH] D71913: [LTO/WPD] Enable aggressive WPD under LTO option

evgeny777 accepted this revision. evgeny777 added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71913/new/ https://reviews.llvm.org/D71913

[PATCH] D71913: [LTO/WPD] Enable aggressive WPD under LTO option

evgeny777 added a comment. Looks good so far. See remaining comment in D71907 Comment at: clang/lib/CodeGen/CGVTables.cpp:1050 - if (getCodeGenOpts().LTOVisibilityPublicStd) { -const DeclContext *DC = RD; -while (1) { - auto

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

evgeny777 added inline comments. Comment at: llvm/lib/Transforms/IPO/GlobalSplit.cpp:116 +if (GV.hasMetadata(LLVMContext::MD_vcall_visibility)) + SplitGV->setVCallVisibilityMetadata(GV.getVCallVisibility()); } I think this needs a test. Removal of

[PATCH] D71913: [LTO/WPD] Enable aggressive WPD under LTO option

evgeny777 added inline comments. Comment at: clang/test/CodeGenCXX/lto-visibility-inference.cpp:73 c1->f(); - // ITANIUM-NOT: type.test{{.*}}!"_ZTS2C2" + // ITANIUM: type.test{{.*}}!"_ZTS2C2" // MS: type.test{{.*}}!"?AUC2@@" tejohnson wrote: > evgeny777

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

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

[PATCH] D71911: [ThinLTO] Summarize vcall_visibility metadata

evgeny777 added inline comments. Comment at: llvm/include/llvm/IR/ModuleSummaryIndex.h:762 +bool ReadOnly, bool WriteOnly, bool Constant, +GlobalObject::VCallVisibility Vis = GlobalObject::VCallVisibilityPublic) : MaybeReadOnly(ReadOnly),

[PATCH] D71913: [LTO/WPD] Enable aggressive WPD under LTO option

evgeny777 added inline comments. Comment at: clang/test/CodeGenCXX/lto-visibility-inference.cpp:73 c1->f(); - // ITANIUM-NOT: type.test{{.*}}!"_ZTS2C2" + // ITANIUM: type.test{{.*}}!"_ZTS2C2" // MS: type.test{{.*}}!"?AUC2@@" tejohnson wrote: > evgeny777

[PATCH] D71913: [LTO/WPD] Enable aggressive WPD under LTO option

evgeny777 added inline comments. Comment at: llvm/test/Transforms/WholeProgramDevirt/import-indir.ll:2 ; Test that we correctly import an indir resolution for type identifier "typeid1". -; RUN: opt -S -wholeprogramdevirt -wholeprogramdevirt-summary-action=import

[PATCH] D71913: [LTO/WPD] Enable aggressive WPD under LTO option

evgeny777 added a comment. Thanks, I'm still in process of testing (now fixing issue which however is most likely related to devirtualization itself, not to this patch). Meanwhile some of my comments below. Comment at:

[PATCH] D71911: [ThinLTO] Summarize vcall_visibility metadata

evgeny777 added a comment. I think this has to be rebased - I see multiple failures when trying to apply Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71911/new/ https://reviews.llvm.org/D71911 ___

[PATCH] D71913: [LTO/WPD] Enable aggressive WPD under LTO option

evgeny777 added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:559 + if (!CodeGenOpts.ThinLTOIndexFile.empty()) +MPM.add(createLowerTypeTestsPass(/*ExportSummary=*/nullptr, + /*ImportSummary=*/nullptr,

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

evgeny777 added a comment. Looks good at first sight. Do you have linker patch for me to try this out? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71907/new/ https://reviews.llvm.org/D71907 ___

[PATCH] D71911: [ThinLTO] Summarize vcall_visibility metadata

evgeny777 added inline comments. Comment at: llvm/include/llvm/IR/ModuleSummaryIndex.h:794 + } + GlobalObject::VCallVisibility vCallVisibility() const { +return (GlobalObject::VCallVisibility)VarFlags.VCallVisibility; getVCallVisibility() ? Repository:

[PATCH] D69173: [clang][ThinLTO][Legacy] Don't add -fsplit-lto-unit for legacy thin LTO builds

This revision was automatically updated to reflect the committed changes. Closed by commit rG1ae8e8d25fd8: Dont add -fsplit-lto-unit for thin LTO builds with PS4 and Darwin toolchains (authored by evgeny777). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to

[PATCH] D54905: [AddressSanitizer] Add flag to disable linking with CXX runtime

evgeny777 added a comment. > ASan uses -Wl,-whole-archive to pull all its members Yep, I forgot about this. Looks like weak new/delete will do the job then. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54905/new/ https://reviews.llvm.org/D54905

[PATCH] D54905: [AddressSanitizer] Add flag to disable linking with CXX runtime

evgeny777 added a comment. > don't think so. It's just symbol interposition. Every module in the process > gets the same definition. If I have two weak symbols in lib1.a and lib2.a and doesn't define strong one, which one will be chosen at link time? CHANGES SINCE LAST ACTION

[PATCH] D54905: [AddressSanitizer] Add flag to disable linking with CXX runtime

evgeny777 added a comment. > How do you do this now? Sometimes with this patch, sometimes simply issuing linker command line manually > Is that related to https://github.com/google/sanitizers/issues/295 ? I don't think so. I'm experiencing link error, not C++ standard discrepancy. > Would

[PATCH] D55048: [ThinLTO] Allow importing of multiple symbols with same GUID

evgeny777 accepted this revision. evgeny777 added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55048/new/ https://reviews.llvm.org/D55048 ___

[PATCH] D54905: [AddressSanitizer] Add flag to disable linking with CXX runtime

evgeny777 added a comment. Unfortunately, this is not an option. I have very custom heap implementation (in fact multiple heap types sharing contiguous memory block), so ASAN heap can't be a drop-in replacement. I'm using manual poisoning and it works pretty well, but have to disable C++

[PATCH] D54905: [AddressSanitizer] Add flag to disable linking with CXX runtime

evgeny777 created this revision. evgeny777 added reviewers: kcc, samsonov. This prevents link errors when operators new/delete are overridden by application. https://reviews.llvm.org/D54905 Files: include/clang/Driver/Options.td lib/Driver/SanitizerArgs.cpp test/Driver/sanitizer-ld.c