[PATCH] D131980: [Passes] Don't run tail-call-elim in -O1

aeubanks updated this revision to Diff 453070. aeubanks added a comment. delete comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131980/new/ https://reviews.llvm.org/D131980 Files: clang/test/CodeGen/aarch64-ls64-inline-asm.c clang/test/C

[PATCH] D131980: [Passes] Don't run tail-call-elim in -O1

aeubanks added a comment. In D131980#3726750 , @efriedma wrote: > Are you concerned about tail-call marking, or the recursive call->loop > transform? specifically tail call marking we have symbolizers that stopped displaying some frames with this chang

[PATCH] D131986: [inlining] Add a clang option to control inlining of functions based on stack size

aeubanks accepted this revision. aeubanks added a comment. This revision is now accepted and ready to land. perhaps a test that "inline-max-stacksze" doesn't appear if the flag isn't specified? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131986/new/ https://reviews.llvm.org/D131986

[PATCH] D131980: [Passes] Don't run tail-call-elim in -O1

aeubanks abandoned this revision. aeubanks added a comment. will use Eli's suggestion instead Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131980/new/ https://reviews.llvm.org/D131980 ___ cfe-commits ma

[PATCH] D128830: [Pipelines] Introduce DAE after ArgumentPromotion

aeubanks accepted this revision. aeubanks added a comment. This revision is now accepted and ready to land. sorry for the big delay, I did run the benchmarks and it looks fine ran this through llvm-compile-time-tracker, looks good https://llvm-compile-time-tracker.com/compare.php?from=552b59b9e6

[PATCH] D128830: [Pipelines] Introduce DAE after ArgumentPromotion

aeubanks added a comment. I'd try compiling with `clang -g2` to see the effects of this patch on the debug info in the IR Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128830/new/ https://reviews.llvm.org/D128830 _

[PATCH] D128830: [Pipelines] Introduce DAE after ArgumentPromotion

aeubanks added a comment. reduced: define void @a() { entry: %call = call float @strtof(ptr noundef null, ptr noundef null) ret void } define internal float @strtof(ptr noundef %0, ptr noundef %1) nounwind { entry: ret float 0.0 } `./build/rel/bin/opt -passes='inline,a

[PATCH] D128830: [Pipelines] Introduce DAE after ArgumentPromotion

aeubanks added a comment. sent out https://reviews.llvm.org/D132764 to fix the CGSCC crash Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128830/new/ https://reviews.llvm.org/D128830 ___ cfe-commits maili

[PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

aeubanks updated this revision to Diff 462245. aeubanks added a comment. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. "fix" lldb test (hopefully) skip when msan add flag to disable (to be removed in the future) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

[PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

aeubanks updated this revision to Diff 462247. aeubanks added a comment. add flag to disable (to be removed in the future) "fix" lldb test skip on msan instrumented functions Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133036/new/ https://reviews

[PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

aeubanks updated this revision to Diff 462249. aeubanks added a comment. extra parens Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133036/new/ https://reviews.llvm.org/D133036 Files: clang/test/CodeGenOpenCL/overload.cl lldb/test/API/function

[PATCH] D128830: [Pipelines] Introduce DAE after ArgumentPromotion

aeubanks added a comment. that was fixed and I've relanded the patch hopefully there aren't any more CGSCC issues this uncovers Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128830/new/ https://reviews.llvm.org/D128830 ___

[PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

aeubanks added a comment. @vitalybuka can we turn on -fsanitize-memory-param-retval by default upstream? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133036/new/ https://reviews.llvm.org/D133036 ___ cfe

[PATCH] D134669: [clang][msan] Turn on -fsanitize-memory-param-retval by default

aeubanks created this revision. aeubanks added a reviewer: vitalybuka. Herald added a project: All. aeubanks requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This eagerly reports use of undef values when passed to noundef parameters or retur

[PATCH] D134669: [clang][msan] Turn on -fsanitize-memory-param-retval by default

aeubanks added a comment. hmm, looks like this breaks a bunch of msan tests with `-DLLVM_ENABLE_RUNTIMES=compiler-rt` and `ninja && ninja check-runtimes` MemorySanitizer-X86_64 :: Linux/swapcontext_annotation_reset.cpp MemorySanitizer-X86_64 :: bsearch.cpp MemorySanitizer-X86_64 :: chained

[PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

aeubanks added a comment. In D133036#3816002 , @aeubanks wrote: > @vitalybuka can we turn on -fsanitize-memory-param-retval by default upstream? attempt in https://reviews.llvm.org/D134669 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D134669: [clang][msan] Turn on -fsanitize-memory-param-retval by default

aeubanks added a comment. before looking into those, is it ok to proceed with this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134669/new/ https://reviews.llvm.org/D134669 ___ cfe-commits mailing list

[PATCH] D134669: [clang][msan] Turn on -fsanitize-memory-param-retval by default

aeubanks added a comment. do you want the tests to all properly work with -fsanitize-memory-param-retval, or just pin them to -fno-sanitize-memory-param-retval if they're failing? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134669/new/ https://r

[PATCH] D134669: [clang][msan] Turn on -fsanitize-memory-param-retval by default

aeubanks updated this revision to Diff 463061. aeubanks added a comment. add release notes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134669/new/ https://reviews.llvm.org/D134669 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Driver

[PATCH] D134669: [clang][msan] Turn on -fsanitize-memory-param-retval by default

aeubanks updated this revision to Diff 463096. aeubanks added a comment. Herald added subscribers: pcwang-thead, s.egerton, simoncook. update test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134669/new/ https://reviews.llvm.org/D134669 Files:

[PATCH] D134669: [clang][msan] Turn on -fsanitize-memory-param-retval by default

aeubanks updated this revision to Diff 463322. aeubanks added a comment. rebase (to run through presubmits with test fixes) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134669/new/ https://reviews.llvm.org/D134669 Files: clang/docs/ReleaseNotes

[PATCH] D134669: [clang][msan] Turn on -fsanitize-memory-param-retval by default

aeubanks updated this revision to Diff 463327. aeubanks added a comment. mention -fno-sanitize-memory-param-retval in release notes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134669/new/ https://reviews.llvm.org/D134669 Files: clang/docs/Rele

[PATCH] D134669: [clang][msan] Turn on -fsanitize-memory-param-retval by default

This revision was automatically updated to reflect the committed changes. Closed by commit rG44ad67031cc1: [clang][msan] Turn on -fsanitize-memory-param-retval by default (authored by aeubanks). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134669/ne

[PATCH] D134825: [clang] Add debug info in MicrosoftCXXABI::EmitVirtualMemPtrThunk()

aeubanks created this revision. aeubanks added a reviewer: rnk. Herald added a project: All. aeubanks requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fixes https://crbug.com/1355639 Repository: rG LLVM Github Monorepo https://reviews.l

[PATCH] D134825: [clang] Add debug info in MicrosoftCXXABI::EmitVirtualMemPtrThunk()

aeubanks updated this revision to Diff 463661. aeubanks added a comment. update test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134825/new/ https://reviews.llvm.org/D134825 Files: clang/lib/CodeGen/MicrosoftCXXABI.cpp clang/test/CodeGenCXX/

[PATCH] D134825: [clang] Add debug info in MicrosoftCXXABI::EmitVirtualMemPtrThunk()

aeubanks updated this revision to Diff 463669. aeubanks edited the summary of this revision. aeubanks added a comment. update test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134825/new/ https://reviews.llvm.org/D134825 Files: clang/lib/CodeGe

[PATCH] D134825: [clang] Add debug info in MicrosoftCXXABI::EmitVirtualMemPtrThunk()

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2f3d7c2cc770: [clang] Add debug info in MicrosoftCXXABI::EmitVirtualMemPtrThunk() (authored by aeubanks). Repository: rG LLVM Github Monorepo CHA

[PATCH] D135461: [LLDB] Fix crash when printing a struct with a static wchar_t member

aeubanks created this revision. aeubanks added a reviewer: DavidSpickett. Herald added a project: All. aeubanks requested review of this revision. Herald added projects: clang, LLDB. Herald added subscribers: lldb-commits, cfe-commits. Similar to D135170 . Repos

[PATCH] D135461: [LLDB] Fix crash when printing a struct with a static wchar_t member

aeubanks added inline comments. Comment at: clang/lib/AST/StmtPrinter.cpp:1298 + case BuiltinType::WChar_U: +break; // no suffix. } DavidSpickett wrote: > Is there any reason to have a suffix here? > > I admit this function is puzzling to me anyway given

[PATCH] D135461: [LLDB] Fix crash when printing a struct with a static wchar_t member

aeubanks updated this revision to Diff 466622. aeubanks added a comment. update python test, although those changes don't actually test the code path changed here, the code path here is tested via the existing `self.expect("image lookup -t A")` Repository: rG LLVM Github Monorepo CHANGES SI

[PATCH] D135461: [LLDB] Fix crash when printing a struct with a static wchar_t member

This revision was automatically updated to reflect the committed changes. Closed by commit rGeab5c2f94f5a: [LLDB] Fix crash when printing a struct with a static wchar_t member (authored by aeubanks). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1354

[PATCH] D128567: [WIP][Fuchsia] Set LLVM_TOOL_LLD_BUILD to allow some extra runtimes tests to run

aeubanks added a comment. I've noticed that we're skipping compiler-rt tests with `REQUIRED: lld-available` because `LLVM_TOOL_LLD_BUILD` needs to be defined. any update on the proposed alternative? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D12

[PATCH] D128567: [WIP][Fuchsia] Set LLVM_TOOL_LLD_BUILD to allow some extra runtimes tests to run

aeubanks added a comment. In D128567#3863222 , @abrachet wrote: > In D128567#3863178 , @aeubanks > wrote: > >> I've noticed that we're skipping compiler-rt tests with `REQUIRED: >> lld-available` because `LLVM_T

[PATCH] D136474: [CodeView][clang] Add flag to disable emitting command line into CodeView

This revision was automatically updated to reflect the committed changes. Closed by commit rG29a500b346bd: [CodeView][clang] Add flag to disable emitting command line into CodeView (authored by aeubanks). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D137583: [lldb] Fix simple template names and template params with scope qualifiers

aeubanks created this revision. Herald added a project: All. aeubanks requested review of this revision. Herald added projects: clang, LLDB. Herald added subscribers: lldb-commits, cfe-commits. Followup to D134378 . With PrintingPolicy::SuppressScope, we'd also n

[PATCH] D137269: [Clang][AArch64][Darwin] Enable GlobalISel by default for Darwin ARM64 platforms.

aeubanks added a comment. In D137269#3916916 , @nikic wrote: > I'd like to object to enabling this via the frontend. This means that > non-clang frontends will now use a non-default configuration that is not > extensively tested by upstream anymore. > >

[PATCH] D137583: [lldb] Fix simple template names and template params with scope qualifiers

aeubanks updated this revision to Diff 474848. aeubanks added a comment. use getNameForDiagnostics Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137583/new/ https://reviews.llvm.org/D137583 Files: lldb/source/Plugins/TypeSystem/Clang/TypeSystemC

[PATCH] D137583: [lldb] Fix simple template names and template params with scope qualifiers

aeubanks updated this revision to Diff 474850. aeubanks added a comment. add comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137583/new/ https://reviews.llvm.org/D137583 Files: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp lld

[PATCH] D137583: [lldb] Fix simple template names and template params with scope qualifiers

aeubanks added a comment. In D137583#3917735 , @dblaikie wrote: > In D137583#3917706 , @aaron.ballman > wrote: > >>> ...we expect template params to be fully qualified when comparing them for >>> simple template

[PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

aeubanks updated this revision to Diff 475017. aeubanks added a comment. I somehow missed the previous comments rebased on top of fixing UB in tests changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133036/new/ https://reviews.llvm.org/D133036

[PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

aeubanks added a comment. I'll hold off on submitting this until that bug is figured out Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133036/new/ https://reviews.llvm.org/D133036 ___ cfe-commits mailing

[PATCH] D137116: [AggressiveInstCombine] Remove legacy PM pass

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG70dc3b811e49: [AggressiveInstCombine] Remove legacy PM pass (authored by aeubanks). Changed prior to commit: https://reviews.llvm.org/D137116?vs=4

[PATCH] D137583: [lldb] Fix simple template names and template params with scope qualifiers

This revision was automatically updated to reflect the committed changes. Closed by commit rGcb0ffa529a0f: [lldb] Fix simple template names and template params with scope qualifiers (authored by aeubanks). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D138081: [IR] Split out IR printing passes into IRPrinter

aeubanks accepted this revision. aeubanks added a comment. This revision is now accepted and ready to land. in the description, link to the change that makes this necessary and describe why (need to use an analysis in the pass) Comment at: llvm/include/llvm/IRPrinter/IRPrintin

[PATCH] D138081: [IR] Split out IR printing passes into IRPrinter

aeubanks added inline comments. Comment at: llvm/include/llvm/IRPrinter/IRPrintingPasses.h:33 +/// +/// Note: This pass is for use with the new pass manager. Use the create...Pass +/// functions above to create passes for use with the legacy pass manager. tejohns

[PATCH] D136554: Implement CWG2631

aeubanks added a comment. one more regression bisected to this: https://bugs.chromium.org/p/chromium/issues/detail?id=1408177 `incomplete type 'blink::ResourceClient' used in type trait expression` I'll try to come up with a smaller repro Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[PATCH] D136554: Implement CWG2631

aeubanks added a comment. In D136554#4059723 , @aeubanks wrote: > one more regression bisected to this: > https://bugs.chromium.org/p/chromium/issues/detail?id=1408177 > `incomplete type 'blink::ResourceClient' used in type trait expression` > I'll try t

[PATCH] D141580: [clang] Don't consider a nullptr returned from ActOnTag() as a valid result in ParseClassSpecifier.

aeubanks added a comment. apologies for reverting this, but https://reviews.llvm.org/rGf1f0a0d8e8fdd2e534d9423b2e64c6b8aaa53aee is broken and doesn't revert cleanly without this also being reverted, could you reland? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://re

[PATCH] D126984: [clang] Add initial support for gcc's optimize function attribute

aeubanks added a comment. In D126984#3574033 , @steplong wrote: > In D126984#3571573 , @aeubanks > wrote: > >> IIRC in the past there was a strong preference to not have the pass manager >> support this sort of

[PATCH] D126984: [clang] Add initial support for gcc's optimize function attribute

aeubanks added a comment. In D126984#3574046 , @aaron.ballman wrote: > In D126984#3571573 , @aeubanks > wrote: > >> IIRC in the past there was a strong preference to not have the pass manager >> support this so

[PATCH] D126984: [clang] Add initial support for gcc's optimize function attribute

aeubanks added a comment. In D126984#3574091 , @steplong wrote: > In D126984#3574077 , @aeubanks > wrote: > >> In D126984#3574046 , >> @aaron.ballman wrote: >> >>> In D1

[PATCH] D126984: [clang] Add initial support for gcc's optimize function attribute

aeubanks added a comment. I can't speak for @xbolva00 but the only part I'm against is the user-visible feature of > - Added preliminary support for GCC's attribute `optimize`, which allows > functions to be compiled with different optimization options than what was > specified on the command

[PATCH] D126984: [clang] Add support for optimize function attribute

aeubanks added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:3469 +support the GCC semantics. Optimization levels `-O1` through `-O4` are +ignored. Only "-O0", "-Oz", "-Os", and "-Ofast" are supported. + something about `optimize(-Os)` still

[PATCH] D127876: [clang] Don't emit type test/assume for virtual classes that should never participate in WPD

aeubanks created this revision. aeubanks added reviewers: pcc, tejohnson. Herald added subscribers: ormris, steven_wu, hiraditya. Herald added a project: All. aeubanks requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Gi

[PATCH] D127876: [clang] Don't emit type test/assume for virtual classes that should never participate in WPD

aeubanks updated this revision to Diff 437224. aeubanks added a comment. update some comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127876/new/ https://reviews.llvm.org/D127876 Files: clang/lib/CodeGen/CGClass.cpp clang/lib/CodeGen/CGV

[PATCH] D127876: [clang] Don't emit type test/assume for virtual classes that should never participate in WPD

aeubanks added a comment. I realize that this is not a full fix for the general problem involving `-lto-whole-program-visibility`, but I believe if you ignore `-lto-whole-program-visibility` (which Chrome doesn't use) then this fix makes sense. Repository: rG LLVM Github Monorepo CHANGES S

[PATCH] D127876: [clang] Don't emit type test/assume for virtual classes that should never participate in WPD

aeubanks added a comment. In D127876#3586134 , @pcc wrote: > This diverges from the documented behavior of > `-lto-whole-program-visibility`. The flag is meant to give all classes hidden > LTO visibility, but now it only does that to some of them. perh

[PATCH] D127876: [clang] Don't emit type test/assume for virtual classes that should never participate in WPD

aeubanks updated this revision to Diff 437258. aeubanks added a comment. update docs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127876/new/ https://reviews.llvm.org/D127876 Files: clang/docs/LTOVisibility.rst clang/lib/CodeGen/CGClass.cpp

[PATCH] D127876: [clang] Don't emit type test/assume for virtual classes that should never participate in WPD

aeubanks added a comment. > We documented the feature in D75655 and > there it says "all classes" (and still does). I've updated the documentation. It was already slightly inaccurate in that we weren't emitting type test/assumes for `std`/`stdext` namespace cla

[PATCH] D127876: [clang] Don't emit type test/assume for virtual classes that should never participate in WPD

aeubanks updated this revision to Diff 437420. aeubanks added a comment. update docs to only mention `[[clang::lto_visibility_public]]` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127876/new/ https://reviews.llvm.org/D127876 Files: clang/docs/

[PATCH] D127876: [clang] Don't emit type test/assume for virtual classes that should never participate in WPD

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa70b39abffb4: [clang] Don't emit type test/assume for virtual classes that should never… (authored by aeubanks). Repository: rG LLVM Github Monore

[PATCH] D127579: [clang][WIP] add option to keep types of ptr args for non-kernel functions in metadata

aeubanks added a comment. If I'm understanding correctly, previously the LLVM pointee type was used to represent a frontend OpenCL type. An alternative to marking everything with `elementtype` or adding metadata, can something be done in the frontend to mangle the function name with OpenCL poin

[PATCH] D126984: [clang] Add initial support for gcc's optimize function attribute

aeubanks requested changes to this revision. aeubanks added a comment. This revision now requires changes to proceed. IIRC in the past there was a strong preference to not have the pass manager support this sort of thing if you want to support this, there should be an RFC for how the optimization

[PATCH] D154093: [clang-format] Break long string literals in C#, etc.

aeubanks added a comment. In D154093#4648931 , @eaeltsin wrote: > This might have another issue with Verilog - > > < import "AAA-BBB" foo bar baz > --- > > import {"AAA-", > > "BBB"} foo bar

[PATCH] D154093: [clang-format] Break long string literals in C#, etc.

aeubanks added a comment. In D154093#4648986 , @aeubanks wrote: > In D154093#4648931 , @eaeltsin > wrote: > >> This might have another issue with Verilog - >> >> < import "AAA-BBB" foo bar baz >

[PATCH] D155997: [Phase Ordering] Don't speculate in SimplifyCFG before PGO annotation

aeubanks added a comment. the pipeline change and simplifycfg change should be split into two changes Comment at: llvm/lib/Passes/PassBuilderPipelines.cpp:277 +static cl::opt AlwaysAllowSimplifyCFGSpeculation( +"always-allow-simplify-cfg-speculation", cl::init(false), cl:

[PATCH] D155997: [Phase Ordering] Don't speculate in SimplifyCFG before PGO annotation

aeubanks added inline comments. Comment at: llvm/test/Transforms/SimplifyCFG/fold-branch-to-common-dest.ll:3 ; RUN: opt < %s -S -passes=simplifycfg -simplifycfg-require-and-preserve-domtree=1 -bonus-inst-threshold=10 | FileCheck %s +; RUN: opt < %s -S -passes='simplifycfg' -s

[PATCH] D158223: [clang] Add clang::unnamed_addr attribute that marks globals' address as not significant

aeubanks created this revision. Herald added a reviewer: aaron.ballman. Herald added a project: All. aeubanks requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This simply marks the global with the `unnamed_addr` IR attribute so it be merged

[PATCH] D158223: [clang] Add clang::unnamed_addr attribute that marks globals' address as not significant

aeubanks added a comment. unsure if this should have an RFC or not will add release notes if this is ok to go ahead with Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158223/new/ https://reviews.llvm.org/D158223 __

[PATCH] D158223: [clang] Add clang::unnamed_addr attribute that marks globals' address as not significant

aeubanks added a comment. and naming can be bikeshed. `no_addrsig` follows https://llvm.org/docs/Extensions.html#sht-llvm-addrsig-section-address-significance-table, `unnamed_addr` follows the IR attribute Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D158223: [clang] Add clang::unnamed_addr attribute that marks globals' address as not significant

aeubanks added a comment. merging doesn't have to be done at link time, the optimizer can also do it. so I'd rule out any names with `icf` since that's specifically the name of a linker feature `no_unique_address` popped into my mind but that's already taken :). that's exactly what this is tho

[PATCH] D156799: Update generic scheduling to use A510 scheduling model

aeubanks added inline comments. Comment at: llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll:1 -; RUN: split-file %s %t ; RUN: cat %t/main.ll %t/a.ll > %t/a2.ll ; RUN: cat %t/main.ll %t/b.ll > %t/b2.ll this test is broken, without the `split-file` the other file

[PATCH] D157879: [clang] Report missing designated initializers in C++

aeubanks added a comment. is there any way to suppress this for a specific field? (I believe) I'm seeing user code where a field is intentionally not being initialized Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157879/new/ https://reviews.llvm.

[PATCH] D157879: [clang] Report missing designated initializers in C++

aeubanks added a comment. ah I thought this was in `-Wall` but it's not the struct is struct Foo { const void* buffer; uint32_t capacity; uint32_t reserved; }; where `reserved` isn't explicitly initialized. that seems like reasonable code, but I suppose we can just explicitly i

[PATCH] D157632: [Profile] Allow online merging with debug info correlation.

aeubanks added inline comments. Comment at: compiler-rt/lib/profile/InstrProfilingMerge.c:146 +if (!(__llvm_profile_get_version() & VARIANT_MASK_DBG_CORRELATE)) { + PROF_ERR("%s\n", "Missing profile data section."); + return 1; we're running into th

[PATCH] D157632: [Profile] Allow online merging with debug info correlation.

aeubanks added inline comments. Comment at: compiler-rt/lib/profile/InstrProfilingMerge.c:146 +if (!(__llvm_profile_get_version() & VARIANT_MASK_DBG_CORRELATE)) { + PROF_ERR("%s\n", "Missing profile data section."); + return 1; aeubanks wrote: > we'

[PATCH] D158223: [clang] Add clang::unnamed_addr attribute that marks globals' address as not significant

aeubanks added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:1416-1417 +not significant. This allows global constants with the same contents to be +merged. This can break global pointer identity, i.e. two different globals have +the same address. + --

[PATCH] D157879: [clang] Report missing designated initializers in C++

aeubanks added a comment. In D157879#4606531 , @aaron.ballman wrote: > In D157879#4604288 , @phosek wrote: > >> Note that there's an ongoing discussion on the GCC bug >> https://gcc.gnu.org/bugzilla/show_bug.cgi

[PATCH] D157632: [Profile] Allow online merging with debug info correlation.

aeubanks added inline comments. Comment at: compiler-rt/lib/profile/InstrProfilingMerge.c:146 +if (!(__llvm_profile_get_version() & VARIANT_MASK_DBG_CORRELATE)) { + PROF_ERR("%s\n", "Missing profile data section."); + return 1; ellis wrote: > aeuban

[PATCH] D158223: [clang] Add clang::unnamed_addr attribute that marks globals' address as not significant

aeubanks added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:1416-1417 +not significant. This allows global constants with the same contents to be +merged. This can break global pointer identity, i.e. two different globals have +the same address. + --

[PATCH] D157632: [Profile] Allow online merging with debug info correlation.

aeubanks added inline comments. Comment at: compiler-rt/lib/profile/InstrProfilingMerge.c:146 +if (!(__llvm_profile_get_version() & VARIANT_MASK_DBG_CORRELATE)) { + PROF_ERR("%s\n", "Missing profile data section."); + return 1; aeubanks wrote: > ell

[PATCH] D157632: [Profile] Allow online merging with debug info correlation.

aeubanks reopened this revision. aeubanks added a comment. This revision is now accepted and ready to land. reverted in 9b6b6bbc9127d604881cdc9dcea0e7c74ef821fd Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D137149: Use PassGate from LLVMContext if any otherwise global one

aeubanks added a comment. we don't currently support reusing a pipeline so I'm surprised that you're able to share/reuse pipelines without running into any issues Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137149/new/ https://reviews.llvm.org/D

[PATCH] D153131: [clang analysis][thread-safety] Handle return-by-reference...

aeubanks added a comment. This is finding lots of real issues in code, which is awesome, but could I request that this be put under a separate warning flag so we can toggle off just the new functionality and turn it on as we clean our codebase? Repository: rG LLVM Github Monorepo CHANGES SI

[PATCH] D137149: Use PassGate from LLVMContext if any otherwise global one

aeubanks added a comment. In D137149#4653381 , @vchuravy wrote: > In D137149#4653308 , @aeubanks > wrote: > >> we don't currently support reusing a pipeline so I'm surprised that you're >> able to share/reuse pi

[PATCH] D153131: [clang analysis][thread-safety] Handle return-by-reference...

aeubanks added a comment. If people are passing `-Wthread-safety-reference`, there was clearly some value in the previous checks and it would be unfortunate to turn them off while fixing the codebase. I'm not super familiar with flag families and if what I'm proposing is easily doable, but I th

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

aeubanks abandoned this revision. aeubanks added a comment. reworked into https://github.com/llvm/llvm-project/pull/69030 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149800/new/ https://reviews.llvm.org/D149800 __

[PATCH] D151479: [clang] Use IgnoreParensSingleStep in more places

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe32cde6f41cd: [clang] Use IgnoreParensSingleStep in more places (authored by aeubanks). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D155997: [Phase Ordering] Don't speculate in SimplifyCFG before PGO annotation

aeubanks added inline comments. Comment at: llvm/test/Transforms/PhaseOrdering/simplifycfg-speculate-blocks.ll:1 +;; Check that SimplifyCFG does not attempt speculation until after PGO is +;; annotated in the IR, and then does not perform it when unprofitable. hm

[PATCH] D155997: [Phase Ordering] Don't speculate in SimplifyCFG before PGO annotation

aeubanks added a comment. can we try not gating this on PGO as suggested? minimizing differences between pipelines is nice, and as mentioned it'll help with other cases Comment at: llvm/test/Transforms/PhaseOrdering/simplifycfg-speculate-blocks.ll:1 +;; Check that SimplifyCFG

[PATCH] D157518: Avoid running optimization passes in frontend test

aeubanks accepted this revision. aeubanks added a comment. This revision is now accepted and ready to land. seems fine if nobody else objects Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157518/new/ https://reviews.llvm.org/D157518 __

[PATCH] D158474: [clang][ExtractAPI] Fix bool spelling coming from the macro definition.

aeubanks added inline comments. Comment at: clang/test/ExtractAPI/bool.c:14 +//--- input.h +#include +bool Foo; aeubanks wrote: > clang tests should not be including system headers since that makes the tests > non-hermetic (this test fails in our internal build

[PATCH] D127221: [Clang] Enable -print-pipeline-passes in clang.

aeubanks added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:1102 +outs() << "\n"; +return; + } I wouldn't return here, doesn't seem right that we'll skip running the opt pipeline but continue with compilation. we should either bail out

[PATCH] D127221: [Clang] Enable -print-pipeline-passes in clang.

aeubanks accepted this revision. aeubanks added a comment. lg with one test comment Comment at: clang/lib/CodeGen/BackendUtil.cpp:1102 +outs() << "\n"; +return; + } jcranmer-intel wrote: > aeubanks wrote: > > I wouldn't return here, doesn't seem right

[PATCH] D127221: [Clang] Enable -print-pipeline-passes in clang.

aeubanks added inline comments. Comment at: clang/test/CodeGen/print-pipeline-passes.c:8 +// CHECK: always-inline +// CHECK-SAME: verify +void Foo(void) {} aeubanks wrote: > I believe this will fail in non-assert builds since we don't run the verifier in non-asse

[PATCH] D153924: [OpenMP] Allow exceptions in target regions when offloading to GPUs

aeubanks added inline comments. Comment at: clang/test/OpenMP/amdgpu_throw_trap.cpp:4 +// RUN: %clang_cc1 -fopenmp -triple amdgcn-amd-amdhsa -fopenmp-is-target-device %s -emit-llvm -S -Wno-openmp-target-exception -o - | FileCheck -check-prefix=DEVICE %s +// RUN: %clang_cc1 -fop

[PATCH] D153924: [OpenMP] Allow exceptions in target regions when offloading to GPUs

aeubanks added inline comments. Comment at: clang/test/OpenMP/amdgpu_throw_trap.cpp:4 +// RUN: %clang_cc1 -fopenmp -triple amdgcn-amd-amdhsa -fopenmp-is-target-device %s -emit-llvm -S -Wno-openmp-target-exception -o - | FileCheck -check-prefix=DEVICE %s +// RUN: %clang_cc1 -fop

[PATCH] D127221: [Clang] Enable -print-pipeline-passes in clang.

aeubanks added inline comments. Comment at: clang/test/CodeGen/print-pipeline-passes.c:8 +// CHECK: always-inline +// CHECK-SAME: verify +void Foo(void) {} jcranmer-intel wrote: > aeubanks wrote: > > aeubanks wrote: > > > I believe this will fail in non-assert bu

[PATCH] D130531: [IR] Use Min behavior for module flag "PIC Level"

aeubanks added a comment. I'm not understanding why this doesn't also apply to "PIE Level", doesn't it also follow the same reasoning? pic -> PIC is the same as pie -> PIE e.g. if you merge a small PIC and large PIC file, the resulting file would only be guaranteed to work with a "large PIC exe

[PATCH] D154253: [clang] detect integer overflow through temporary values

aeubanks added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:15215 Exprs.append(Construct->arg_begin(), Construct->arg_end()); +else if (auto Temporary = dyn_cast(E)) + Exprs.push_back(Temporary->getSubExpr()); nit: `clang::` isn't

<    1   2   3   4   5   6   7   8   >