[PATCH] D92617: [DWARF] Allow toolchain to adjust specified DWARF version.

MaskRay added inline comments. Comment at: clang/test/Driver/cuda-unsupported-debug-options.cu:22 +// Make sure we do not see any dwarf version other than 2, regardless of what's used on the host side. +// CHECK-NOT: {{-dwarf-version=[^2]}} // CHECK: "-triple" "x86_64 -

[PATCH] D92633: Add -f[no-]direct-access-external-data to deprecate -mpie-copy-relocations

MaskRay created this revision. MaskRay added reviewers: hjl.tools, rnk, tmsriram. Herald added subscribers: dexonsmith, dang, pengfei, s.egerton, simoncook. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. GCC r218397 "x86-64: Optimi

[PATCH] D92633: Add -f[no-]direct-access-external-data to deprecate -mpie-copy-relocations

MaskRay added a comment. In D92633#2433108 , @tmsriram wrote: > You said : "The name -mpie-copy-relocations is misleading [1] and does not > capture the idea that this option can actually apply to all of > -fno-pic,-fpie, ..." > > Could you please clarif

[PATCH] D92633: Add -f[no-]direct-access-external-data to deprecate -mpie-copy-relocations

MaskRay added a comment. In D92633#2433999 , @tmsriram wrote: > In D92633#2433979 , @MaskRay wrote: > >> In D92633#2433108 , @tmsriram wrote: >> >>> You said : "The name -mpi

[PATCH] D92633: Add -f[no-]direct-access-external-data to deprecate -mpie-copy-relocations

MaskRay added a comment. In D92633#2434166 , @tmsriram wrote: > LGTM. I checked it completely and the patch makes total sense to me. Thanks! The name bothers me a bit. There are two parts "direct-access" (direct access relocations like absolute relocati

[PATCH] D92633: Add -f[no-]direct-access-external-data to deprecate -mpie-copy-relocations

MaskRay added a comment. In D92633#2434231 , @tmsriram wrote: > Sorry just one more thing which is a bit concerning: > > When I do : > > $ clang -fPIC -frxternal-data-access foo.c > > You will omit the GOT but this object can go into a shared object and b

[PATCH] D92633: Add -f[no-]direct-access-external-data to deprecate -mpie-copy-relocations

MaskRay added a comment. In D92633#2434337 , @tmsriram wrote: > In D92633#2434267 , @MaskRay wrote: > >> In D92633#2434231 , @tmsriram wrote: >> >>> Sorry just one more thing

[PATCH] D92633: Add -f[no-]direct-access-external-data to deprecate -mpie-copy-relocations

MaskRay added a comment. In D92633#2434714 , @tmsriram wrote: > Correct me if I am wrong, but I do see that this behavior is touched. Line > 10 in -fdirect-access-external-data.c : > > // RUN: %clang -### -c -target aarch64 %s -fpic -fdirect-access-exter

[PATCH] D92078: [asan] Default to -asan-use-private-alias=1

MaskRay updated this revision to Diff 309675. MaskRay marked 6 inline comments as done. MaskRay edited the summary of this revision. MaskRay added a comment. Pre-committed test improvement in 190b4374c00a9cf090ea73f9eddf3d8f71b11ec8

[PATCH] D92078: [asan] Default to -asan-use-private-alias=1

MaskRay added inline comments. Comment at: compiler-rt/test/asan/TestCases/Linux/odr-vtable.cpp:4-5 -// RUN: %clangxx_asan -fno-rtti -DBUILD_SO1 -fPIC -shared %s -o %dynamiclib1 -// RUN: %clangxx_asan -fno-rtti -DBUILD_SO2 -fPIC -shared %s -o %dynamiclib2 +// RUN: %clangxx_asan

[PATCH] D92714: Make -fno-pic respect -fno-direct-access-external-data

MaskRay created this revision. MaskRay added reviewers: hjl.tools, rnk, tmsriram. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. D92633 added -f[no-]direct-access-external-data to supersede -m[no-

[PATCH] D90507: [Driver] Add DWARF64 flag: -gdwarf64

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4031 +else + CmdArgs.push_back("-gdwarf64"); + } Use `render` (see other `render` in the file) Comment at: clang/test/Driver/debug-options.c:379 +// +//

[PATCH] D90507: [Driver] Add DWARF64 flag: -gdwarf64

MaskRay added inline comments. Comment at: clang/test/Driver/debug-options.c:390 +// GDWARF64_OFF: error: invalid argument '-gdwarf64' only allowed with 'DWARFv3 or greater' +// GDWARF64_32ARCH: error: invalid argument '-gdwarf64' only allowed with '64 bit architecture' --

[PATCH] D90507: [Driver] Add DWARF64 flag: -gdwarf64

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4029 + D.Diag(diag::err_drv_argument_only_allowed_with) + << A->getAsString(Args) << "elf output format"; +else "elf output format" -> "ELF platforms" ELF is usual

[PATCH] D80391: [Driver] Don't make -gsplit-dwarf imply -g2

MaskRay added a comment. In D80391#2437926 , @dblaikie wrote: > Looks like GCC has changed the semantics here despite the backwards > compatibility break - as much as I'd rather not break backwards compatibility > it's probably best on balance to maintai

[PATCH] D80391: [Driver] Don't make -gsplit-dwarf imply -g2

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3718 + } else +DwarfFission = DwarfFissionKind::None; dblaikie wrote: > Rather than undoing the DwarfFission uinitialization, instead could the > DwarfFission initialization

[PATCH] D80391: [Driver] Don't make -gsplit-dwarf imply -g2

MaskRay updated this revision to Diff 310073. MaskRay edited the summary of this revision. MaskRay added a comment. Comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80391/new/ https://reviews.llvm.org/D80391 Files: clang/docs/ReleaseNotes.r

[PATCH] D89799: [clang][driver] Rename DriverOption as NoXarchOption (NFC)

MaskRay added a comment. LGTM. I think a number of options can probably drop the flag, but that can be left as a future clean-up for macOS -Xarch (can someone add a reviewer for macOS?) and CUDA -Xarch* folks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D85474: Add -fbinutils-version= to gate ELF features on the specified binutils version

MaskRay added a comment. In D85474#2342365 , @compnerd wrote: > Is there a reason to not use the existing `-mlinker-version=` option and > expanding that beyond just Darwin targets? I feel like having the same > option would be much nicer. `-mlinker-ve

[PATCH] D89799: [clang][driver] Rename DriverOption as NoXarchOption (NFC)

MaskRay added a subscriber: yaxunl. MaskRay added a comment. @yaxunl Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89799/new/ https://reviews.llvm.org/D89799 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D85474: Add -fbinutils-version= to gate ELF features on the specified binutils version

MaskRay added a comment. In D85474#2343356 , @dexonsmith wrote: > In D85474#2343326 , @MaskRay wrote: > >> In D85474#2342365 , @compnerd wrote: >> >>> Is there a reason to no

[PATCH] D53238: [Driver] Add -static= to unify -static-{libgcc,libstdc++}

MaskRay added a comment. In D53238#2345618 , @Yu wrote: > I accidentally come into this thread when searching for how to statically > link to libc++/libc++abi. I'm really curious if this patch will or will not > get merged. I'm really painful on using c+

[PATCH] D85474: Add -fbinutils-version= to gate ELF features on the specified binutils version

MaskRay added a comment. In D85474#2344823 , @compnerd wrote: > In D85474#2343393 , @MaskRay wrote: > >> In D85474#2343356 , @dexonsmith >> wrote: >> >>> In D85474#2343326 <

[PATCH] D88566: be more specific when testing for no fuse-ld warnings

MaskRay added a comment. In D88566#2317248 , @stuij wrote: > Hi @MaskRay. Yes, so we're seeing a warning specific to our Armcompiler > toolchain, so I'm guessing that isn't relevant to OSS LLVM: > `armclang: warning: '--target=x86_64-unknown-linux' is not

[PATCH] D89799: [clang][driver] Rename DriverOption as NoXarchOption (NFC)

MaskRay added a comment. In D89799#2345004 , @awarzynski wrote: > Thank you all for you comments! Please find my replies below. I've picked 4 > main points raised here. > > 1 > - > > In D89799#2342677 , @rnk wrote:

[PATCH] D75579: Replace MCTargetOptionsCommandFlags.inc and CommandFlags.inc by runtime-registration

MaskRay added inline comments. Comment at: llvm/tools/llvm-mc/CMakeLists.txt:6 AllTargetsInfos + CodeGen MC craig.topper wrote: > Why did this patch need to make llvm-mc dependent on CodeGen? It doesn't. Deleted in 7011a2f3504b43e57eda5dbc1f23a3f9c1b44593

[PATCH] D89799: [clang][driver] Rename DriverOption as NoXarchOption (NFC)

MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Driver/ToolChain.cpp:1203 +unsigned DiagID = +Diags.getCustomDiagID(DiagnosticsEngine::Error, + "invalid

[PATCH] D84623: Remove HAVE_VCS_VERSION_INC, not needed

MaskRay accepted this revision. MaskRay added a comment. Herald added a subscriber: dexonsmith. Thanks! Assuming you don't have a commit bit, I'll commit on your behalf. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84623/new/ https://reviews.llvm.

[PATCH] D84623: Remove HAVE_VCS_VERSION_INC, not needed

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9bb9b737c557: Remove HAVE_VCS_VERSION_INC, not needed (authored by hlopko, committed by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D89087: [MemProf] Pass down memory profile name with optional path from clang

MaskRay added inline comments. Comment at: clang/include/clang/Driver/Options.td:1021 +def fmemory_profile_EQ : Joined<["-"], "fmemory-profile=">, +Group, Flags<[CC1Option, DriverOption]>, MetaVarName<"">, +HelpText<"Enable heap memory profiling and dump results into ">;

[PATCH] D78899: [Driver] Add callback to Command execution

MaskRay added inline comments. Comment at: clang/include/clang/Driver/Compilation.h:118 + /// Callback called after the command has been executed. + std::function PostCallback; Can you comment what `int` means? Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D102090: [CMake][ELF] Add -fno-semantic-interposition and -Bsymbolic-functions

This revision was automatically updated to reflect the committed changes. Closed by commit rG3bf1acab5b45: [CMake][ELF] Add -fno-semantic-interposition and -Bsymbolic-functions (authored by MaskRay). Changed prior to commit: https://reviews.llvm.org/D102090?vs=343758&id=344866#toc Repository:

[PATCH] D101921: [MC] Make it possible for targets to define their own MCObjectFileInfo

MaskRay added a comment. Can `createMCObjectFileInfo` return `MCObjectFileInfo` instead of `std::unique_ptr`? Comment at: clang/lib/Parse/ParseStmtAsm.cpp:590 + if (!MAI || !MII || !MOFI || !STI) { Diag(AsmLoc, diag::err_msasm_unable_to_create_target)

[PATCH] D102090: [CMake][ELF] Add -fno-semantic-interposition and -Bsymbolic-functions

MaskRay added a subscriber: marco-c. MaskRay added a comment. In D102090#2756605 , @Mordante wrote: > I tried to build the libc++ benchmarks locally and the benchmark with target > `to_chars_libcxx` fails to properly execute. Bi-section let to this commi

[PATCH] D102090: [CMake][ELF] Add -fno-semantic-interposition and -Bsymbolic-functions

MaskRay reopened this revision. MaskRay added a comment. This revision is now accepted and ready to land. I'll go simple - just drop -fno-semantic-interposition from this patch, as I cannot figure out how to make -fno-semantic-interposition specific to llvm/ and clang/. -Bsymbolic-functions clai

[PATCH] D102090: [CMake][ELF] Link libLLVM.so and libclang-cpp.so with -Bsymbolic-functions

MaskRay updated this revision to Diff 345264. MaskRay retitled this revision from "[CMake][ELF] Add -fno-semantic-interposition and -Bsymbolic-functions" to "[CMake][ELF] Link libLLVM.so and libclang-cpp.so with -Bsymbolic-functions". MaskRay edited the summary of this revision. MaskRay added a c

[PATCH] D102090: [CMake][ELF] Link libLLVM.so and libclang-cpp.so with -Bsymbolic-functions

MaskRay updated this revision to Diff 345268. MaskRay edited the summary of this revision. MaskRay added a comment. Fix comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102090/new/ https://reviews.llvm.org/D102090 Files: clang/tools/clang-s

[PATCH] D102090: [CMake][ELF] Link libLLVM.so and libclang-cpp.so with -Bsymbolic-functions

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG4f05f4c8e66b: [CMake][ELF] Link libLLVM.so and libclang-cpp.so with -Bsymbolic-functions (authored by MaskRay). Changed prior to commit: https://r

[PATCH] D102090: [CMake][ELF] Link libLLVM.so and libclang-cpp.so with -Bsymbolic-functions

MaskRay added a comment. In D102090#2758316 , @xbolva00 wrote: >>> I cannot figure out how to make -fno-semantic-interposition specific to >>> llvm/ and clang/. > > Maybe @nikic knows? I think he is interested in this work, since the base > (reverted) c

[PATCH] D100509: Support GCC's -fstack-usage flag

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. LG with some nits. After fixes, please wait one day or so in case there are further comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5500 + SmallString<128

[PATCH] D102568: [Driver] Delete -mimplicit-it=

MaskRay created this revision. MaskRay added reviewers: nickdesaulniers, ostannard, raj.khem. Herald added subscribers: dang, kristof.beyls. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This is a GNU as and Clang cc1as option, no

[PATCH] D91628: [SystemZ][NFC] Group SystemZ tests in SystemZ folder

MaskRay added a comment. .ll -> .s tests should be placed in llvm/test/CodeGen/SystemZ, not in clang Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91628/new/ https://reviews.llvm.org/D91628 ___ cfe-commi

[PATCH] D102583: -fno-semantic-interposition: Don't set dso_local on GlobalVariable

MaskRay created this revision. MaskRay added reviewers: foutrelis, rnk, serge-sans-paille, xbolva00. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. `clang -fpic -fno-semantic-interposition` may set dso_local on variables for -fpic

[PATCH] D102568: [Driver] Delete -mimplicit-it=

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2919222d8017: [Driver] Delete -mimplicit-it= (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D102568: [Driver] Delete -mimplicit-it=

MaskRay added a comment. In D102568#2769237 , @mstorsjo wrote: > In D102568#2769053 , > @nickdesaulniers wrote: > >>> But this change did break my build in these places: >>> https://code.videolan.org/videolan/x26

[PATCH] D102568: [Driver] Delete -mimplicit-it=

MaskRay added a comment. I think libvpx's ASFLAGS usage is about GNU as, not the driver option. In D102568#2769296 , @dblaikie wrote: >> I think "waiting for a few releases" is too much and doesn't improve things >> (they will notice issues until you re

[PATCH] D102568: [Driver] Delete -mimplicit-it=

MaskRay added a comment. In D102568#2769340 , @mstorsjo wrote: > In the meantime, wouldn't it be possible to detect the presence of the other > one and check if they match or not, to avoid passing duplicate options to the > backend? I can give that a tr

[PATCH] D102568: [Driver] Delete -mimplicit-it=

MaskRay added a comment. In D102568#2769389 , @dblaikie wrote: > In D102568#2769341 , @MaskRay wrote: > >> I think libvpx's ASFLAGS usage is about GNU as, not the driver option. >> >> In D102568#2769296

[PATCH] D102568: [Driver] Delete -mimplicit-it=

MaskRay added a comment. In D102568#2769401 , @dblaikie wrote: > Ah, thanks for the history. Yeah, still not sure those bugs are on equal > footing - if the patch had been in for a few months before musl was fixed (If > they're interested in building fr

[PATCH] D102469: [sanitizer] Reduce redzone size for small size global objects

MaskRay accepted this revision. MaskRay added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:2556 + uint64_t RZ = 0; + if (SizeInBytes < MinRZ / 2) { +// Reduce redzone size for small size objects, e.g. int, char[1]. MinRZ is -

[PATCH] D102583: -fno-semantic-interposition: Don't set dso_local on GlobalVariable

This revision was automatically updated to reflect the committed changes. Closed by commit rG37561ba89b7d: -fno-semantic-interposition: Don't set dso_local on GlobalVariable (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102583/

[PATCH] D102469: [sanitizer] Reduce redzone size for small size global objects

MaskRay updated this revision to Diff 346614. MaskRay added a comment. Herald added a subscriber: steven_wu. fix test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102469/new/ https://reviews.llvm.org/D102469 Files: clang/test/CodeGen/asan-globa

[PATCH] D102469: [sanitizer] Reduce redzone size for small size global objects

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGdbc641deb988: [sanitizer] Reduce redzone size for small size global objects (authored by condy, committed by MaskRay). Repository: rG LLVM Github

[PATCH] D102812: [clang] Don't pass multiple backend options if mixing -mimplicit-it and -Wa,-mimplicit-it

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2369 -static bool AddARMImplicitITArgs(const ArgList &Args, ArgStringList &CmdArgs, +static bool CheckARMImplicitITArg(StringRef Value) { + return Value == "always" || Value == "never" || Value ==

[PATCH] D102812: [clang] Don't pass multiple backend options if mixing -mimplicit-it and -Wa,-mimplicit-it

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102812/new/ https://reviews.llvm.org/D102812 __

[PATCH] D101921: [MC] Make it possible for targets to define their own MCObjectFileInfo

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. LGTM. This looks like an improvement because it avoids a temporary `MCObjectFileInfo MOFI;` (which appeared to be initialized in two subsequent calls) in numerous places. Repository: rG

[PATCH] D101921: [MC] Make it possible for targets to define their own MCObjectFileInfo

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc2f819af73c5: [MC] Refactor MCObjectFileInfo initialization and allow targets to create… (authored by flip1995, committed by MaskRay). Repository:

[PATCH] D101921: [MC] Make it possible for targets to define their own MCObjectFileInfo

MaskRay added inline comments. Comment at: llvm/include/llvm/Support/TargetRegistry.h:26 #include "llvm/ADT/iterator_range.h" +#include "llvm/MC/MCObjectFileInfo.h" #include "llvm/Support/CodeGen.h" `include/llvm/Support/TargetRegistry.h now has cyclic dependen

[PATCH] D102873: [clang] [MinGW] Fix gcc version detection/picking

MaskRay accepted this revision. MaskRay added a comment. LGTM. Comment at: clang/test/Driver/mingw-sysroot.cpp:21 // RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" %clang -target x86_64-w64-mingw32 -rtlib=platform -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck -check

[PATCH] D101921: [MC] Make it possible for targets to define their own MCObjectFileInfo

MaskRay added subscribers: compnerd, dblaikie. MaskRay added a comment. Because of `new MCObjectFileInfo`, we cannot use a forward declaration (incomplete class) to replace `#include "llvm/MC/MCObjectFileInfo.h"` in `TargetRegistry.h`. I thought about moving `TargetRegistry.{h,cpp}` from Suppor

[PATCH] D80421: [Mips] use correct ld.so for musl soft float

MaskRay added a comment. Such driver tests are in `clang/test/Driver/`. You may want to read a few existing mips driver tests and musl tests to figure out whether reusing an existing file or creating a new file makes more sense. You can run a test with `$build/bin/llvm-lit -vv test.c`. `check-c

[PATCH] D85474: Add -fbinutils-version= to gate ELF features on the specified binutils version

MaskRay added a comment. Herald added a reviewer: jansvoboda11. Ping:) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85474/new/ https://reviews.llvm.org/D85474 ___ cfe-commits mailing list cfe-commits@li

[PATCH] D93579: [clang][AVR] Improve avr-ld command line options

MaskRay requested changes to this revision. MaskRay added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Driver/ToolChains/AVR.cpp:37 } MCUInfo[] = { {"at90s1200", "", "avr1"}, {"attiny11", "", "avr1"}, This may c

[PATCH] D93579: [clang][AVR] Improve avr-ld command line options

MaskRay added a comment. If you remove your system GCC avr, does the test still work? This is the criteria whether the patch is acceptable. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93579/new/ https://reviews.llvm.org/D93579 ___ cfe-comm

[PATCH] D93579: [clang][AVR] Improve avr-ld command line options

MaskRay added inline comments. Comment at: clang/test/Driver/avr-ld.c:46 + +int main() { return 0; } You can entirely delete the code. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93579/new/ https://reviews.llvm.org/D93579 ___

[PATCH] D93579: [clang][AVR] Improve avr-ld command line options

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/D93579/new/ https://reviews.llvm.org/D93579 ___ cfe-commits mailing list cfe-commits@

[PATCH] D94391: CGDebugInfo: Drop Loc.isInvalid() special case from getLineNumber

MaskRay added a comment. In D94391#2507144 , @MaskRay wrote: > In D94391#2495056 , @dblaikie wrote: > >> I'll leave this one to @aprantl > > @aprantl :) :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[PATCH] D90646: [clang] Add warning when `-include-pch` is passed multiple times

MaskRay added a comment. There are many options whether the latter one overrides the previous ones. How is this special? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90646/new/ https://reviews.llvm.org/D90646

[PATCH] D85474: Add -fbinutils-version= to gate ELF features on the specified binutils version

MaskRay updated this revision to Diff 319174. MaskRay marked 8 inline comments as done. MaskRay edited the summary of this revision. MaskRay added a comment. Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85474/new/ https://reviews.

[PATCH] D85474: Add -fbinutils-version= to gate ELF features on the specified binutils version

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4642 +int Num; +if (V == "future") + A->render(Args, CmdArgs); rsmith wrote: > dblaikie wrote: > > ro wrote: > > > MaskRay wrote: > > > > phosek wrote: > > > > > Another

[PATCH] D95417: [NFC] Disallow unused prefixes under clang/test/CodeGen

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Thanks! Comment at: clang/test/CodeGen/catch-alignment-assumption-attribute-align_value-on-lvalue.cpp:2 +// RUN: %clang_cc1 -emit-llvm %s -o - -triple x86_64-linux-gnu | F

[PATCH] D94806: [OpenMP] Add support for mapping names in mapper API

MaskRay added a comment. In your commit the message has just `Reviewers:`. The `Reviewers:` list does not necessarily mean all the people on the list have acknowledged the patch so `Reviewers:` is mostly useless. Many people agree that both `Reviewed:` & `Differential Revision:` should be prese

[PATCH] D95075: [hip] Fix `` compilation on Windows with VS2019.

MaskRay added a comment. In your commit the message does not include `Reviewed by:`. Many people agree that both `Reviewed by:` & `Differential Revision:` should be present. The `Reviewed by:` list indicates people who acknowledged the patch. (The `Reviewers:` list does not necessarily mean all

[PATCH] D95001: [CodeView] Emit function types in -gline-tables-only.

MaskRay added a comment. In your commit the message does not include `Reviewed by:`. Many people agree that both `Reviewed by:` & `Differential Revision:` should be present. The `Reviewed by:` list indicates people who acknowledged the patch. (The `Reviewers:` list does not necessarily mean all

[PATCH] D85474: Add -fbinutils-version= to gate ELF features on the specified binutils version

MaskRay updated this revision to Diff 319337. 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/D85474/new/ https://reviews.llvm.org/D85474 Files: clang/docs/ReleaseN

[PATCH] D94391: CGDebugInfo: Drop Loc.isInvalid() special case from getLineNumber

This revision was automatically updated to reflect the committed changes. Closed by commit rG31d375f178c2: CGDebugInfo: Drop Loc.isInvalid() special case from getLineNumber (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94391/ne

[PATCH] D94735: CGDebugInfo CreatedLimitedType: Drop file/line for RecordType with invalid location

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG189f311130da: CGDebugInfo CreatedLimitedType: Drop file/line for RecordType with invalid… (authored by MaskRay). Repository: rG LLVM Github Monore

[PATCH] D85474: Add -fbinutils-version= to gate ELF features on the specified binutils version

MaskRay added a comment. In D85474#2523439 , @rsmith wrote: > Clang side looks good to me. > > On the LLVM side, is it intended that invalid `-binutils-version` values are > silently accepted? Yes. In `llvm/tools/llc/llc.cpp`, there is no validity check.

[PATCH] D85474: Add -fbinutils-version= to gate ELF features on the specified binutils version

MaskRay updated this revision to Diff 319378. MaskRay added a comment. Add llc validity check and tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85474/new/ https://reviews.llvm.org/D85474 Files: clang/docs/ReleaseNotes.rst clang/include/c

[PATCH] D85474: Add -fbinutils-version= to gate ELF features on the specified binutils version

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG34b60d8a5684: Add -fbinutils-version= to gate ELF features on the specified binutils version (authored by MaskRay). Repository: rG LLVM Github Mon

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

MaskRay added inline comments. Comment at: clang/docs/UsersManual.rst:2271 +.. option:: -fprofile-list= + This can be added below `-fprofile-exclude-files=` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94820/new

[PATCH] D95119: Prefer /usr/bin/env xxx over /usr/bin/xxx where xxx = perl, python, awk

MaskRay added a comment. `#!/usr/bin/env perl -w` (2 arguments) unfortunately does not work on Linux and many other systems. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95119/new/ https://reviews.llvm.org/D95119

[PATCH] D95119: Prefer /usr/bin/env xxx over /usr/bin/xxx where xxx = perl, python, awk

MaskRay added a comment. Several `llvm/utils/` scripts can probably just be removed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95119/new/ https://reviews.llvm.org/D95119 ___ cfe-commits mailing list

[PATCH] D76802: [InstrProfiling] Use !associated metadata for counters, data and values

MaskRay added a comment. Looks quite good. In D76802#2526382 , @phosek wrote: > @MaskRay does this look good to you? Looks like GNU ld has an infinite loop problem w.r.t self-link SHF_LINK_ORDER: https://sourceware.org/bugzilla/show_bug.cgi?id=27259 >

[PATCH] D76802: [InstrProfiling] Use !associated metadata for counters, data and values

MaskRay added a comment. Please also comment (and update the description) that self-link may be used. This is subtle. The reason is that the ELF section names (e.g. `__llvm_prf_cnts`) are C identifiers and considered GC roots in the absence of the `SHF_LINK_ORDER` flag. Repository: rG LLVM

[PATCH] D91927: [X86] Add x86_amx type for intel AMX.

MaskRay added inline comments. Comment at: llvm/include/llvm-c/Core.h:163 LLVMX86_MMXTypeKind, /**< X86 MMX */ + LLVMX86_AMXTypeKind, /**< X86 AMX */ LLVMTokenTypeKind, /**< Tokens */ cuviper wrote: > This is a breaking change to the C ABI -- can w

[PATCH] D94627: [PowerPC][PC Rel] Implement option to omit P10 instructions from stubs

MaskRay added a comment. > Implement option to omit P10 instructions from > stubs Please mention the option name directly. Please avoid P10 - the abbreviation is confusing. Comment at: lld/ELF/Config.h:74 +// For -

[PATCH] D95660: [NFC] Disallow unused prefixes under clang/test/Driver

MaskRay added inline comments. Comment at: clang/test/Driver/rocm-device-libs.cl:82 // RUN: %s \ -// RUN: 2>&1 | FileCheck --check-prefixes=COMMON,COMMON-UNSAFE,GFX803,WAVE64 %s +// RUN: 2>&1 | FileCheck --check-prefixes=COMMON,GFX803,WAVE64 %s Since you

[PATCH] D95635: [CMake] Actually require python 3.6 or greater

MaskRay added a comment. https://lists.llvm.org/pipermail/llvm-dev/2021-January/148229.html "Python version requirement" for the version discussion. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95635/new/ https://reviews.llvm.org/D95635

[PATCH] D95660: [NFC] Disallow unused prefixes under clang/test/Driver

MaskRay added inline comments. Comment at: clang/test/Driver/rocm-device-libs.cl:82 // RUN: %s \ -// RUN: 2>&1 | FileCheck --check-prefixes=COMMON,COMMON-UNSAFE,GFX803,WAVE64 %s +// RUN: 2>&1 | FileCheck --check-prefixes=COMMON,GFX803,WAVE64 %s mtrofin wr

[PATCH] D93023: Replace deprecated %T in 2 tests.

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

[PATCH] D76802: [InstrProfiling] Use !associated metadata for counters, data and values

MaskRay added a comment. > The !associated metadata may be attached to a global object declaration with > a single argument that references another global object. This metadata > prevents discarding of the global object in linker GC unless the referenced > object is also discarded. "prevents d

[PATCH] D76802: [InstrProfiling] Use !associated metadata for counters, data and values

MaskRay accepted this revision. MaskRay added a comment. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76802/new/ https://reviews.llvm.org/D76802 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/m

[PATCH] D76802: [InstrProfiling] Use !associated metadata for counters, data and values

MaskRay added a subscriber: akuegel. MaskRay added inline comments. Comment at: compiler-rt/test/profile/instrprof-gc-sections.c:30 +// RUN: llvm-nm -jgU %t | grep -vE "main|_start|_IO_stdin_used|__libc_.*" > %t.code.syms +// RUN: diff %t.nocode.syms %t.code.syms + -

[PATCH] D95849: [FileCheck] Default --allow-unused-prefixes to false

MaskRay created this revision. MaskRay added reviewers: jdoerfert, jhenderson, mtrofin. Herald added subscribers: okura, kuter, thopre. MaskRay requested review of this revision. Herald added a reviewer: sstefan1. Herald added a reviewer: baziotis. Herald added subscribers: llvm-commits, cfe-commit

[PATCH] D95849: [FileCheck] Default --allow-unused-prefixes to false

MaskRay added a comment. In D95849#2536559 , @nikic wrote: > Just to be clear here: Only the small handful where a spelling mistake was > fixed were actually bugs. All other unused check prefixes were there for > convenience, and are now casualties of th

[PATCH] D95849: [FileCheck] Default --allow-unused-prefixes to false

MaskRay updated this revision to Diff 321524. MaskRay edited the summary of this revision. MaskRay added a comment. Rebase to re-cycle bots Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95849/new/ https://reviews.llvm.org/D95849 Files: clang/tes

[PATCH] D95849: [FileCheck] Default --allow-unused-prefixes to false

MaskRay added subscribers: pengfei, RKSimon. MaskRay added a comment. @nikic D95849 has already changed `llvm/test/` to default to `--allow-unused-prefixes`. I updated a few directories to be `--allow-unused-prefixes` clean (this uncovered improvement to many

[PATCH] D96070: [clang] [driver] Enable static linking to libc++

MaskRay requested changes to this revision. MaskRay added a comment. This revision now requires changes to proceed. It is difficult for clang driver to make the decision because libc++ has multiple C++ ABI implementations as @mstorsjo said. Persoanlly I use: `clang++ -stdlib=libc++ -static-libs

[PATCH] D96070: [clang] [driver] Enable static linking to libc++

MaskRay added a comment. The problem is that we have `-lc++ (optional -lc++abi/-lcxxrt/others) (optional -lpthread))` dependency. While we can control `-lpthread` with `-pthread` (which does extra things on its own), we cannot control `(optional -lc++abi/-lcxxrt/others)`, which is depended by t

<    11   12   13   14   15   16   17   18   19   20   >