[PATCH] D56353: Replace cc1 options '-mdisable-fp-elim' and '-momit-leaf-frame-pointer' with'-mframe-pointer='

tabloid.adroit created this revision. tabloid.adroit added reviewers: chandlerc, rnk, t.p.northover. Herald added a subscriber: cfe-commits. set cc1 '-mframe-pointer=' according to driver options m(no-)omit-leaf-frame-pointer and f(no-)omit-frame-pointer This makes -fomit-frame-pointer imply

[PATCH] D55915: [Driver] Make -fno-omit-frame-pointer imply -mno-omit-leaf-frame-pointer

tabloid.adroit added a comment. https://reviews.llvm.org/D56351 https://reviews.llvm.org/D56353 Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55915/new/ https://reviews.llvm.org/D55915 ___ cfe-commits mailing list

[PATCH] D56353: Replace cc1 options '-mdisable-fp-elim' and '-momit-leaf-frame-pointer' with'-mframe-pointer='

tabloid.adroit updated this revision to Diff 182213. tabloid.adroit marked an inline comment as done. tabloid.adroit added a comment. Thanks for the review! - address comments Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56353/new/

[PATCH] D56353: Replace cc1 options '-mdisable-fp-elim' and '-momit-leaf-frame-pointer' with'-mframe-pointer='

tabloid.adroit marked 2 inline comments as done. tabloid.adroit added inline comments. Comment at: lib/Driver/ToolChains/Clang.cpp:576-579 + Arg *FP = Args.getLastArg(options::OPT_fno_omit_frame_pointer, +options::OPT_fomit_frame_pointer); + Arg

[PATCH] D55915: [Driver] Make -fno-omit-frame-pointer imply -mno-omit-leaf-frame-pointer

tabloid.adroit marked 2 inline comments as done. tabloid.adroit added a comment. Thanks for the review! Comments inline. Comment at: lib/CodeGen/CGCall.cpp:1739 FuncAttrs.addAttribute("no-frame-pointer-elim", "true"); -

[PATCH] D55915: [Driver] Make -fno-omit-frame-pointer imply -mno-omit-leaf-frame-pointer

tabloid.adroit updated this revision to Diff 179447. tabloid.adroit marked an inline comment as done. tabloid.adroit added a comment. - update test Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55915/new/ https://reviews.llvm.org/D55915 Files:

[PATCH] D55915: [Driver] Make -fno-omit-frame-pointer imply -mno-omit-leaf-frame-pointer

tabloid.adroit marked 3 inline comments as done. tabloid.adroit added inline comments. Comment at: lib/CodeGen/CGCall.cpp:1739 FuncAttrs.addAttribute("no-frame-pointer-elim", "true"); - FuncAttrs.addAttribute("no-frame-pointer-elim-non-leaf"); }

[PATCH] D55915: [Driver] Make -fno-omit-frame-pointer imply -mno-omit-leaf-frame-pointer

tabloid.adroit updated this revision to Diff 179325. tabloid.adroit added a comment. - [Driver] Make -fno-omit-frame-pointer imply -mno-omit-leaf-frame-pointer - add test - update Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55915/new/

[PATCH] D56353: Replace cc1 options '-mdisable-fp-elim' and '-momit-leaf-frame-pointer' with'-mframe-pointer='

tabloid.adroit added a comment. gentle ping Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56353/new/ https://reviews.llvm.org/D56353 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D55915: [Driver] Make -fno-omit-frame-pointer imply -mno-omit-leaf-frame-pointerand make -fomit-frame-pointer imply -momit-leaf-frame-pointer.This matches GCC's behavior.

tabloid.adroit created this revision. Herald added a subscriber: cfe-commits. llvm.org/PR9825 Repository: rC Clang https://reviews.llvm.org/D55915 Files: lib/CodeGen/CGCall.cpp lib/Driver/ToolChains/Clang.cpp test/Driver/cl-options.c test/Driver/frame-pointer-elim.c Index:

[PATCH] D56353: Replace cc1 options '-mdisable-fp-elim' and '-momit-leaf-frame-pointer' with'-mframe-pointer='

tabloid.adroit added a comment. Herald added a project: clang. ping Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56353/new/ https://reviews.llvm.org/D56353 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D64294: [Driver] Consolidate shouldUseFramePointer() and shouldUseLeafFramePointer()

ychen added inline comments. Comment at: lib/Driver/ToolChains/Clang.cpp:585 + (A && A->getOption().matches(options::OPT_fno_omit_frame_pointer)) || + (!(A && A->getOption().matches(options::OPT_fomit_frame_pointer)) && + (Args.hasArg(options::OPT_pg) ||

[PATCH] D64294: [Driver] Consolidate shouldUseFramePointer() and shouldUseLeafFramePointer()

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

[PATCH] D56353: Replace cc1 options '-mdisable-fp-elim' and '-momit-leaf-frame-pointer' with'-mframe-pointer='

ychen updated this revision to Diff 209550. ychen added a comment. - rebase after D64294 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56353/new/ https://reviews.llvm.org/D56353 Files:

[PATCH] D64294: [Driver] Consolidate shouldUseFramePointer() and shouldUseLeafFramePointer()

ychen added a comment. In D56353 , I remember @chandlerc thought `-f(no-)omit-frame-pointer` should win over `-m(no-)omit-leaf-frame-pointer`. I'm not sure what his thoughts on this now. @chandlerc ? Repository: rC Clang CHANGES SINCE LAST ACTION

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

ychen added a comment. Since Charlie has completed his internship before LGTM, I'll commit this on his behalf. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62829/new/ https://reviews.llvm.org/D62829 ___ cfe-commits mailing list

[PATCH] D67141: [DebugInfo] Emit DW_TAG_enumeration_type for referenced global enumerator.

ychen created this revision. ychen added reviewers: akhuang, rnk, probinson. Herald added a project: clang. Herald added a subscriber: cfe-commits. This essentially reverts changes from r361400. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D67141 Files:

[PATCH] D65527: Avoid assemble step in verbose-output-quoting.c

ychen updated this revision to Diff 218588. ychen added a comment. Address comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65527/new/ https://reviews.llvm.org/D65527 Files: clang/test/Driver/verbose-output-quoting.c Index:

[PATCH] D65527: Avoid assemble step in verbose-output-quoting.c

ychen added a comment. > Would that work for this test also? Yes, that sounds good. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65527/new/ https://reviews.llvm.org/D65527 ___ cfe-commits mailing

[PATCH] D67141: [DebugInfo] Emit DW_TAG_enumeration_type for referenced global enumerator.

ychen updated this revision to Diff 218769. ychen added a comment. - address comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67141/new/ https://reviews.llvm.org/D67141 Files: clang/lib/CodeGen/CGDebugInfo.cpp clang/test/CodeGen/enum2.c

[PATCH] D67141: [DebugInfo] Emit DW_TAG_enumeration_type for referenced global enumerator.

ychen added a comment. In D67141#1658238 , @rnk wrote: > lgtm, thanks! Thank you! Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67141/new/ https://reviews.llvm.org/D67141

[PATCH] D65527: Avoid assemble step in verbose-output-quoting.c

ychen created this revision. ychen added a reviewer: hans. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D65527 Files: clang/test/Driver/verbose-output-quoting.c Index:

[PATCH] D56353: Replace cc1 options '-mdisable-fp-elim' and '-momit-leaf-frame-pointer' with '-mframe-pointer'

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

[PATCH] D56353: Replace cc1 options '-mdisable-fp-elim' and '-momit-leaf-frame-pointer' with '-mframe-pointer'

ychen marked an inline comment as done and an inline comment as not done. ychen added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:845 + if (Args.hasArg(OPT_pg)) +Opts.setFramePointer(CodeGenOptions::FP_All); + MaskRay wrote: > I

[PATCH] D56353: Replace cc1 options '-mdisable-fp-elim' and '-momit-leaf-frame-pointer' with '-mframe-pointer'

ychen updated this revision to Diff 209973. ychen marked an inline comment as not done. ychen edited the summary of this revision. ychen added a comment. - Make `FramePointerKind` enum class. - Replace two if-else-if blocks to switch statements. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D56353: Replace cc1 options '-mdisable-fp-elim' and '-momit-leaf-frame-pointer' with '-mframe-pointer'

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

[PATCH] D64294: [Driver] Consolidate shouldUseFramePointer() and shouldUseLeafFramePointer()

ychen added inline comments. Comment at: lib/Driver/ToolChains/Clang.cpp:579 -static bool shouldUseFramePointer(const ArgList , - const llvm::Triple ) { - if (Arg *A = Args.getLastArg(options::OPT_fno_omit_frame_pointer, -

[PATCH] D69327: [Clang][ThinLTO] Add a cache for compile phase output.

ychen added a comment. Sorry for the confusion @steven_wu. By `stable` I mean the probability that the `-fthin-link-bitcode` option is replaced with some other thinlink mechanism under the distributed build environment. Since at least for ccache, the compilation output caching depends on the

[PATCH] D69327: [Clang][ThinLTO] Add a cache for compile phase output.

ychen added a comment. In D69327#1719419 , @steven_wu wrote: > In D69327#1719411 , @ychen wrote: > > > Thanks for the inputs @steven_wu @tejohnson. Totally agree with the points > > you brought up. One last thing

[PATCH] D69327: [Clang][ThinLTO] Add a cache for compile phase output.

ychen added a comment. Thank you @steven_wu @tejohnson. I created D69406 to promote the flag to the driver. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69327/new/ https://reviews.llvm.org/D69327

[PATCH] D69406: [clang][ThinLTO] Promote cc1 -fthin_link_bitcode to driver -fthinlto_link_bitcode

ychen created this revision. ychen added reviewers: tejohnson, steven_wu. Herald added subscribers: cfe-commits, dexonsmith, inglorion, mehdi_amini. Herald added a project: clang. A necessary step to let build system caching work for its output. I renamed it to match existing driver options for

[PATCH] D69406: [clang][ThinLTO] Promote cc1 -fthin_link_bitcode to driver -fthinlto_link_bitcode

ychen updated this revision to Diff 226347. ychen added a comment. Address reviewer's comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69406/new/ https://reviews.llvm.org/D69406 Files: clang/include/clang/Driver/CC1Options.td

[PATCH] D69406: [clang][ThinLTO] Promote cc1 -fthin_link_bitcode to driver -fthinlto_link_bitcode

This revision was automatically updated to reflect the committed changes. Closed by commit rGef7a154d17f2: [clang][ThinLTO] Promote cc1 -fthin_link_bitcode to driver… (authored by ychen). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69406/new/

[PATCH] D69327: [Clang][ThinLTO] Add a cache for compile phase output.

ychen added a comment. In D69327#1719109 , @tejohnson wrote: > I haven't read through the patch in detail yet, but from the description it > sounds like a cache is being added for the compile step outputs, e.g. the > bitcode output object of the "clang

[PATCH] D69327: [Clang][ThinLTO] Add a cache for compile phase output.

ychen added a comment. Thanks for the inputs @steven_wu @tejohnson. Totally agree with the points you brought up. One last thing I'm not quite sure is the caching of `-fthin-link-bitcode`. It is a `-cc1` option since it is a kind of implementation of ThinLTO, right? I'm a little hesitant to

[PATCH] D69327: [Clang][ThinLTO] Add a cache for compile phase output.

ychen created this revision. ychen added reviewers: mehdi_amini, pcc, tejohnson. Herald added subscribers: llvm-commits, cfe-commits, dang, dexonsmith, steven_wu, aheejin, hiraditya, inglorion. Herald added projects: clang, LLVM. Currently the link phase has a object file cache whereas the

[PATCH] D67847: [Support] make report_fatal_error `abort` instead of `exit`

ychen added a comment. In D67847#1691898 , @jyknight wrote: > The `abort()` function raises SIGABRT, for which the default behavior is to > trigger a coredump. Do we actually want that behavior? > > Either `_exit()` (long available extension, which lld

[PATCH] D68482: [clang] fix a typo from r372531

ychen created this revision. ychen added a reviewer: xbolva00. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D68482 Files: clang/lib/Sema/SemaChecking.cpp

[PATCH] D74456: Reland "[Support] make report_fatal_error `abort` instead of `exit`"

ychen added a comment. In D74456#1871238 , @MaskRay wrote: > `ninja check-llvm check-clang` passed on a powerpc64le machine. Thanks for the review. Fingers crossed sanitizer bots not complain. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D67847: [Support] make report_fatal_error `abort` instead of `exit`

ychen added a comment. In D67847#1847757 , @rnk wrote: > If we can reland this patch, it should fix this new clang bug: > https://bugs.llvm.org/show_bug.cgi?id=44705 > > This was the list of failures you noted on the commit: > > In

[PATCH] D67847: [Support] make report_fatal_error `abort` instead of `exit`

ychen added a comment. Apologies that it takes so long. I'll fix the test and land it today. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67847/new/ https://reviews.llvm.org/D67847 ___ cfe-commits

[PATCH] D44352: [Concepts] Type Constraints

ychen added a comment. I saw a null pointer dereference for the `p3-2a.cpp` file added here. It is not always reproducible for me, I was able to reproduce it once offline. Any idea? Crash stack is here: https://results.llvm-merge-guard.org/amd64_debian_testing_clang8-1220/console-log.txt From

[PATCH] D72404: [WIP][ThinLTO] Support Os and Oz

ychen updated this revision to Diff 238042. ychen added a comment. - Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72404/new/ https://reviews.llvm.org/D72404 Files: clang/lib/CodeGen/BackendUtil.cpp

[PATCH] D72404: [WIP][ThinLTO] Support Os and Oz

ychen updated this revision to Diff 238028. ychen added a comment. Herald added subscribers: cfe-commits, jfb. Herald added a project: clang. - when optnone is not present, add `optsize` for Os and `minsize` for Oz - add Os Oz function attribute test. - add Os Oz pipeline test Repository: rG

[PATCH] D72404: [WIP][ThinLTO] Support Os and Oz

ychen updated this revision to Diff 238034. ychen added a comment. - fix a typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72404/new/ https://reviews.llvm.org/D72404 Files: clang/lib/CodeGen/BackendUtil.cpp

[PATCH] D72404: [ThinLTO/FullLTO] Support Os and Oz

ychen updated this revision to Diff 238071. ychen added a comment. - rebase & clang-format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72404/new/ https://reviews.llvm.org/D72404 Files: clang/lib/CodeGen/BackendUtil.cpp

[PATCH] D72404: [ThinLTO/FullLTO] Support Os and Oz

ychen added a comment. In D72404#1820628 , @tycho wrote: > In D72404#1820461 , > @merge_guards_bot wrote: > > > {icon times-circle color=red} Unit tests: fail. 61858 tests passed, 1 > > failed and 781 were

[PATCH] D72404: [ThinLTO/FullLTO] Support Os and Oz

ychen updated this revision to Diff 238142. ychen edited the summary of this revision. ychen added a comment. - fix test when clang is the host compiler Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72404/new/ https://reviews.llvm.org/D72404

[PATCH] D67847: [Support] make report_fatal_error `abort` instead of `exit`

ychen added a comment. There are some interesting failures that need to be investigated. https://reviews.llvm.org/rG647c3f4e47de8a850ffcaa897db68702d8d2459a Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67847/new/ https://reviews.llvm.org/D67847

[PATCH] D80356: `cc1as -fdebug-compilation-dir` needs -integrated-as

ychen created this revision. ychen added a reviewer: thakis. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D80356 Files: clang/test/Driver/clang_f_opts.c Index: clang/test/Driver/clang_f_opts.c

[PATCH] D85676: [NewPM] collapsing nested pass mangers of the same type

ychen added a comment. In D85676#2245787 , @asbirlea wrote: > Seems like a good idea. I'm curious if you saw any impact from this change. Yeah, I'm curious too but I didn't measure it yet. What's the recommended ways to measure it? Repository: rG

[PATCH] D85676: [NewPM] collapsing nested pass mangers of the same type

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2c94d88e0769: [NewPM] collapsing nested pass mangers of the same type (authored by ychen). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D85676: [NewPM] collapsing nested pass mangers of the same type

ychen added a comment. Herald added subscribers: wenlei, danielkiss. ping? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85676/new/ https://reviews.llvm.org/D85676 ___ cfe-commits mailing list

[PATCH] D86008: [test] Fix aggregate-assign-call.c in preparation for -enable-npm-optnone

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

[PATCH] D80454: [Clang][test] fix tests when using external assembler

This revision was automatically updated to reflect the committed changes. Closed by commit rG3a2df3bad07f: [Clang][test] fix tests when using external assembler. (authored by ychen). Changed prior to commit: https://reviews.llvm.org/D80454?vs=265792=266088#toc Repository: rG LLVM Github

[PATCH] D80944: Add begin source location for the attributed statement created from PragmaLoopHint decorated loop

ychen created this revision. ychen added reviewers: ABataev, jdenny, lebedev.ri, aaron.ballman. Herald added a project: clang. Herald added a subscriber: cfe-commits. Right now it is a '' for cases like this. CounterCoverageMappingBuilder relies on the information to decide the region for a

[PATCH] D80454: [Clang] fix limiting -fintegrated-cc1 to only one TU

ychen planned changes to this revision. ychen added a comment. This may not be complete. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80454/new/ https://reviews.llvm.org/D80454 ___ cfe-commits

[PATCH] D80454: [Clang] fix limiting -fintegrated-cc1 to only one TU

ychen created this revision. ychen added a reviewer: aganea. Herald added a project: clang. Herald added a subscriber: cfe-commits. ychen planned changes to this revision. ychen added a comment. This may not be complete. For D74490 . For cases like where

[PATCH] D80454: [Clang][test] fix tests when using external assembler

ychen updated this revision to Diff 265791. ychen added a comment. - update Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80454/new/ https://reviews.llvm.org/D80454 Files: clang/test/Driver/cc1-spawnprocess.c Index:

[PATCH] D80454: [Clang][test] fix tests when using external assembler

ychen added a comment. In D80454#2051524 , @aganea wrote: > Could you please attach a test to demonstrate the issue? When a toolchain is not using integrated-as, the test would fail because using external assember would increase job counts. Most

[PATCH] D80454: [Clang][test] fix tests when using external assembler

ychen updated this revision to Diff 265792. ychen added a comment. - update Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80454/new/ https://reviews.llvm.org/D80454 Files: clang/test/Driver/cc1-spawnprocess.c Index:

[PATCH] D80454: [Clang][test] fix tests when using external assembler

ychen added a comment. In D80454#2051637 , @aganea wrote: > LGTM. > > Are you planning on adding another patch for the change that you've sent > initially? Ideally, I'd like to relax a bit that constraint in `Driver.cpp`, > as in D74447

[PATCH] D74447: [Clang] After integrated-cc1, ignore -disable-free when there are more than one job in the queue

ychen added a comment. In D74447#1872211 , @thakis wrote: > I'd think that everyone debugging clang always passes a single TU to it, so > I'm not sure debuggability does much here :) > > The `-disable-free` code has never been used in normal

[PATCH] D80454: [Clang][test] fix tests when using external assembler

ychen added a comment. In D80454#2051668 , @MaskRay wrote: > In D80454#2051527 , @ychen wrote: > > > In D80454#2051524 , @aganea wrote: > > > > > Could you please attach a

[PATCH] D89158: [NewPM] Run all EP callbacks under -O0

ychen added a comment. IIUC, this is the NPM version of EP_EarlyAsPossible in legacy PM for O0, right? Between the choice of (1) [this patch and ] reusing all existing EP callbacks and letting optnone filtering out non-required passes (which I think does not work for `-disable-O0-optnone`) and

[PATCH] D89158: [NewPM] Run all EP callbacks under -O0

ychen added inline comments. Comment at: llvm/include/llvm/Passes/PassBuilder.h:626 + /// typically run at the very beginning. + void runRegisteredO0EPCallbacks(ModulePassManager , bool DebugLogging); + invokeO0EPCallbacks? I saw there is

[PATCH] D89158: [NewPM] Run all EP callbacks under -O0

ychen added inline comments. Comment at: llvm/lib/Passes/PassBuilder.cpp:1659 + bool DebugLogging) { + for (auto : PipelineStartEPCallbacks) +C(MPM); aeubanks wrote: > ychen wrote: > > asbirlea wrote: > > >

[PATCH] D89158: [NewPM] Run all EP callbacks under -O0

ychen added inline comments. Comment at: llvm/lib/Passes/PassBuilder.cpp:1659 + bool DebugLogging) { + for (auto : PipelineStartEPCallbacks) +C(MPM); asbirlea wrote: > aeubanks wrote: > > ychen wrote: > > > What

[PATCH] D83519: [NewPM] Support optnone under new pass manager

ychen added inline comments. Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:57 +class OptNoneInstrumentation { +public: I may probably name this `SkipPassInstrumentation`. But it is up to you. Comment at:

[PATCH] D84886: Create LoopNestPass

ychen added a comment. Hello, sorry for the late reply. I have some high-level questions - What are existing and future loop passes/analyses that could benefit from using LoopNestPass? - I'd feel more comfortable if existing loop pass/analysis managers could be extended to handle LoopNestPass,

[PATCH] D85676: [NewPM] collapsing nested pass mangers of the same type

ychen created this revision. ychen added reviewers: asbirlea, chandlerc. Herald added subscribers: llvm-commits, cfe-commits, dexonsmith, steven_wu, hiraditya. Herald added projects: clang, LLVM. ychen requested review of this revision. This is one of the reason for extra invalidations in D84959

[PATCH] D85457: [NewPM][optnone] Mark various passes as required

ychen added a comment. Looks reasonable to me. Please let @asbirlea double-check. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85457/new/ https://reviews.llvm.org/D85457 ___ cfe-commits mailing list

[PATCH] D84886: Create LoopNestPass

ychen added a comment. I think this needs unit tests like LoopPassManagerTest.cpp (in that file or a similarly named file) to demonstrate the expected behavior: the expected usage comparing with LoopPass/LoopAnalysisPass, how the adaptor is used and the invalidation interaction within its pass

[PATCH] D83519: [NewPM] Support optnone under new pass manager

ychen added a comment. High-level request: how about split this patch into two, the first for the `require` pass part; the second for the PassInstrument callback. Then we could discuss the choices of first patch and D82344 . Comment at:

[PATCH] D83519: [NewPM] Support optnone under new pass manager

ychen added inline comments. Comment at: llvm/include/llvm/IR/PassInstrumentation.h:150 for (auto : Callbacks->BeforePassCallbacks) - ShouldRun &= C(Pass.name(), llvm::Any()); + ShouldRun &= C(Pass.name(), Pass.isRequired(), llvm::Any()); return ShouldRun;

[PATCH] D81970: [Clang][Driver] Remove gold linker support for PS4 toolchain

This revision was automatically updated to reflect the committed changes. Closed by commit rG2956cc50f340: [Clang][Driver] Remove gold linker support for PS4 toolchain (authored by ychen). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81970/new/

[PATCH] D81970: [Clang][Driver] Remove gold linker support for PS4 toolchain

ychen updated this revision to Diff 271394. ychen added a comment. - address comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81970/new/ https://reviews.llvm.org/D81970 Files: clang/lib/Driver/ToolChains/PS4CPU.cpp

[PATCH] D80944: Add begin source location for the attributed statement created from PragmaLoopHint decorated loop

ychen added a comment. In D80944#2095564 , @erichkeane wrote: > Note that this causes a regression (reported here) > https://bugs.llvm.org/show_bug.cgi?id=46336 > > The assert added here breaks in the case where a pragma loop attribute is > combined

[PATCH] D81970: [Clang][Driver] Remove gold linker support for PS4 toolchain

ychen updated this revision to Diff 271218. ychen added a comment. - update test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81970/new/ https://reviews.llvm.org/D81970 Files: clang/lib/Driver/ToolChains/PS4CPU.cpp

[PATCH] D81970: [Clang][Driver] Remove gold linker support for PS4 toolchain

ychen created this revision. ychen added a reviewer: probinson. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D81970 Files: clang/lib/Driver/ToolChains/PS4CPU.cpp clang/test/Driver/ps4-linker-non-win.c

[PATCH] D81970: [Clang][Driver] Remove gold linker support for PS4 toolchain

ychen updated this revision to Diff 271237. ychen added a comment. - clang-format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81970/new/ https://reviews.llvm.org/D81970 Files: clang/lib/Driver/ToolChains/PS4CPU.cpp

[PATCH] D80944: Add begin source location for the attributed statement created from PragmaLoopHint decorated loop

ychen updated this revision to Diff 268958. ychen added a comment. - Address feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80944/new/ https://reviews.llvm.org/D80944 Files: clang/lib/Parse/ParsePragma.cpp

[PATCH] D80944: Add begin source location for the attributed statement created from PragmaLoopHint decorated loop

ychen marked 4 inline comments as done. ychen added inline comments. Comment at: clang/lib/Parse/ParsePragma.cpp:2941 LoopHintTok.setKind(tok::annot_pragma_loop_hint); -LoopHintTok.setLocation(PragmaName.getLocation()); +LoopHintTok.setLocation(Introducer.Loc);

[PATCH] D80944: Add begin source location for the attributed statement created from PragmaLoopHint decorated loop

ychen updated this revision to Diff 269282. ychen marked 3 inline comments as done. ychen added a comment. - add comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80944/new/ https://reviews.llvm.org/D80944 Files:

[PATCH] D80944: Add begin source location for the attributed statement created from PragmaLoopHint decorated loop

ychen added inline comments. Comment at: clang/test/AST/sourceranges.cpp:116 + void f() { +// CHECK: AttributedStmt {{.*}} +DO_PRAGMA (unroll(2)) jdenny wrote: > Is there any reason not to check for `LoopHintAttr` here and on the `_Pragma` > case

[PATCH] D80944: Add begin source location for the attributed statement created from PragmaLoopHint decorated loop

ychen updated this revision to Diff 269586. ychen added a comment. - address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80944/new/ https://reviews.llvm.org/D80944 Files: clang/lib/Parse/ParsePragma.cpp

[PATCH] D80944: Add begin source location for the attributed statement created from PragmaLoopHint decorated loop

This revision was automatically updated to reflect the committed changes. Closed by commit rG9c2e770034d0: Add begin source location for the attributed statement created from… (authored by ychen). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D80944: Add begin source location for the attributed statement created from PragmaLoopHint decorated loop

ychen updated this revision to Diff 269380. ychen added a comment. - update Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80944/new/ https://reviews.llvm.org/D80944 Files: clang/lib/Parse/ParsePragma.cpp clang/lib/Parse/ParseStmt.cpp

[PATCH] D83519: [NewPM] Support optnone under new pass manager

ychen added a comment. In D83519#2144616 , @aeubanks wrote: > In D83519#2144463 , @aeubanks wrote: > > > In D83519#2144403 , @ychen wrote: > > > > > High-level request: how

[PATCH] D84774: [NewPM][PassInstrument] Add PrintPass callback to StandardInstrumentations

ychen updated this revision to Diff 281399. ychen added a comment. - Update `-debug-pass-manager-verbose` test. Some output should be ignored. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84774/new/ https://reviews.llvm.org/D84774 Files:

[PATCH] D84774: [NewPM][PassInstrument] Add PrintPass callback to StandardInstrumentations

ychen updated this revision to Diff 281395. ychen added a comment. - Add `-debug-pass-manager-verbose` to print adaptor passes. (test case in pass-pipeline-parsing.ll where we could ask for adaptor pass from `-passes`) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D84774: [NewPM][PassInstrument] Add PrintPass callback to StandardInstrumentations

ychen created this revision. ychen added reviewers: asbirlea, aeubanks, hans. Herald added subscribers: llvm-commits, cfe-commits, dexonsmith, steven_wu, zzheng, hiraditya. Herald added projects: clang, LLVM. ychen requested review of this revision. Problem: Right now, our "Running pass" is not

[PATCH] D84774: [NewPM][PassInstrument] Add PrintPass callback to StandardInstrumentations

ychen added a comment. In D84774#2179410 , @asbirlea wrote: > Does it make sense to have the option to enable seeing where the adaptors are > run? Adding a static:cl at the top of StandardInstrumentation.cpp should do it. Do you want me to add it now

[PATCH] D84707: [DFSan][NewPM] Port DataFlowSanitizer to NewPM

ychen accepted this revision. ychen added a comment. This revision is now accepted and ready to land. LGTM with two nits. Please wait one day or two in case other reviewers want to have a look. Comment at: llvm/include/llvm/Transforms/Instrumentation/DataFlowSanitizer.h:8

[PATCH] D83519: [NewPM] Support optnone under new pass manager

ychen accepted this revision. ychen added a comment. This revision is now accepted and ready to land. LGTM with one nit. Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:57 +class OptNoneInstrumentation { +public: ychen wrote: > I may probably

[PATCH] D84774: [NewPM][PassInstrument] Add PrintPass callback to StandardInstrumentations

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG555cf42f380d: [NewPM][PassInstrument] Add PrintPass callback to StandardInstrumentations (authored by ychen). Repository: rG LLVM Github Monorepo

[PATCH] D92436: [Time-report] Add a flag -ftime-report={per-pass,per-pass-run} to control the pass timing aggregation

ychen created this revision. ychen added reviewers: efriedma, philip.pfaffe, chandlerc. Herald added subscribers: llvm-commits, cfe-commits, dexonsmith, dang, hiraditya, mgorny. Herald added projects: clang, LLVM. ychen requested review of this revision. Currently, -ftime-report + new pass

[PATCH] D81958: [clangd] Add library to semantically strip flags by name.

ychen added inline comments. Comment at: clang-tools-extra/clangd/CompileCommands.cpp:356 + HELP, METAVAR, VALUES) \ + if (DriverID::OPT_##ALIAS != DriverID::OPT_INVALID && ALIASARGS == nullptr) \ +

[PATCH] D87216: [NewPM] Support --print-before/after in NPM

ychen added a comment. In D87216#2431626 , @aeubanks wrote: > In D87216#2431508 , @ychen wrote: > >> It is very unfortunate that we have to manage and translate between two sets >> of names (one pass name and one

[PATCH] D87216: [NewPM] Support --print-before/after in NPM

ychen accepted this revision. ychen added a comment. It is very unfortunate that we have to manage and translate between two sets of names (one pass name and one type name). This makes me wonder if we just keep the pass name as the return value of PassInfoMixin::name and get rid of class name

  1   2   3   4   5   6   7   >