[PATCH] D146603: [docs] Document -fomit-frame-pointer

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. MaskRay marked 2 inline comments as done. Closed by commit rG5f883cdbfbe2: [docs] Document -fomit-frame-pointer (authored by MaskRay). Changed prior to commit:

[PATCH] D146603: [docs] Document -fomit-frame-pointer

MaskRay updated this revision to Diff 507792. MaskRay added a comment. Adjust some terms. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146603/new/ https://reviews.llvm.org/D146603 Files: clang/include/clang/Driver/Options.td Index:

[PATCH] D146686: [Driver] Fix rpath for compiler-rt

MaskRay added a comment. In D146686#4216612 , @yaxunl wrote: > In D146686#4215577 , @MaskRay wrote: > >> This change is correct for Linux. `llvm/CMakeLists.txt` says: >> >> if(CMAKE_SYSTEM_NAME MATCHES

[PATCH] D146686: [Driver] Fix rpath for compiler-rt

MaskRay added a comment. This change is correct for Linux. `llvm/CMakeLists.txt` says: if(CMAKE_SYSTEM_NAME MATCHES "BSD|Linux|OS390") set(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR_default ON) Some rpath using OSes (notably macOS) use LLVM_ENABLE_PER_TARGET_RUNTIME_DIR_default=OFF. Is the rpath

[PATCH] D144190: [AIX][clang] Storage Locations for Constant Pointers

MaskRay added inline comments. Comment at: clang/test/Driver/ppc-roptr.c:1 +// RUN: %clang -### -target powerpc-ibm-aix-xcoff -mroptr %s 2>&1 | \ +// RUN: FileCheck %s --check-prefixes=ROPTR,LINK Prefer `--target=` to `-target ` for new tests.

[PATCH] D144190: [AIX][clang] Storage Locations for Constant Pointers

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/AIX.cpp:125 + // `-mroptr` implies the `-bforceimprw` linker option. + // The `-mroptr` option places constants in RO sections as much as possible. Delete `// `-mroptr` implies the

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to S11

MaskRay added inline comments. Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:58 const auto *RVFI = MF.getInfo(); if (RVFI->useSaveRestoreLibCalls(MF)) { paulkirth wrote: > craig.topper wrote: > > paulkirth wrote: > > > craig.topper wrote: > >

[PATCH] D146155: [clang][NFC] Fix location of 2>&1 in a few -print tests

MaskRay added a comment. Thank you! (A somewhat common practice is to prefer `[test]` over `[NFC]` for pure-test changes (no code change) :) I see that " in a few -print tests" in the subject may render `[test]` unneeded.. perhaps what tag is used isn't really important.) Repository: rG

[PATCH] D145646: [clang][driver] Enable '-flto' on AVR

MaskRay added a comment. In D145646#4213547 , @gulfem wrote: > We started seeing a test failure in `avr-ld.c`. > > /b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/avr-ld.c:48:11: error: > // LINKP: {{".*ld.*"}} {{.*}}

[PATCH] D145646: [clang][driver] Enable '-flto' on AVR

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. "Close" seems more appropriate to me as this isn't a bug (previously not an intended use case). Comment at: clang/test/Driver/avr-ld.c:59 +// LINKS: {{".*ld.*"}} {{.*}}

[PATCH] D146603: [docs] Document -fomit-frame-pointer

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

[PATCH] D145840: [Docs] Added -fomit-frame-pointer and -fno-omit-frame-pointer flag documentation

MaskRay added a comment. Sorry for my belated response as I was on a trip for quite a few days. We don't need to document the negative form. "Produce better stack traces," is not necessarily correct. See https://maskray.me/blog/2020-11-08-stack-unwinding I created D146603

[PATCH] D146603: [docs] Document -fomit-frame-pointer

MaskRay created this revision. MaskRay added reviewers: aabhinavg, vitalybuka. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D146603

[PATCH] D146054: [RISCV] Add -print-supported-marchs and -march=help support

MaskRay requested changes to this revision. MaskRay added inline comments. This revision now requires changes to proceed. Comment at: clang/test/Driver/print-supported-marchs.c:17 +// CHECK-RISCV:i 2.0 +// CHECK-RISCV:e 1.9 +//

[PATCH] D143587: [Docs] Multilib design

MaskRay accepted this revision. MaskRay added inline comments. Comment at: clang/docs/Multilib.rst:241 + +multilib.yaml and -print-multi-selection-flags-experimental are new interfaces +to Clang. In order for them to be usable over time and across LLVM versions

[PATCH] D145883: [Flang][RISCV] Emit target features for RISC-V

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Flang.cpp:108 case llvm::Triple::aarch64: [[fallthrough]]; + case llvm::Triple::riscv64: Remove `[[fallthrough]]` and just list the 3 `case` consecutively. Comment

[PATCH] D145999: [RISCV] Reserve X18 by default for Android

MaskRay accepted this revision. MaskRay added inline comments. Comment at: clang/test/Driver/riscv-fixed-x-register.c:343 + +// Check that x18 is reserved on Android by default +// RUN: %clang --target=riscv64-linux-android -### %s 2> %t asb wrote: >

[PATCH] D145725: [Driver] Make -X default for baremetal riscv

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

[PATCH] D145849: [Driver][xray] Allow XRay on Apple Silicon

MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: clang/test/Driver/XRay/xray-instrument-macos.c:1 +// RUN: %clang -o /dev/null -v -fxray-instrument -target aarch64-apple-darwin20 -c %s // RUN: %clang -o

[PATCH] D145941: [Clang] Always use -zdefs when linking AMDGPU images

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. While `--no-undefined` is an alias for `-z defs`, be consistent in the subject and the body. > Always use -zdefs `--no-undefined` Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D145848: [Driver] Correct -f(no-)xray-function-index behavior

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Thanks! Comment at: clang/test/CodeGen/xray-function-index.cpp:1 +// RUN: %clang_cc1 -fxray-instrument -x c++ -std=c++11 -triple

[PATCH] D145848: [Driver] Correct -f(no-)xray-function-index behavior

MaskRay requested changes to this revision. MaskRay added inline comments. This revision now requires changes to proceed. Comment at: clang/test/CodeGen/xray-function-index.cpp:1 +// RUN: %clang_cc1 -fxray-instrument -x c++ -std=c++11 -triple

[PATCH] D145146: [Driver][NFC] Remove some redundant code in Driver.cpp.

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. `computeTargetTriple` has several users. You probably want to check all these code paths do not need `if (const Arg *A = Args.getLastArg(options::OPT_target))`. For the uses in

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

MaskRay added a comment. In D143306#4183759 , @tstellar wrote: > Should we apply this patch to the release/16.x branch to create a more smooth > transition since the option has been removed in main? Yes, it will be a good idea to apply this to

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

MaskRay added a comment. > Due to this we started seeing assembler errors with certain .c and .cpp files > - > "Error: file number 1 already allocated" What are the certain `.c` and `.cpp` files? The behavior is correct for the following two commands. clang --target=arm-linux-gnueabihf

[PATCH] D118493: Set rpath on openmp executables

MaskRay added a comment. In D118493#4182583 , @JonChesterfield wrote: > Duplicating a comment from the commit thread so it's easier for me to find > later. > > You've applied this to the release branches going back as far as 14. It's a > user facing

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

MaskRay abandoned this revision. MaskRay added a comment. Abandoned by a revert of D118493 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143306/new/ https://reviews.llvm.org/D143306

[PATCH] D145567: [Driver] Rename multilib flags to tags

MaskRay added a comment. Since the touched code is still in review, you need to merge the changes into the prior patches. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145567/new/ https://reviews.llvm.org/D145567

[PATCH] D144179: [Clang] Added functionality to provide config file name via env variable

MaskRay requested changes to this revision. MaskRay added a comment. This revision now requires changes to proceed. In D144179#4166909 , @mgabka wrote: > In D144179#4146599 , @MaskRay wrote: > >> This looks like

[PATCH] D118493: Set rpath on openmp executables

MaskRay added a comment. In D118493#4178250 , @JonChesterfield wrote: > Reporting after another round of discussion. I don't have much support from > the llvm openmp working group that we should maintain the current divergence > from libc++ and the

[PATCH] D145173: Make section attribute and -ffunction-sections play nicely

MaskRay added a comment. The inconsistency stem from the fact that with the default `.text`, the compiler may generate `.text.$suffix` if `-ffunction-sections` is specified or if a COMDAT is needed. While with an explicit section, there is no such suffix appending mechanism used together with

[PATCH] D142174: [OpenMP] Don't set rpath for system paths

MaskRay added a comment. In D142174#4171731 , @JonChesterfield wrote: > I'm happy with this but agree that "what might be a system path?" is a tricky > heuristic. What we want is to exclude the places that the application will > search anyway, but

[PATCH] D145391: [HIP] Supports env var HIP_PATH

MaskRay added inline comments. Comment at: clang/test/Driver/rocm-detect.hip:42 + +// RUN: rm -rf %T/myhip +// RUN: rm -rf %T/myhip_nouse `%T` is not recommended. https://llvm.org/docs/CommandGuide/lit.html "parent directory of %t (not unique, deprecated, do

[PATCH] D145393: [HIP] Make `--offload-add-rpath` alias of `-frtlib-add-rpath`

MaskRay added a comment. Seems fine. Should we eventually remove `--offload-add-rpath` and `-fopenmp-implicit-rpath`? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145393/new/ https://reviews.llvm.org/D145393 ___ cfe-commits mailing list

[PATCH] D145141: [Driver] Reject -march= for ppc

This revision was automatically updated to reflect the committed changes. Closed by commit rG7370b9c8ea00: [Driver] Reject -march= for ppc (authored by MaskRay). Changed prior to commit: https://reviews.llvm.org/D145141?vs=501759=502686#toc Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D145173: Make section attribute and -ffunction-sections play nicely

MaskRay added a comment. > the programmer is indicating the function has some special purpose, and is > not expected to be garbage collected. There are use cases that GC semantics are desired. They may place the address function in a variable. I don't think changing the behavior by default is

[PATCH] D138539: Use std::nullopt_t instead of NoneType (NFC)

MaskRay added a comment. In D138539#4164313 , @steakhal wrote: > Oh, I forgot to mention why this change breaks the equality operator of > `llvm::StringSet`. It's because the `StringMap::operator==` will try to > compare the `value` as well, which is

[PATCH] D145164: [clang][RISCV] Enable -fasynchronous-unwind-tables by default on Linux

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. LG since GCC made a similar change and this matches several major architectures on Linux/other ELF based operating systems. (At heart I think `-fasynchronous-unwind-tables` is not a good

[PATCH] D144967: [PowerPC] Recognize long CPU name for -mtune in Clang

MaskRay added a comment. In D144967#4164858 , @nathanchance wrote: > Could this be merged into `main` and backported to `release/16.x`? If this > makes 16.0.0 final, I think the kernel can avoid working around this issue > altogether, as `-mtune` was

[PATCH] D145141: [Driver] Reject -march= for ppc

MaskRay created this revision. MaskRay added reviewers: PowerPC, nemanjai. Herald added subscribers: shchenz, fedor.sergeev, kbarton, jyknight. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Clang

[PATCH] D145021: [Clang][AIX][p] Claim -p in front end

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6342 +A->claim(); +if (TC.getTriple().isOSAIX()) { + if (!Args.hasArgNoClaim(options::OPT_pg)) Use `&&` Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D145021: [Clang][AIX][p] Claim -p in front end

MaskRay added inline comments. Comment at: clang/test/Driver/ibm-profiling.c:1 +// RUN: %clang -### 2>&1 \ +// RUN:--target=s390x-none-zos \ This is too verbose. Driver tests prefer packing many options on one line. The decreased number of lines

[PATCH] D144934: [clang] drop buggy use of `-serialize-diagnostics` flag

MaskRay added a comment. For pure test updates, best to include `[test] `in the subject :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144934/new/ https://reviews.llvm.org/D144934 ___ cfe-commits

[PATCH] D144878: __builtin_FILE_NAME()

MaskRay added a comment. It'd be nice to have compiler feature parity. I created a GCC feature request for `__builtin_FILE_NAME`: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108978 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144878/new/

[PATCH] D144533: [clang][driver] Do not emit default '-Tdata' for AVR devices

MaskRay added a comment. This change is an improvement but why is `__DATA_REGION_ORIGIN__` defined? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144533/new/ https://reviews.llvm.org/D144533 ___

[PATCH] D144967: [PowerPC] Recognize long CPU name for -mtune in Clang

MaskRay accepted this revision. MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Arch/PPC.cpp:80 -return llvm::StringSwitch(CPUName) -.Case("common", "generic") -.Case("440fp", "440") -.Case("630", "pwr3") -.Case("G3",

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

MaskRay added inline comments. Comment at: clang/test/Driver/config-file3.c:227 +// +// RUN: HOME=%S/Inputs/config %clang --config-user-dir=~ -v 2>&1 | FileCheck %s -check-prefix CHECK-TILDE +// CHECK-TILDE: User configuration file directory: {{.*}}/Inputs/config

[PATCH] D144914: [Clang][Driver] Add -mcpu=help to clang

MaskRay added a comment. Since `llc` has `-mcpu=help`, it seems fine to have `clang -mcpu=help`. I made a bad suggestion there about `-mcpu=?`. Perhaps I should revert de94ac9357750fdba45e09eefa8f67a650ae6a64 . This `-mcpu=help` patch is good. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D144914: [Clang][Driver] Add -mcpu=help to clang

MaskRay added a comment. FWIW I think this should be reverted. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144914/new/ https://reviews.llvm.org/D144914 ___ cfe-commits mailing list

[PATCH] D144914: [Clang][Driver] Add -mcpu=help to clang

MaskRay added a comment. Sorry I just saw this but I am not sure this is a good idea. Why can't the user use `--print-supported-cpus` instead? The additional alias doesn't seem useful. If you can make GCC add this as well, it will be different. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D144853: [Clang][RISCV] Add CMake options to configure default CPU

MaskRay added a comment. I object to this change which further complicates the build system and makes the difference difficult to observe by users. If your `clang` executable is at `bin/clang`, just create `bin/riscv64-unknown-linux-gnu.cfg` with `-mcpu=xxx`. You may configure

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

MaskRay added a comment. Herald added a project: All. The used code sequence has 2 bugs so the output will crash. I fixed them in 7198c87f42f6c15d76b127c1c63530e9b4d5dd39 and added my notes to

[PATCH] D140925: [CMake] Use Clang to infer the target triple

MaskRay added a comment. In D140925#4151524 , @smeenai wrote: > Ping @ldionne, would you be able to take a look at this soon (or are you okay > waiving the libc++ blocking requirement for it)? This is really useful for > Android armv7, where the triple

[PATCH] D144620: [clang][driver] Handle '-mrelax' and '-mno-relax' for AVR

MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Driver/ToolChains/AVR.cpp:548 +// '-mrelax' is default unless '-mno-relax' is specified. +if (Args.hasFlag(options::OPT_mrelax,

[PATCH] D144179: [Clang] Added functionality to provide config file name via env variable

MaskRay added a comment. This looks like introducing a footgun (when something behaves differently from an upstream Clang, it would be difficult for toolchain maintainers to know why). Why can't your user specify `CLANG_CONFIG_FILE_SYSTEM_DIR`? Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D144620: [clang][driver] Handle '-mrelax' and '-mno-relax' for AVR

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/AVR.cpp:549 +// '-mrelax' is default unless '-mno-relax' is specified. +if (Args.hasFlag(options::OPT_mrelax, options::OPT_mno_relax, true)) + CmdArgs.push_back("--relax"); If

[PATCH] D144035: [llvm] Ensure SCEV does not replace aliases with their aliasees

MaskRay accepted this revision. MaskRay added a comment. > `[llvm] ` `[SCEV]` is a more common tag for ScalarEvolution patches. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144035/new/ https://reviews.llvm.org/D144035

[PATCH] D144035: [llvm] Ensure SCEV does not replace aliases with their aliasees

MaskRay added a comment. > Full context at https://github.com/llvm/llvm-project/issues/60668. Best to leave a simplified description in the summary so that a reader doesn't have to go to the external link and summarize the discussions themselves. This SCEV function is invoked by `FunctionPass

[PATCH] D144035: [llvm] Ensure SCEV does not replace aliases with their aliasees

MaskRay added a comment. LG Comment at: clang/test/CodeGenCXX/pr60668.cpp:1 +// RUN: %clang --target=aarch64-unknown-linux-gnu -O1 -fvisibility=hidden \ +// RUN: -fsanitize=hwaddress -fsanitize=undefined -std=c++17 -fno-exceptions \ We try avoiding .cc to

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

MaskRay added a comment. In D143306#4144541 , @jdoerfert wrote: > I'm worried this makes use of LLVM on HPC machines even harder. That said, > I'm open to suggestions and I am not well versed in all the ways we can make > it work. > Our problem is that

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

MaskRay added a comment. In D143306#4144518 , @JonChesterfield wrote: > Marking this as "no" because as far as I can tell it'll stop anyone running > openmp built from source which constitutes a severe regression and I am > struggling to find

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

MaskRay added a comment. In D143306#4144396 , @JonChesterfield wrote: > So what is this configuration file? Joseph found a Gentoo blog post > https://blogs.gentoo.org/mgorny/2022/10/07/clang-in-gentoo-now-sets-default-runtimes-via-config-file/ > and I

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

MaskRay added inline comments. Comment at: clang/include/clang/Driver/Options.td:4218-4223 defm openmp_implicit_rpath: BoolFOption<"openmp-implicit-rpath", LangOpts<"OpenMP">, - DefaultTrue, + DefaultFalse, PosFlag, NegFlag, BothFlags<[NoArgumentUnused]>>;

[PATCH] D144444: [PowerPC] Use member function to determine PowerPC Secure PLT

MaskRay accepted this revision. MaskRay added a comment. `isPPC32SecurePlt` is probably more accurate. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D14/new/ https://reviews.llvm.org/D14 ___

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

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

[PATCH] D144157: Add 128-bit integer support to enum element

MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: clang/test/CodeGen/enum2.c:30 +// CHECK: ![[E]] = !DIEnumerator(name: "b0", value: 24197857203266734864629346612071973665, isUnsigned: true) +

[PATCH] D144157: Add 128-bit integer support to enum element

MaskRay added inline comments. Comment at: clang/test/CodeGen/enum3.c:1 +// RUN: %clang_cc1 -triple x86_64-linux-gnu %s -debug-info-kind=limited -emit-llvm -o - | FileCheck %s + We don't need a new test file. Reusing can improve test discoverability and

[PATCH] D144011: [clang]Fix warning for signed conversion

MaskRay added a comment. The description needs to adjusted to say that this confusing warning is for LP64 systems. It makes sense to match -m32 and GCC. If someone thinks a warning is useful, that can be a separate change. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144011/new/

[PATCH] D144011: [clang]Fix warning for signed conversion

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Thanks! Comment at: clang/lib/Sema/SemaChecking.cpp:14296 +if (SourceBT && SourceBT->isInteger() && TargetBT && +TargetBT->isInteger() &&

[PATCH] D144157: Add 128-bit integer support to enum element

MaskRay added a comment. We also need a codegen test. `clang/test/CodeGen/enum2.c` may be a good one. It tests that debug information is correct as well. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144157/new/ https://reviews.llvm.org/D144157

[PATCH] D144179: [Clang] Added functionality to provide config file name via env variable

MaskRay added a comment. Behaviors due to a new environment variable should be very careful. Why is this useful? If you want this, you can add a wrapper around `clang` to specify `--config=` by yourself. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D144011: [clang]Fix warning for signed conversion

MaskRay added a comment. I think it makes sense for `-Wsign-conversion` to not warn for this case. I do not know whether we need another diagnostic like `-Wshorten-64-to-32` for this case, but am inclined to no. `-m32` doesn't emit a warning. I wonder whether the newly added condition can be

[PATCH] D143745: Make section attribute and -ffunction-sections play nicely

MaskRay added a comment. If one TU has more than one section of the same name, there is no guarantee they are adjacent after linking. Linker scripts and `--shuffle-sections` can adjust them. I don't mind a behavior change if GCC is willing to do the same. But without, it's safer to use an

[PATCH] D142986: Enable multilib.yaml in the BareMetal ToolChain

MaskRay added inline comments. Comment at: clang/test/Driver/baremetal-multilib.cpp:19 +// CHECK-NEXT: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" "{{.*}}.o" "-Bstatic" +// CHECK-SAME:

[PATCH] D143763: [Clang] Add clangMinimumVersion to multilib.yaml

MaskRay added a comment. I think this patch can be merged into a previous one that introduces `variants` and `flagMap` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143763/new/ https://reviews.llvm.org/D143763

[PATCH] D142933: Add -print-multi-selection-flags argument

MaskRay added a comment. > Add -print-multi-selection-flags argument "option" is a better term. Since the option doesn't take an argument, "flag" applies as well. `[Driver] Add -print-multi-selection-flags` The summary is empty. Is that intended? We need some description what the new option

[PATCH] D142905: Change multilib selection algorithm

MaskRay added a comment. Mention `MultilibSet::select` in the summary. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142905/new/ https://reviews.llvm.org/D142905 ___ cfe-commits mailing list

[PATCH] D142905: Change multilib selection algorithm

MaskRay added inline comments. Comment at: clang/lib/Driver/Multilib.cpp:53 + for (StringRef Flag : PrintArgs) { +OS << "@" << Flag.substr(1); } remove braces Comment at: clang/lib/Driver/Multilib.cpp:99 + multilib_list Result =

[PATCH] D142905: Change multilib selection algorithm

MaskRay added a comment. Use `[Driver] ` for pure driver patches. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142905/new/ https://reviews.llvm.org/D142905 ___ cfe-commits mailing list

[PATCH] D142893: [NFC] Class for building MultilibSet

MaskRay added inline comments. Comment at: clang/include/clang/Driver/MultilibBuilder.h:117 + /// Add an optional Multilib segment + MultilibSetBuilder (const MultilibBuilder ); + Use `camelCase` for new function names. Repository: rG LLVM Github Monorepo

[PATCH] D142986: Enable multilib.yaml in the BareMetal ToolChain

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/BareMetal.cpp:167 + std::vector ArgsFlags = TC.getMultiSelectionFlags(Args); + + if (!Result.Multilibs.parse(*MB.get())) { delete blank line Comment at:

[PATCH] D142932: [NFC] Multilib YAML parsing

MaskRay added a comment. Adding new APIs is usually not considered `NFC`. Just remove the tag. Comment at: clang/lib/Driver/Multilib.cpp:115 +if (!StringRef(M.Regex).starts_with("^")) { + RegexString.insert(RegexString.begin(), '^'); +}

[PATCH] D143587: [Docs] Multilib design

MaskRay added inline comments. Comment at: clang/docs/Multilib.rst:41 + +The available libraries can be hard-coded in clang. Typically this is done +using the ``MultilibBuilder`` interface. There are many examples of this in `s/clang/Clang/`

[PATCH] D143745: Make section attribute and -ffunction-sections play nicely

MaskRay added a comment. > [...] all functions with that attribute end up in a single section, defeating > the linker GC. This can be made preciser. Say we have __attribute__((section("foo"))) void f(i) {} __attribute__((section("foo"))) void g() {} `f` and `g` don't use COMDAT,

[PATCH] D143692: [clang][driver] Emit error when enabling emulated tls on unsupported architectures

MaskRay accepted this revision. MaskRay added a comment. With nits Comment at: clang/test/Driver/emulated-tls.cpp:49 +// RISCV-NOT: unsupported option '-fno-emulated-tls' for target 'riscv64-unknown-linux-android' +// RISCV-ERROR: unsupported option '-femulated-tls' for

[PATCH] D143692: [clang][driver] Emit error when enabling emulated tls on unsupported architectures

MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6170 + + if (Arg *A = Args.getLastArg(options::OPT_femulated_tls)) { +// LLVM does not support Emulated TLS for

[PATCH] D131230: [RISCV] Allow mismatched SmallDataLimit and use Min for conflicting values

MaskRay added a comment. I updated more `i32 1, !"SmallDataLimit"` and re-landed this patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131230/new/ https://reviews.llvm.org/D131230 ___ cfe-commits

[PATCH] D131230: [RISCV] Allow mismatched SmallDataLimit and use Min for conflicting values

This revision was automatically updated to reflect the committed changes. Closed by commit rGaf1287914648: [RISCV] Allow mismatched SmallDataLimit and use Min for conflicting values (authored by MaskRay). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to

[PATCH] D140270: MIPS: fix build from IR files, nan2008 and FpAbi

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb4b95dee3140: MIPS: fix build from IR files, nan2008 and FpAbi (authored by wzssyqa, committed by MaskRay). Repository: rG LLVM Github Monorepo

[PATCH] D137756: [z/OS][pg] Throw error when using -pg on z/OS

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Thanks! Comment at: clang/test/Driver/zos-profiling-error.c:1 +// Check failed cases + Delete this comment. It does not help reading the test.

[PATCH] D142606: Lazyly initialize uncommon toolchain detector

MaskRay added a comment. (The first line of 0ffaffcaac97de31e1b0e7e80c4f7cab724eda20 should have mentioned `Lazyly initialize uncommon toolchain detector`. You can add `Reland` prefix or add the message in the body of the

[PATCH] D143300: [randstruct] Don't allow implicit forward decl to stop struct randomization

MaskRay added inline comments. Comment at: clang/test/CodeGen/init-randomized-struct-fwd-decl.c:2 +// RUN: %clang_cc1 -triple=x86_64-unknown-linux -emit-llvm -frandomize-layout-seed=1234567890abcdef < %s | FileCheck %s +// RUN: %clang_cc1 -triple=x86_64-unknown-linux -emit-llvm

[PATCH] D143300: [randstruct] Don't allow implicit forward decl to stop struct randomization

MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: clang/test/CodeGen/init-randomized-struct-fwd-decl.c:23 + +// CHECK-LABEL: define {{.*}}@t1 +// CHECK-NOT: getelementptr inbounds %struct.foo, ptr %3, i32

[PATCH] D143318: [Support] Move ItaniumManglingCanonicalizer and SymbolRemappingReader from Support to ProfileData

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

[PATCH] D143355: [RISCV] Default to -ffixed-x18 for Fuchsia

MaskRay added inline comments. Comment at: llvm/lib/Target/RISCV/RISCVSubtarget.cpp:16 +#include "GISel/RISCVLegalizerInfo.h" +#include "GISel/RISCVRegisterBankInfo.h" #include "RISCV.h" mcgrathr wrote: > jrtc27 wrote: > > Unrelated change > arcanist requested

[PATCH] D142606: Lazyly initialize uncommon toolchain detector

MaskRay added a comment. LGTM Comment at: clang/lib/Driver/ToolChains/LazyDetector.h:36 +} +return (); + } tbaeder wrote: > Is the `.value()` here permitted? AFAIK the convention is to always use > `operator*`. > > And I think @aaron.ballman would

[PATCH] D143325: [Driver] Add -mllvm= as an alias for -mllvm

MaskRay added a comment. In D143325#4105356 , @Meinersbur wrote: > This patch broke the Flang buildbot, e.g. > https://lab.llvm.org/buildbot/#/builders/172/builds/23305 > > The tests >

[PATCH] D143325: [Driver] Add -mllvm= as an alias for -mllvm

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG90094ab8850e: [Driver] Add -mllvm= as an alias for -mllvm (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D143300: [randstruct] Don't allow implicit forward decl to stop struct randomization

MaskRay added a comment. `clang/test/CodeGen/init-randomized-struct-fwd-decl.c` passes without this patch. Is it correct? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143300/new/ https://reviews.llvm.org/D143300

[PATCH] D143325: [Driver] Add -mllvm= as an alias for -mllvm

MaskRay updated this revision to Diff 494844. MaskRay added a comment. fix test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143325/new/ https://reviews.llvm.org/D143325 Files: clang/include/clang/Driver/Options.td

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