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

MaskRay added a comment. (I substantially edited my previous comment.) It now says this: > If the link mixes bitcode files and ELF relocatable files, for a function in > a bitcode file, `F.hasAddressTaken()` doesn't indicate that its address is > not taken by an ELF relocatable file. Reposito

[PATCH] D140332: [ADT] Alias llvm::Optional to std::optional

MaskRay added a comment. Can you push `using OptionalFileEntryRef = CustomizableOptional;` and the renaming as a separate commit to make this patch smaller? There is a smaller that this rename may be reverted. 205c0589f918f95d2f2c586a01bea2716d73d603

[PATCH] D140035: [X86] Prevent -mibt-seal to work together with -flto=thin

MaskRay added a comment. I did not notice this patch (and https://github.com/ClangBuiltLinux/linux/issues/1737#issuecomment-1310741237) but make a comment yesterday while studying CFI schemes in llvm-project: https://reviews.llvm.org/D116070#4004060 As mentioned, the `isUsedInRegularObj` issue

[PATCH] D138337: Add support for kcfi-seal optimization with LTO

MaskRay added a comment. (Sorry for my belated response.) If we make ThinLTO properly track combined the address-taken property, and combine precise `addressTaken` and `VisibleToRegularObj`, it seems that we can use this condition to decide whether ENDBR is needed with an appropriate code mode

[PATCH] D140363: Remove incorrectly implemented -mibt-seal

MaskRay created this revision. MaskRay added reviewers: joaomoreira, kees, nickdesaulniers, pcc, samitolvanen, xiangzhangllvm. Herald added subscribers: StephenFan, pengfei, hiraditya. Herald added a project: All. MaskRay requested review of this revision. Herald added projects: clang, LLVM. Heral

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

MaskRay added a comment. Sent D140363 to remove this incorrectly implemented option. Comment at: llvm/test/CodeGen/X86/ibtseal-small.ll:1 +; RUN: llc < %s -O2 -mtriple=x86_64-unknown-linux-gnu -x86-indirect-branch-tracking --code-model=small

[PATCH] D127812: [AArch64] FMV support and necessary target features dependencies.

MaskRay added a comment. In D127812#4011437 , @hctim wrote: > LDFLAGS="$LDFLAGS -Wl,--rpath=/tmp/2/lib" # < use the instrumented libcxx > from step 2 I think -Wl,-rpath and -isystem are somewhat different. sanitizer_ldflags="-Wl,--rpath=$libcxx_o

[PATCH] D140011: [clang][compiler-rt] Support LLVM_ENABLE_PER_TARGET_RUNTIME_DIR on Arm Linux and BSD

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

[PATCH] D140363: Remove incorrectly implemented -mibt-seal

This revision was automatically updated to reflect the committed changes. Closed by commit rG69243cdb926b: Remove incorrectly implemented -mibt-seal (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140363/new/ https://reviews.llv

[PATCH] D140224: [Driver] Revert D139717 and add -Xparser instead

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. I'll reject [-\Xparser for a while as well. This is a valid amendment to D139717 , so I don't think it needs more approval. Repository: rG LLVM Github

[PATCH] D140224: [Driver] Revert D139717 and add -Xparser instead

This revision was automatically updated to reflect the committed changes. Closed by commit rG46cd3127fe54: [Driver] Revert D139717 and add -Xparser/-Xcompiler instead (authored by MaskRay). Changed prior to commit: https://reviews.llvm.org/D140224?vs=483565&id=484938#toc Repository: rG LLVM

[PATCH] D108265: .clang-tidy: Push variable related readability-identifier-naming options down to projects

MaskRay updated this revision to Diff 484955. MaskRay edited the summary of this revision. MaskRay added a comment. Herald added a reviewer: bollu. Herald added subscribers: yota9, ayermolo, StephenFan. Herald added a reviewer: rafauler. Herald added a reviewer: Amir. Herald added a reviewer: maksf

[PATCH] D108265: .clang-tidy: Push variable related readability-identifier-naming options down to projects

MaskRay added a comment. Created D140585 to update `llvm/docs/CodingStandards.rst`. Changed the summary to mention https://discourse.llvm.org/t/top-level-clang-tidy-options-and-variablename-suggestion-on-codingstandards/58783 Repository: rG LLVM Github Mono

[PATCH] D140363: Remove incorrectly implemented -mibt-seal

MaskRay added a comment. In D140363#4014131 , @joaomoreira wrote: > FWIIW, agreed on removing this until we figure out how to make it work > properly. Thanks for the patch @MaskRay. Thanks for accepting the removal:) Repository: rG LLVM Github Mono

[PATCH] D140224: [Driver] Revert D139717 and add -Xparser instead

MaskRay added a comment. In D140224#4014203 , @davide wrote: > @MaskRay Roy hasn't replied. We found other spellings that break (e.g. > `-Xcctests` or something). Revert this patch until we find an agreement. D139717 (

[PATCH] D140224: [Driver] Revert D139717 and add -Xparser instead

MaskRay added a comment. In D140224#4014234 , @davide wrote: > In D140224#4014230 , @MaskRay wrote: > >> In D140224#4014203 , @davide wrote: >> >>> @MaskRay Roy hasn't rep

[PATCH] D140224: [Driver] Revert D139717 and add -Xparser instead

MaskRay added a comment. In D140224#4014243 , @rsundahl wrote: >> I'll reject [-\Xparser for a while as well. This is a valid amendment to >> D139717 , so I don't think it needs more >> approval. > > We have projects t

[PATCH] D140224: [Driver] Revert D139717 and add -Xparser instead

MaskRay added a comment. In D140224#4014246 , @davide wrote: > In D140224#4014245 , @MaskRay wrote: > >> In D140224#4014234 , @davide wrote: >> >>> In D140224#4014230

[PATCH] D78441: Delete NaCl support

MaskRay added a comment. Herald added subscribers: llvm-commits, StephenFan, pengfei. Herald added projects: LLVM, All. @dschuff Is NaCL dead in 2022 ? :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78441/new/ https://reviews.llvm.org/D78

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

MaskRay requested changes to this revision. MaskRay added a comment. This revision now requires changes to proceed. Most targets reject `-p` now. It's unnecessary to have another z/OS specific diagnostic. So this patch can be abandoned. % fclang -p a.cc clang-16: error: unsupported option '-

[PATCH] D113638: [xray] Add support for hexagon architecture

MaskRay added a comment. Herald added a subscriber: Enna1. Herald added a project: All. `recordSled(CurSled, MI, Kind, 0);` is not intended. All new ports should use version 2. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113638/new/ https://revi

[PATCH] D140690: [compiler-rt][dfsan] Enable loongarch64 and add test support

MaskRay added a comment. In D140690#4018617 , @browneee wrote: > In D140690#4018043 , @SixWeining > wrote: > >> We notice that DFSan is a work in progress >>

[PATCH] D140743: [DFSan] Fix overuse of REQUIRES: in tests.

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

[PATCH] D136554: Implement CWG2631

MaskRay added a comment. In D136554#4019135 , @rupprecht wrote: > [...] > The undefined symbols before are all provided by libc++, so those are fine. > After, the new undefined symbol for the lambda cannot be resolved. Depending > on how the linker is i

[PATCH] D140743: [DFSan] Fix overuse of REQUIRES: in tests.

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Linux.cpp:766 Res |= SanitizerKind::SafeStack; - if (IsX86_64 || IsMIPS64 || IsAArch64) + if (IsX86_64) Res |= SanitizerKind::DataFlow; This change is still useful and worth submitti

[PATCH] D136554: Implement CWG2631

MaskRay added a comment. @rupprecht You may consider contributing some interesting tests in a separate patch:) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136554/new/ https://reviews.llvm.org/D136554

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

MaskRay added inline comments. Comment at: llvm/test/CodeGen/Mips/abiflags-2008-fp64.ll:1 +; RUN: llc %s -o - | FileCheck %s + Add `;; ` file-level comment what this test does. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D140868: [C] Make (c ? e1 : e2) noreturn only if both e1 and e2 are noreturn

MaskRay created this revision. MaskRay added reviewers: aaron.ballman, mizvekov, rjmccall. 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. In C mode, if e1 is noret

[PATCH] D140868: [C] Make (c ? e1 : e2) noreturn only if both e1 and e2 are noreturn

MaskRay updated this revision to Diff 485912. MaskRay added a comment. use `%itanium_abi_triple`. remove `-S` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140868/new/ https://reviews.llvm.org/D140868 Files: clang/include/clang/AST/ASTContext.h

[PATCH] D140817: [Driver] move NetBSD header search path management to the driver

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

[PATCH] D140868: [C] Make (c ? e1 : e2) noreturn only if both e1 and e2 are noreturn

MaskRay updated this revision to Diff 486061. MaskRay marked an inline comment as done. MaskRay added a comment. use @rjmccall's comment (thanks a lot!) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140868/new/ https://reviews.llvm.org/D140868 Fil

[PATCH] D108265: .clang-tidy: Push variable related readability-identifier-naming options down to projects

MaskRay added a comment. I'll wait a bit and land this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108265/new/ https://reviews.llvm.org/D108265 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D140896: [WIP] Move from llvm::makeArrayRef to ArrayRef deduction guides

MaskRay accepted this revision. MaskRay added a comment. LGTM if `makeArrayRef` is kept in this patch. Do we foresee compiler bugs (for supported compilers) in this area? If there may be a risk, consider migrate a part of the code base first. CHANGES SINCE LAST ACTION https://reviews.llvm.or

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

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/ZOS.cpp:22 +: ToolChain(D, Triple, Args) { + for (Arg *A : Args.filtered(options::OPT_pg)) { +auto ArgString = A->getAsString(Args); francii wrote: > @MaskRay we still need `-pg` to e

[PATCH] D140868: [C] Make (c ? e1 : e2) noreturn only if both e1 and e2 are noreturn

MaskRay added a comment. I am not a C language lawyer :) I wonder what else should be done to move this patch forward. The https://github.com/llvm/llvm-project/issues/59792 has got some traction and has been added a candidate for the next 15.x patch release https://github.com/llvm/llvm-project/

[PATCH] D136554: Implement CWG2631

MaskRay added a comment. In D136554#4021754 , @cor3ntin wrote: > In D136554#4020387 , @MaskRay wrote: > >> @rupprecht You may consider contributing some interesting tests (which work >> before this patch) in a se

[PATCH] D140868: [C] Make (c ? e1 : e2) noreturn only if both e1 and e2 are noreturn

MaskRay updated this revision to Diff 486453. MaskRay added a comment. show that C23 `[[noreturn]]` codegen is unaffected. The diff base improves `attr-noreturn.c` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140868/new/ https://reviews.llvm.org/

[PATCH] D140757: [Hexagon][VE][WebAssembly] Define __GCC_HAVE_SYNC_COMPARE_AND_SWAP macros

MaskRay accepted this revision. MaskRay added inline comments. Comment at: clang/test/Preprocessor/predefined-arch-macros.c:4337 +// RUN: %clang -E -dM %s -o - 2>&1 \ +// RUN: -target hexagon-unknown-linux \ +// RUN: | FileCheck -match-full-lines %s -check-prefix=CHECK_HEXA

[PATCH] D140868: [C] Make (c ? e1 : e2) noreturn only if both e1 and e2 are noreturn

MaskRay updated this revision to Diff 486641. MaskRay edited the summary of this revision. MaskRay added a comment. add release note (this makes it slightly difficult to backport) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140868/new/ https://re

[PATCH] D140868: [C] Make (c ? e1 : e2) noreturn only if both e1 and e2 are noreturn

This revision was automatically updated to reflect the committed changes. Closed by commit rGcf8fd210a35c: [C] Make (c ? e1 : e2) noreturn only if both e1 and e2 are noreturn (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140868

[PATCH] D141070: [LoongArch] Define __GCC_HAVE_SYNC_COMPARE_AND_SWAP macros

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Wait for LoongArch folks:) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141070/new/ https://reviews.llvm.org/D141070 ___

[PATCH] D140999: [NFC][TargetParser] Deprecate llvm/Support/AArch64TargetParser.h

MaskRay accepted this revision. MaskRay added a comment. I don't think it is necessary to deprecate the old header then delete it after 16.0.0 is branched. llvm/Support/AArch64TargetParser.h has very few open-source out-of-tree uses. Perhaps only ldc `driver/targetmachine.cpp` uses the header. S

[PATCH] D138329: [-Wunsafe-buffer-usage] Add a new recursive matcher to replace `forEachDescendant` in unsafe buffer check

MaskRay added a comment. For relands, you can just use `Differential Revision: https://reviews.llvm.org/D138321` instead of `Related differential revision: https://reviews.llvm.org/D138329`. `Revert "Revert ` can be replaced with `Reland` to emphasize that it is a reland (it can be omitted as w

[PATCH] D80392: [mips][mc][clang] Use pc-relative relocations in .eh_frame

MaskRay added a comment. In D80392#4028011 , @wzssyqa wrote: > When link with ld.bfd, we get: > > ld/ld-new: .eh_frame_hdr entry overflow > ld/ld-new: final link failed: bad value > > It seems due to the `initial_loc` in the object from LLVM is quite b

[PATCH] D141172: [ModuleUtils][KCFI] Set patchable-function-prefix for synthesized functions

MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: llvm/lib/Transforms/Utils/ModuleUtils.cpp:169 +unsigned Offset = MD->getZExtValue(); +if (Offset) + F.addFnAttr("patchable-function-prefix", st

[PATCH] D137381: [clang][compiler-rt] Exception escape out of an non-unwinding function is an undefined behaviour

MaskRay added a comment. I am unfamiliar with Clang codegen so I am just commenting about what a user may feel about this feature. `compiler-rt/test/ubsan/TestCases/Misc/exception-escape.cpp` cannot demonstrate the value of the new UBSan check `-fsanitize=exception-escape`, as the executable w

[PATCH] D139114: [Clang][Sema] Enabled implicit conversion warning for CompoundAssignment operator.

MaskRay added inline comments. Comment at: libunwind/src/AddressSpace.hpp:249 const uint8_t *pend = (uint8_t *)end; - int64_t result = 0; + uint64_t result = 0; int bit = 0; Thanks. LGTM for this file. You can just fix libunwind in a separate commit so t

[PATCH] D139114: [Clang][Sema] Enabled implicit conversion warning for CompoundAssignment operator.

MaskRay added inline comments. Comment at: clang/docs/ReleaseNotes.rst:354 ^^^ +- Clang now supports implicit conversion warnings (-Wsign-conversion, + Wshorten-64-to-32, etc) for compound assignment operators (like +=, >=, etc)

[PATCH] D140693: [Driver][RISCV] Adjust the priority between -mcpu, -mtune and -march

MaskRay added a comment. (Haven't looked carefully ) but the Pipeline model: matches AArch64 (ideal model in my view https://community.arm.com/arm-community-blogs/b/tools-software-ides-blog/posts/compiler-flags-across-architectures-march-mtune-and-mcpu) (I probably need to update my https://mas

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

MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Herald added a subscriber: StephenFan. Comment at: clang/test/Driver/x86-march.c:91 // +// RUN: %clang --target=x86_64-unknown-unknown -c -### %s -march=raptorlake 2>

[PATCH] D135411: Add generic KCFI operand bundle lowering

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChain.cpp:1082 SanitizerKind::CFICastStrict | SanitizerKind::FloatDivideByZero | - SanitizerKind::UnsignedIntegerOverflow | + SanitizerKind::KCFI | SanitizerKind::UnsignedIntegerOverflow | San

[PATCH] D135411: Add generic KCFI operand bundle lowering

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChain.cpp:1082 SanitizerKind::CFICastStrict | SanitizerKind::FloatDivideByZero | - SanitizerKind::UnsignedIntegerOverflow | + SanitizerKind::KCFI | SanitizerKind::UnsignedIntegerOverflow | San

[PATCH] D135411: Add generic KCFI operand bundle lowering

MaskRay added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/KCFI.cpp:47 + // Find call instructions with KCFI operand bundles. + SmallVector KCFICalls; + for (Instruction &I : instructions(F)) { You can omit `, 8` to use the default (also 8)

[PATCH] D136146: [Clang][LoongArch] Handle -march/-m{single,double,soft}-float/-mfpu options

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Arch/LoongArch.cpp:31 + options::OPT_msingle_float, + options::OPT_msoft_float)) { +if (A->getOption().matches(options::OPT_mdouble_f

[PATCH] D137227: [asan] Default to -fsanitize-address-use-odr-indicator

MaskRay created this revision. MaskRay added reviewers: Sanitizers, eugenis, kcc, kstoimenov, vitalybuka. Herald added subscribers: Enna1, StephenFan, sunfish, hiraditya, dschuff. Herald added a project: All. MaskRay requested review of this revision. Herald added subscribers: llvm-commits, cfe-com

[PATCH] D137227: [asan] Default to -fsanitize-address-use-odr-indicator for non-Windows

MaskRay updated this revision to Diff 472533. MaskRay retitled this revision from "[asan] Default to -fsanitize-address-use-odr-indicator" to "[asan] Default to -fsanitize-address-use-odr-indicator for non-Windows". MaskRay edited the summary of this revision. MaskRay added a comment. Herald adde

[PATCH] D137263: add boundary check for ASTUnresolvedSet::erase

MaskRay added a comment. The summary and comments use MarkDown. You comment is currently formatted strangely because you did not use fenced code blocks :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137263/new/ https://reviews.llvm.org/D137263 ___

[PATCH] D137227: [asan] Default to -fsanitize-address-use-odr-indicator for non-Windows

MaskRay updated this revision to Diff 472827. MaskRay marked 2 inline comments as done. MaskRay added a comment. address comments Thanks for the quick review:) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137227/new/ https://reviews.llvm.org/D137

[PATCH] D137227: [asan] Default to -fsanitize-address-use-odr-indicator for non-Windows

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG1ada819c237b: [asan] Default to -fsanitize-address-use-odr-indicator for non-Windows (authored by MaskRay). Repository: rG LLVM Github Monorepo C

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

MaskRay abandoned this revision. MaskRay added a comment. Herald added subscribers: Enna1, jeroen.dobbelaere, StephenFan. Herald added a project: All. Obsoleted by D137227 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D130586: [cmake] Use `CMAKE_INSTALL_LIBDIR` too

MaskRay added a comment. Herald added a subscriber: Moerafaat. > We held off on this before as `LLVM_LIBDIR_SUFFIX` conflicted with it. Now we > return this. > > I imagine this is too potentially-breaking to make LLVM 15. That's fine. ... These sentences are no longer relevant and should be remo

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

MaskRay added inline comments. 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 + MaskRay wrote: > Move this to `test/Preprocessor/init-arm.c` test/Prepro

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

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

[PATCH] D137375: [AIX][pg] Add Correct Search Paths for Profiled Libraries

MaskRay added a comment. This needs a clang/test/Driver test. And, does this work with --sysroot= ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137375/new/ https://reviews.llvm.org/D137375 ___ cfe-comm

[PATCH] D137268: [clang][Headers] Do not define varargs macros for __need___va_list

MaskRay added inline comments. Comment at: clang/lib/Headers/stdarg.h:17 +#ifndef __GNUC_VA_LIST +#define __GNUC_VA_LIST 1 +typedef __builtin_va_list __gnuc_va_list; To match gcc stdarg.h, `#define __GNUC_VA_LIST` Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D136846: [Driver] Add -fsample-profile-use-profi

MaskRay requested changes to this revision. MaskRay added inline comments. This revision now requires changes to proceed. Comment at: clang/docs/UsersManual.rst:2246 + blocks. Profi (profile inference) algorithm can infer block and edge counts + to fix them. For applying this,

[PATCH] D137381: [clang][compiler-rt] Exception escape out of an non-unwinding function is an undefined behaviour

MaskRay added a comment. In your example, `clang++ a.cc; ./a.out` gives a libstdc++ error: terminate called after throwing an instance of 'int' libc++'s is similar. footgun is nounwind (due to the GNU pure attribute), so Clang uses `call` instead of `invoke` and the function call does not ge

[PATCH] D137227: [asan] Default to -fsanitize-address-use-odr-indicator for non-Windows

MaskRay added a comment. In D137227#3908994 , @sbc100 wrote: > At least for WebAssembly object files, it looks like the symbol table now > contains (what look like) demangled symbols. e.g.: > > $ llvm-nm /tmp/emscripten_temp/command_0.o > 3ef5

[PATCH] D137227: [asan] Default to -fsanitize-address-use-odr-indicator for non-Windows

MaskRay added a comment. In D137227#3909056 , @sbc100 wrote: >> The asan instrumentation just prepends `__odr_asan_gen_` to the symbol name >> to form a new symbol name. For ELF every byte except `\0` can be used in a >> symbol name, and this is totally

[PATCH] D135411: Add generic KCFI operand bundle lowering

MaskRay added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/KCFI.cpp:61 + if (F.hasFnAttribute("patchable-function-prefix")) +report_fatal_error("-fpatchable-function-entry=N,M, where M>0 is not " + "compatible with -fsanitize=kcfi on

[PATCH] D135411: Add generic KCFI operand bundle lowering

MaskRay accepted this revision. MaskRay added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/KCFI.cpp:42 +namespace { +class KCFIDiagnosticInfo : public DiagnosticInfo { + const Twine &Msg; If you look at existing DiagnosticInfo subclasses, the

[PATCH] D125860: [clang] Only use major version in resource dir

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. LGTM. Considering https://discourse.llvm.org/t/should-we-continue-embed-the-full-llvm-version-in-lib-clang/62094 and this thread, I think overall people favor this patch. If a distribution

[PATCH] D137263: add boundary check for ASTUnresolvedSet::erase

MaskRay added a comment. We need a regression test. See `clang/test/SemaCXX/using-decl*` for some other using declaration tests. Find a file which is appropriate and add a reduced case there. Use `ninja check-clang-semacxx` to run `clang/test/SemaCXX` tests. Use `$build/bin/llvm-lit -v clang/te

[PATCH] D136846: [Driver] Add -fsample-profile-use-profi

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. In D136846#3890699 , @wenlei wrote: > Did you see measurable perf boost with profi on autofdo? Or what motivated > you to turn on profi? In most cas

[PATCH] D137268: [clang][Headers] Do not define varargs macros for __need___va_list

MaskRay requested changes to this revision. MaskRay added a comment. This revision now requires changes to proceed. LGTM, but changes are needed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137268/new/ https://reviews.llvm.org/D137268 __

[PATCH] D137267: [clang][Headers] Only define FLT_EVAL_METHOD for C99 and later

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Thanks! C99 and C++11 look good and the new behavior matches GCC. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137267/new/ https://reviews.ll

[PATCH] D137263: add boundary check for ASTUnresolvedSet::erase

MaskRay accepted this revision. MaskRay added a comment. Thanks! Comment at: clang/include/clang/AST/ASTUnresolvedSet.h:73 + void erase(unsigned I) { +if (I == (Decls.size() - 1)) + Decls.pop_back(); `if (I == Decls.size() - 1)` Com

[PATCH] D137263: add boundary check for ASTUnresolvedSet::erase

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb84fd822fa7e: Add boundary check for ASTUnresolvedSet::erase (authored by zhouyizhou, committed by MaskRay). Changed prior to commit: https://revi

[PATCH] D137511: [PPC] Undefine __ppc64__ to match GCC

MaskRay created this revision. MaskRay added reviewers: q66, thesamesam. Herald added subscribers: StephenFan, shchenz, kbarton, nemanjai. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. GCC only defines

[PATCH] D137511: [PPC] Undefine __ppc64__ to match GCC

MaskRay added subscribers: Bdragon28, jhibbits, dim. MaskRay added a comment. In D137511#3911052 , @thesamesam wrote: > This needs to go in Breaking Changes in the release notes, not least so we > can link to it when updating upstreams. > > What do you

[PATCH] D137511: [PPC] Undefine __ppc64__ to match GCC

MaskRay updated this revision to Diff 473543. MaskRay edited the summary of this revision. MaskRay added a comment. Herald added a subscriber: steven.zhang. update releasenote Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137511/new/ https://review

[PATCH] D137511: [PPC] Undefine __ppc64__ to match GCC

MaskRay added a comment. In D137511#3911149 , @nemanjai wrote: > I am going to block this since the uses of the macros in > `clang/lib/Headers/ppc_wrappers` will likely cause build bot failures. clang/lib/Headers/ppc_wrappers was fixed yesterday. The on

[PATCH] D137268: [clang][Headers] Do not define varargs macros for __need___va_list

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

[PATCH] D137659: [Driver] Refactor err_drv_unsupported_option_argument call sites to use llvm::opt::Arg::getSpelling

MaskRay created this revision. MaskRay added a reviewer: lenary. Herald added subscribers: StephenFan, frasercrmck, luismarques, apazos, sameer.abuasal, s.egerton, Jim, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, atanasyan, edward-jones, zzheng, jrtc27, niosHD, sabuasal, simoncook

[PATCH] D137659: [Driver] Refactor err_drv_unsupported_option_argument call sites to use llvm::opt::Arg::getSpelling

This revision was automatically updated to reflect the committed changes. Closed by commit rG8c2c62282fca: [Driver] Refactor err_drv_unsupported_option_argument call sites to use llvm… (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D137153: [X86] Support -march=sierraforest, grandridge, graniterapids.

MaskRay accepted this revision. MaskRay added a comment. Drop trailing period from subject. Comment at: clang/test/Preprocessor/predefined-arch-macros.c:1792 +// RUN: %clang -march=graniterapids -m32 -E -dM %s -o - 2>&1 \ +// RUN: -target i386-unknown-linux \ +// RUN: | F

[PATCH] D136146: [Clang][LoongArch] Handle -march/-m{single,double,soft}-float/-mfpu options

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. In D136146#3911137 , @SixWeining wrote: > Sorry for the late reply. > > Should we choose not to implement the `-mfpu=` option which is not mandatory

[PATCH] D137724: [CMake] Warn when the version is older than 3.20.0.

MaskRay accepted this revision as: MaskRay. MaskRay added a comment. I think `if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)` checks for standalone builds is not necessary. The check in `llvm/CMakeLists.txt` suffices. It's unlikely the users will use different cmake versions to configure

[PATCH] D137838: [RFC][Support] Move TargetParsers to new component

MaskRay added a comment. Moving target-specific parsers outside LLVMSupport LGTM. I even objected a bit when more stuff of this kind was introduced into LLVMSupport. +1 for adding temporary forwarding headers for now to avoid update all `#include` users. Repository: rG LLVM Github Monorepo

[PATCH] D137885: [modules] Support zstd in .pcm file

MaskRay created this revision. MaskRay added reviewers: aaron.ballman, jansvoboda11, rsmith. 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. Extend SM_SLOC_BUFFER_B

[PATCH] D137885: [modules] Support zstd in .pcm file

MaskRay updated this revision to Diff 474953. MaskRay edited the summary of this revision. MaskRay added a comment. use level 9 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137885/new/ https://reviews.llvm.org/D137885 Files: clang/lib/Serializa

[PATCH] D137901: [Clang] `nothrow`-implying attributes should actually manifest `nothrow` attribute (PR58798)

MaskRay added a comment. I think improving diagnostic is useful but `-fsanitize=` is probably not a good place. Instrumenting call sites with `callq __ubsan_handle_exception_escape@PLT` wastes code size. The functionality is better handled somewhere in libc++abi personality related code with

[PATCH] D137381: [clang][compiler-rt] Exception escape out of an non-unwinding function is an undefined behaviour

MaskRay added a comment. I think improving diagnostic is useful but `-fsanitize=` is probably not a good place. Instrumenting call sites with `callq __ubsan_handle_exception_escape@PLT` wastes code size. The functionality is better handled somewhere in libc++abi personality related code with

[PATCH] D136354: [Driver] Enable nested configuration files

MaskRay added inline comments. Comment at: llvm/lib/Support/CommandLine.cpp:1215-1222 +} else if (ArgStr == "--config") { + if (I + 1 == E) +return createStringError(std::errc::invalid_argument, + "Option '--config' requires an arg

[PATCH] D136354: [Driver] Enable nested configuration files

MaskRay accepted this revision. MaskRay added a comment. > To solve such problems, the option --config is allowed inside ... `--config=` Comment at: clang/docs/UsersManual.rst:989 +``linux.opts`` is searched for in the directory ``~/.llvm/os``. Another way to +include a file c

[PATCH] D137217: [LTO][COFF] Use bitcode file names in lto native object file names.

MaskRay added a comment. In D137217#3926601 , @tejohnson wrote: > @MaskRay wondering if this is a good change to make for ELF as well, wdyt? Yes, I think this is a good idea and improves debuggability. The change is non-trivial so so this patch focusing

[PATCH] D137217: [LTO][COFF] Use bitcode file names in lto native object file names.

MaskRay added inline comments. Comment at: lld/COFF/LTO.cpp:229 +StringRef ltoObjName; +if (bitcodeFilePath == "ld-temp.o") { + ltoObjName = MaskRay wrote: > tejohnson wrote: > > zequanwu wrote: > > > tejohnson wrote: > > > > This case should always

[PATCH] D138183: [Driver] move FreeBSD header search path management to the driver

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/FreeBSD.cpp:442 + + addExternCSystemInclude(DriverArgs, CC1Args, + concat(D.SysRoot, "/usr/include")); I think Fuchsia way of checking `if (!D.SysRoot.empty()) {` bef

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

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

[PATCH] D138183: [Driver] move FreeBSD header search path management to the driver

MaskRay added inline comments. Comment at: clang/lib/Lex/InitHeaderSearch.cpp:234 + if (!ShouldAddDefaultIncludePaths(triple)) { llvm_unreachable("Include management is handled in the driver."); } drop braces around simple single statements ===

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