[PATCH] D146764: [clang] Make predefined expressions string literals under -fms-extensions

aeubanks updated this revision to Diff 518548. aeubanks added a comment. address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146764/new/ https://reviews.llvm.org/D146764 Files: clang/include/clang/AST/Expr.h clang/include/clang/AST/

[PATCH] D146764: [clang] Make predefined expressions string literals under -fms-extensions

aeubanks marked an inline comment as done. aeubanks added a comment. In D146764#4310398 , @aaron.ballman wrote: > I think you're missing changes in ASTReaderStmt.cpp and ASTWriterStmt.cpp, so > serialization through modules or PCH won't work without tha

[PATCH] D149187: [clang] Canonicalize system headers in dependency file when -canonical-prefixes

aeubanks added a comment. In D149187#4311354 , @chapuni wrote: > I am afraid this would be incompatible to bazel due to deep symlinks for > sandbox. I assume bazel would use `-no-canonical-prefixes`? Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D146764: [clang] Make predefined expressions string literals under -fms-extensions

aeubanks updated this revision to Diff 518777. aeubanks added a comment. add module test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146764/new/ https://reviews.llvm.org/D146764 Files: clang/include/clang/AST/Expr.h clang/include/clang/AST/I

[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

aeubanks added inline comments. Comment at: llvm/include/llvm/IR/DiagnosticInfo.h:1122 } + void + getInliningDecisions(SmallVectorImpl &InliningDecisions) const; can this return a SmallVector instead of taking one as a mutable param? Comme

[PATCH] D146764: [clang] Make predefined expressions string literals under -fms-extensions

aeubanks updated this revision to Diff 518887. aeubanks added a comment. add warning Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146764/new/ https://reviews.llvm.org/D146764 Files: clang/include/clang/AST/Expr.h clang/include/clang/AST/Ignor

[PATCH] D146764: [clang] Make predefined expressions string literals under -fms-extensions

aeubanks added a comment. added the warning, not sure if this needs more tests for testing the interaction between `-pedantic`, `-Wmicrosoft`, `-Wmicrosoft-init-from-predefined` or if that's already assumed to work Comment at: clang/test/Modules/predefined.cpp:6 +// RUN: %cla

[PATCH] D143624: Inlining: Run the legacy AlwaysInliner before the regular inliner.

aeubanks added a comment. In D143624#4315508 , @nikic wrote: > In D143624#4315468 , @dmgreen wrote: > >> It looks like there is quite a lot more optimization that happens to the >> function being always-inlined (

[PATCH] D146764: [clang] Make predefined expressions string literals under -fms-extensions

aeubanks updated this revision to Diff 519215. aeubanks added a comment. add -verify to module test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146764/new/ https://reviews.llvm.org/D146764 Files: clang/include/clang/AST/Expr.h clang/include/

[PATCH] D149800: [WIP][PGO] Add ability to mark cold functions as optsize/minsize/optnone

aeubanks created this revision. Herald added subscribers: wlei, Enna1, ormris, wenlei, steven_wu, hiraditya. Herald added a project: All. aeubanks requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. The performance of cold fun

[PATCH] D149800: [WIP][PGO] Add ability to mark cold functions as optsize/minsize/optnone

aeubanks added a comment. looking for feedback if this makes sense at all Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149800/new/ https://reviews.llvm.org/D149800 ___ cfe-commits mailing list cfe-commi

[PATCH] D139296: [msan][CodeGen] Set noundef for C return value

aeubanks added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:1828 !Module.MayDropFunctionReturn(Module.getContext(), RetTy) || Module.getLangOpts().Sanitize.has(SanitizerKind::Memory) || Module.getLangOpts().Sanitize.has(SanitizerKind::Re

[PATCH] D136554: Implement CWG2631

aeubanks added a comment. the following now produces a link error: $ cat /tmp/a.cc #include #include struct S { std::map a; }; using T = std::array; class C { T t{}; }; int main() { C c; } $ ./build/rel/bin/clang++ -o /dev/null /tm

[PATCH] D136554: Implement CWG2631

aeubanks reopened this revision. aeubanks added a comment. This revision is now accepted and ready to land. I've bisected another crash to this change. Reverting, and currently creducing a repro. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136554

[PATCH] D136554: Implement CWG2631

aeubanks added a comment. a really bad reduced repro which also has random syntax errors: using string = struct basic_string { constexpr basic_string(char *) {} constexpr ~basic_string( } struct AutocompleteParsingResult struct optional { template constexpr optional(U

[PATCH] D136554: Implement CWG2631

aeubanks added a comment. yes, it was chrome I went ahead and tried the latest patch, it successfully compiles the file that crashed before. I built all of chrome, and now I'm getting one last linker error, I'll try to get some more info about that Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D136554: Implement CWG2631

aeubanks added a comment. no, I got $ ninja -C out/MyClang/ content_unittests ld.lld: error: undefined symbol: mojo::Receiver>::Receiver(content::mojom::TestInterfaceForDefer*) >>> referenced by mojo_binder_policy_applier_unittest.cc:90 (../../content/browser/mojo_binder_policy_applier_un

[PATCH] D136554: Implement CWG2631

aeubanks added a comment. I'm fine with relanding and seeing if anything else pops up Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136554/new/ https://reviews.llvm.org/D136554 ___ cfe-commits mailing li

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

aeubanks added a comment. # no asserts, no debuginfo, -O3 $ du -b clang-stable clang-patched 83539528clang-stable 83528456clang-patched clang's code size actually goes down with this patch llvm-compile-time-tracker: https://llvm-compile-time-tracker.com/compare.php?from=a

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

aeubanks added inline comments. Comment at: clang/lib/CodeGen/Address.h:30 + // Int portion stores lower 3 bits of the log of the alignment. + llvm::PointerIntPair ElementType; nikic wrote: > Are we guaranteed 3 bits even on 32-bit architectures? Apparently n

[PATCH] D117965: [AlwaysInliner] Enable call site inlining to make flatten attribute working again (PR53360)

aeubanks added a comment. The existing implementation only inlines top level call sites, which doesn't match gcc where all calls are recursively inlined [1]. gcc's implementation makes more sense IMO, only inlining top level call sites doesn't seem super useful. I'd vote for properly implement

[PATCH] D117965: [AlwaysInliner] Enable call site inlining to make flatten attribute working again (PR53360)

aeubanks added a comment. sounds good for the purposes of going back to what we had before, but we should reconsider the attribute support Comment at: llvm/lib/Transforms/IPO/AlwaysInliner.cpp:95 - // Remember to try and delete this function afterward. This both avoids

[PATCH] D117965: [AlwaysInliner] Enable call site inlining to make flatten attribute working again (#53360)

aeubanks accepted this revision. aeubanks added inline comments. This revision is now accepted and ready to land. Comment at: llvm/test/Transforms/Coroutines/coro-retcon-once-private.ll:6 -; CHECK: define internal { i8*, i32 } @f(i8* %buffer, i32* %array) -; CHECK-NEXT: entry:

[PATCH] D118169: [NFC][CodeGen] Use llvm::DenseMap for DeferredDecls

aeubanks accepted this revision. aeubanks added a comment. This revision is now accepted and ready to land. https://llvm-compile-time-tracker.com/compare.php?from=c39d22d1968cf07e54b5816ba76dccef8acaace1&to=1958f5a5fb5be3fb5b6bad5079a2f3485db0b0e9&stat=instructions mostly neutral but a tiny bit of

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

aeubanks updated this revision to Diff 403070. aeubanks added a comment. only use PointerIntPair when alignof(Value*) >= 8 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117262/new/ https://reviews.llvm.org/D117262 Files: clang/lib/CodeGen/Addres

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

aeubanks updated this revision to Diff 403323. aeubanks added a comment. address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117262/new/ https://reviews.llvm.org/D117262 Files: clang/lib/CodeGen/Address.h Index: clang/lib/CodeGen/Add

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

aeubanks added inline comments. Comment at: clang/lib/CodeGen/Address.h:29 +// so we fallback to storing the alignment separately. +template = 8> class AddressImpl {}; + nikic wrote: > Why do we need the extra T parameter? without it we end up instantiating `Addr

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb1613f05ae0c: [NFC] Store Address's alignment into PointerIntPairs (authored by aeubanks). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

aeubanks added inline comments. Comment at: clang/lib/CodeGen/Address.h:70 +unsigned AlignLog = (Pointer.getInt() << 3) | ElementType.getInt(); +return CharUnits::fromQuantity(1UL << AlignLog); + } LegalizeAdulthood wrote: > This is causing warnings to b

[PATCH] D118313: [Driver] Remove -fno-experimental-new-pass-manager

aeubanks added a comment. can we also remove the -cc1 option? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118313/new/ https://reviews.llvm.org/D118313 ___ cfe-commits mailing list cfe-commits@lists.llv

[PATCH] D118313: [Driver] Remove -fno-experimental-new-pass-manager

aeubanks accepted this revision. aeubanks added a comment. This revision is now accepted and ready to land. I think this is fine, it gives clang users another chance to report new PM blockers Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118313/new

[PATCH] D118744: [clang] Don't cache function type after clearing clang->llvm type cache

aeubanks created this revision. aeubanks requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. We clear the type cache when SkippedLayout is true and we're converting a function type. However, we then immediately put the computed entry in the cac

[PATCH] D118744: [clang] Don't cache function type after clearing clang->llvm type cache

aeubanks updated this revision to Diff 405129. aeubanks added a comment. update Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118744/new/ https://reviews.llvm.org/D118744 Files: clang/lib/CodeGen/CodeGenTypes.cpp clang/test/CodeGenCXX/type-cac

[PATCH] D118744: [clang] Don't cache function type after clearing clang->llvm type cache

aeubanks planned changes to this revision. aeubanks added a comment. I'm seeing a similar crash with struct z { static z dc(z); z (*di)(z); }; z bar = z::dc({}); going to try to fix that here as well Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D120527: [OpaquePtr][AArch64] Use elementtype on various intrinsics

aeubanks updated this revision to Diff 411274. aeubanks added a comment. update Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120527/new/ https://reviews.llvm.org/D120527 Files: clang/lib/CodeGen/CGBuiltin.cpp clang/test/CodeGen/arm_acle.c c

[PATCH] D120666: [docs] Add note about interaction between clang plugins and -clear-ast-before-backend

aeubanks created this revision. aeubanks 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/D120666 Files: clang/docs/ClangPlugins.rst Index: clang/docs/ClangPlugins.rst

[PATCH] D120666: [docs] Add note about interaction between clang plugins and -clear-ast-before-backend

aeubanks updated this revision to Diff 411825. aeubanks added a comment. add header Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120666/new/ https://reviews.llvm.org/D120666 Files: clang/docs/ClangPlugins.rst Index: clang/docs/ClangPlugins.rs

[PATCH] D120666: [docs] Add note about interaction between clang plugins and -clear-ast-before-backend

aeubanks updated this revision to Diff 411829. aeubanks added a comment. reword Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120666/new/ https://reviews.llvm.org/D120666 Files: clang/docs/ClangPlugins.rst Index: clang/docs/ClangPlugins.rst ==

[PATCH] D120666: [docs] Add note about interaction between clang plugins and -clear-ast-before-backend

aeubanks updated this revision to Diff 411851. aeubanks added a comment. reword Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120666/new/ https://reviews.llvm.org/D120666 Files: clang/docs/ClangPlugins.rst Index: clang/docs/ClangPlugins.rst ==

[PATCH] D120666: [docs] Add note about interaction between clang plugins and -clear-ast-before-backend

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf1315c61a03e: [docs] Add note about interaction between clang plugins and -clear-ast-before… (authored by aeubanks). Repository: rG LLVM Github Mo

[PATCH] D120757: [NFC][Clang][OpaquePtr] Remove the call to Address::deprecated in CreatePointerBitCastOrAddrSpaceCast

aeubanks accepted this revision. aeubanks added a comment. This revision is now accepted and ready to land. lg if check-clang passes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120757/new/ https://reviews.llvm.org/D120757 ___

[PATCH] D101017: [NewPM] Make GlobalsAA available earlier in the pipeline

aeubanks added a comment. this seems to have some compile time improvements? http://llvm-compile-time-tracker.com/compare.php?from=a62cbd9a0211d08bace8794b435996890feb44d4&to=7805d7f72c337bfcc2fbc2dc9d2b9ac23474d5d9&stat=instructions Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D100917: [NewPM] Only invalidate modified functions' analyses in CGSCC passes

aeubanks updated this revision to Diff 339782. aeubanks added a comment. Herald added subscribers: cfe-commits, steven_wu. Herald added a project: clang. add test use CFGAnalyses uncovered another place in updateCGAndAnalysisManager() to change depends on D101017

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

aeubanks added a comment. Looks good in terms of false positives on Chrome. This did uncover a lot of issues, although many of them are due to only using some variable in some config with `#ifdef` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1005

[PATCH] D101017: [NewPM] Make GlobalsAA available earlier in the pipeline

aeubanks added a comment. Looking at one of the examples in llvm-test-suite that regressed, this seems to be regressing some very simple code: void f(double *a) { double c[20] = { 0.1051, 0.0157, 0.0185, 0.0089, 0.0219, 0.0141, 0.0097, 0.0758, 0.0168, 0.1188, 0.1635, 0.0112

[PATCH] D101017: [NewPM] Make GlobalsAA available earlier in the pipeline

aeubanks added a comment. Actually, the InstCombine transform allows LoopFullUnrollPass to unroll the copy loop which results in a bunch of stores. Previously during LoopFullUnrollPass we wouldn't see the global within the loop. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D101017: [NewPM] Make GlobalsAA available earlier in the pipeline

aeubanks added a comment. ah the issue is LoopIdiomRecognize can't recognize the following as a memcpy: @__const.f.c = private unnamed_addr constant [20 x double] [double 1.051000e-01, double 1.57e-02, double 1.85e-02, double 0x3F823A29C779A6B5, double 2.19e-02, double 1.41e-0

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

aeubanks added a comment. I can land this for you, but could you update the description to be a bit more descriptive? Explaining what the warnings do. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100581/new/ https://reviews.llvm.org/D100581

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

aeubanks added a comment. yeah you have to manually go to "Edit Revision" and update the message. I'll just amend it locally though Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100581/new/ https://reviews.llvm.org/D100581 ___

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

This revision was automatically updated to reflect the committed changes. Closed by commit rG9b0501abc7b5: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable (authored by mbenfield, committed by aeubanks). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://re

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

aeubanks added a comment. In D100581#2718840 , @uabelho wrote: > I noticed that with this patch > > libcxx/test/std/strings/basic.string/string.cons/iter_alloc_deduction.fail.cpp > fails: > > error: 'warning' diagnostics seen but not expected: >

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

aeubanks added a comment. In D100581#2718947 , @Abpostelnicu wrote: > I think this added a regression, where you have this context: > > uint32_t LocalAccessible::SelectedItemCount() { > uint32_t count = 0; > AccIterator iter(this, filters::GetS

[PATCH] D101378: [llvm, clang] Remove stdlib includes from .h files without `std::`

aeubanks accepted this revision. aeubanks added a comment. This revision is now accepted and ready to land. lgtm CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101378/new/ https://reviews.llvm.org/D101378 ___ cfe-commits mailing list cfe-commi

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

aeubanks added a comment. Sorry about that, although I don't think emails went to me since the email was somebody else's. But point taken, I'll coordinate better in the future when landing someone else's patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D101017: [NewPM] Make GlobalsAA available earlier in the pipeline

aeubanks abandoned this revision. aeubanks added a comment. turns out it was an issue with GlobalsAA not being recalculated and being too conservative anyway, I don't think adding GlobalsAA early on really does much, adding it to the inliner pipeline seems good enough https://reviews.llvm.org/D

[PATCH] D101480: Revert "[Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable"

aeubanks added a comment. https://reviews.llvm.org/rG6d8d1338629ceeaf6f56dc9eabc72e1a63f05169 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101480/new/ https://reviews.llvm.org/D101480 ___ cfe-commits ma

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

aeubanks added a comment. reverted regarding having a reviewer who is knowledgable in clang diagnostics, I assumed that george.burgess.iv was knowledgable and was happy with the change after his comments, perhaps I should have waited for an explicit LGTM Repository: rG LLVM Github Monorepo

[PATCH] D101561: [Prototype] Introduce attribute for ignoring C++ ABI

aeubanks created this revision. Herald added a reviewer: aaron.ballman. aeubanks 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/D101561 Files: clang/include/clang/Basic/Att

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

aeubanks added a comment. regarding adding these to a warning group in a separate change, it's easier to revert a smaller second change that adds this to warning groups, and easier to iterate upon in tree when there are discovered issues it's not dead code if it has tests Repository: rG LLVM

[PATCH] D101561: [Prototype] Introduce attribute for ignoring C++ ABI

aeubanks added a comment. I'm first testing out if there are even noticeable benefits to doing this, then if there are I'll put something together to post to cfe-dev. https://crbug.com/1028731 for some background. I'm aware of trivial_abi and will consider wrapping this up into that, although w

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

aeubanks added a comment. casting to void is the normal way to get around these warnings, these new warnings should also not diagnose anything with a cast to void Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100581/new/ https://reviews.llvm.org/D

[PATCH] D101561: [Prototype] Introduce attribute for ignoring C++ ABI

aeubanks added a comment. ah I wasn't aware of that, thanks! sorry for the extra noise Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101561/new/ https://reviews.llvm.org/D101561 ___ cfe-commits mailing l

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

aeubanks added a comment. As you said in the description, this overlaps a lot with -debug-pass-manager. What exactly is too verbose with the existing debug logging? I'd strongly prefer to have this consolidated into one pass instrumentation rather than two. Having a new pipeline test is very te

[PATCH] D101379: [NewPM] Invalidate AAManager after populating GlobalsAA

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2df3426fd15e: [NewPM] Invalidate AAManager after populating GlobalsAA (authored by aeubanks). Herald added a project: clang. Herald added a subscribe

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

aeubanks added a comment. I've already run into having to update these two golden file tests twice, we really shouldn't be having multiple types of golden file tests for the same thing in different locations and with different formats. I'd like to revert this change. Perhaps something like htt

[PATCH] D100917: [NewPM] Only invalidate modified functions' analyses in CGSCC passes

aeubanks updated this revision to Diff 342602. aeubanks added a comment. Herald added a subscriber: nikic. rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100917/new/ https://reviews.llvm.org/D100917 Files: clang/test/CodeGen/thinlto-distri

[PATCH] D100917: [NewPM] Only invalidate modified functions' analyses in CGSCC passes

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd14d84af2f5e: [NewPM] Only invalidate modified functions' analyses in CGSCC passes (authored by aeubanks). Repository: rG LLVM Github Monorepo CH

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

aeubanks added a comment. In D99599#2735582 , @evgeny777 wrote: >> 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

[PATCH] D100917: [NewPM] Only invalidate modified functions' analyses in CGSCC passes

aeubanks added a comment. In D100917#2735651 , @nikic wrote: > An unfortunate side-effect of this change is that NewPM uses even more > memory. tramp3d-v4 is up 20% in max-rss > (https://llvm-compile-time-tracker.com/compare.php?from=4ef1f90e4d564b872e3

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

aeubanks accepted this revision. aeubanks added a comment. If you'd like you can split this into separate changes, one for adding the warning and another for adding it into warning groups, either way is fine. Seems like the reported false positives have been addressed. Repository: rG LLVM Git

[PATCH] D102093: [NewPM] Move analysis invalidation/clearing logging to instrumentation

aeubanks created this revision. aeubanks added reviewers: asbirlea, ychen. Herald added subscribers: dexonsmith, steven_wu, zzheng, hiraditya. aeubanks requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. We're trying to move D

[PATCH] D102093: [NewPM] Move analysis invalidation/clearing logging to instrumentation

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6f7131002b6a: [NewPM] Move analysis invalidation/clearing logging to instrumentation (authored by aeubanks). Repository: rG LLVM Github Monorepo

[PATCH] D102196: [NewPM] Add options to PrintPassInstrumentation

aeubanks created this revision. Herald added subscribers: nikic, hiraditya. aeubanks requested review of this revision. Herald added a reviewer: jdoerfert. Herald added subscribers: llvm-commits, cfe-commits, sstefan1. Herald added projects: clang, LLVM. To bring D99599

[PATCH] D102196: [NewPM] Add options to PrintPassInstrumentation

aeubanks updated this revision to Diff 344201. aeubanks added a comment. assert Indent >= 0 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102196/new/ https://reviews.llvm.org/D102196 Files: clang/lib/CodeGen/BackendUtil.cpp clang/test/Driver/d

[PATCH] D114908: [clang] Don't call inheritDefaultTemplateArguments() on CXXDeductionGuideDecl's template parameters

aeubanks added inline comments. Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:3784-3790 + if (auto *TD = dyn_cast(D)) { +// CXXDeductionGuideDecls reference the class template parameters so we need +// to make sure not to call this twice on the same template par

[PATCH] D116983: Clone constructor template parameters when creating deduction guide

aeubanks created this revision. rsmith added inline comments. aeubanks published this revision for review. Herald added a project: clang. Herald added a subscriber: cfe-commits. Comment at: clang/lib/Sema/SemaTemplate.cpp:2174 + Args.addOuterTemplateArguments(SubstArgs); +

[PATCH] D116983: Clone constructor template parameters when creating deduction guide

aeubanks updated this revision to Diff 399514. aeubanks added a comment. update Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116983/new/ https://reviews.llvm.org/D116983 Files: clang/lib/Sema/SemaTemplate.cpp clang/test/Modules/Inputs/ctad/a.

[PATCH] D116983: Clone constructor template parameters when creating deduction guide

aeubanks added inline comments. Comment at: clang/lib/Sema/SemaTemplate.cpp:2174 + Args.addOuterTemplateArguments(SubstArgs); + Args.addOuterRetainedLevel(); + NamedDecl *NewParam = transformTemplateParameter(Param, Args); rsmith wrote: > This oute

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

aeubanks created this revision. aeubanks requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This mitigates the extra memory caused by D115725 . Repository: rG LLVM Github Monorepo https://reviews.llvm.or

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

aeubanks updated this revision to Diff 399840. aeubanks added a comment. update Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117262/new/ https://reviews.llvm.org/D117262 Files: clang/lib/CodeGen/Address.h Index: clang/lib/CodeGen/Address.h ==

[PATCH] D114908: [clang] Don't call inheritDefaultTemplateArguments() on CXXDeductionGuideDecl's template parameters

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

[PATCH] D115790: [Coroutines] Run CoroEarly Pass in front of AlwaysInliner in O0 pass and warn for always_inline coroutine

aeubanks added a comment. pass manager changes seem fine, but I'll let somebody else lgtm Comment at: llvm/test/Other/new-pm-O0-defaults.ll:34 ; CHECK-DIS-NEXT: Running pass: AddDiscriminatorsPass +; CHECK-CORO: Running pass: CoroEarlyPass ; CHECK-DIS-NEXT: Running pass: Alwa

[PATCH] D115798: Provide SmallAttrBuilder as a lightweight alternative to AttrBuilder

aeubanks added a comment. I agree that we really should only have one attribute builder class. A SmallVector does seem nicer than having a static array the size of all possible attributes. We should avoid creating copies of AttributeLists/Sets into AttrBuilders and just have AttrBuilder be a li

[PATCH] D94355: [Passes] Add relative lookup table converter pass

aeubanks added inline comments. Comment at: llvm/test/Other/opt-O3-pipeline-enable-matrix.ll:322-323 ; CHECK-NEXT: Simplify the CFG +; CHECK-NEXT: Relative Lookup Table Converter +; CHECK-NEXT: FunctionPass Manager ; CHECK-NEXT: Annotation Remarks --

[PATCH] D94355: [Passes] Add relative lookup table converter pass

aeubanks added inline comments. Comment at: llvm/test/Other/opt-O3-pipeline-enable-matrix.ll:322-323 ; CHECK-NEXT: Simplify the CFG +; CHECK-NEXT: Relative Lookup Table Converter +; CHECK-NEXT: FunctionPass Manager ; CHECK-NEXT: Annotation Remarks --

[PATCH] D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1

aeubanks added a comment. could you clang-format the patch? Comment at: clang/lib/CodeGen/CGException.cpp:1666-1667 + ++J) { + if (isa(J)) { +auto LI = dyn_cast(J); +LI->setVolatile(true); ``` if (auto*LI = dyn_cast<...>(J)) { ...

[PATCH] D100231: [NewPM] Cleanup IR printing instrumentation

aeubanks updated this revision to Diff 337572. aeubanks marked 2 inline comments as done. aeubanks added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D100231: [NewPM] Cleanup IR printing instrumentation

aeubanks added a comment. I'm not sure that having a helper method with 4 lambdas is any cleaner. Normal control flow in a function is easier to read and understand, and not really that much more code. Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:222 if (any_

[PATCH] D100231: [NewPM] Cleanup IR printing instrumentation

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc8f0a7c215ab: [NewPM] Cleanup IR printing instrumentation (authored by aeubanks). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

aeubanks added a comment. the description should be clearer about what these warnings do running this over an existing codebase to see what fires is probably a good idea (if you haven't already done that) Comment at: clang/test/Sema/vector-gcc-compat.c:38 v2i64 v2i64_a = (

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

aeubanks added a comment. Running this over Chromium, I'm getting $ cat /tmp/a.cc struct A { int i; A(int i): i(i) {} }; $ ./build/rel/bin/clang++ -fsyntax-only /tmp/a.cc -Wunused-but-set-parameter /tmp/a.cc:3:9: warning: parameter 'i' set but not used [-Wunused-but-set-para

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

aeubanks added a comment. -Wunused-but-set-variable is firing on `x` at [1] [1] https://source.chromium.org/chromium/chromium/src/+/master:third_party/abseil-cpp/absl/synchronization/mutex.h;drc=07cb7b184515ad207d30f00d0b00b8ce96d0a750;l=947 ../../third_party/abseil-cpp/absl/synchronization/m

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

aeubanks added a comment. another false positive: ../../third_party/abseil-cpp/absl/strings/internal/str_format/float_conversion.cc:879:20: error: variable 'kBufSizeForHexFloatRepr' set but not used [-Werror,-Wunused-but-set-variable] constexpr size_t kBufSizeForHexFloatRepr = https://s

[PATCH] D101017: [NewPM] Make GlobalsAA available earlier in the pipeline

aeubanks created this revision. Herald added subscribers: wenlei, steven_wu, hiraditya. aeubanks requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. A future change will cause fewer analyses to be invalidated. Currently, the p

[PATCH] D93656: Moving UniqueInternalLinkageNamesPass to the start of IR pipelines.

aeubanks accepted this revision. aeubanks added a comment. This revision is now accepted and ready to land. LGTM with nit, but somebody else more familiar with this should LGTM to make sure this makes sense Comment at: llvm/test/Transforms/UniqueLinkageNames/unique-internal-l

[PATCH] D93656: Moving UniqueInternalLinkageNamesPass to the start of IR pipelines.

aeubanks added inline comments. Comment at: llvm/tools/opt/NewPMDriver.cpp:136-138 +static cl::opt PseudoProbeForProfiling( +"new-pm-pseudo-probe-for-profiling", cl::init(false), cl::Hidden, +cl::desc("Emit pseudo probes to enable PGO profile generation.")); -

[PATCH] D92936: [Sema] Fix deleted function problem in implicitly movable test

aeubanks added a comment. This broke struct A { A(); A(A&&)=delete; private: A(const A&); friend class B; }; struct B { A foo() { A a; return a; } }; with /tmp/a.cc:12:12: error: call to deleted constructor of 'A' return a; ^

[PATCH] D92936: [Sema] Fix deleted function problem in implicitly movable test

aeubanks added a comment. Thanks, and checking other compilers with godbolt is a good idea (I had tried gcc locally but it was likely too old) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92936/new/ https://reviews.llvm.org/D92936 __

[PATCH] D94019: Switching Clang UniqueInternalLinkageNamesPass scheduling to using the LLVM one with newpm.

aeubanks added a comment. I don't think a test is necessary for an NFC change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94019/new/ https://reviews.llvm.org/D94019 ___ cfe-commits mailing list cfe-co

[PATCH] D94019: Switching Clang UniqueInternalLinkageNamesPass scheduling to using the LLVM one with newpm.

aeubanks added a comment. Oh sorry, yeah this isn't NFC. But I still don't think this needs a new test. We're going from a custom Clang implementation of adding passes to something more generic that's also more tested within LLVM. So IMO we just need an end to end test, which we already have in

<    2   3   4   5   6   7   8   >