[PATCH] D148757: [clang] Follow user-provided order for prefix map

MaskRay added a comment. In D148757#4285735 , @gulfem wrote: > I compared Clang and GCC behavior. > **1) Following user-specified prefix mapping order** > GCC uses a linked list to store the prefix mappings, so it applies the prefix > mappings based on t

[PATCH] D148757: [clang] Follow user-provided order for prefix map

MaskRay added a comment. Created https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109591 to discuss GCC's behavior. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148757/new/ https://reviews.llvm.org/D148757 ___

[PATCH] D148757: [clang] Follow user-provided order for prefix map

MaskRay added a comment. I'm creating a patch to change DebugPrefixMap to respect the command line option order... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148757/new/ https://reviews.llvm.org/D148757

[PATCH] D148975: -fdebug-prefix-map=: make the last win when multiple prefixes match

MaskRay created this revision. MaskRay added reviewers: debug-info, dankm, gulfem, phosek. Herald added subscribers: hiraditya, emaste. Herald added a project: All. MaskRay requested review of this revision. Herald added a reviewer: jdoerfert. Herald added subscribers: llvm-commits, cfe-commits, jp

[PATCH] D148757: [clang] Follow user-provided order for prefix map

MaskRay added a comment. In D148757#4289076 , @MaskRay wrote: > I'm creating a patch to change DebugPrefixMap to respect the command line > option order... Created D148975 to follow GCC's behavior (the last applicable

[PATCH] D149019: [Clang] Accept and forward `-fconvergent-functions` in the driver

MaskRay added inline comments. Comment at: clang/include/clang/Driver/Options.td:972 + LangOpts<"ConvergentFunctions">, DefaultFalse, + NegFlag, + PosFlag>; Only one of the positive and negative options need to be a cc1 option. Comment at: c

[PATCH] D148975: -fdebug-prefix-map=: make the last win when multiple prefixes match

MaskRay marked 2 inline comments as done. MaskRay added inline comments. Comment at: clang/include/clang/Basic/CodeGenOptions.h:209 - std::map DebugPrefixMap; + llvm::SmallVector, 0> DebugPrefixMap; std::map CoveragePrefixMap; scott.linder wrote: > What be

cfe-commits@lists.llvm.org

MaskRay added inline comments. Comment at: lld/ELF/Symbols.cpp:50 +std::string root = symName.str(); +return demangle(root); + } `return demangle(symName.str());` perhaps `return demangle(symName);` works as well? Repository: rG LLVM Github Monorepo

[PATCH] D148975: -fdebug-prefix-map=: make the last win when multiple prefixes match

MaskRay updated this revision to Diff 516863. MaskRay marked 3 inline comments as done. MaskRay added a comment. remove a DebugPrefixMap variable Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148975/new/ https://reviews.llvm.org/D148975 Files: c

[PATCH] D148975: -fdebug-prefix-map=: make the last win when multiple prefixes match

MaskRay marked an inline comment as done. MaskRay added inline comments. Comment at: clang/include/clang/Basic/CodeGenOptions.h:209 - std::map DebugPrefixMap; + llvm::SmallVector, 0> DebugPrefixMap; std::map CoveragePrefixMap; scott.linder wrote: > MaskRay

[PATCH] D148094: [DRAFT][clang][CodeGen] Break up TargetInfo.cpp [6/6]

MaskRay added a comment. This refactoring looks reasonable to me as well. In `clang/lib/Driver`, we have D30372 that splits some huge files into target-specific files. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D149193: [Driver] -gsplit-dwarf: derive .dwo names from -o for link actions

MaskRay created this revision. MaskRay added reviewers: dblaikie, phosek, yaxunl. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. When the final phase is linking, Clang currently places auxiliary files i

[PATCH] D148975: -fdebug-prefix-map=: make the last win when multiple prefixes match

MaskRay updated this revision to Diff 516924. MaskRay marked an inline comment as done. MaskRay added a comment. Update HelpText Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148975/new/ https://reviews.llvm.org/D148975 Files: clang/include/clan

[PATCH] D148975: -fdebug-prefix-map=: make the last win when multiple prefixes match

MaskRay added a comment. In D148975#4296625 , @scott.linder wrote: > LGTM, thank you! > > Does this warrant a release note, as it is changing the behavior in a > backwards-incompatible manner? I do think changing to match GCC is > worthwhile, even if i

[PATCH] D148665: Change -fsanitize=function to place two words before the function entry

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

[PATCH] D148975: -fdebug-prefix-map=: make the last win when multiple prefixes match

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGdaad48d6b236: -fdebug-prefix-map=: make the last win when multiple prefixes match (authored by MaskRay). Repository: rG LLVM Github Monorepo CHAN

[PATCH] D148757: [clang] Apply -fcoverage-prefix-map reverse order

MaskRay added a comment. LGTM Comment at: clang/include/clang/Basic/CodeGenOptions.h:211 + + /// Prefix replacement map for coverage. + llvm::SmallVector, 0> CoveragePrefixMap; While adding a comment, clarify what coverage it is? There are multiple coverage

[PATCH] D125272: [clang] Add -fcheck-new support

MaskRay added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:4117 + if (const Arg *A = Args.getLastArg(OPT_fcheck_new)) +Opts.CheckNew = true; + heatd wrote: > MaskRay wrote: > > Use `CodeGenOpts<"CheckNew">` and avoid change to this

[PATCH] D149193: [Driver] -gsplit-dwarf: derive .dwo names from -o for link actions

MaskRay added a comment. In D149193#4297043 , @scott.linder wrote: > I'm a bit confused after trying to work out the rules for the GCC version of > `-dumpdir` over at > https://gcc.gnu.org/onlinedocs/gcc/Overall-Options.html#index-dumpdir but it > at

[PATCH] D149193: [Driver] -gsplit-dwarf: derive .dwo names from -o for link actions

MaskRay updated this revision to Diff 516980. MaskRay edited the summary of this revision. MaskRay added a comment. Herald added a subscriber: ormris. add notes and more test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149193/new/ https://reviews

[PATCH] D149193: [Driver] Add -dumpdir and change -gsplit-dwarf .dwo names for linking

MaskRay updated this revision to Diff 517004. MaskRay retitled this revision from "[Driver] -gsplit-dwarf: derive .dwo names from -o for link actions" to "[Driver] Add -dumpdir and change -gsplit-dwarf .dwo names for linking". MaskRay edited the summary of this revision. MaskRay added a comment.

[PATCH] D149193: [Driver] Add -dumpdir and change -gsplit-dwarf .dwo names for linking

MaskRay added inline comments. Comment at: clang/lib/Driver/Driver.cpp:3884 + nullptr, getOpts().getOption(options::OPT_dumpdir), + Args.MakeArgString(Args.getLastArgValue(options::OPT_o, "a") + "-")); + Arg->claim(); dblaikie wrote: > woul

[PATCH] D149193: [Driver] Add -dumpdir and change -gsplit-dwarf .dwo names for linking

MaskRay added a comment. In D149193#4300501 , @phosek wrote: > Do we have any way to check if there are projects out there that use > `-dumpbase`? It shouldn't be too difficult to support it, but we should find > out first if it's needed. I spot checke

[PATCH] D148757: [clang] Apply -fcoverage-prefix-map reverse order

MaskRay added inline comments. Comment at: clang/include/clang/Basic/CodeGenOptions.h:211 + + /// Prefix replacement map for coverage. + llvm::SmallVector, 0> CoveragePrefixMap; MaskRay wrote: > While adding a comment, clarify what coverage it is? There are mul

[PATCH] D149193: [Driver] Add -dumpdir and change -gsplit-dwarf .dwo names for linking

MaskRay marked an inline comment as done. MaskRay added a comment. I think the patch as-is implements all the useful parts of GCC's complex rules and in the absence of `-dumpbase` (we deliberately don't implement), the rule almost exactly matches GCC unless we do `gcc -g -gsplit-dwarf d/a.c -o e

[PATCH] D125272: [clang] Add -fcheck-new support

MaskRay added inline comments. Comment at: clang/test/CodeGenCXX/fcheck-new.cpp:2 +// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py +// RUN: %clang_cc1 -fcheck-new -triple x86_64-linux-gnu -S -disable-O0-optnone \ +// RUN: -emit-llvm -o - %s | opt -S

[PATCH] D149193: [Driver] Add -dumpdir and change -gsplit-dwarf .dwo names for linking

MaskRay marked an inline comment as done. MaskRay added a comment. In D149193#4302337 , @scott.linder wrote: > In D149193#4300885 , @MaskRay wrote: > >> I think the patch as-is implements all the useful parts of

[PATCH] D125272: [clang] Add -fcheck-new support

MaskRay added inline comments. Comment at: clang/test/CodeGenCXX/fcheck-new.cpp:2 +// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py +// RUN: %clang_cc1 -fcheck-new -triple x86_64-linux-gnu -S -disable-O0-optnone \ +// RUN: -emit-llvm -o - %s | opt -S

[PATCH] D125272: [clang] Add -fcheck-new support

MaskRay added inline comments. Comment at: clang/test/CodeGenCXX/fcheck-new.cpp:7 +// CHECK-NEXT: entry: +// CHECK-NEXT:[[CALL:%.*]] = call noalias noundef ptr @_Znwm(i64 noundef 4) #[[ATTR2:[0-9]+]] +// CHECK-NEXT:ret ptr [[CALL]] ` #[[ATTR2:[0-9]+]]`

[PATCH] D146269: MIPS: allow o32 abi with 64bit CPU and 64 abi with 32bit triple

MaskRay added inline comments. Comment at: clang/test/Driver/mips-cpu64abi32.c:1 +// Check handling the CPU is 64bit while ABI is O32. +// when build for MIPS platforms. Use `///` for non-CHECK non-RUN comments. Comment at: clang/test/Driver/mi

[PATCH] D148944: [clang][Driver] Fix crash with unsupported architectures in MinGW and CrossWindows.

MaskRay accepted this revision. MaskRay added a comment. Looks great! Comment at: clang/test/Driver/unsupported-target-arch.c:33 +// RUN: FileCheck --input-file=%t.err --check-prefix=CHECK-NOARCH-CROSSWINDOWS %s +// CHECK-NOARCH-CROSSWINDOWS: error: unknown target triple 'noa

[PATCH] D148944: [clang][Driver] Fix crash with unsupported architectures in MinGW and CrossWindows.

This revision was automatically updated to reflect the committed changes. Closed by commit rGecad12100927: [clang][Driver] Fix crash with unsupported architectures in MinGW and… (authored by k-mana, committed by MaskRay). Changed prior to commit: https://reviews.llvm.org/D148944?vs=515809&id=51

cfe-commits@lists.llvm.org

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Looks great! Comment at: llvm/docs/ReleaseNotes.rst:289 +* ``llvm::demangle`` now takes a ``std::string_view`` rather than a + ``const std::string&``. Be careful passing

[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

MaskRay added inline comments. Comment at: clang/lib/CodeGen/CodeGenAction.cpp:863 + InliningDecisions.push_back(D.getCaller().str()); + for (size_t i = 0, e = InliningDecisions.size(); i != e; ++i) { +std::string S = llvm::demangle(InliningDecisions[i]); a

[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

MaskRay added a comment. I have read prior discussions and https://discourse.llvm.org/t/rfc-improving-clangs-middle-and-back-end-diagnostics/69261 . The current approach seems reasonable. The summary should incorporate more information. `inlined.from` isn't mentioned at all. ==

[PATCH] D148767: Restore CodeGen/LowLevelType

MaskRay added a comment. > This is rework of; > > D30046 (LLT) Add some information why this is restored? Assume that people may not read the depended patches (`Depends on D145937, D146352, and D148768.`). Repository: rG LLVM Github Monorepo CHANGES SINCE L

[PATCH] D149695: MS inline asm: remove obsolete code adding AOK_SizeDirective (e.g. dword ptr)

MaskRay created this revision. MaskRay added reviewers: alvinhochun, epastor, hans, thakis, ayzhao. Herald added subscribers: pengfei, hiraditya. Herald added a project: All. MaskRay requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-c

[PATCH] D149193: [Driver] Add -dumpdir and change -gsplit-dwarf .dwo names for linking

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

[PATCH] D148665: Change -fsanitize=function to place two words before the function entry

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

[PATCH] D149695: MS inline asm: remove obsolete code adding AOK_SizeDirective (e.g. dword ptr)

MaskRay added a comment. In D149695#4315194 , @hans wrote: >> The AOK_SizeDirective part from 5b37c181291210bedfbb7a6af5d51229f3652ef0 >> (2014-08) seems unneeded nowadays (the root cause has likely been fixed >> elsewhere). > > Would it be possible to co

[PATCH] D149193: [Driver] Add -dumpdir and change -gsplit-dwarf .dwo names for linking

MaskRay added a comment. In D149193#4316293 , @dblaikie wrote: > I guess my main question is: What's the motivation for implementing this? Do > you have a need/use for this? (it doesn't seem to be motivated by GCC > compatibility - as discussed, looks l

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

MaskRay added a comment. `-march=` `-mcpu=` `-mtune=` are from GCC and they traditionally have many opinions on the features. If GCC is going to have `-march=help`, I think adding `-march=help` to Clang to match is fine. But I'd really like to avoid aliases for non-compatibility reasons. They ju

[PATCH] D152090: [clang][Driver] Add -fcaret-diagnostics-max-lines as a driver option

MaskRay added a comment. `def fcaret_diagnostics_max_lines` in `Options.td` has the `NoDriverOption` flag. Move it to other places with `BooleainFFlag` should work. Then in `Clang.cpp` you can just write `Args.AddLastArg(...)` Can you add some description that this is related to D147875

[PATCH] D148385: [RISCV] Implement KCFI operand bundle lowering

MaskRay added a comment. `KCFI_CHECK` lowering has some complexity to allocate a temporary register. This needs to following the calling convention which can be modified by many compiler options and function attributes. I wonder whether we can move the if-condition part of the expanded code se

[PATCH] D151863: [x86][MC] Fix movdir64b addressing

MaskRay added inline comments. Comment at: llvm/test/CodeGen/X86/movdir64b-inline-asm-x86_64.ll:1 +; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 2 +; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+movdir64b | FileCheck

[PATCH] D152021: [clang][AIX] Fix Overly Strick LTO Option Checking against `data-sections` when `mxcoff-roptr` is in Effect

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:749 if (HasRoptr) { if (!Args.hasFlag(options::OPT_fdata_sections, +options::OPT_fno_data_sections, UseSeparateSections) && Consider the `has

[PATCH] D152021: [clang][AIX] Fix Overly Strick LTO Option Checking against `data-sections` when `mxcoff-roptr` is in Effect

MaskRay added a comment. > The LTO mxcoff-roptr check against data `-mxcoff-roptr`. Consider not dropping the leading `-`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152021/new/ https://reviews.llvm.org/D152021

[PATCH] D57497: [RISCV] Passing small data limitation value to RISCV backend

MaskRay added inline comments. Herald added subscribers: jobnoorman, luke, pcwang-thead, eopXD, VincentWu, vkmr, frasercrmck, arichardson. Herald added a project: All. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1970 + // Get small data limitation. + if (Args.getLastArg(

[PATCH] D152279: [Driver] Default -msmall-data-limit= to 0

MaskRay created this revision. MaskRay added reviewers: apazos, asb, craig.topper, hiraditya, jrtc27, shiva0217. Herald added subscribers: luke, frasercrmck, luismarques, sameer.abuasal, s.egerton, Jim, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, niosHD, sabu

[PATCH] D152279: [Driver] Default -msmall-data-limit= to 0

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2087 // Not support linker relaxation for PIC. -SmallDataLimit = "0"; if (Args.hasArg(options::OPT_G)) { D.Diag(diag::warn_drv_unsupported_sdata); The code is wr

[PATCH] D150902: [ARM][Driver] Warn if -mhard-float is incompatible

MaskRay added inline comments. Comment at: clang/test/Driver/arm-no-float-regs.c:23 +// HARDFLOAT: warning: '-mhard-float': selected processor lacks floating point registers +// NOWARN-NOT: selected processor lacks floating point registers For future changes lik

[PATCH] D99201: [HIP] Diagnose unaligned atomic for amdgpu

MaskRay added inline comments. Comment at: clang/test/Driver/hip-options.hip:144 + +// RUN: %clang -### -target x86_64-unknown-linux-gnu -nogpuinc -nogpulib \ +// RUN: --cuda-gpu-arch=gfx906 %s 2>&1 | FileCheck -check-prefix=WARN-ATOMIC %s Prefer `--target=` t

[PATCH] D152090: [clang][Driver] Add -fcaret-diagnostics-max-lines as a driver option

MaskRay added a comment. In D152090#4398097 , @tbaeder wrote: > In D152090#4395827 , @MaskRay wrote: > >> `def fcaret_diagnostics_max_lines` in `Options.td` has the `NoDriverOption` >> flag. Move it to other plac

[PATCH] D152285: Add support for the NO_COLOR environment variable

MaskRay added a comment. Thanks for the detailed description. I am generally conservative when it comes to new use cases for environment variables. However, `NO_COLOR` seems to become a standard and a large number of tools respect it, I think it is the right call to support it. If we don't int

[PATCH] D151863: [x86][MC] Fix movdir64b addressing

MaskRay added inline comments. Comment at: llvm/test/CodeGen/X86/movdir64b-inline-asm-x86_64.ll:1 +; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 2 +; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+movdir64b | FileCheck

[PATCH] D152285: Add support for the NO_COLOR environment variable

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. LGTM. Glad that my comment on https://github.com/llvm/llvm-project/issues/23983 might bring attention to users. I'd like to make my stance softer: if we find `CLICOLOR_FORCE` useful, we can

[PATCH] D152285: Add support for the NO_COLOR environment variable

MaskRay added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2348 + llvm::sys::Process::GetEnv("NO_COLOR"); + NoColor && !NoColor->empty() && NoColor->at(0) != '\0') { +// If the user set the NO_COLOR environment variable, we'll honor tha

[PATCH] D152285: Add support for the NO_COLOR environment variable

MaskRay added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2348 + llvm::sys::Process::GetEnv("NO_COLOR"); + NoColor && !NoColor->empty() && NoColor->at(0) != '\0') { +// If the user set the NO_COLOR environment variable, we'll honor tha

[PATCH] D152207: [HIP] Instruct lld to go through all archives

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/HIPAMD.cpp:165 + // --whole-archive flag such that all global symbols would be linked in. + LldArgs.push_back("--whole-archive"); + Though not strictly required, it's usually better to end `

[PATCH] D152207: [HIP] Instruct lld to go through all archives

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/HIPAMD.cpp:184 + // pair with the --whole-archive being added previously + LldArgs.push_back("--no-whole-archive"); For complete sentences in comments, capitalize and add a full stop. Act

[PATCH] D148385: [RISCV] Implement KCFI operand bundle lowering

MaskRay added a comment. Thanks for the description. The code generally looks good. If the kernel wants a specific code sequence, implementing conditions in the LLVMCodeGen looks good to me. Consider adding a link to the discussion. I wonder whether it will be more useful to give more context t

[PATCH] D148385: [RISCV] Implement KCFI operand bundle lowering

MaskRay added inline comments. Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:14427 SDValue Callee = CLI.Callee; + bool IsCFICall = CLI.CB && CLI.CB->isIndirectCall() && CLI.CFIType; bool &IsTailCall = CLI.IsTailCall; If `CLI.CFIType != 0`, is `CL

[PATCH] D151730: [RISCV] Support target attribute for function

MaskRay added inline comments. Comment at: clang/test/CodeGen/RISCV/riscv-func-attr-target.c:2 +// REQUIRES: riscv-registered-target +// RUN: %clang -target riscv64 -march=rv64g -S %s -o - | FileCheck %s --check-prefix=CHECK-ASM +// RUN: %clang -target riscv64 -march=rv64g -emit

[PATCH] D152279: [Driver] Default -msmall-data-limit= to 0

MaskRay added a comment. In D152279#4405901 , @asb wrote: > One of the key things we've been discussing on this at the LLVM call is that > we probably want to keep the small data limit for embedded targets. It'd be useful to hear from some concrete embe

[PATCH] D142933: [Driver] Add -print-multi-flags-experimental option

MaskRay accepted this revision. MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChain.cpp:266 + + // Sort and remove duplicates + std::sort(Result.begin(), Result.end()); Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

[PATCH] D152090: [clang][Driver] Add -fcaret-diagnostics-max-lines as a driver option

MaskRay added inline comments. Comment at: clang/include/clang/Driver/Options.td:2303 + Group, Flags<[NoXarchOption, CC1Option, CoreOption]>, + HelpText<"Set the maximum number of source lines to show in a caret diagnostic">, + MarshallingInfoInt, "DiagnosticOptions::Default

[PATCH] D152090: [clang][Driver] Add -fcaret-diagnostics-max-lines as a driver option

MaskRay added a comment. > Add -fcaret-diagnostics-max-lines as a driver option `-fcaret-diagnostics-max-lines=` is more accurate. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152090/new/ https://reviews.llvm.org/D152090 ___ cfe-commits mai

[PATCH] D149946: [LoongArch] Define `ual` feature and override `allowsMisalignedMemoryAccesses`

MaskRay added a comment. > [1]: > https://github.com/torvalds/linux/blob/master/arch/loongarch/include/asm/cpu.h#L77 > [2]: > https://github.com/torvalds/linux/blob/master/arch/loongarch/kernel/proc.c#L75 `master` will point to different commits. It'd be better to use a specific commit for fut

[PATCH] D149946: [LoongArch] Define `ual` feature and override `allowsMisalignedMemoryAccesses`

MaskRay added a comment. I'll make `-munaligned-access` `TargetSpecific` (D151590 ) to report errors for unsupported targets. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149946/new/ https://reviews.llvm.org/D149

[PATCH] D152090: [clang][Driver] Add -fcaret-diagnostics-max-lines as a driver option

MaskRay added a comment. In D152090#4407632 , @dyung wrote: > If we do not accept the form "`-fcaret-diagnostics-max-lines n`", can we add > a negative for that? Or if we do a positive test for it as well? I think this is not very useful. This is a new

[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

MaskRay added a comment. In D141907#4094748 , @MaskRay wrote: > [...] > edeaf16f2c2f02d6e43312d48d26d354d87913f3 (2011) added the CMake variable > `CLANG_RESOURCE_DIR` but did not explain why. > In the long term, the CMake variable `CLANG_RESOURCE_DIR`

[PATCH] D152433: [ARM,AArch64] Add a full set of -mtp= options.

MaskRay added inline comments. Comment at: clang/test/Driver/clang-translation.c:132 +// ARMv7_THREAD_POINTER-HARD: "-target-feature" "+read-tp-tpidruro" + +// RUN: %clang -target armv7-linux -mtp=tpidruro -### -S %s 2>&1 | \ `clang-translation.c`'s job is shifti

[PATCH] D151683: [clang] Enable C++11-style attributes in all language modes

MaskRay added a comment. In D151683#4409633 , @aaron.ballman wrote: > In D151683#4384017 , @erichkeane > wrote: > >> In D151683#4382408 , @philnik >> wrote: >> >>> No.

[PATCH] D152604: [Driver] Default -fsanitize-address-globals-dead-stripping to true for ELF

MaskRay created this revision. MaskRay added reviewers: Sanitizers, aeubanks, rnk, eugenis, phosek, probinson. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. -fsanitize-address-globals-dead-stripping is

[PATCH] D152650: [docs] Improve UndefinedBehaviorSanitizer.rst

MaskRay created this revision. MaskRay added reviewers: Sanitizers, vitalybuka. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. - Mention that -fsanitize= and -fno-sanitize= apply to check groups. - Ment

[PATCH] D152090: [clang][Driver] Add -fcaret-diagnostics-max-lines= as a driver option

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. LGTM. Comment at: clang/include/clang/Driver/Options.td:2303 + Group, Flags<[NoXarchOption, CC1Option, CoreOption]>, + HelpText<"Set the maximum number of source lines to

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

MaskRay added inline comments. Comment at: clang/test/CodeGen/xray-function-index.cpp:1 +// REQUIRES: x86-registered-target +// RUN: %clang_cc1 -fxray-instrument -x c++ -std=c++11 -triple x86_64-unknown-linux-gnu -S -o - %s | FileCheck %s --check-prefix

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

MaskRay added a comment. This is a kinda odd case. Normally we should enable the option in the driver when the feature actually works. However, `Triple.isMacOSX()` is allowed before the feature actually works and `compiler-rt/test/xray/lit.cfg.py` tests it. `clang/test/Driver/XRay/` is broken a

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

MaskRay added a comment. > This option has undergone several refactorings and got inverted along the > way. The XRayOmitFunctionIndex flag governs codegen behavior, *omitting* > "xray_fn_idx" section if it is set. But the command-line flag behavior was > not adjusted at the time. Right now it's

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

MaskRay added inline comments. Comment at: clang/include/clang/Driver/Options.td:2219 defm xray_function_index : BoolFOption<"xray-function-index", - CodeGenOpts<"XRayOmitFunctionIndex">, DefaultTrue, - NegFlag, DefaultFalse, + NegFlaghttps://reviews.llvm.org/D145848/new/ ht

[PATCH] D152650: [docs] Improve UndefinedBehaviorSanitizer.rst

MaskRay added inline comments. Comment at: clang/docs/UndefinedBehaviorSanitizer.rst:62 + # -fno-sanitize=undefined nullifies the previous -fsanitize=undefined. + % clang -fsanitize=undefined -fno-sanitize=undefined -fsanitize=alignment a.c + vitalybuka wrote:

[PATCH] D152650: [docs] Improve UndefinedBehaviorSanitizer.rst

MaskRay updated this revision to Diff 530645. MaskRay marked an inline comment as done. MaskRay edited the summary of this revision. MaskRay added a comment. better -fsanitize=... -fno-sanitize example Add "If the signal is not caught, the program will typically terminate due to a ``SIGILL`` or `

[PATCH] D152650: [docs] Improve UndefinedBehaviorSanitizer.rst

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGee0367ef13f8: [docs] Improve UndefinedBehaviorSanitizer.rst (authored by MaskRay). Changed prior to commit: https://reviews.llvm.org/D152650?vs=53

[PATCH] D149444: [ARM] Allow codegen for Armv6m eXecute-Only (XO) sections

MaskRay accepted this revision. MaskRay added a comment. > Actual implementation of code-gen for v6m will follow in follow-up patches, > which will include an implementation of relocations needed to support this. Normally we implement the feature in llvm/ first, and the `clang/lib/Driver` chang

[PATCH] D152604: [Driver] Default -fsanitize-address-globals-dead-stripping to true for ELF

MaskRay added a comment. In D152604#4415392 , @rnk wrote: > I think there's a fair bit more cleanup and simplification to be done, see > asanUsesGlobalsGC > > and t

[PATCH] D152604: [Driver] Default -fsanitize-address-globals-dead-stripping to true for ELF

This revision was automatically updated to reflect the committed changes. Closed by commit rGa8d3ae712290: [Driver] Default -fsanitize-address-globals-dead-stripping to true for ELF (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D125272: [clang] Add -fcheck-new support

MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: clang/test/CodeGenCXX/fcheck-new.cpp:3 +// RUN: %clang_cc1 -fcheck-new -triple x86_64-linux-gnu -S -disable-O0-optnone \ +// RUN: -emit-llvm -o - %s | opt

[PATCH] D152762: [clang][docs] Update SanitizerSpecialCaseList docs

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. > "The meaning of * in regular expression for entity names is different - it is > treated as in shell wildcarding." Right. See `SpecialCaseList.cpp` > I've removed fun:MyFooBar since it mad

[PATCH] D152762: [clang][docs] Update SanitizerSpecialCaseList docs

MaskRay added a comment. For this patch, the best test is actually `llvm/unittests/Support/SpecialCaseListTest.cpp`. `clang/test/CodeGen/profile-filter-new.c` is a bit loosely connected to the gist of the patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

[PATCH] D152279: [Driver] Default -msmall-data-limit= to 0

MaskRay added a comment. In D152279#4414574 , @hiraditya wrote: > In D152279#4406896 , @MaskRay wrote: > >> In D152279#4405901 , @asb wrote: >> >>> One of the key things w

[PATCH] D152570: [clang] Fix filename remapping in template arguments

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Looks great! > [clang] Fix filename remapping in template arguments Personally I don't consider this a bug fix, but this changes does move into a desired direction, by making `-fmacro-prefi

[PATCH] D151590: [Driver] Add ClangFlags::TargetSpecific to simplify err_drv_unsupported_opt_for_target processing

MaskRay added a comment. @jansvoboda11 For ELF operating systems (that traditionally use GCC), I think unsupported `-m*` options as errors are very desired as that's the GCC behavior and some configure system will prefer it. I think x86 `-m*` options are the most possible options to be misused

[PATCH] D152856: [Driver] Allow warning for unclaimed TargetSpecific options

MaskRay created this revision. MaskRay added reviewers: jansvoboda11, mstorsjo, xen0n, SixWeining. Herald added a subscriber: hiraditya. Herald added a project: All. MaskRay requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits.

[PATCH] D152570: [clang] Apply -fmacro-prefix-map to anonymous tags in template arguments

MaskRay added a comment. In D152570#4417752 , @dankm wrote: > @MaskRay if this meets your approval, are you able to commit it for me? LG, but give @aaron.ballman and @shafik some time if they have opinions. In D152570#4416201

[PATCH] D152856: [Driver] Allow warning for unclaimed TargetSpecific options

MaskRay added inline comments. Comment at: llvm/include/llvm/Option/Arg.h:53 + /// This is used for generating an "argument unused" warning (without + /// clang::driver::options::TargetSpecific) or "unsupported option" error + /// (with TargetSpecific). mstors

[PATCH] D148827: -fsanitize=function: support C

MaskRay added a comment. In D148827#4422498 , @glandium wrote: > In D148827#4379764 , @MaskRay wrote: > >> These cases are UB and should be caught. It's not an excuse that they use C. > > Are they really though?

[PATCH] D148827: -fsanitize=function: support C

MaskRay added a comment. In D148827#4422709 , @glandium wrote: > OTOH. 6.2.7.3: > >> A composite type can be constructed from two types that are compatible; it >> is a type that is compatible with both of the two types (...) > > 6.2.7.5: > >> EXAMPLE Giv

[PATCH] D152279: [Driver] Default -msmall-data-limit= to 0

MaskRay added a comment. In D152279#4420312 , @asb wrote: > In D152279#4415974 , @MaskRay wrote: > >> However, RISC-V `-msmall-data-limit=` is probably a case warranting a >> difference. >> The global pointer rel

[PATCH] D148385: [RISCV] Implement KCFI operand bundle lowering

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Will be good to wait a few days to give regular RISC-V backend developers a chance to leave any final comments. Comment at: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp:332 +

[PATCH] D152279: [Driver] Default -msmall-data-limit= to 0

MaskRay added a comment. Thank you for chiming in. I disagree that we keep default=8 for embedded without understanding (a) why 8 provides values and (b) justifying that the value is significant enough for embedded to be different. I think Alex's argument "I think we can generally expect more w

<    29   30   31   32   33   34   35   36   37   38   >