[PATCH] D144006: [DebugMetadata][DwarfDebug] Support function-local types in lexical block scopes (4/7)

aeubanks added a comment. In D144006#4656383 , @dzhidzhoev wrote: > In D144006#4656371 , @aeubanks > wrote: > >> Is there any issue with mixing IR built with >> `LLVMContext::enableDebugTypeODRUniquing()` and

[PATCH] D144006: [DebugMetadata][DwarfDebug] Support function-local types in lexical block scopes (4/7)

aeubanks added a comment. F30438826: i.ll Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144006/new/ https://reviews.llvm.org/D144006 ___ cfe-commits mailing list

[PATCH] D144006: [DebugMetadata][DwarfDebug] Support function-local types in lexical block scopes (4/7)

aeubanks added a comment. another observation, `opt` complained about some debug info due to setting `LLVMContext::enableDebugTypeODRUniquing()` and stripped it away, but `llc` happily accepted the debug info and crashed with the assertion above when emitting assembly. Repository: rG LLVM

[PATCH] D144006: [DebugMetadata][DwarfDebug] Support function-local types in lexical block scopes (4/7)

aeubanks added a comment. Is there any issue with mixing IR built with `LLVMContext::enableDebugTypeODRUniquing()` and without? I'm suspecting that ThinLTO is mixing Rust and Clang IR which set that differently. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D144006: [DebugMetadata][DwarfDebug] Support function-local types in lexical block scopes (4/7)

aeubanks added a comment. `../../llvm/lib/CodeGen/AsmPrinter/DwarfFile.cpp:110: void llvm::DwarfFile::addScopeVariable(LexicalScope *, DbgVariable *): Assertion 'Ret.second' failed.` is the crash btw Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D144006: [DebugMetadata][DwarfDebug] Support function-local types in lexical block scopes (4/7)

aeubanks added a comment. Heads up, we're seeing issues from this in Chrome again, will try to post a reduced repro (https://crbug.com/1500022) The reduced repro I found crashed very far back, so I'm guessing this patch caused bad debug info which caused an existing issue that doesn't handle

[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

[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] 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

[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

[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

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

[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] 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] 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

[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

[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

[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

[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

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

[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

[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; clang tests should not be including system headers since that makes the tests non-hermetic (this test fails in our internal builds that provide more

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

aeubanks added inline comments. Comment at: clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp:147-148 -// CHECK: ![[PROF_LIKELY]] = !{!"branch_weights", i32 [[UNLIKELY]], i32 [[LIKELY]]} -// CHECK: ![[PROF_UNLIKELY]] = !{!"branch_weights", i32 [[LIKELY]], i32

[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] D153924: [OpenMP] Allow exceptions in target regions when offloading to GPUs

aeubanks added inline comments. Comment at: clang/test/OpenMP/amdgpu_exceptions.cpp:10 + +// RUN: %clang_cc1 -fopenmp -triple amdgcn-amd-amdhsa -fopenmp-is-target-device -fcxx-exceptions -fexceptions %s -emit-llvm -S -verify=with -Wopenmp-target-exception -analyze +// RUN:

[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] D61670: [clang] [MinGW] Add the option -fno-autoimport

aeubanks added a comment. I don't have all the context here, but seems fine once the commit description is updated with the new spelling Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61670/new/ https://reviews.llvm.org/D61670

[PATCH] D159000: Reland "[Profile] Allow online merging with debug info correlation."

aeubanks added a comment. looks fine to me but I'll let ellis lgtm Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159000/new/ https://reviews.llvm.org/D159000 ___ cfe-commits mailing list

[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] D61670: [clang] [MinGW] Add the option -fno-autoimport

aeubanks added a comment. In D61670#4607313 , @mstorsjo wrote: >> I expected the answer would be "yes", so I said "lgtm" and then phrased my >> question very awkwardly. > > Ah, thanks for the clarification! > > Any opinion on the name, `-fno-autoimport`

[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] 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] D158223: [clang] Add clang::unnamed_addr attribute that marks globals' address as not significant

aeubanks updated this revision to Diff 552889. aeubanks edited the summary of this revision. aeubanks added a comment. add warning Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158223/new/ https://reviews.llvm.org/D158223 Files:

[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

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

[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; ellis wrote: >

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

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

[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

[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

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

[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

[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

[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

[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 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] 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] 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] 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.

[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'

[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),

[PATCH] D148654: Modify BoundsSan to improve debuggability

aeubanks added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:3594 +TrapCall->addFnAttr(A); + } + TrapCall->setDoesNotReturn(); oskarwirga wrote: > vitalybuka wrote: > > wouldn't be you issues solved with > >

[PATCH] D145265: [Pipeline] Remove GlobalCleanupPM

aeubanks added a comment. @nikic could you try running this over the rust tests again? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145265/new/ https://reviews.llvm.org/D145265 ___ cfe-commits mailing

[PATCH] D145265: [Pipeline] Remove GlobalCleanupPM

aeubanks updated this revision to Diff 537871. aeubanks added a comment. rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145265/new/ https://reviews.llvm.org/D145265 Files: clang/test/Headers/mm_malloc.c

[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

[PATCH] D153468: [clang][LTO] Add flag to run verifier after every pass

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGda7f212f4a7a: [clang][LTO] Add flag to run verifier after every pass (authored by aeubanks). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D153468: [clang][LTO] Add flag to run verifier after every pass

aeubanks updated this revision to Diff 533627. aeubanks marked an inline comment as done. aeubanks added a comment. update test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153468/new/ https://reviews.llvm.org/D153468 Files:

[PATCH] D153468: [clang][LTO] Add flag to run verifier after every pass

aeubanks added inline comments. Comment at: clang/test/CodeGen/verify-each.c:5 +// RUN: %clang_cc1 -O2 -o /dev/null -triple x86_64-unknown-linux-gnu -emit-llvm-bc %s -fdebug-pass-manager 2>&1 | FileCheck %s --check-prefix=NO +// NO-NOT: Verifying + tejohnson

[PATCH] D153468: [clang][LTO] Add flag to run verifier after every pass

aeubanks created this revision. aeubanks added a reviewer: tejohnson. Herald added subscribers: ormris, steven_wu, hiraditya, inglorion. Herald added a project: All. aeubanks requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits.

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

aeubanks created this revision. aeubanks added a reviewer: rsmith. Herald added a project: All. aeubanks requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Addresses a post-commit comment on D146764 .

[PATCH] D145403: [Pipeline] Don't run EarlyFPM in LTO post link

This revision was automatically updated to reflect the committed changes. Closed by commit rG13e3d4aa5a4e: [Pipeline] Dont run EarlyFPM in LTO post link (authored by aeubanks). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145403/new/

[PATCH] D145403: [Pipeline] Don't run EarlyFPM in LTO post link

aeubanks updated this revision to Diff 524406. aeubanks added a comment. rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145403/new/ https://reviews.llvm.org/D145403 Files: llvm/lib/Passes/PassBuilderPipelines.cpp

[PATCH] D145403: [Pipeline] Don't run EarlyFPM in LTO post link

aeubanks added a comment. I was trying to get this landed with https://reviews.llvm.org/D145265 so I could measure the impact of the two together. But also now there are some merge conflicts that I'd rather deal with after submitting D145265 . But that patch

[PATCH] D148851: Disable llvm-symbolizer on some of the driver tests that are timing out

aeubanks added a comment. `not` will set `LLVM_DISABLE_SYMBOLIZATION` if we use `not --crash` are these tests supposed to crash or gracefully fail? if they're supposed to crash then we should use `not --crash` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D150645: [Driver] Support multi /guard: options

aeubanks added a comment. if you add a test for the repro, relanding lgtm Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150645/new/ https://reviews.llvm.org/D150645 ___ cfe-commits mailing list

[PATCH] D150645: [Driver] Support multi /guard: options

aeubanks added a comment. this causes `./build/rel/bin/clang-cl /c /tmp/a.cc /Fo/tmp/a /guard:cf,nochecks` to go from no warnings to clang-cl: warning: argument unused during compilation: '/guard:cf,nochecks' [-Wunused-command-line-argument] Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D145265: [Pipeline] Remove GlobalCleanupPM

aeubanks added inline comments. Comment at: llvm/lib/Passes/PassBuilderPipelines.cpp:476 + FPM.addPass(EarlyCSEPass()); + nikic wrote: > Why the extra EarlyCSE pass in the `O1` pipeline? forgot to update the commit message, see that Comment

[PATCH] D145265: [Pipeline] Remove GlobalCleanupPM

aeubanks updated this revision to Diff 523182. aeubanks edited the summary of this revision. aeubanks added a comment. update commit message Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145265/new/ https://reviews.llvm.org/D145265 Files:

[PATCH] D145265: [Pipeline] Remove GlobalCleanupPM

aeubanks updated this revision to Diff 523175. aeubanks added a comment. rebase keep the existing EarlyCSE/InstCombine ordering Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145265/new/ https://reviews.llvm.org/D145265 Files:

[PATCH] D150326: [WPD] Update llvm.public.type.test after importing functions

aeubanks accepted this revision. aeubanks added inline comments. This revision is now accepted and ready to land. Comment at: llvm/test/ThinLTO/X86/public-type-test.ll:31 ; HIDDEN-NOT: call {{.*}}@llvm.public.type.test ; HIDDEN: call {{.*}}@llvm.type.test

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG878e590503df: Reland [clang] Make predefined expressions string literals under -fms-extensions (authored by aeubanks). Changed prior to commit: https://reviews.llvm.org/D146764?vs=520204=521039#toc

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

aeubanks added a comment. In D149800#4330831 , @davidxl wrote: > This patch uses a cleaner method than the previous effort. There is some > differences: > > 1. yamauchi's patch works for both iFDO and autoFDO, while this patch limits > to iFDO > 2.

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

aeubanks reopened this revision. aeubanks added a comment. This revision is now accepted and ready to land. the following crashes with this patch: struct StringRef { StringRef(const char *); }; template StringRef getTypeName() { StringRef s = __func__; } `clang -cc1

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG856f384bf945: [clang] Make predefined expressions string literals under -fms-extensions (authored by aeubanks). Changed prior to commit:

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

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

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

aeubanks updated this revision to Diff 519613. aeubanks added a comment. only diagnose if we're initializing an array Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146764/new/ https://reviews.llvm.org/D146764 Files:

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

aeubanks added a comment. In D149800#4317275 , @mtrofin wrote: > high level question, why not have it as a pass that runs after profiles (of > whatever kind - instrumented or sample-based) are ingested. The pass would > attribute functions as

[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

[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

[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

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

[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

[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 ) const; can this return a SmallVector instead of taking one as a mutable param? Comment at:

[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

[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

[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

[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

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG8f0dd4ef3ed2: [clang] Canonicalize system headers in dependency file when -canonical-prefixes (authored by aeubanks). Repository: rG LLVM Github

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

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

[PATCH] D129700: [clang] Don't emit type tests for dllexport/import classes

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf34ecb50e2c0: [clang] Dont emit type tests for dllexport/import classes (authored by aeubanks). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D129700: [clang] Don't emit type tests for dllexport/import classes

aeubanks added a comment. Herald added a subscriber: bd1976llvm. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129700/new/ https://reviews.llvm.org/D129700 ___ cfe-commits mailing list

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

aeubanks created this revision. Herald added a project: All. aeubanks requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. Clang was writing paths to the dependency file that don't exist when using a sysroot with symlinks, causing

[PATCH] D145265: [Pipeline] Remove GlobalCleanupPM

aeubanks added a comment. this regresses clang/test/Headers/mm_malloc.c (added here ) because we now don't run instcombine before the inliner, and we lose return value alignment info when inlining. perhaps this is still a tradeoff worth making Repository:

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

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

[PATCH] D145265: [Pipeline] Remove GlobalCleanupPM

aeubanks added a comment. In D145265#4225332 , @asbirlea wrote: > Nit: the diffs would be easier to analyze with "-fno-discard-value-names". done, see commit description the IR diffs look pretty good now, I'm not seeing any major regressions

[PATCH] D145265: [Pipeline] Remove GlobalCleanupPM

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

  1   2   3   4   5   6   7   8   >