[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 http://lists.llvm.org/cgi-b

[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/ https://reviews.llvm.org/D563

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

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

[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: clang/include/clang/Basic

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

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

[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: clang/include/clang/Basic/CodeGen

[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 https://lists.ll

[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: clang/test/Driver/verbose-output-qu

[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 https://r

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

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

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

[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: test/Dri

[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/ https://reviews.llvm.org/D

[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: lib/CodeGen/CGCa

[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"); - FuncAttrs.addAttribute("no-frame-pointer-elim-

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

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

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

[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 clang/test/Sema/warn-integer-constants-in-ternary

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

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

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

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

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

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

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

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

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

[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: clang/test/Driver/cc1-spawn

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

[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: clang/test/Driver/cc1-spawn

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

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

[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&id=266088#toc Repository: rG LLVM Github M

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

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

[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: llvm/include/llvm/Pa

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

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

[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 https://r

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

[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 clang/lib/Parse/ParseStmt.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: clang/lib/Parse/ParsePragma.

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

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

[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 clang/lib/Parse/ParseStmt.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 https://reviews.llvm.org/D80944/n

[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 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 clang/test/Driver/ps4-linke

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

[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 clang/test/Driver/ps4-link

[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 clang/test/Driver/ps4-l

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

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

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

[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: llvm/i

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

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

[PATCH] D131820: [PS4][driver] make -fjmc work with LTO driver linking stage

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

[PATCH] D132592: [Clang] Implement function attribute nouwtable

ychen updated this revision to Diff 456403. ychen added a comment. - add a release note. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132592/new/ https://reviews.llvm.org/D132592 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/At

[PATCH] D131820: [PS4][driver] make -fjmc work with LTO driver linking stage

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

[PATCH] D131820: [PS4][driver] make -fjmc work with LTO driver linking stage

ychen updated this revision to Diff 456426. ychen added a comment. - use `--target` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131820/new/ https://reviews.llvm.org/D131820 Files: clang/lib/Driver/ToolChains/PS4CPU.cpp clang/test/Driver/ps4-

[PATCH] D131820: [PS4][driver] make -fjmc work with LTO driver linking stage

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

[PATCH] D132592: [Clang] Implement function attribute nouwtable

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG70248bfdea6c: [Clang] Implement function attribute nouwtable (authored by ychen). Changed prior to commit: https://reviews.llvm.org/D132592?vs=456

[PATCH] D131820: [PS4][driver] make -fjmc work with LTO driver linking stage

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG288576f474f2: [PS4][driver] make -fjmc work with LTO driver linking stage (authored by ychen). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

ychen updated this revision to Diff 456528. ychen added a comment. This revision is now accepted and ready to land. - handle constrained placeholder [pack] types Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128750/new/ https://reviews.llvm.org/D12

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

ychen requested review of this revision. ychen added a comment. @royjacobson: really appreciate it if you could take another look. @mizvekov: I saw you mentioned in the Discourse that you want to review template-related stuff and you have been actively working in that area, do you mind having a

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

ychen updated this revision to Diff 456743. ychen marked an inline comment as done. ychen added a comment. - For function templates, compare canonical types of funtion parameters directly. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128750/new/

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

ychen added a comment. In D128750#3758750 , @mizvekov wrote: > Just a first glance at the patch, will try to do a more comprehensive review > later. Thanks! Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:1838 -QualType

[PATCH] D133052: [clang] Avoid crash when expanding conversion templates in concepts.

ychen added a comment. Like mentioned in https://stackoverflow.com/questions/68853472/is-this-a-bug-in-clangs-c20-concepts-implementation-unnecessary-checking-of, could we not go down the path of considering conversion candidates? It seems that's blessed by the standard. Repository: rG LLVM

[PATCH] D133052: [clang] Avoid crash when expanding conversion templates in concepts.

ychen added a comment. In D133052#3763041 , @luken-google wrote: > In D133052#3762596 , @ychen wrote: > >> Like mentioned in >> https://stackoverflow.com/questions/68853472/is-this-a-bug-in-clangs-c20-concepts-i

[PATCH] D133052: [clang] Avoid crash when expanding conversion templates in concepts.

ychen accepted this revision. ychen added a comment. This revision is now accepted and ready to land. In D133052#3763983 , @luken-google wrote: > In D133052#3763201 , @ychen wrote: > >> In D133052#3763041

[PATCH] D133052: [clang] Avoid crash when expanding conversion templates in concepts.

ychen added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:4012 + // Avoid an infinite template expansion loop in requirements checking by + // skipping the conversion functions check. Would be better to check this lazily (inside the if statement be

[PATCH] D133052: [clang] Avoid crash when expanding conversion templates in concepts.

ychen requested changes to this revision. ychen added a comment. This revision now requires changes to proceed. Oh, one more thing, we probably need to handle nested levels too, for example, `foo(a);` might be triggered by a template which may be in turn triggered by the concept check. So only c

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

ychen marked 4 inline comments as done. ychen added a comment. @mizvekov thanks for the detailed review. It helped me a lot in reconsidering the auto type handling. Comment at: clang/include/clang/Sema/SemaConcept.h:48-52 + if (ArgA.getKind() == TemplateArgument::Expressi

[PATCH] D111283: [clang] template / auto deduction deduces common sugar

ychen added a comment. Hi @mizvekov , I noticed that deduction for partial ordering also inherits this new behavior. Do you think partial ordering could opt out of this, or is it better for partial ordering to deal with the new sugared types? Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D133683: [c++] implements tentative DR1432 for partial ordering of function template

ychen created this revision. ychen added reviewers: aaron.ballman, erichkeane. Herald added a project: All. ychen requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. D128745 handled DR1432 for the partial ord

[PATCH] D111283: [clang] template / auto deduction deduces common sugar

ychen added a comment. In D111283#3783338 , @mizvekov wrote: > In D111283#3783250 , @ychen wrote: > >> Hi @mizvekov , I noticed that deduction for partial ordering also inherits >> this new behavior. Do you think

[PATCH] D111283: [clang] template / auto deduction deduces common sugar

ychen added a comment. In D111283#3784648 , @mizvekov wrote: > In D111283#3784615 , @ychen wrote: > >> `A` is currently modeled as ElaboratedType. It was >> `TemplateSpecializationType` before. Reading comments f

[PATCH] D111283: [clang] template / auto deduction deduces common sugar

ychen added a comment. In D111283#3784702 , @mizvekov wrote: > In D111283#3784663 , @ychen wrote: > >> Thanks for the link. I'm not blocked by any of these patches, instead just >> trying to have a mental model o

[PATCH] D133683: [c++] implements tentative DR1432 for partial ordering of function template

ychen added a comment. @mizvekov Thanks for taking a look. Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5195-5196 +for (int i = 0, e = std::min(NumParams1, NumParams2); i < e; ++i) { + QualType T1 = FD1->getParamDecl(i)->getType().getDesugaredType(Context); +

[PATCH] D133341: [C++] [Coroutines] Prefer aligned (de)allocation for coroutines - implement the option2 of P2014R0

ychen added a comment. It surprises me that Option 2 does not change https://eel.is/c++draft/dcl.fct.def.coroutine#10. For consistency, I think it should. And according to your test case, it deals with alignment as expected. Probably we should change the P2014R0 wording accordingly. Before that

[PATCH] D133341: [C++] [Coroutines] Prefer aligned (de)allocation for coroutines - implement the option2 of P2014R0

ychen added inline comments. Comment at: clang/test/SemaCXX/coroutine-alloc-4.cpp:49 +void return_value(int) {} +void *operator new(std::size_t, std::align_val_t) noexcept; +void *operator new(std::size_t) noexcept; Like this test case, please add add

[PATCH] D133341: [C++] [Coroutines] Prefer aligned (de)allocation for coroutines - implement the option2 of P2014R0

ychen added a comment. Looks great. Just a few more nits. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11280 +def warn_non_aligned_allocation_function : Warning < + "found non aligned allocation function for coroutine %0">, + InGroup; "under `

[PATCH] D115844: [ubsan] Using metadata instead of prologue data for function sanitizer

ychen updated this revision to Diff 440329. ychen marked an inline comment as done. ychen added a comment. Update LangRef. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115844/new/ https://reviews.llvm.org/D115844 Files: clang/lib/CodeGen/CodeGe

[PATCH] D115844: [ubsan] Using metadata instead of prologue data for function sanitizer

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6678f8e505b1: [ubsan] Using metadata instead of prologue data for function sanitizer (authored by ychen). Repository: rG LLVM Github Monorepo CHA

  1   2   3   4   5   6   7   >