[PATCH] D85948: [HeapProf] Clang and LLVM support for heap profiling instrumentation

2020-08-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/include/clang/Driver/Options.td:995 +def fheapprof : Flag<["-"], "fheapprof">, Group, + Flags<[CoreOption, CC1Option]>, HelpText<"Enable heap profiling">; `OptInFFlag<"heapprof", "Enable", "Disable", " heap prof

[PATCH] D85948: [HeapProf] Clang and LLVM support for heap profiling instrumentation

2020-08-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/include/clang/Basic/CodeGenOptions.def:148 CODEGENOPT(MergeFunctions, 1, 0) ///< Set when -fmerge-functions is enabled. +CODEGENOPT(HeapProf , 1, 0) ///< Set when -fmemprof is enabled. CODEGENOPT(MSVolatile

[PATCH] D85948: [HeapProf] Clang and LLVM support for heap profiling instrumentation

2020-08-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/test/Instrumentation/HeapProfiling/basic.ll:1 +; Test basic address sanitizer instrumentation. +; The implementation file is HeapProfiler.cpp Naming the test directory `Heapprofiler` can reduce the number of terms

[PATCH] D85948: [HeapProf] Clang and LLVM support for heap profiling instrumentation

2020-08-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. I am still trying to read the logic. Some comments to the tests. Some comments are applicable to other tests. Comment at: llvm/test/Instrumentation/HeapProfiling/basic.ll:22 +; CHECK: %[[LOAD_ADDR:[^ ]*]] = ptrtoint i32* %a to i64 +; CHECK: %[[MASK

[PATCH] D85324: [SystemZ][z/OS] Add z/OS Target and define macros

2020-08-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Preprocessor/init.c:1041 // +// RUN: %clang_cc1 -E -dM -ffreestanding -triple=s390x-none-zos -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix S390X-ZOS %s +// RUN: %clang_cc1 -x c -E -dM -ffreestandi

[PATCH] D85810: [clang] Pass-through remarks options to linker

2020-08-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: lld/ELF/Options.td:595 HelpText<"Alias for --lto-obj-path=">; +def: J<"plugin-opt=opt-remarks-filename=">, + Alias, The change should be moved to a previous patch which will add `--opt-remarks-*` options. Reposito

[PATCH] D79495: [Sema] Allow function attribute patchable_function_entry on aarch64_be

2020-05-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: mrutland, nickdesaulniers. Herald added subscribers: cfe-commits, danielkiss, kristof.beyls. Herald added a reviewer: aaron.ballman. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D79495 Files:

[PATCH] D79495: [Sema] Allow function attribute patchable_function_entry on aarch64_be

2020-05-06 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG57a1c1be53ae: [Sema] Allow function attribute patchable_function_entry on aarch64_be (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D794

[PATCH] D79495: [Sema] Allow function attribute patchable_function_entry on aarch64_be

2020-05-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D79495#2023108 , @nickdesaulniers wrote: > @MaskRay can you please cherry pick this to release-10? cc @tstellar Already done. https://github.com/llvm/llvm-project/commit/98f9f73f6d2367aa8001c4d16de9d3b347febb08 Verified wit

[PATCH] D79210: Let clang print registered targets for --version

2020-05-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. @yaxunl The plural form `--print-targets` may make more sense to be consistent with other plural form options (--print-search-dirs, --print-diagnostic-categories, etc) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79210/ne

[PATCH] D79629: [Clang][Driver]Pass LLVM options to lld in case of LTO

2020-05-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay requested changes to this revision. MaskRay added a comment. This revision now requires changes to proceed. `-mllvm,foobar` is for compilation (.c/.cc -> .o) `-Wl,-mllvm,foobar` for LTO options. For linking, `-mllvm,foobar` is not used and thus warned. Comment at: cla

[PATCH] D79565: Add -print-targets to print the registered targets

2020-05-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79565/new/ https://reviews.llvm.org/D79565 ___ cfe-commits mailing list cfe-comm

[PATCH] D79916: Map -O to -O1 instead of -O2

2020-05-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added a reviewer: hans. Herald added a project: clang. Herald added a subscriber: cfe-commits. MaskRay added a child revision: D79919: [Driver] Pass -plugin-opt=O2 for -Os -Oz and -plugin-opt=O1 for -Og. rL187583 mapped -O

[PATCH] D79919: [Driver] Pass -plugin-opt=O2 for -Os -Oz and -plugin-opt=O1 for -Og

2020-05-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: mehdi_amini, pcc, phosek, tejohnson. Herald added subscribers: cfe-commits, dexonsmith, steven_wu, hiraditya. Herald added a project: clang. MaskRay added a parent revision: D79916: Map -O to -O1 instead of -O2. Fixes PR42445 (-Os -Oz transla

[PATCH] D79919: [Driver] Pass -plugin-opt=O2 for -Os -Oz and -plugin-opt=O1 for -Og

2020-05-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay marked an inline comment as done. MaskRay added a comment. Thanks for the review. Will remove D79916 as a parent and commit this one first. Comment at: clang/test/Driver/lto.c:52 +// RUN: %clang -target x86_64-unknown-linux-gnu --sysro

[PATCH] D79919: [Driver] Pass -plugin-opt=O2 for -Os -Oz and -plugin-opt=O1 for -Og

2020-05-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 264027. MaskRay edited the summary of this revision. MaskRay added a comment. -O => -O2 (temporarily, pending on the resolution of D79916 ) Add a test for -Ofast Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D79919: [Driver] Pass -plugin-opt=O2 for -Os -Oz and -plugin-opt=O1 for -Og

2020-05-14 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG5ecb51414637: [Driver] Pass -plugin-opt=O2 for -Os -Oz and -plugin-opt=O1 for -Og (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79919/

[PATCH] D79916: Map -O to -O1 instead of -O2

2020-05-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 264029. MaskRay edited the summary of this revision. MaskRay added a comment. Herald added subscribers: dexonsmith, steven_wu, hiraditya. Update description Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79916/ne

[PATCH] D79961: [PGO] Fix computation of fuction Hash

2020-05-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. This patch is correct. A clarification of the description: > Previous implementation was incorrectly passing an integer, that got > converted to a pointer, to finalize the hash computation. Working (an `uint64_t`) was truncated to an `uint8_t`, converted to a one-eleme

[PATCH] D79961: [PGO] Fix computation of fuction Hash

2020-05-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/CodeGen/CodeGenPGO.cpp:759 MD5.final(Result); using namespace llvm::support; return Result.low(); Can the using be deleted? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D79916: Map -O to -O1 instead of -O2

2020-05-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D79916#2042782 , @dexonsmith wrote: > In D79916#2039842 , @dexonsmith > wrote: > > > IOW, this LGTM if Alex and Gerolf are happy. > > > Gerolf told me he has no concerns. Thanks! (I wa

[PATCH] D79916: Map -O to -O1 instead of -O2

2020-05-18 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG82904401e327: Map -O to -O1 instead of -O2 (authored by MaskRay). Changed prior to commit: https://reviews.llvm.org/D79916?vs=264029&id=264749#toc Repository: rG LLVM Github Monorepo CHANGES SINCE L

[PATCH] D80225: [Driver] Recognize -fuse-ld={bfd, gold, lld} but don't prepend "ld." or "ld64." for other values

2020-05-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: martell, rnk, sbc100, theraven. Herald added subscribers: cfe-commits, sunfish, aheejin. Herald added a project: clang. - https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55470 added support for -fuse-ld=bfd to mean `ld.bfd` and -fuse-ld=gold to

[PATCH] D80225: [Driver] Recognize -fuse-ld={bfd, gold, lld} but don't prepend "ld." or "ld64." for other values

2020-05-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 264995. MaskRay added a comment. Fix fuse-ld-windows.c (I don't have Windows (REQUIRES: system-windows), so I failed to catch the issue earlier) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80225/new/ https:/

[PATCH] D80225: [Driver] Recognize -fuse-ld={bfd, gold, lld} but don't prepend "ld." or "ld64." for other values

2020-05-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 265026. MaskRay added a comment. Fix fuse-ld-windows.c Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80225/new/ https://reviews.llvm.org/D80225 Files: clang/lib/Driver/ToolChain.cpp clang/test/Driver/Input

[PATCH] D132801: [driver] Additional ignoring of module-map related flags, if modules are disabled

2022-08-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. Herald added a subscriber: StephenFan. I sometimes need to debug builds by disabling modules. This behavior will make debugging easier. Comment at: clang/test/Driver/modules.m:81 // RUN: %clang -fno-modules -fmodules-

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

2022-08-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/ps4-ps5-linker-jmc.c:3 + +// RUN: %clang -target x86_64-scei-ps4 -fjmc %s -### 2>&1 | FileCheck --check-prefixes=CHECK-PS4,CHECK-PS4-LIB %s +// RUN: %clang -target x86_64-scei-ps4 -flto=thin -fjmc %s -### 2>&1 | FileC

[PATCH] D131464: [test] Make tests pass regardless of gnu++14/gnu++17 default

2022-08-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 456524. MaskRay marked 6 inline comments as done. MaskRay added a comment. address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131464/new/ https://reviews.llvm.org/D131464 Files: clang/test/AST/as

[PATCH] D131464: [test] Make tests pass regardless of gnu++14/gnu++17 default

2022-08-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/CodeGenCXX/exception-spec-decay.cpp:1 -// RUN: %clang_cc1 -fcxx-exceptions -fexceptions %s -triple=i686-unknown-linux -emit-llvm -o - | FileCheck %s +// RUN: %clang_cc1 %stdcxx_98- -fcxx-exceptions -fexceptions -Wno-dynamic-

[PATCH] D131464: [test] Make tests pass regardless of gnu++14/gnu++17 default

2022-08-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay marked 5 inline comments as done. MaskRay added inline comments. Comment at: llvm/utils/lit/lit/llvm/config.py:570 +clang_std_values = ('98', '11', '14', '17', '20', '2b') +def add_stdcxx(s): +t = s[8:] aaron.ballman wrote: > I

[PATCH] D131992: [Support] compression proposal for a enum->spec->impl approach

2022-08-30 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. (I was out of town last week so I did not respond then.) In D131992#3748306 , @dblaikie wrote: > @MaskRay I think this change is probably the best point of > comparison/demonstration of this patch direction (taking some minor li

[PATCH] D131153: AArch64: disable asynchronous unwind by default for MachO.

2022-08-30 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/include/clang/Driver/ToolChain.h:501 + /// IsAsyncUnwindTablesDefault - Does this tool chain use + /// -fasync-unwind-tables by default. + virtual bool smeenai wrote: > I believe the option is spelled `-fasynchr

[PATCH] D131563: [clang] Fix clang multiarch isssue with musl

2022-08-31 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay requested changes to this revision. MaskRay added a comment. This revision now requires changes to proceed. This may not be the right direction. If GCC is not configured with `--enable-multi-arch`, `--print-multiarch` output is an empty line. The `x86_64-linux-gnu` style output is for Deb

[PATCH] D131464: [test] Make tests pass regardless of gnu++14/gnu++17 default

2022-09-01 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 457367. MaskRay marked 2 inline comments as done. MaskRay edited the summary of this revision. MaskRay added a comment. Fix a test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131464/new/ https://reviews.llvm.

[PATCH] D131465: C++/ObjC++: switch to gnu++17 as the default standard

2022-09-01 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 457390. MaskRay retitled this revision from "C++/ObjC++: switch to gnu++17 as the default dialect" to "C++/ObjC++: switch to gnu++17 as the default standard". MaskRay edited the summary of this revision. MaskRay added a comment. Skip PS4/PS5. Add `// UNSUPPOR

[PATCH] D132810: [clang][MinGW] Add `-mguard=cf` and `-mguard=cf-nochecks`

2022-09-01 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. The risk conflicting with a GCC option is probably quite low. If there is something, the GCC option will likely be `-mguard-*=` instead of `-mguard=` (IMHO confusing). Comment at: clang/lib/Driver/ToolChains/MinGW.cpp:627 +StringRef GuardArgs = A-

[PATCH] D133170: [Driver] Unsupport --print-multiarch

2022-09-01 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: jrtc27, phosek. Herald added subscribers: StephenFan, pengfei. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. - If GCC is configured with `--disab

[PATCH] D101400: [Driver] Add -print-multiarch-triple

2022-09-01 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Herald added subscribers: abrachet, StephenFan. Herald added a project: All. In D101400#2990254 , @jrtc27 wrote: > Answering the last part myself: GCC only has MULTIARCH_DIRNAME definitions > for various linux-gnu*, linux-musl, l

[PATCH] D133180: [MinGW] Skip -fvisibility/-fvisibility-inlines-hidden for dllexport

2022-09-01 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: mstorsjo, mati865, rnk. Herald added a subscriber: StephenFan. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Similar to 123ce97fac78bc4519afd5d2a

[PATCH] D133180: [MinGW] Ignore -fvisibility/-fvisibility-inlines-hidden for dllexport

2022-09-01 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 457483. MaskRay added a comment. improve test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133180/new/ https://reviews.llvm.org/D133180 Files: clang/lib/CodeGen/CodeGenModule.cpp clang/test/CodeGenCXX/dll

[PATCH] D133170: [Driver] Unsupport --print-multiarch

2022-09-02 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGe05edb19adbf: [Driver] Unsupport --print-multiarch (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133170/new/ https://reviews.llvm.org

[PATCH] D133180: [MinGW] Ignore -fvisibility/-fvisibility-inlines-hidden for dllexport

2022-09-02 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 457625. MaskRay marked an inline comment as done. MaskRay added a comment. rename test to dllstorage-visibility.cpp use -fdeclspec instead of -fms-extensions Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133180/

[PATCH] D133180: [MinGW] Ignore -fvisibility/-fvisibility-inlines-hidden for dllexport

2022-09-02 Thread Fangrui Song via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG1a4d851d272d: [MinGW] Ignore -fvisibility/-fvisibility-inlines-hidden for dllexport (authored by MaskRay). Repository: rG LLVM Github Monorepo CH

[PATCH] D130888: [Clang] Introduce -fexperimental-sanitize-metadata=

2022-09-02 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. Comment at: clang/include/clang/Driver/Options.td:1687 +def fexperimental_sanitize_metadata_EQ : CommaJoined<["-"], "fexperimental-sanitize-metadata=">, + Group, + HelpText<"Specify the type of metadata to emit for

[PATCH] D133157: Add -sanitizer-coverage-control-flow

2022-09-03 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/docs/SanitizerCoverage.rst:338 + +With ``-fsanitize-coverage=control-flow`` the compiler will create a table to collect +control flow for each function. More specifically, for each basic block in the function, Th

[PATCH] D133157: Add -sanitizer-coverage-control-flow

2022-09-03 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > Add -sanitizer-coverage-control-flow Add -fsanitizer-coverage=control-flow Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133157/new/ https://reviews.llvm.org/D133157 ___ cfe-c

[PATCH] D133266: [MinGW] Reject explicit non-default visibility applied to dllexport/dllimport declaration

2022-09-03 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: mstorsjo, mati865, rnk. Herald added a subscriber: StephenFan. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. dllimport/dllexport is incompatible

[PATCH] D131464: [test] Make tests pass regardless of gnu++14/gnu++17 default

2022-09-03 Thread Fangrui Song via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG83ea47acd711: [test] Make tests pass regardless of gnu++14/gnu++17 default (authored by MaskRay). Changed prior to commit: https://reviews.llvm.or

[PATCH] D111367: [Driver] Change -dumpmachine to respect --target/LLVM_DEFAULT_TARGET_TRIPLE verbatim

2022-09-03 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay abandoned this revision. MaskRay added a comment. Herald added a subscriber: StephenFan. Herald added a project: All. Abandon since we decide to try using normalized target triple and just hard code the Debian multiarch (omitted 'vendor') in few places. Repository: rG LLVM Github Mono

[PATCH] D133266: [MinGW] Reject explicit non-default visibility applied to dllexport/dllimport declaration

2022-09-05 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. MaskRay marked an inline comment as done. Closed by commit rG91d8324366f4: [MinGW] Reject explicit non-default visibility applied to dllexport/dllimport… (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D131465: C++/ObjC++: switch to gnu++17 as the default standard

2022-09-05 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Ping:) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131465/new/ https://reviews.llvm.org/D131465 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/c

[PATCH] D133329: [Driver] Add --gcc-install-dir=

2022-09-05 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. Herald added subscribers: StephenFan, pengfei. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This option specifies a GCC installation directory such as /usr/lib/gcc/x86_6

[PATCH] D133329: [Driver] Add --gcc-install-dir=

2022-09-05 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 458065. MaskRay added a comment. improve test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133329/new/ https://reviews.llvm.org/D133329 Files: clang/include/clang/Basic/DiagnosticDriverKinds.td clang/incl

[PATCH] D133325: [Driver] Allow search of included response files as configuration files

2022-09-05 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > Normally a file included into configuration file by directive @file is > searched for relative the including configuration file. In some cases it > is not convenient. Sorry, I am puzzled by the description. A configuration file may specify `--option=file` and `@cfg`. C

[PATCH] D133325: [Driver] Allow search of included response files as configuration files

2022-09-05 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/include/llvm/Support/CommandLine.h:2132 + + bool getSearchAsConfig() const { return SearchAsConfig; } + ExpansionContext &setSearchAsConfig(bool X) { Unused Repository: rG LLVM Github Monorepo CHANGES SINCE L

[PATCH] D131465: C++/ObjC++: switch to gnu++17 as the default standard

2022-09-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 458241. MaskRay added a subscriber: sammccall. MaskRay added a comment. Herald added subscribers: kadircet, arphaman. Herald added a project: clang-tools-extra. Rebase. This shall fix a clang/test test. Update clang-tools-extra/clangd/unittests/SelectionTests

[PATCH] D131465: C++/ObjC++: switch to gnu++17 as the default standard

2022-09-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a subscriber: mgorny. MaskRay added inline comments. Comment at: clang/test/lit.site.cfg.py.in:27 config.clang_enable_opaque_pointers = @CLANG_ENABLE_OPAQUE_POINTERS_INTERNAL@ +config.clang_default_std_cxx = "@CLANG_DEFAULT_STD_CXX@" config.clang_default_cxx_stdli

[PATCH] D133375: [CMake] Remove CLANG_DEFAULT_STD_C/CLANG_DEFAULT_STD_CXX

2022-09-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: aaron.ballman, mgorny, thesamesam. Herald added a subscriber: StephenFan. Herald added a project: All. MaskRay requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. When Cl

[PATCH] D131465: C++/ObjC++: switch to gnu++17 as the default standard

2022-09-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/lit.site.cfg.py.in:27 config.clang_enable_opaque_pointers = @CLANG_ENABLE_OPAQUE_POINTERS_INTERNAL@ +config.clang_default_std_cxx = "@CLANG_DEFAULT_STD_CXX@" config.clang_default_cxx_stdlib = "@CLANG_DEFAULT_CXX_STDLIB@" ---

[PATCH] D131563: [clang] Fix clang multiarch isssue with musl

2022-09-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. This can be abandoned as `--print-multiarch` was removed. See also https://github.com/llvm/llvm-project/issues/51469#issuecomment-1236163566 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131563/new/ https://reviews.llvm.org/D131563

[PATCH] D133358: [compiler-rt] [test] Handle missing ld.gold gracefully

2022-09-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Python 3.7 can use `subprocess.run` with `capture_output`: https://docs.python.org/3/library/subprocess.html#:~:text=capture_output But llvm-project requires Python >= 3.6 now, so we have to

[PATCH] D133325: [Driver] Allow search of included response files as configuration files

2022-09-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. As mentioned, if the new search algorithm makes sense, perhaps use the new algorithm by default and provide a temporary driver option for keeping the old behavior? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133325/new/

[PATCH] D128372: [Clang-Tidy] Empty Check

2022-09-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Drive-by comments: this patch may need a review from common reviewers like @njames93. I assume that @LegalizeAdulthood's comment (about the file hierarchy since there was a big reorganization few months ago) has been addressed. If you are concerned with unable to conta

[PATCH] D133266: [MinGW] Reject explicit non-default visibility applied to dllexport/dllimport declaration

2022-09-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D133266#3775189 , @bd1976llvm wrote: > This approach doesn't account for the implementation of > -fvisibility-global-new-delete-hidden. The following case now fails after > this change: > > >type new_del.cpp > > names

[PATCH] D131465: C++/ObjC++: switch to gnu++17 as the default standard

2022-09-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D131465#3774552 , @sammccall wrote: > In D131465#3774276 , @aaron.ballman > wrote: > >> In D131465#3772803 , @MaskRay >> wrote: >> >>> Update

[PATCH] D131465: C++/ObjC++: switch to gnu++17 as the default standard

2022-09-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 458556. MaskRay edited the summary of this revision. MaskRay added a comment. revert a clangd change after sammccall's change to make it immune to gnu++14/gnu++17 default change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

[PATCH] D131465: C++/ObjC++: switch to gnu++17 as the default standard

2022-09-07 Thread Fangrui Song via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe321c8dd2cea: C++/ObjC++: switch to gnu++17 as the default standard (authored by MaskRay). Changed prior to commit: https://reviews.llvm.org/D1314

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

2022-09-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. I appreciate the detailed summary. It has sufficient information but I think it can be rephased to be conciser. I request changes for the verbosity and the possibly functionality issue. > We can use -ftime-trace or -ftime-trace= to switch on the TimeProfiler. "We can us

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

2022-09-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/Driver.cpp:4514 +// Infer data storing path of the options `-ftime-trace`, `-ftime-trace=` +void InferTimeTracePath(Compilation &C) { + bool HasTimeTrace = Use `static`. See https://llvm.org/docs/Codin

[PATCH] D133266: [MinGW] Reject explicit non-default visibility applied to dllexport/dllimport declaration

2022-09-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D133266#3775384 , @MaskRay wrote: > In D133266#3775189 , @bd1976llvm > wrote: > >> This approach doesn't account for the implementation of >> -fvisibility-global-new-delete-hidden. Th

[PATCH] D133405: [Linux] Hack around Linux/sparc

2022-09-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. It's not a compiler's job to define this workaround... If a platform want to provide a built-in macro, you can use https://clang.llvm.org/docs/UsersManual.html#configuration-files and add `-D__NO_INLINE__` there. To avoid magic behaviors, `${triple}-clang` loads `${trip

[PATCH] D133266: [MinGW] Reject explicit non-default visibility applied to dllexport/dllimport declaration

2022-09-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D133266#3776051 , @bd1976llvm wrote: > In D133266#3775832 , @MaskRay wrote: > >> In D133266#3775384 , @MaskRay >> wrote: >> >>> In D133266#37

[PATCH] D133266: [MinGW] Reject explicit hidden visibility applied to dllexport and hidden/protected applied to dllimport

2022-09-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 458627. MaskRay retitled this revision from "[MinGW] Reject explicit non-default visibility applied to dllexport/dllimport declaration" to "[MinGW] Reject explicit hidden visibility applied to dllexport and hidden/protected applied to dllimport". MaskRay edi

[PATCH] D131465: C++/ObjC++: switch to gnu++17 as the default standard

2022-09-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D131465#3776515 , @nikic wrote: > I've reverted this change because it causes major llvm-test-suite breakage. > You likely need to pin many tests to use `-std=c++14`. https://github.com/llvm/llvm-test-suite/commit/b3a89445dc4

[PATCH] D132727: [clang][Interp] Implement array initializers and subscript expressions

2022-09-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Heads-up: after D132832 and D132727 , clang/test/AST/Interp/arrays.cpp.test has a msan failure. It can be pre-existing or can be newly introduced. There seems another issue about an integer overflow in

[PATCH] D111509: [clang] use getCommonSugar in an assortment of places

2022-09-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Since the fix was not merged, I will revert this soon. I saw a clang segfault with `clang++ libstdc++-v3/src/c++98/complex_io.cc` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111509/new/ https://reviews.llvm.org/D111509

[PATCH] D111509: [clang] use getCommonSugar in an assortment of places

2022-09-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D111509#3778882 , @mizvekov wrote: > In D111509#3778879 , @MaskRay wrote: > >> Since the fix was not merged, I will revert this soon. >> >> I saw a clang segfault with `clang++ libstdc+

[PATCH] D131153: AArch64: disable asynchronous unwind by default for MachO.

2022-09-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Herald added a subscriber: StephenFan. You may want to split the patch, with refactoring as the first, and the Mach-O specific change as the second one. Comment at: clang/

[PATCH] D133405: [Linux] Hack around Linux/sparc

2022-09-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a subscriber: zatrazz. MaskRay added a comment. This revision is now accepted and ready to land. So, sparc64 gcc does seem to define the macro by default. ¯\_(ツ)_/¯ % sparc64-linux-gnu-gcc -E -dM -xc /dev/null -nostdinc | grep NO_INLINE #define __

[PATCH] D132975: [CMake] Add clang-bolt target

2022-09-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/CMakeLists.txt:930-937 +-DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX} +-DCMAKE_C_COMPILER=${CLANG_INSTRUMENTED} +-DCMAKE_CXX_COMPILER=${CLANGXX_INSTRUMENTED} +-DCMAKE

[PATCH] D132727: [clang][Interp] Implement array initializers and subscript expressions

2022-09-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D132727#3779237 , @tbaeder wrote: > Thanks @MaskRay, I thought this was fixed by > https://github.com/llvm/llvm-project/commit/86271798e51a7866dd2af44e0ee183d1331089e6, > at least all the emails I got about failing msan build

[PATCH] D131465: C++/ObjC++: switch to gnu++17 as the default standard

2022-09-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D131465#3780717 , @AaronLiu wrote: > This cause a large amount failures in our testing with errors such as: > > error: ISO C++17 does not allow 'register' storage class specifier > [-Wregister] > > error: ISO C++17 does no

[PATCH] D131465: C++/ObjC++: switch to gnu++17 as the default standard

2022-09-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D131465#3776841 , @nikic wrote: > In D131465#3776599 , @nikic wrote: > >> It looks like the switch to `-std=c++17` has some major compile-time impact: >> http://llvm-compile-time-track

[PATCH] D133587: Loop names used in reporting can grow very large

2022-09-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133587/new/ https://reviews.llvm.org/D133587 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi

[PATCH] D131465: C++/ObjC++: switch to gnu++17 as the default standard

2022-09-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D131465#3780833 , @AaronLiu wrote: > In D131465#3780767 , @aaron.ballman > wrote: > >> In D131465#3780717 , @AaronLiu >> wrote: >> >>> This c

[PATCH] D133266: [MinGW] Reject explicit hidden visibility applied to dllexport and hidden/protected applied to dllimport

2022-09-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay marked an inline comment as done. MaskRay added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1116 +if (GV->hasDLLExportStorageClass()) { + // Reject explicit hidden visibility on dllexport. + if (LV.getVisibility() == HiddenVisibility) --

[PATCH] D133266: [MinGW] Reject explicit hidden visibility applied to dllexport and hidden/protected applied to dllimport

2022-09-12 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. MaskRay marked an inline comment as done. Closed by commit rG6f9c4851ab7c: [MinGW] Reject explicit hidden visibility applied to dllexport and… (authored by MaskRay). Changed prior to commit: https://reviews.llvm.org/D1332

[PATCH] D133771: Add a "Potentially Breaking Changes" section to the Clang release notes

2022-09-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. Herald added a subscriber: StephenFan. Comment at: clang/docs/ReleaseNotes.rst:49 + present in major C++ implementations (including Clang), this error has the + ability to be downgraded into a warning (via: -Wno-er

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

2022-09-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Driver/Driver.cpp:4516 + bool HasTimeTrace = C.getArgs().hasArg(options::OPT_ftime_trace); + bool HasTimeTraceFile = C.getArgs().hasArg(options:

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

2022-09-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay requested changes to this revision. MaskRay added a comment. This revision now requires changes to proceed. Sorry, we should compare this approach with D133662 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D133662: [Clang] WIP: Change -ftime-trace storing path and support multiple compilation jobs

2022-09-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. At a glance the approach looks more elegant to me, but I haven't studied this carefully yet. Also thanks for checking how to handle offloading. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133662/new/ https://reviews.llvm

[PATCH] D133662: [Clang] WIP: Change -ftime-trace storing path and support multiple compilation jobs

2022-09-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/include/clang/Driver/Compilation.h:134 + + llvm::SmallDenseMap TimeTraceFiles; + I don't think we should use a non-zero inline element for these members. They are, in 99.9% cases, empty. We should optimize for th

[PATCH] D133092: [clang] fix generation of .debug_aranges with LTO

2022-09-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. This is not correct. GNU ld and gold don't accept `-mllvm`. You need to use `-plugin-opt=-generate...` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133092/new/ https://reviews.llvm.org/D133092 ___

[PATCH] D128465: [llvm] add zstd to `llvm::compression` namespace

2022-09-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/include/llvm/Support/Compression.h:48 + +constexpr int NoCompression = -5; +constexpr int BestSpeedCompression = 1; MaskRay wrote: > I missed the values here. Why is -5 picked for NoCompression? What does it > mean

[PATCH] D133875: [clang] fix generation of .debug_aranges with LTO (resubmit)

2022-09-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/debug-options-aranges.c:1 +// Check that lld will emit dwarf aranges. + The message is outdated now. This just checks how -gdwarf-aranges ias passed to the compiler and linker. Comme

[PATCH] D83015: [Driver] Add --ld-path= and deprecate -fuse-ld=/abs/path and -fuse-ld=rel/path

2022-09-15 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D83015#3792397 , @thakis wrote: > `LinkerIsLLD` isn't getting set when `--ld-path` is used. (It also isn't > getting set when using `-fuse-ld=` with an absolute path.) > > How do you imagine using an absolute path and telling c

[PATCH] D154396: [clang] Add support for SerenityOS

2023-11-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/serenity.cpp:20 +// SERENITY_X86_64: "{{(.*[^-.0-9A-Z_a-z])?}}ld.lld" +// SERENITY_X86_64: "-pie" +// SERENITY_X86_64: "-dynamic-linker" "/usr/lib/Loader.so" "--eh-frame-hdr" Prefer `-SAME:` whenever ap

[PATCH] D154396: [clang] Add support for SerenityOS

2023-11-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Serenity.h:61 + bool isPICDefault() const override { return true; } + bool isPIEDefault(const llvm::opt::ArgList &) const override { return true; } + bool isPICDefaultForced() const override { return false;

[PATCH] D147073: [Coverage] Handle invalid end location of an expression/statement.

2023-11-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:607 +// compiler has broken. +assert((!StartLoc || StartLoc->isValid()) && "Start location is not valid"); +assert((!EndLoc || EndLoc->isValid()) && "End location is not valid");

[PATCH] D154774: [Sema] Respect instantiated-from template's VisibilityAttr for two implicit/explicit instantiation cases

2023-11-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/AST/Decl.cpp:380 +->getTemplatedDecl() +->hasAttr(); } rjmccall wrote: > Okay, this change seems wrong. A visibi

<    2   3   4   5   6   7   8   9   10   11   >