[PATCH] D93668: [clang] Override the Fuchsia platform ABI using the triple environment

phosek added a comment. I agree that if we want to allow selecting C++ ABI only, a flag like `-fc++abi=` with some additional checks to disallow invalid combinations is better. The question is whether we want to allow that in the first place? The motivation behind this change is to support gene

[PATCH] D94820: Support for instrumenting only selected files or functions

phosek created this revision. phosek added reviewers: davidxl, vsk, gulfem. Herald added subscribers: dexonsmith, wenlei, dang, jdoerfert, steven_wu, hiraditya, mgorny. phosek requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits

[PATCH] D94820: Support for instrumenting only selected files or functions

phosek updated this revision to Diff 317796. phosek marked 3 inline comments as done. Herald added a reviewer: jansvoboda11. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94820/new/ https://reviews.llvm.org/D94820 Files: clang/docs/SourceBasedCodeCoverage.rst clang/include/clang/AST/

[PATCH] D94820: Support for instrumenting only selected files or functions

phosek added inline comments. Comment at: clang/docs/SourceBasedCodeCoverage.rst:86 +.. code-block:: none + + # all functions whose name starts with foo will be instrumented. davidxl wrote: > what is the default section name? It's `*` which always matches. ===

[PATCH] D83154: clang: Add -fcoverage-prefix-map

phosek added a comment. Herald added a reviewer: jansvoboda11. @keith do you plan on landing this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83154/new/ https://reviews.llvm.org/D83154 ___ cfe-commits

[PATCH] D93668: [clang] Override the Fuchsia platform ABI using the triple environment

phosek added a comment. In D93668#2510463 , @leonardchan wrote: > Also, this probably isn't very important, but just as a heads up: if we were > to depend on the environment component in the triple as the flag for > selecting which multilib to use, then

[PATCH] D94820: Support for instrumenting only selected files or functions

phosek updated this revision to Diff 318132. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94820/new/ https://reviews.llvm.org/D94820 Files: clang/docs/SourceBasedCodeCoverage.rst clang/include/clang/AST/ASTContext.h clang/include/clang/Basic/LangOptions.h clang/include/clang/Bas

[PATCH] D94820: Support for instrumenting only selected files or functions

phosek marked an inline comment as not done. phosek added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2591 + } + return false; +} davidxl wrote: > phosek wrote: > > davidxl wrote: > > > If the profile list contains only one line of exclude

[PATCH] D108592: [clang][Fuchsia] Support __attribute__((availability)) on Fuchsia

phosek added inline comments. Comment at: clang/lib/Basic/Targets/OSTargets.h:893 Builder.defineMacro("_GNU_SOURCE"); +Builder.defineMacro("FUCHSIA_API_LEVEL", Twine(Opts.FuchsiaAPILevel)); +this->PlatformName = "fuchsia"; aaron.ballman wrote: > I

[PATCH] D111366: [CMake] Include llvm-libtool-darwin in Fuchsia toolchain

phosek created this revision. phosek added reviewers: leonardchan, haowei, gulfem. Herald added subscribers: abrachet, mgorny. phosek requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. We want to use this tool in our build. Repository: rG

[PATCH] D111366: [CMake] Include llvm-libtool-darwin in Fuchsia toolchain

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGebcfd3ae8cc5: [CMake] Include llvm-libtool-darwin in Fuchsia toolchain (authored by phosek). Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D111371: [Support][ThinLTO] Move ThinLTO caching to LLVM Support library.

phosek added inline comments. Comment at: llvm/lib/Support/Caching.cpp:137 + sys::path::append(TempFilenameModel, CacheDirectoryPath, +"Caching-%%.tmp.o"); Expected Temp = sys::fs::TempFile::create( I would make this config

[PATCH] D111371: [Support][ThinLTO] Move ThinLTO caching to LLVM Support library.

phosek added inline comments. Comment at: llvm/lib/Support/Caching.cpp:32 + AddBufferFn AddBuffer, + StringRef CacheNameRef) { if (std::error_code EC = sys::fs::create_directories(CacheDire

[PATCH] D108592: [clang][Fuchsia] Support __attribute__((availability)) on Fuchsia

phosek accepted this revision. phosek added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/include/clang/Basic/LangOptions.def:432 +VALUE_LANGOPT(FuchsiaAPILevel, 32, 0, "Fuchsia API Level") + Comment at

[PATCH] D110663: [Driver] Support Debian multiarch style lib/clang/14.0.0/x86_64-linux-gnu runtime path and include/x86_64-linux-gnu/c++/v1 libc++ path

phosek added a comment. I'm still worried that this solution as implemented is going to break existing users, including major one like Rust. I'm not also not a fan of special casing individual targets like Fuchsia directly inside `Driver`, I believe that any per-target logic should live in the

[PATCH] D110663: [Driver] Support Debian multiarch style lib/clang/14.0.0/x86_64-linux-gnu runtime path and include/x86_64-linux-gnu/c++/v1 libc++ path

phosek added a comment. In D110663#3055813 , @phosek wrote: > I'm still worried that this solution as implemented is going to break > existing users, including major one like Rust. > > I'm not also not a fan of special casing individual targets like Fuch

[PATCH] D111371: [Support][ThinLTO] Move ThinLTO caching to LLVM Support library.

This revision was automatically updated to reflect the committed changes. Closed by commit rG92b8cc52bbc8: [Support][ThinLTO] Move ThinLTO caching to LLVM Support library (authored by noajshu, committed by phosek). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[PATCH] D111371: [Support][ThinLTO] Move ThinLTO caching to LLVM Support library.

This revision was automatically updated to reflect the committed changes. Closed by commit rGe678c5117710: [Support][ThinLTO] Move ThinLTO caching to LLVM Support library (authored by noajshu, committed by phosek). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[PATCH] D112271: [clang][Fuchsia] Add additional unit test for availability attr

phosek accepted this revision. phosek added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/test/Sema/attr-availability-fuchsia.c:19 void f7(int) __attribute__((availability(fuchsia, introduced = 16.1))); // expected-warning {{Fuchsia API Le

[PATCH] D109977: LLVM Driver Multicall tool

phosek added a comment. @beanz Is this ready to land or do you plan on making any more changes? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109977/new/ https://reviews.llvm.org/D109977 ___ cfe-commits

[PATCH] D108494: [Driver] Remove discouraged -gcc-toolchain

phosek accepted this revision. phosek 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/D108494/new/ https://reviews.llvm.org/D108494 ___

[PATCH] D108775: [CMake] Change -DENABLE_EXPERIMENTAL_NEW_PASS_MANAGER=off to -DLLVM_ENABLE_NEW_PASS_MANAGER=off

phosek accepted this revision. phosek added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108775/new/ https://reviews.llvm.org/D108775 ___ cfe-commits mailing list cfe-commits@lists.llvm.

[PATCH] D109199: [compiler-rt][Fuchsia] Support building + running compiler-rt tests on fuchsia's host toolchain

phosek added a comment. @leonardchan would it be possible to break this up into 3 changes? Comment at: clang/cmake/caches/Fuchsia-stage2.cmake:123 set(RUNTIMES_${target}_COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "") +set(RUNTIMES_${target}_COMPILER_RT_BUILD_BUILTI

[PATCH] D109199: [compiler-rt][Fuchsia] Support building + running compiler-rt tests on fuchsia's host toolchain

phosek added inline comments. Comment at: clang/cmake/caches/Fuchsia-stage2.cmake:123 set(RUNTIMES_${target}_COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "") +set(RUNTIMES_${target}_COMPILER_RT_BUILD_BUILTINS ON CACHE BOOL "") +set(RUNTIMES_${target}_COMPILER_RT_CA

[PATCH] D109199: [compiler-rt][Fuchsia] Support building + running compiler-rt tests on fuchsia's host toolchain

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

[PATCH] D108592: [clang][Fuchsia] Support __attribute__((availability)) on Fuchsia

phosek added inline comments. Comment at: clang/include/clang/Basic/LangOptions.def:431 +VALUE_LANGOPT(FuchsiaVersion, 32, 0, "Fuchsia Version") + This is more consistent with other options. Comment at: clang/lib/Basic/Targets/OSTargets.h:888

[PATCH] D109550: [clang_format] Add fallback_style flag to clang-format-diff.py

phosek accepted this revision. phosek added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/tools/clang-format/clang-format-diff.py:62 + parser.add_argument('-fallback-style', + dest='fallback_style', +

[PATCH] D109624: [clang] Make the driver not diagnose errors on nonexistent linker inputs

phosek added a comment. `Clang :: Driver/cl.c` started failing on our Windows bots after this change landed. Here

[PATCH] D109624: [clang] Make the driver not diagnose errors on nonexistent linker inputs

phosek added a comment. In D109624#2998419 , @thakis wrote: > Does it fail consistently? Or was it a one-time flake? It's been failing consistently. It might be related to our toolchain setting lld as the default linker: https://github.com/llvm/llvm-pr

[PATCH] D109624: [clang] Make the driver not diagnose errors on nonexistent linker inputs

phosek added a comment. In D109624#2998774 , @saghir wrote: > In D109624#2998665 , @thakis wrote: > >> In D109624#2998550 , @saghir wrote: >> >>> Hi, this change also brea

<    12   13   14   15   16   17