[PATCH] D113372: [Driver] Add CLANG_DEFAULT_PIE to emulate GCC --enable-default-pie

MaskRay updated this revision to Diff 389305. MaskRay added a comment. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Add a release note Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113372/new/ https://reviews.llvm.org/D113

[PATCH] D112913: Misleading bidirectional detection

MaskRay added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.cpp:49 + if (C == '\n' || C == '\r' || C == '\f' || C == '\v' || + C == 0x85 /*next line*/) +EmbeddingOverride = Isolate = 0; `/*next line=*

[PATCH] D114394: Compile-time computation of string attribute hashes

MaskRay added inline comments. Comment at: llvm/lib/IR/AttributeImpl.h:214 - DenseMap StringAttrs; + std::unordered_map StringAttrs; std::unordered_map is inefficient. Why is this change? Comment at: llvm/lib/IR/Attributes.cpp:862 + auto

[PATCH] D114497: [Driver] Stop adding stdlib paths in -ffreestanding

MaskRay added a comment. GCC -ffreestanding doesn't change libstdc++ search paths. Clang's behavior matches GCC. To drop /usr/include/c++/v1, you need -nostdinc++. If you want to drop /usr/include etc, you need to specify --sysroot= Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D113250: [clang][driver] Add -fplugin-arg- to pass arguments to plugins

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

[PATCH] D105869: [Driver] fix PowerPC SPE musl dynamic linker name

MaskRay requested changes to this revision. MaskRay added a comment. This revision now requires changes to proceed. This needs a test like `clang/test/Driver/linux-ld.c` `CHECK-MUSL-PPC`. Use `ninja check-clang-driver` to test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D105869: [Driver] fix PowerPC SPE musl dynamic linker name

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. I have confirmed that this matches musl `#define FP_SUFFIX "-sf"` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105869/new/ https://reviews.ll

[PATCH] D105869: [Driver] Support PowerPC SPE musl dynamic linker name ld-musl-powerpc-sf.so.1

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb3163c1cdde0: [Driver] Support PowerPC SPE musl dynamic linker name ld-musl-powerpc-sf.so.1 (authored by pattop, committed by MaskRay). Repository:

[PATCH] D114842: [lld-macho] Remove old macho darwin lld

MaskRay accepted this revision. MaskRay added a comment. Consider moving the clang driver change to a separate change so that the removal is a more pure file deletion (other than some build system changes) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D113372: [Driver] Add CLANG_DEFAULT_PIE to emulate GCC --enable-default-pie

MaskRay added a subscriber: felixonmars. MaskRay added a comment. I will wait until Dec 14 before committing. Hope Arch Linux folks can test this, too: @felixonmars @foutrelis Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113372/new/ https://revie

[PATCH] D114974: [clang][Darwin] Remove old lld implementation handling

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

[PATCH] D115045: [Clang] Ignore CLANG_DEFAULT_LINKER for custom-linker toolchains

MaskRay accepted this revision. MaskRay added a comment. LGTM. Comment at: clang/lib/Driver/ToolChain.cpp:545 +const char *ToolChain::getDefaultLinker() const { + if (StringRef(CLANG_DEFAULT_LINKER).empty()) +return "ld"; `CLANG_DEFAUALT_LINKER[0] == '\0'`

[PATCH] D114497: [PowerPC] Drop stdlib paths in freestanding tests

MaskRay added a comment. LGTM (in a trip, slow to reply) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114497/new/ https://reviews.llvm.org/D114497 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

MaskRay added a comment. To add on rnk's comment (deduplication of vague linkage data which may be included in multiple shared objects), another big reason is -Bsymbolic data does not work with copy relocations. -fno-pic programs generally cannot avoid copy relocations (except mips; mips did th

[PATCH] D135440: [SourceManager] Speedup getFileIDLocal with a separate Offset Table.

MaskRay added a comment. > Speedup: Linux kernel: getFileIDLocal overhead is reduced to 1.66% (~30% > saving) What's the fraction related to the whole parsing time? > The sad bit is thatit increases SourceManager memory usage, however it is a > small fraction (< 10%) of clang AST memory usage,

[PATCH] D136041: [clang][DebugInfo] Emit DISubprogram for extern functions with reserved names

MaskRay added a comment. Nice. This also fixes DW_AT_call_all_calls for functions which call builtins/reserved name functions. Comment at: llvm/test/Verifier/callsite-dbgloc.ll:49 +; CHECK-NOT: inlinable function call in a function with debug info must have a !dbg location +;

[PATCH] D136041: [clang][DebugInfo] Emit DISubprogram for extern functions with reserved names

MaskRay added inline comments. Comment at: llvm/lib/IR/Verifier.cpp:3473 !Call.getCalledFunction()->isInterposable() && + !Call.getCalledFunction()->isDeclaration() && Call.getCalledFunction()->getSubprogram()) Removing this check does not brea

[PATCH] D136309: [clang][Toolchains][Gnu] pass -g through to assembler

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. LGTM. Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:980 + if (Arg *A = Args.getLastArg(options::OPT_g_Flag)) +A->render(Args, CmdArgs); + Use `Args.

[PATCH] D136309: [clang][Toolchains][Gnu] pass -g through to assembler

MaskRay added a comment. Many -g options unfortunately enable/disable debug info emission: -g, -g[0123], -ggdb[0123], etc. The full rules are very complex. I think it makes sense to support a subset which is mostly likely used, i.e. -g, -g[0123]. So you may check `OPT_g_Group` and possibly reus

[PATCH] D136309: [clang][Toolchains][Gnu] pass -g through to assembler

MaskRay added inline comments. Comment at: clang/test/Driver/as-options.s:120 + +// Test that -g is passed through to GAS. +// RUN: %clang -fno-integrated-as -g %s -### 2>&1 | \ nickdesaulniers wrote: > MaskRay wrote: > > -g can be tested along with other pass-th

[PATCH] D136309: [clang][Toolchains][Gnu] pass -g through to assembler

MaskRay added a comment. (Note that assembler synthesized debug info for assembly files is a very minor feature. I don't know why the Linux kernel is so fond of it..). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136309/new/ https://reviews.llvm.

[PATCH] D135937: [WIP][X86] Support -march=raptorlake, meteorlake

MaskRay added inline comments. Comment at: clang/test/Driver/x86-march.c:120 +// RUN: | FileCheck %s -check-prefix=meteorlake +// meteorlake: "-target-cpu" "meteorlake" // RKSimon wrote: > Move these after alderlake instead of the old atom cores? And use `--ta

[PATCH] D136474: [CodeView][clang] Add flag to disable emitting command line into CodeView

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4354 + +// Emit codeview command line if requested. +if (Args.hasFlag(options::OPT_gcodeview_command_line, This needs a test in clang/test/Driver/ Use `addOptInFlag` Repos

[PATCH] D136435: [clang][Driver] Add gcc-toolset-12 and devtoolset-12 prefixes

MaskRay added a comment. > If clang16 stops doing the gcc/devtoolset detection and relies on a config > file instead, that's fine but for the time being (and for a backport to > clang15), we need to fix this problem. I think this is fine. The test removal is fine if the future direction is to

[PATCH] D135284: [Driver] select alternative target containing . in executable name

MaskRay added a comment. > The gcc compatible driver has support for selecting an alternative target > based on the driver's executable name, for instance > x86_64-unknown-linux-gnu-clang will set the target to linux on x86_64. I think the somewhat canonical term is target override (not selecti

[PATCH] D136436: [Clang][LoongArch] Add register alias handling without `$` prefix

MaskRay added a comment. In D136436#3876684 , @SixWeining wrote: > In D136436#3873987 , @xen0n wrote: > >> In D136436#3873949 , @SixWeining >> wrote: >> >>> How about th

[PATCH] D136660: [clang] Replace BACKEND_PACKAGE_STRING with LLVM_VERSION_STRING

MaskRay created this revision. MaskRay added reviewers: mgorny, thieta, tstellar. 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. 420d7ccbac0f499a

[PATCH] D136660: [clang] Replace BACKEND_PACKAGE_STRING with LLVM_VERSION_STRING

MaskRay added a comment. In D136660#3881602 , @mgorny wrote: > I suppose you want to remove it from `clang/CMakeLists.txt` as well. I see > Flang has copied the same logic too, so might be worthwhile to remove it > there as well. `set(BACKEND_PACKAGE_S

[PATCH] D136660: [clang] Replace BACKEND_PACKAGE_STRING with LLVM_VERSION_STRING

MaskRay updated this revision to Diff 470383. MaskRay edited the summary of this revision. MaskRay added a comment. Remove cmake variable BACKEND_PACKAGE_STRING Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136660/new/ https://reviews.llvm.org/D136

[PATCH] D136660: [clang] Replace BACKEND_PACKAGE_STRING with LLVM_VERSION_STRING

MaskRay added a comment. Since the patch subject is called `[clang] ` I am deferring the flang cmake change to a separate patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136660/new/ https://reviews.llvm.org/D136660 __

[PATCH] D136660: [clang] Replace BACKEND_PACKAGE_STRING with LLVM_VERSION_STRING

MaskRay added a comment. In D136660#3881647 , @mgorny wrote: > In D136660#3881617 , @MaskRay wrote: > >> In D136660#3881602 , @mgorny wrote: >> >>> I suppose you want to r

[PATCH] D136660: [clang] Replace BACKEND_PACKAGE_STRING with LLVM_VERSION_STRING

This revision was automatically updated to reflect the committed changes. Closed by commit rG3a3603ff99ff: [clang] Replace BACKEND_PACKAGE_STRING with LLVM_VERSION_STRING (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136660/new

[PATCH] D136615: [Instrumentation] Remove legacy passes

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. dfsan removal can use the dedicated D136615 . Other legacy instrumentation passes can use this patch:) Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D124594: [LegacyPM] Remove DataFlowSanitizerLegacyPass

MaskRay added a subscriber: aeubanks. MaskRay added a comment. Herald added a subscriber: Enna1. In D124594#3479202 , @MaskRay wrote: > In D124594#3479195 , @browneee > wrote: > >> I know this is currently used i

[PATCH] D124594: [LegacyPM] Remove DataFlowSanitizerLegacyPass

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa527bda520f0: [LegacyPM] Remove DataFlowSanitizerLegacyPass (autho

[PATCH] D136615: [Instrumentation] Remove legacy passes

MaskRay added a comment. In D136615#3883220 , @aeubanks wrote: > did you mean to point to a different patch? My fault. Updated my previous comment. Pushed it:) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D133998: [HIP][test] Avoid %T

MaskRay added a comment. In D133998#3883517 , @probinson wrote: > @MaskRay I see that you restored the `clang-driver` keyword in > hip-link-bc-to-bc.hip, and of course that keyword is not defined anywhere so > the test is never being run. > Is there an

[PATCH] D136707: [clang][Toolchains][Gnu] pass -gdwarf-* through to assembler

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:7966 } + unsigned DwarfVersion = GetDwarfVersion(getToolChain(), Args); RenderDebugEnablingArgs(Args, CmdArgs, DebugInfoKind, DwarfVersion, Consider `const` if this variable i

[PATCH] D135583: [LLVM] Use DWARFv4 bitfields when tuning for GDB

MaskRay added inline comments. Comment at: clang/docs/ReleaseNotes.rst:619 +Previously when emitting DWARFv4 and tuning for GDB, Clang would use DWARFv2's +``DW_AT_bit_offset`` and ``DW_AT_data_member_location``. Clang now uses DWARFv4's "DWARF v4" is more com

[PATCH] D136707: [clang][Toolchains][Gnu] pass -gdwarf-* through to assembler

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:979 + unsigned DwarfVersion = GetDwarfVersion(getToolChain(), Args); + std::string DV = "-gdwarf-" + std::to_string(DwarfVersion); + CmdArgs.push_back(Args.MakeArgString(DV)); --

[PATCH] D136080: [flang] Add -ffp-contract option processing

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Flang.cpp:89 +const StringRef Val = A->getValue(); +if (Val.equals("fast") || Val.equals("off")) { + FPContract = Val; We prefer `==` to `equals` CHANGES SINCE LAST ACTION htt

[PATCH] D135284: [Driver] select alternative target containing . in executable name

MaskRay added a comment. In D135284#3886469 , @dankm wrote: > Thanks for the feedback. I've added a testcase for this, I'll push that > change and update the summary. Did you upload a new patch to the differential? I cannot see it. Repository: rG LL

[PATCH] D135284: [Driver] allow target override containing . in executable name

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/D135284/new/ https://reviews.llvm.org/D135284 __

[PATCH] D135284: [Driver] allow target override containing . in executable name

This revision was automatically updated to reflect the committed changes. Closed by commit rG0d7eba1aeed8: [Driver] Allow target override containing . in executable name (authored by dankm, committed by MaskRay). Changed prior to commit: https://reviews.llvm.org/D135284?vs=470918&id=470970#toc

[PATCH] D136789: [clang] Remove no-op -fexperimental-new-pass-manager/-fno-legacy-pass-manager flags

MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: clang/docs/ReleaseNotes.rst:161 + - The -fexperimental-new-pass-manager and -fno-legacy-pass-manager flags have +been removed. These have been no-ops

[PATCH] D136846: [Driver] Enable profi flag as user-facing flag

MaskRay requested changes to this revision. MaskRay added a comment. This revision now requires changes to proceed. Herald added a subscriber: StephenFan. > [Driver] Enable profi flag as user-facing flag Please avoid unclear term "profi" and use "-fsample-profile-use-profi" instead. ==

[PATCH] D136854: [HIP] add -fhiplib-add-rpath

MaskRay added a comment. If an option does not affect compilation, I prefer `--` to `-f` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136854/new/ https://reviews.llvm.org/D136854 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:

[PATCH] D136572: Harmonize cmake_policy() across standalone builds of all projects

MaskRay added a comment. It'll be good if we have a wiki describing how downstream users invoke cmake, so that large cmake refactoring can be verified beforehand. (like usage verification https://github.com/opencollab/llvm-toolchain-integration-test-suite, but for cmake invocations) CHANGES S

[PATCH] D136888: Move getenv for AS_SECURE_LOG_FILE to clang

MaskRay added inline comments. Comment at: clang/test/Driver/AS_SECURE_LOG_FILE.s:1 +// RUN: env AS_SECURE_LOG_FILE=%t %clang -target x86_64-apple-darwin -c %s -o %t.o -### 2>&1 | FileCheck %s -DLOG_FILE=%t +// CHECK: "-cc1as" `--target=` for new tests remove u

[PATCH] D136712: Define _GNU_SOURCE for arm baremetal in C++ mode.

MaskRay added a comment. Is this an upstream GCC behavior or a behavior from some gcc-none-eabi distributions? GCC's C++ mode defines `_GNU_SOURCE` historically due to a libstdc++ limitation https://gcc.gnu.org/onlinedocs/libstdc++/faq.html#faq.predefined and nowadays "the ship has sailed" and

[PATCH] D136712: Define _GNU_SOURCE for arm baremetal in C++ mode.

MaskRay added a comment. The `_GNU_SOURCE` definition needs to be guarded by `if (Opts.CPlusPlus)`. Comment at: clang/test/Driver/arm-baremetal-defines.cpp:1 +// RUN: %clang --target=arm-none-eabi -march=armv7-m %s -emit-llvm -S -c -o - 2>&1 | FileCheck %s + M

[PATCH] D136888: Move getenv for AS_SECURE_LOG_FILE to clang

MaskRay accepted this revision. MaskRay added inline comments. Herald added a subscriber: StephenFan. Comment at: llvm/lib/MC/MCContext.cpp:77 AutoReset(DoAutoReset), TargetOptions(TargetOpts) { - SecureLogFile = AsSecureLogFileName; + SecureLogFile = TargetOptions ? Tar

[PATCH] D136707: [clang][Toolchains][Gnu] pass -gdwarf-* through to assembler

MaskRay added a comment. In D136707#3892300 , @jyknight wrote: > This change has caused `clang -Wall -fdebug-default-version=5 -c -xc > /dev/null -o /dev/null` to print the following warning, because it has moved > the read of the argument into a condit

[PATCH] D136940: [clang][Driver] allow tilde in user config dir

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. `clang/test/Driver/config-file3.c` tests `--config-user-dir=` but I cannot think of a portable way testing this. `RUN: echo ~; %clang -### --config-user-dir=~ ...` doesn't work as the selecte

[PATCH] D136940: [clang][Driver] allow tilde in user config dir

MaskRay added a comment. In D136940#3893638 , @paperchalice wrote: > I would appreciate if someone can commit this change. I am waiting on @mgorny 's opinion for https://blogs.gentoo.org/mgorny/2022/10/07/clang-in-gentoo-now-sets-default-runtimes-via-c

[PATCH] D131351: [C] Default implicit function pointer conversions diagnostic to be an error

MaskRay added a comment. Nit: I think it is useful mentioning `-Wincompatible-function-pointer-types` in the commit message, not just in `clang/docs/ReleaseNotes.rst`, so that `git log --grep incompatible-function-pointer-types` will reveal something when the user knows incompatible-function-po

[PATCH] D129160: libclang.so: Make SONAME the same as LLVM version

MaskRay added a comment. In D129160#3716016 , @isuruf wrote: > As a downstream packager of libclang, I really like the SONAME not changing > in libclang. It makes it possible for us to package things like qt that > depend on libclang, but not have to re

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

MaskRay added a comment. Sorry, my previous main comment had been written before I introduced `LIT_CLANG_STD_GROUP` in `llvm/utils/lit/lit/llvm/config.py`. The multiple `%clang_cc1` approach actually looks like the following. Note the use of `%stdcxx_17-` to make the test future-proof. (It is no

[PATCH] D131717: [ADT] Replace STLForwardCompat.h's C++17 equivalents

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. The llvm/include/llvm/ADT/STLForwardCompat.h and llvm/unittests/ADT/STLForwardCompatTest.cpp removal can be in a separate patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D131533: [Flang][Driver] Enable PIC in the frontend

MaskRay added inline comments. Comment at: clang/include/clang/Driver/Options.td:6320-6325 +def pic_level : Separate<["-"], "pic-level">, + HelpText<"Value for __PIC__">, + MarshallingInfoInt>; +def pic_is_pie : Flag<["-"], "pic-is-pie">, + HelpText<"File is for a position ind

[PATCH] D131533: [Flang][Driver] Enable PIC in the frontend

MaskRay added inline comments. Comment at: flang/lib/Frontend/CompilerInvocation.cpp:151 +static llvm::Reloc::Model getRelocationFromName(llvm::StringRef model) { + if (model.equals("static")) +return llvm::Reloc::Static; awarzynski wrote: > Only `-fpic` and

[PATCH] D131808: [clang,flang] Add missing options fsyntax-only in help

MaskRay accepted this revision. MaskRay added a comment. > Subject: [clang,flang] Add missing options fsyntax-only in help Prefer `-fsyntax-only` to `fsyntax-only` since the former is what a user writes. > this pull request contains a fix for the problem with displaying options > -fsyntax_only

[PATCH] D131808: [clang,flang] Add missing options fsyntax-only in help

MaskRay added a comment. > https://github.com/llvm/llvm-project/issues/57033 Use `Fix https://github.com/llvm/llvm-project/issues/57033` or `Fixes https://github.com/llvm/llvm-project/issues/57033` or (I usually prefer this for non-bugs but still some issues worth addressing) `Close ...`, then

[PATCH] D131808: [clang,flang] Add missing options fsyntax-only in help

MaskRay added a comment. To be clear, some issues should be addressed before someone lands this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131808/new/ https://reviews.llvm.org/D131808 ___ cfe-commits

[PATCH] D130531: [IR] Use Min behavior for module flag "PIC Level"

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

[PATCH] D130531: [IR] Use Min behavior for module flag "PIC Level"

MaskRay updated this revision to Diff 453806. MaskRay added a comment. add Bitcode/upgrade-pic-level.ll Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130531/new/ https://reviews.llvm.org/D130531 Files: clang/test/CodeGen/piclevels.c llvm/lib/I

[PATCH] D130531: [IR] Use Min behavior for module flag "PIC Level"

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc2a38887932e: [IR] Use Min behavior for module flag "PIC Level" (authored by MaskRay). Changed prior to commit: https://reviews.llvm.org/D130531?v

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

MaskRay added a comment. Both D131638 and this patch want to use classes with inheritance to support different compression algorithms. There have been many versions but I think I have never received a convincing argument how such an inheritance based design loo

[PATCH] D119296: KCFI sanitizer

MaskRay added a comment. > OK, so we could just not make the symbols weak and end up failing at link > time if there's a mismatch. That sounds reasonable to me. For `STB_WEAK SHN_ABS` `__kcfi_typeid_*`, there is no duplicate definition error. Is this behavior intentional? Note: I don't think we

[PATCH] D119296: KCFI sanitizer

MaskRay added inline comments. Comment at: llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp:330 + // comparing the hashes. + unsigned ScratchRegs[] = {AArch64::W16, AArch64::W17}; + const? Comment at: llvm/lib/Target/AArch64/AArch64AsmPrinter.cp

[PATCH] D131533: [Flang][Driver] Enable PIC in the frontend

MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: flang/test/Driver/pic-flags.f90:69 +! CHECK-ROPI: "-mrelocation-model" "ropi" +! CHECK-ROPI-NOT: -pic + I'd use `! CHECK-ROPI-NOT: "-pic`

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

MaskRay updated this revision to Diff 454282. MaskRay marked 6 inline comments as done. MaskRay added a comment. fix tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131464/new/ https://reviews.llvm.org/D131464 Files: clang/test/AST/ast-dump-

[PATCH] D130255: [Clang][LoongArch] Add initial LoongArch target and driver support

MaskRay accepted this revision. MaskRay added inline comments. Comment at: clang/test/Preprocessor/init-loongarch.c:1 +// RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 -triple=loongarch32 /dev/null \ +// RUN: | FileCheck --match-full-lines --check-prefix=LA32 %s -

[PATCH] D132324: [RFC] Remove support for building libc++ with `LLVM_ENABLE_PROJECTS`

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Linux.cpp:310 - // The deprecated -DLLVM_ENABLE_PROJECTS=libcxx configuration installs - // libc++.so in D.Dir+"/../lib/". Detect this path. @mgorny FYI, this has been removed. Repositor

[PATCH] D132324: [RFC] Remove support for building libc++ with `LLVM_ENABLE_PROJECTS`

MaskRay added a subscriber: vitalybuka. MaskRay added a comment. @vitalybuka `-DLLVM_ENABLE_PROJECTS='libcxx;libcxxabi'` in `zorg/buildbot/builders/sanitizers/buildbot_functions.sh` needs adjustment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

[PATCH] D130020: [OpenMP] Deprecate the old driver for OpenMP offloading

MaskRay accepted this revision. MaskRay added a comment. This is "remove" instead of "deprecate"? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130020/new/ https://reviews.llvm.org/D130020 ___ cfe-commit

[PATCH] D133157: Add -fsanitizer-coverage=control-flow

MaskRay accepted this revision. MaskRay added inline comments. Herald added a subscriber: StephenFan. Comment at: llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp:155 +cl::desc("collect control flow for each function"), cl::Hidden, +cl::in

[PATCH] D133998: [HIP][test] Avoid %T

MaskRay created this revision. MaskRay added reviewers: tra, yaxunl. 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. %T is a deprecated lit feature. It refers to th

[PATCH] D133998: [HIP][test] Avoid %T

MaskRay updated this revision to Diff 460569. MaskRay added a comment. fix all hip-* Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133998/new/ https://reviews.llvm.org/D133998 Files: clang/test/Driver/hip-link-bc-to-bc.hip clang/test/Driver/hi

[PATCH] D133705: [HIP] Fix unbundling archive

MaskRay added a comment. I know very little about HIP, but I am concerned with relying on extensions as well. For example, I've seen `libc++.a.1` (we use this for the real archive while `libc++.a` is a linker script) and `.la` (libtool). A `.so` file is sometimes a linker script and it may refer

[PATCH] D133998: [HIP][test] Avoid %T

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9e1c1ecb148e: [HIP][test] Avoid %T (authored by MaskRay). Changed prior to commit: https://reviews.llvm.org/D133998?vs=460569&id=460608#toc Reposi

[PATCH] D132300: [clang][lldb][cmake] Use new `*_INSTALL_LIBDIR_BASENAME` CPP macro

MaskRay added a comment. In the long term we should just remove the `CLANG_INSTALL_LIBDIR_BASENAME` customization. This is supposed for GCC multilib lib32 lib64 names but we don't necessarily use it for Clang + compiler-rt files. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D129824: [RISCV] Set triple based on -march flag which can be deduced in more generic way

MaskRay added a comment. Both D54214 and this look like a surprising behavior to me. Do we still have time to go back the state before D54214 and make mismatching --target & -march= an error? > clang -target=riscv32-- -march=r

[PATCH] D129824: [RISCV] Set triple based on -march flag which can be deduced in more generic way

MaskRay added a comment. In D129824#3794229 , @jrtc27 wrote: > In D129824#3794221 , @MaskRay wrote: > >> Both D54214 and this look like a >> surprising behavior to me. Do we stil

[PATCH] D134018: [clang] [Driver] Add an option to disable default config filenames

MaskRay added a comment. This makes sense. Comment at: clang/lib/Driver/Driver.cpp:1027 - // If config file is not specified explicitly, try to deduce configuration - // from executable name. For instance, an executable 'armv7l-clang' will - // search for config file 'armv

[PATCH] D134018: [clang] [Driver] Add an option to disable default config filenames

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. LGTM. Comment at: clang/lib/Driver/Driver.cpp:1027 - // If config file is not specified explicitly, try to deduce configuration - // from executable name. For instance,

[PATCH] D134018: [clang] [Driver] Add an option to disable default config filenames

MaskRay accepted this revision. MaskRay added inline comments. Comment at: clang/test/Driver/config-file3.c:44 + +//--- --no-default-config disables config search +// Append `.` after a complete sentence in a comment. Comment at: clang/test/Dri

[PATCH] D109621: [clang][Driver] Default to loading clang.cfg if config file not specified

MaskRay added inline comments. Comment at: clang/docs/UsersManual.rst:878 Another way to specify a configuration file is to encode it in executable name. For example, if the Clang executable is named `armv7l-clang` (it may be a +symbolic link to `clang`), then Clang will search

[PATCH] D134063: [clang] Make --ld-path= work with -fuse-ld=lld

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

[PATCH] D109621: [clang][Driver] Default to loading clang.cfg if config file not specified

MaskRay added a subscriber: joerg. MaskRay added a comment. Thanks for taking over the patch! I've CCed (in https://discourse.llvm.org/t/rfc-adding-a-default-file-location-to-config-file-support/63606/33) some folks who had strong opinions in the initial support patch. (Cc @joerg manually whom

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

MaskRay created this revision. MaskRay added reviewers: DavidSpickett, dmgreen, john.brawn. Herald added subscribers: StephenFan, kristof.beyls. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. `__builtin

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

MaskRay added a comment. In D133329#3798166 , @mgorny wrote: > Hmm, apparently this could be achieved by making it a "CoreOption". I don't think it is appropriate to have `CoreOption`. The Windows dxc support appears to have nothing to do with gcc insta

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

MaskRay added a comment. In D133329#3798371 , @mgorny wrote: > I think you misunderstood me. The aforementioned tests fail if I put > `--gcc-install-dir=` in the config file. There is no way to put it in the > config file that wouldn't affect these test

[PATCH] D134191: [clang] Make config-related options CoreOptions

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

[PATCH] D134191: [clang] Make config-related options CoreOptions

MaskRay added a comment. They don't need to be `NoXarchOption` if I am not mistaken. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134191/new/ https://reviews.llvm.org/D134191 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://li

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

MaskRay added a comment. In D134127#3798442 , @dmgreen wrote: > This looks like a subset of D133359 ? D133359 looks like it is for AArch64 and may change the diagnostic in a more aggre

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

MaskRay added a comment. zstd provides GNU Makefile, CMake, and Meson. The CMake files are only installed when configured with CMake. Debian and Ubuntu lack such files. The pkg-config file libzstd.pc is probably the most portable file. (I have also used it for a binutils-gdb patch.) I think we

[PATCH] D134208: [clang] [Driver] Do not transform explicit --config filename

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

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

MaskRay added a comment. If the two patches are basically identical with just some fixes for the problem, the convention is to reuse the original Differential. You can click "Add Action - Open" (or Reopen?), then you can upload a new patch to update the Differential. This keeps all discussions i

[PATCH] D134271: [clang] [docs] Improve formatting & fix typo in config docs

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

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