[PATCH] D134127: [ARM] Check target feature support for __builtin_arm_crc*

MaskRay added a comment. In D134127#3805748 , @dmgreen wrote: > Whether you do that here or in another patch, the change LGTM. Thanks:) I'll learn more about arm_acle.h Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D134127: [ARM] Check target feature support for __builtin_arm_crc*

This revision was automatically updated to reflect the committed changes. Closed by commit rG069ecd0c6e2c: [ARM] Check target feature support for __builtin_arm_crc* (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134127/new/ htt

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

MaskRay added a comment. In D131465#3804893 , @glandium wrote: > This didn't change the default for msvc targets, was that expected? I think it is expected (but I did not know when I made the change) https://learn.microsoft.com/en-us/cpp/build/reference

[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs (WIP)

MaskRay added inline comments. Comment at: clang/lib/Driver/Driver.cpp:6195 + + assert(false && "Unhandled Mode"); + return "clang"; If the switch covers all enum members, we typically use llvm_unreachable to work around a GCC -Wswitch warning. Clang correctly

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG23e4299261d9: [Driver] Add --gcc-install-dir= (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133329/new/ https://reviews.llvm.org/D133

[PATCH] D134468: [Driver] Drop MSVC<2015 tweaking

MaskRay created this revision. MaskRay added reviewers: aganea, hans, rnk, STL_MSFT, zequanwu. 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. 8db9176d688fd89a7578d

[PATCH] D134468: [Driver] Ignore -fmsc-version= -fms-compatibility-version= values smaller than 19

MaskRay updated this revision to Diff 462292. MaskRay retitled this revision from "[Driver] Drop MSVC<2015 tweaking" to "[Driver] Ignore -fmsc-version= -fms-compatibility-version= values smaller than 19". MaskRay edited the summary of this revision. MaskRay added a comment. . Repository: rG

[PATCH] D128142: [MemProf] Memprof profile matching and annotation

MaskRay added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:1247 +uint32_t Column) { + return hash_combine(Function, LineOffset, Column); +} You may use BLAKE3 instead of MD5. BLAKE3 is mu

[PATCH] D128142: [MemProf] Memprof profile matching and annotation

MaskRay added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:1247 +uint32_t Column) { + return hash_combine(Function, LineOffset, Column); +} MaskRay wrote: > You may use BLAKE3 instead of

[PATCH] D134468: [Driver] Ignore -fmsc-version= -fms-compatibility-version= values smaller than 19

MaskRay added a comment. I am not a Windows driver user. This patch is entirely motivated by Windows discussions in D131465 and I want to make `/std:` default rule simple (say, clang 16 uses C++17 for every mode for Windows). If the `-fno-threadsafe-statics` c

[PATCH] D134550: [Clang] Make Clang driver suggest '-Xclang' for CC1 options passed to the driver

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. I think this is fine. Driver options take precedence and cc1 options cannot be suggested. If the user writes a cc1-only option without `-Xclang`, they likely have the particular need and wil

[PATCH] D134657: [Driver] pass -fcrash-diagnostics-dir to LTO

MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:635 + // Setup crash diagnostics dir. + if (Args.hasArg(options::OPT_fcrash_diagnostics_dir)) { +StringRef

[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs (WIP)

MaskRay added a comment. In D134337#3815412 , @mgorny wrote: > In D134337#3815339 , @sepavloff > wrote: > >> In D134337#3805368 , @mgorny wrote: >> >>> For target, it wil

[PATCH] D134468: [Driver] Ignore -fmsc-version= -fms-compatibility-version= values smaller than 19

MaskRay added a comment. I am not a user of the MSVC toolchain, so leaving this work to someone working on it is better:) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134468/new/ https://reviews.llvm.org/D134468 _

[PATCH] D134454: [Driver][Distro] Fix ArchLinux triplet and sysroot detection

MaskRay added a comment. I am nervous as well as Arch Linux has many derivatives. They likely don't use `ID=arch` in `/etc/os-release`. The patch won't work for them. The last few comments of https://discourse.llvm.org/t/rfc-adding-a-default-file-location-to-config-file-support/63606 discuss a

[PATCH] D134671: [Driver] Prevent Mips specific code from claiming -mabi argument on other targets.

MaskRay added inline comments. Comment at: clang/test/Driver/mabi.c:1 +// RUN: %clang -target i386-unknown-linux -mabi=ms -S %s -### 2>&1 | FileCheck --check-prefix=CHECK %s + `-target ` is legacy. Use `--target=` Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D134638: [Clang][LoongArch] Add inline asm support for constraints k/m/ZB/ZC

MaskRay added inline comments. Comment at: clang/lib/Basic/Targets/LoongArch.cpp:103 + Info.setAllowsMemory(); + Name++; // Skip over 'Z'. + return true; `++Name` Comment at: clang/lib/Basic/Targets/LoongArch.cpp:117 +// pars

[PATCH] D134638: [Clang][LoongArch] Add inline asm support for constraints k/m/ZB/ZC

MaskRay added inline comments. Comment at: llvm/lib/Target/LoongArch/LoongArchISelDAGToDAG.cpp:92 + case InlineAsm::Constraint_m: { +SDValue Base, Offset; +Base = Op; Prefer initializing the variable when it is declared Comment at: llv

[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs

MaskRay added a comment. Thanks. The previous behavior is too magical. I like this version. Comment at: clang/docs/ReleaseNotes.rst:243 + ``.cfg`` and ``.cfg`` if the former is not found. `Target` + is always the effective target (with respect to ``-target``, ``-m32``, etc.)

[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs

MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Driver/Driver.cpp:6195 + + assert(false && "Unhandled Mode"); + return "clang"; MaskRay wrote: > If the switch covers all enum

[PATCH] D134790: [Driver] Add --config= as canonical spelling of --config

MaskRay created this revision. MaskRay added reviewers: mgorny, sepavloff. 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. Driver options usually use `Joined` inste

[PATCH] D134790: [Driver] Add --config= as canonical spelling of --config

MaskRay updated this revision to Diff 463427. MaskRay added a comment. . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134790/new/ https://reviews.llvm.org/D134790 Files: clang/docs/UsersManual.rst clang/include/clang/Driver/Options.td clang

[PATCH] D134790: [Driver] Add --config= as canonical spelling of --config

MaskRay added a comment. `--config ` is deliberately not a `CoreOption` as we want to discourage the form. `--config ` was not `CoreOption` in previous releases. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134790/new/ https://reviews.llvm.org/D1

[PATCH] D119209: Implement -mctor_dtor_return_this ABI option.

MaskRay added inline comments. Comment at: clang/include/clang/Driver/Options.td:3471 HelpText<"Set the deployment target to be the specified OS and OS version">; +def mctor_dtor_return_this : Flag<["-"], "mctor-dtor-return-this">, Group, Flags<[CC1Option]>, + HelpText<"Chan

[PATCH] D134717: [Clang][AArch64] Fix va_arg with -mgeneral-regs-only

MaskRay added a comment. It seems that the intention of `-mgeneral-regs-only` is to prevent FP and SIMD register use. aarch64-linux-gnu-gcc has such an error error: ‘-mgeneral-regs-only’ is incompatible with the use of floating-point types Clang doesn't reject FP in Sema yet, but I think ch

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

MaskRay added inline comments. Comment at: clang/test/Driver/debug-options-aranges.c:5 +// +// RUN: %clang -### -g -target x86_64-linux -flto -fuse-ld=lld -gdwarf-aranges %s 2>&1 | FileCheck %s +// RUN: %clang -### -g -target x86_64-linux -flto=thin -fuse-ld=lld -gdwarf-ar

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

MaskRay added inline comments. Comment at: clang/test/Driver/debug-options-aranges.c:5 +// +// RUN: %clang -### -g -target x86_64-linux -flto -fuse-ld=lld -gdwarf-aranges %s 2>&1 | FileCheck %s +// RUN: %clang -### -g -target x86_64-linux -flto=thin -fuse-ld=lld -gdwarf-ar

[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs

MaskRay added inline comments. Comment at: clang/include/clang/Driver/Driver.h:758 + /// Return the typical executable name for the specified driver \p Mode. + static const char *getExecutableForDriverMode(DriverMode Mode); }; This should be private

[PATCH] D134790: [Driver] Add --config= as canonical spelling of --config

This revision was automatically updated to reflect the committed changes. Closed by commit rG08af5ba37135: [Driver] Add --config= as canonical spelling of --config (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134790/new/ http

[PATCH] D134638: [Clang][LoongArch] Add inline asm support for constraints k/m/ZB/ZC

MaskRay added a comment. `Constraint_k` somehow broke AArch64 and X86 inlineasm. Please run `check-llvm` for generic changes which may have farreaching effects to other targets. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134638/new/ https://rev

[PATCH] D134880: [clang] [test] Use %clang_cc1 substitution consistently

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134880/new/ https://reviews.llvm.org/D134880 ___ cfe-commits mailing list cfe-commit

[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs

MaskRay added inline comments. Comment at: clang/lib/Driver/Driver.cpp:1065-1080 + StringRef EffectiveDriverMode; + if (CLOptions) +EffectiveDriverMode = CLOptions->getLastArgValue(options::OPT_driver_mode); + if (EffectiveDriverMode.empty()) +EffectiveDriverMode = Cla

[PATCH] D134454: [Driver][Distro] Fix ArchLinux sysroot detection

MaskRay added a comment. Since this decreases complexity, it's probably fine. But we really need some tests in clang/test/Driver see linux-cross.cpp CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134454/new/ https://reviews.llvm.org/D134454 _

[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs

MaskRay added inline comments. Comment at: clang/test/Driver/config-file3.c:176 // -// SHORT-NAME: Configuration file: {{.*}}/testdmode/qqq.cfg -// SHORT-NAME: -Werror -// SHORT-NAME-NOT: -Wundefined-func-template +// FULL3-NOT: Configuration file: +// FULL3: Configuration file:

[PATCH] D134454: [Driver][Distro] Fix ArchLinux sysroot detection

MaskRay requested changes to this revision. MaskRay added a comment. This revision now requires changes to proceed. I'll grab an Arch Linux machine for testing, but I don't think this is currently in a form for submitting. This adds new functionality for non-MIPS and we need some fake file hierar

[PATCH] D134454: [Driver][Distro] Fix ArchLinux sysroot detection

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Linux.cpp:369 + // Path = $GCCToolchainPath/csky-linux-gnuabiv2/libc + if (getTriple().isCSKY()) +Path += "/libc"; If the `else` branch uses braces, the `then` branch needs brances as we

[PATCH] D134454: [Driver][Distro] Fix ArchLinux sysroot detection

MaskRay added a comment. Another opinion is whether we actually need the more and more complex sysroot computation logic. https://reviews.llvm.org/D134337 will soon land and we can move to require distributions to provide a config file instead. Yes, I see mips and csky special cases. To be fai

[PATCH] D121445: [Clang][CSKY] Add the CSKY target and compiler driver

MaskRay added a comment. mips computed sysroot from GCCInstallation very early in 2013 rG08450bd55ccdc4aee4f5f73cde97e25b3c4ce5b9 and Android followed up in 2018 (D45291 ), but I am not sure

[PATCH] D134880: [clang] [test] Use %clang_cc1 substitution consistently

MaskRay added inline comments. Comment at: clang/test/Driver/lit.local.cfg:8 -('%clang_cc1', - """*** Do not use 'clang -cc1' in Driver tests. ***""") ) probinson wrote: > But we _don't_ want to use `%clang_cc1` in Driver tests; if we do, then we're >

[PATCH] D124751: [HLSL] Support -E option for HLSL.

MaskRay added inline comments. Comment at: clang/test/Driver/hlsl-entry.cpp:1 +// RUN:not %clang -cc1 -triple dxil-pc-shadermodel6.3-compute -x c++ -hlsl-entry foo %s 2>&1 | FileCheck %s --check-prefix=NOTHLSL + Note, test/Driver deliberately discourages `%cla

[PATCH] D52290: [driver][mips] Adjust target triple accordingly to provided ABI name

MaskRay added inline comments. Herald added a project: All. Comment at: cfe/trunk/lib/Driver/Driver.cpp:486 + // accordingly to provided ABI name. + A = Args.getLastArg(options::OPT_mabi_EQ); + if (A && Target.isMIPS()) getLastArg claims the option for non-mip

[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs

MaskRay added a comment. In D134337#3824683 , @ojeda wrote: > Thanks Nick for the ping. Indeed, `rustc` allows to use a "target > specification file" to create new targets that are not built-in, as a JSON > file. > > They are unstable though, since they

[PATCH] D134671: [Driver] Prevent Mips specific code from claiming -mabi argument on other targets.

MaskRay added a comment. In D134671#3824672 , @aaron.ballman wrote: > In D134671#3824644 , > @nickdesaulniers wrote: > >> I don't think it's an issue for us to work around downstream, but this did >> regress su

[PATCH] D134880: [clang] [test] Use %clang_cc1 substitution consistently

MaskRay added a subscriber: yaxunl. MaskRay added inline comments. Comment at: clang/test/Driver/lit.local.cfg:8 -('%clang_cc1', - """*** Do not use 'clang -cc1' in Driver tests. ***""") ) mgorny wrote: > MaskRay wrote: > > probinson wrote: > > > But we

[PATCH] D134880: [clang] [test] Use %clang_cc1 substitution consistently

MaskRay added inline comments. Comment at: clang/test/Driver/lit.local.cfg:8 -('%clang_cc1', - """*** Do not use 'clang -cc1' in Driver tests. ***""") ) MaskRay wrote: > mgorny wrote: > > MaskRay wrote: > > > probinson wrote: > > > > But we _don't_ want

[PATCH] D134905: [clang] [Driver] Disable default configs via envvar during testing

MaskRay added a comment. The concept LGTM. Comment at: clang/lib/Driver/Driver.cpp:1073 + if (const char *NoConfigEnv = ::getenv("CLANG_NO_DEFAULT_CONFIG")) { +if (*NoConfigEnv) + return false; Looking at LLDB_LAUNCH_FLAG_DISABLE_ASLR/LIBCLANG_DISABLE

[PATCH] D134820: [LTO][clang] Teaching Clang to Pass Plugin Options to the AIX Linker

MaskRay added inline comments. Comment at: clang/test/Driver/lto-aix.c:2 +// Test LTO path, mcpu and opt level options +// RUN: %clang -target powerpc-ibm-aix -### %s -flto -fuse-ld=ld -O3 2>&1 \ +// RUN: | FileCheck -check-prefixes=LTOPATH,MCPUOPTLEVEL %s Repl

[PATCH] D125142: [clang][auto-init] Remove -enable flag for "zero" mode

MaskRay added a comment. > [clang][auto-init] Remove -enable flag for "zero" mode The subject should be updated to mention the exact option name, not a vague `-enable` and `"zero" mode` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125142/new/ ht

[PATCH] D134905: [clang] [Driver] Disable default configs via envvar during testing

MaskRay added a comment. > during testing This part of the subject is probably not needed. I think exposing the idea to user can probably be useful, too. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134905/new/ https://reviews.llvm.org/D134905 ___

[PATCH] D134905: [clang] [Driver] Disable default configs via envvar during testing

MaskRay requested changes to this revision. MaskRay added a comment. This revision now requires changes to proceed. Wait. The effect can be achieved today with `CCC_OVERRIDE_OPTIONS=+--no-default-config` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134905/new/ https://reviews.llvm.org

[PATCH] D134905: [clang] [Driver] Disable default configs via envvar during testing

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. But `CCC_OVERRIDE_OPTIONS=+--no-default-config` leaves an annoying notice, so I guess introducing a new environment variable is still fine. CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D134454: [Driver][Distro] Fix ArchLinux sysroot detection

MaskRay added a comment. In D134454#3826143 , @10ne1 wrote: > In D134454#3824571 , @MaskRay wrote: > >> I'll grab an Arch Linux machine for testing, but I don't think this is >> currently in a form for submitting

[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs

MaskRay added a comment. In D134337#3827522 , @nickdesaulniers wrote: > In D134337#3824862 , @MaskRay wrote: > >> It's unclear Clang wants to support GCC style specs file and whether GCC >> wants to adopt anothe

[PATCH] D134454: [Driver][Distro] Fix ArchLinux sysroot detection

MaskRay added a comment. > While cross-compiling Linux kernel tools with Clang on ArchLinux, it was > revealed the sysroot is not properly detected and instead of fixing the root > cause in Clang, kernel developers started doing workarounds [1] upon > workarounds [2] in the kernel build to be a

[PATCH] D125142: [clang][auto-init] Deprecate -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang

MaskRay added a comment. Add the option to `Group` instead of adding a custom diagnostic code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125142/new/ https://reviews.llvm.org/D125142 ___ cfe-commits

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

MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: clang/test/Driver/debug-options-aranges.c:1 +// REQUIRES: lld + Remove `// REQUIRES: lld` Comment at: clang/test/Driver/

[PATCH] D135045: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH

MaskRay created this revision. MaskRay added reviewers: clang-language-wg, aaron.ballman, appcs, efriedma, emaste. 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.

[PATCH] D135045: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH

MaskRay updated this revision to Diff 464581. MaskRay edited the summary of this revision. MaskRay added a comment. Herald added a reviewer: ctetreau. https://wiki.debian.org/ReproducibleBuilds/TimestampsFromCPPMacros says > We missed out __TIMESTAMP__ by mistake, but we will probably send in ano

[PATCH] D135076: [Clang] Make offloading flags accept '-' and '--'

MaskRay added a comment. Single-dash long options starting with `-o` conflict with the short option `-o` so I am unsure I like this direction. > This is similar to many other driver arguments beginning with -o. What options? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[PATCH] D135076: [Clang] Make offloading flags accept '-' and '--'

MaskRay added a comment. I think this should be reverted. It's moving in the backward direction. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135076/new/ https://reviews.llvm.org/D135076 ___ cfe-commits

[PATCH] D135076: [Clang] Make offloading flags accept '-' and '--'

MaskRay added a comment. There are traditionally some single-dash long options (perhaps classical Mac OS style) which conflict with short options. I think nowadays we try to avoid such single-dash short options. (For example, GNU ld now requires all new long options to use two-dashes, after kno

[PATCH] D135076: [Clang] Make offloading flags accept '-' and '--'

MaskRay added a comment. In D135076#3830989 , @jhuber6 wrote: > In D135076#3830972 , @MaskRay wrote: > >> There are traditionally some single-dash long options (perhaps classical Mac >> OS style) which conflict w

[PATCH] D135076: [Clang] Make offloading flags accept '-' and '--'

MaskRay added a comment. We really want these `--offload-*` users to stick with one canonical form, not `-offload-*` in some places while `--offload-*` in other places. Another angle is that people find `-offload-*` working with a new clang may try `-offload-*` on an old clang and get `-o ffloa

[PATCH] D135076: [Clang] Make offloading flags accept '-' and '--'

MaskRay added a comment. In D135076#3831307 , @jhuber6 wrote: > In D135076#3831298 , @MaskRay wrote: > >> We really want these `--offload-*` users to stick with one canonical form, >> not `-offload-*` in some pla

[PATCH] D135076: [Clang] Make offloading flags accept '-' and '--'

MaskRay added a comment. In D135076#3831347 , @jhuber6 wrote: > In D135076#3831340 , @MaskRay wrote: > >> In D135076#3831307 , @jhuber6 >> wrote: >> >>> In D135076#383129

[PATCH] D135093: Revert D135076 "[Clang] Make offloading flags accept '-' and '--'"

MaskRay created this revision. MaskRay added reviewers: jhuber6, jdoerfert, JonChesterfield. 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. This reverts commit 11a

[PATCH] D135076: [Clang] Make offloading flags accept '-' and '--'

MaskRay added a comment. In D135076#3831371 , @jhuber6 wrote: > In D135076#3831363 , @MaskRay wrote: > >> My idea is to just disallow `Joined` `-o` when targeting a specific >> environment (e.g. when offloading t

[PATCH] D135076: [Clang] Make offloading flags accept '-' and '--'

MaskRay added a comment. Another idea is to reject multiple `-o` if some are used as the `Joined` form. Note: multiple `Separate` `-o` should be allowed. It will not catch `-offload-*` when `-o output` is not specified, but is probably useful enough. Repository: rG LLVM Github Monorepo CHAN

[PATCH] D119209: Implement -fctor_dtor_return_this ABI option.

MaskRay added a comment. > Implement -fctor_dtor_return_this ABI option. Typo in the subject. I'd use: `Add cc1 option -fctor-dtor-return-this` Comment at: clang/include/clang/Driver/Options.td:5608 +def fctor_dtor_return_this : Flag<["-"], "fctor-dtor-return-this">, + HelpTe

[PATCH] D119209: [clang] Add cc1 option -fctor-dtor-return-this

MaskRay added inline comments. Comment at: clang/include/clang/Basic/CodeGenOptions.def:495 +/// Modify C++ ABI to returning `this` pointer from constructors and +/// non-deleting destructors. (No effect on Microsoft ABI.) Modify `Itanium C++ ABI` and delete `

[PATCH] D119209: [clang] Add cc1 option -fctor-dtor-return-this

MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Driver/Options.td:5608 +def fctor_dtor_return_this : Flag<["-"], "fctor-dtor-return-this">, + HelpText<"Change the C++ ABI to returning

[PATCH] D135045: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH

MaskRay updated this revision to Diff 464815. MaskRay added a comment. Use `%if clang-target-64-bits` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135045/new/ https://reviews.llvm.org/D135045 Files: clang/docs/ReleaseNotes.rst clang/include/c

[PATCH] D135076: [Clang] Make offloading flags accept '-' and '--'

MaskRay added a comment. Sorry, but I'll revert this patch shortly. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135076/new/ https://reviews.llvm.org/D135076 ___ cfe-commits mailing list cfe-commits@lis

[PATCH] D135093: Revert D135076 "[Clang] Make offloading flags accept '-' and '--'"

MaskRay abandoned this revision. MaskRay added a comment. Pushed in f6797056fc4163e6fe5bd57bebd19aa7fd83a778 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135093/new/ https://reviews.llvm.org/D135093 ___

[PATCH] D134478: BareMetal: detect usr/include/c++/v1 path in sysroot

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. LGTM. Comment at: clang/test/Driver/baremetal.cpp:77 +// RUN: mkdir -p %T/baremetal_cxx_sysroot/usr/include/c++/v1 +// RUN: %clangxx -no-canonical-prefixes %s -### -o %t.o

[PATCH] D134454: [Driver][Distro] Fix ArchLinux sysroot detection

MaskRay added a comment. - This patch should be abandoned. Inferring sysroot from gcc-toolchain is backward. - The D644 (`Support Sourcery CodeBench MIPS toolchain`) `computeSysRoot` should not be used by new support. - `--gcc-toolchain=` is discouraged. - New cro

[PATCH] D134820: [LTO][clang] Teaching Clang to Pass Plugin Options to the AIX Linker

MaskRay added a comment. LGTM Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:72 +CmdArgs.push_back(Args.MakeArgString( +Twine(PluginOptPrefix) + Twine("-pass-remarks=") + A->getValue())); Adding `Twine` to one operand of `+` suffices. ==

[PATCH] D135045: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH

MaskRay updated this revision to Diff 465501. MaskRay marked an inline comment as done. MaskRay added a comment. Make error message precise Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135045/new/ https://reviews.llvm.org/D135045 Files: clang/d

[PATCH] D135045: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH

MaskRay marked an inline comment as done. MaskRay added inline comments. Comment at: clang/lib/Lex/PPMacroExpansion.cpp:1092 +TT = *PP.getPreprocessorOpts().SourceDateEpoch; +TM = std::gmtime(&TT); + } else { ychen wrote: > Why not use localtime as the e

[PATCH] D135045: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH

MaskRay marked 2 inline comments as done. MaskRay added inline comments. Comment at: clang/lib/Lex/PPMacroExpansion.cpp:1092 +TT = *PP.getPreprocessorOpts().SourceDateEpoch; +TM = std::gmtime(&TT); + } else { ychen wrote: > MaskRay wrote: > > ychen wrote

[PATCH] D135045: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH

MaskRay updated this revision to Diff 465649. MaskRay marked an inline comment as done. MaskRay edited the summary of this revision. MaskRay added a comment. Use `??? ?? ` or `??:??:??` is gmtime/localtime returns null Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:

[PATCH] D135045: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH

MaskRay marked an inline comment as done. MaskRay added inline comments. Comment at: clang/lib/Lex/PPMacroExpansion.cpp:1092 +TT = *PP.getPreprocessorOpts().SourceDateEpoch; +TM = std::gmtime(&TT); + } else { ychen wrote: > MaskRay wrote: > > ychen wrote

[PATCH] D129446: [clang][driver] Find Apple default SDK path

MaskRay added a comment. (With configuration file improvement, you can express the intention with a default configuration file.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129446/new/ https://reviews.llvm.org/D129446 __

[PATCH] D135097: Remove redundant option '-menable-unsafe-fp-math'.

MaskRay added a comment. Thanks for noticing the issue. There are some formatting issues in the description:) And a typo in the table cell `initeMathOnly` -menable-unsafe-fp-math looks like a CC1 for a long time, probably from 2012. In 2020 D82574 accidentally

[PATCH] D134668: [LTO][clang] Using Single Dash Consistently when Passing LTO Options

MaskRay added a comment. I think I want to revert this change. `--plugin-opt=` is entirely fine with lld and GNU gold/ld... And generally the double-dash long options are preferred. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134668/new/ https:/

[PATCH] D135400: [clang][LTO] Remove the use of `--` for arange option

MaskRay added a comment. I missed https://reviews.llvm.org/D134668 . See my comment there I don't think the change is necessary. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135400/new/ https://reviews.llvm.org/D135400 ___ cfe-commits maili

[PATCH] D135389: [Clang] Emit a warning for ambiguous joined '-o' arguments

MaskRay accepted this revision. MaskRay added inline comments. Comment at: clang/test/Driver/unknown-arg.c:74 + +// RUN: %clang -### --offload-arch=sm_70 -o ffload-device-only -Werror=unknown-argument %s jhuber6 wrote: > tra wrote: > > jhuber6 wrote: > > > tra w

[PATCH] D134668: [LTO][clang] Using Single Dash Consistently when Passing LTO Options

MaskRay added a comment. Err since `-plugin-opt=` was used a lot of before this change, I think this is fine for consistency. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134668/new/ https://reviews.llvm.org/D134668 _

[PATCH] D135400: [clang][LTO] Remove the use of `--` for arange option

MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: clang/test/Driver/debug-options-aranges.c:6 // RUN: %clang -### -g --target=x86_64-linux -flto=thin -gdwarf-aranges %s 2>&1 | FileCheck %s -// CHECK: --pl

[PATCH] D134454: [Driver][Distro] Fix ArchLinux sysroot detection

MaskRay added a comment. In D134454#3844627 , @zixuan-wu wrote: > It's fine for CSKY to use config file. I only have 2 points. Thanks. It'll be nice for CSKY to move away from changing `computeSysRoot`. > 1. I agree sysroot should be separated from GCC

[PATCH] D135045: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH

MaskRay marked an inline comment as done. MaskRay added inline comments. Comment at: clang/lib/Lex/PPMacroExpansion.cpp:1612 + time_t TT = *getPreprocessorOpts().SourceDateEpoch; + std::tm *TM = std::gmtime(&TT); Result = asctime(TM); ychen wrote

[PATCH] D135701: [clang] Do not override libclang.so's SOVERSION if CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION

MaskRay accepted this revision. MaskRay added a comment. I don't understand this well, though. Is `CLANG_VERSION_MAJOR` not defined in some cases? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135701/new/ https://reviews.llvm.org/D135701 ___

[PATCH] D135045: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH

MaskRay marked 2 inline comments as done. MaskRay added inline comments. Comment at: clang/lib/Lex/PPMacroExpansion.cpp:1612 + time_t TT = *getPreprocessorOpts().SourceDateEpoch; + std::tm *TM = std::gmtime(&TT); Result = asctime(TM); ychen wrote

[PATCH] D135045: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH

MaskRay updated this revision to Diff 466891. MaskRay marked an inline comment as done. MaskRay added a comment. Don't test SOURCE_DATE_EPOCH=253402300799 on Windows (which only supports upto 32536850399) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D135045: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH

This revision was automatically updated to reflect the committed changes. Closed by commit rG2c090162746a: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135045/new/ h

[PATCH] D135097: Remove redundant option '-menable-unsafe-fp-math'.

MaskRay added inline comments. Comment at: clang/docs/ReleaseNotes.rst:410-412 +- The driver option ``-menable-unsafe-fp-math`` has been removed. Passing it, will +result in a hard error. To enable unsafe floating-point optimizations, the compiler +options ``-funsafe-math-optim

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

MaskRay updated this revision to Diff 467840. MaskRay marked an inline comment as done. MaskRay edited the summary of this revision. MaskRay added a comment. remove lit feature default_std_cxx I think some other CLANG_DEFAULT_* probably should go away as well, but we can start with STD_C STD_CXX

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3baadff896ed: [CMake] Remove CLANG_DEFAULT_STD_C/CLANG_DEFAULT_STD_CXX (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE L

[PATCH] D135439: Keep configuration file search directories in ExpansionContext. NFC

MaskRay added a comment. I am not familiar with `ExpansionContext`. Is there any behavior difference which can be demonstrated by a test? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135439/new/ https://reviews.llvm.org/D135439 _

[PATCH] D116070: [X86] Enable ibt-seal optimization when LTO is used in Kernel

MaskRay added a comment. I have a question why the option is needed. if (IBTSeal) { return F.hasAddressTaken(); } // if !IBTSeal, fall into default case. LLVM_FALLTHROUGH; // Address taken or externally linked functions may be reachable. default: return (F.hasAddress

<    16   17   18   19   20   21   22   23   24   25   >