[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

2023-08-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. Comment at: clang/include/clang/Basic/SizedDeallocation.h:23 +namespace clang { +inline llvm::VersionTuple sizedDeallocMinVersion(llvm::Triple::OSType OS) { + switch (OS) { Does this need to be in Ba

[PATCH] D159010: [Driver,X86] Ignore -mfpmath= for assembler input

2023-08-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Tested by parona on Libera Chat IRC. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159010/new/ https://reviews.llvm.org/D159010 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D159010: [Driver,X86] Ignore -mfpmath= for assembler input

2023-08-28 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG081afa3d04a4: [Driver,X86] Ignore -mfpmath= for assembler input (authored by MaskRay). Changed prior to commit: https://reviews.llvm.org/D159010?vs=554010&id=554037#toc Repository: rG LLVM Github Mon

[PATCH] D158071: [clang] Remove rdar links; NFC

2023-08-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158071/new/ https://reviews.llvm.org/D158071 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.o

[PATCH] D159054: [Driver] Removal of C_INCLUDE_DIRS feature

2023-08-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Technically there is some risk but I think the blast radius, if present, is extremely small. `C_INCLUDE_DIRS` is not recognized by GCC. If I use https://sourcegraph.com/search?q=context:gl

[PATCH] D158688: [Driver,ARM,AArch64] Ignore -mbranch-protection= diagnostics for assembler input

2023-08-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D158688#4624267 , @simon_tatham wrote: > The change LGTM, and "agree with gcc" seems like a reasonable justification > in this case. Thank you both! > But I'm curious more generally about what options should / shouldn't be

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

2023-08-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/include/clang/Basic/SizedDeallocation.h:23 +namespace clang { +inline llvm::VersionTuple sizedDeallocMinVersion(llvm::Triple::OSType OS) { + switch (OS) { wangpc wrote: > MaskRay wrote: > > Does this need to be in

[PATCH] D159137: [AIX] Fix Link Issue when `-fprofile-update=[atomic|prefer-atomic]` is in Effect

2023-08-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/AIX.cpp:440 + StringRef Val = A->getValue(); + if (Val == "atomic" || Val == "prefer-atomic") +CmdArgs.push_back("-latomic"); qiongsiwu1 wrote: > This check here is copied fr

[PATCH] D157813: [Driver][VE] Support VPU flag as a feature for VE

2023-08-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Arch/VE.cpp:22 + std::vector &Features) { + // Defaults. + bool EnableVPU = true; kaz7 wrote: > MaskRay wrote: > > Delete the comm

[PATCH] D158614: [UBSan] Disable the function sanitizer on an execute-only target.

2023-08-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Thanks! Comment at: clang/lib/Basic/Sanitizers.cpp:126 + // execute-only output (no data access to code sections). + if (const llvm::opt::Arg *A = + Args.getLast

[PATCH] D158376: [Driver] move DragonFly header search path management to the driver

2023-08-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Lex/InitHeaderSearch.cpp:331 - case llvm::Triple::DragonFly: -AddPath("/usr/include/c++/5.0", CXXSystem, false); -break; 5.0 becomes 8.0 after move? Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D158376: [Driver] move DragonFly header search path management to the driver

2023-08-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Looks great! Comment at: clang/lib/Lex/InitHeaderSearch.cpp:331 - case llvm::Triple::DragonFly: -AddPath("/usr/include/c++/5.0", CXXSystem, false); -break; ---

[PATCH] D117929: [XRay] Add support for RISCV

2023-08-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: compiler-rt/lib/xray/xray_trampoline_riscv32.S:54 + // Handler address will be null if it is not set + beq a2, x0, FunctionEntry_restore + This local symbol doesn't seem useful. We can just use numbers (`

[PATCH] D159016: [clang] Fix assertion failure using -MJ with -fsyntax-only

2023-08-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Looks great! Comment at: clang/test/Driver/compilation_database_fsyntax_only.c:1 +// RUN: mkdir -p %t.workdir && cd %t.workdir +// RUN: %clang -fsyntax-only %s -MJ - 2>&1 |

[PATCH] D159173: [Driver] Report warnings for unlaimed TargetSpecific options for assembler input

2023-08-30 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: jansvoboda11, thesamesam, chill, peter.smith, simon_tatham, stuij. Herald added subscribers: kadircet, kristof.beyls. Herald added a reviewer: ctetreau. Herald added a project: All. MaskRay requested review of this revision. Herald added subs

[PATCH] D158688: [Driver,ARM,AArch64] Ignore -mbranch-protection= diagnostics for assembler input

2023-08-30 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D158688#4625839 , @MaskRay wrote: > In D158688#4624267 , @simon_tatham > wrote: > >> The change LGTM, and "agree with gcc" seems like a reasonable justification >> in this case. > > T

[PATCH] D158963: [CodeGen] Function multi-versioning: don't set comdat for internal linkage resolvers

2023-08-30 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 554756. MaskRay edited the summary of this revision. MaskRay added a comment. add release note Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158963/new/ https://reviews.llvm.org/D158963 Files: clang/docs/Rel

[PATCH] D158963: [CodeGen] Function multi-versioning: don't set comdat for internal linkage resolvers

2023-08-30 Thread Fangrui Song via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG651b2fbc1c7e: [CodeGen] Function multi-versioning: don't set comdat for internal linkage… (authored by MaskRay). Repository: rG LLVM Github Monore

[PATCH] D152495: [Clang][SemaCXX] Add unused warning for variables declared in condition expressions

2023-08-30 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D152495#4628903 , @smeenai wrote: > In D152495#4628877 , @hans wrote: > >> In D152495#4628870 , @goncharov >> wrote: >> >>> due to this change

[PATCH] D159054: [Driver] Removal of C_INCLUDE_DIRS feature

2023-08-30 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D159054#4628278 , @probinson wrote: > In D159054#4626772 , @brad wrote: > >> Just FYI I am not in a rush to commit this. I am posting this more so as a >> means of prodding for discuss

[PATCH] D85309: [Driver] Support GNU ld on Solaris

2023-08-30 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:305 +bool tools::isLinkerGnuLd(const ToolChain &TC, const ArgList &Args) { + // Only used if targetting Solaris. I suppose that this should be in a Solaris specific file to i

[PATCH] D146054: [RISCV] Add --print-supported-extensions support

2023-08-30 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Driver/Options.td:4586 +def print_supported_extensions : Flag<["-", "--"], "print-supported-extensions">, + Group, Flags<[CC1Option, C

[PATCH] D158614: [UBSan] Disable the function sanitizer on an execute-only target.

2023-08-30 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Basic/Sanitizers.cpp:130 +if (A->getOption().matches(clang::driver::options::OPT_mexecute_only) && +llvm::ARM::supportedExecuteOnly(Triple)) { + return true; MaskRay wrote: > I don't think we n

[PATCH] D154014: [SpecialCaseList] Use Globs instead of Regex

2023-08-30 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. > [SpecialCaseList] Use Globs instead of Regex Since Glob is enabled under a non-default header `#!special-case-list-v2`, this subject should be changed. Repository: rG LLVM Github Monor

[PATCH] D154014: [SpecialCaseList] Use Globs instead of Regex

2023-08-30 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > This is a breaking change since some SCLs might use .* or (abc|def) which are > supported regexes but not valid globs. Since we have just cut clang 16.x this > is a good time to make this change. This should be updated, too. 16.x => 17.x Repository: rG LLVM Github

[PATCH] D143305: [clang] Fix -Xarch_ for -mllvm and alike

2023-08-30 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay requested changes to this revision. MaskRay added a comment. This revision now requires changes to proceed. Herald added a subscriber: jplehr. I assume that this is unneeded after :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143305/new/ https://reviews.llvm.org/D143305

[PATCH] D130777: Enable embedded lto for XCOFF.

2023-08-30 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. This needs rebase after fatLTO support. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130777/new/ https://reviews.llvm.org/D130777 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D85309: [Driver] Support GNU ld on Solaris

2023-08-31 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/hip-link-bundle-archive.hip:59 // RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \ -// RUN: --target=x86_64-pc-windows-msvc \ +// RUN: --target=x86_64-pc-windows-msvc -fuse-ld= \ // RUN: -nogpuinc

[PATCH] D159335: [Object] Change OffloadBinary::write to return SmallString<0>

2023-08-31 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: jhuber6, JonChesterfield. 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. SmallString<0> is

[PATCH] D159335: [Object] Change OffloadBinary::write to return SmallString<0>

2023-08-31 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 555258. MaskRay added a comment. update clang-offload-packager Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159335/new/ https://reviews.llvm.org/D159335 Files: clang/tools/clang-linker-wrapper/ClangLinkerWr

[PATCH] D159173: [Driver] Report warnings for unclaimed TargetSpecific options for assembler input

2023-08-31 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Thank you. I am testing more thoroughly. We need this to catch up 17.0.0rc4... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159173/new/ https://reviews.llvm.org/D159173 ___ cfe-

[PATCH] D159173: [Driver] Report warnings for unclaimed TargetSpecific options for assembler input

2023-08-31 Thread Fangrui Song via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe9d454d1c195: [Driver] Report warnings for unclaimed TargetSpecific options for assembler… (authored by MaskRay). Repository: rG LLVM Github Monor

[PATCH] D158688: [Driver,ARM,AArch64] Ignore -mbranch-protection= diagnostics for assembler input

2023-08-31 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay abandoned this revision. MaskRay added a comment. Obsoleted by D159173 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158688/new/ https://reviews.llvm.org/D158688 _

[PATCH] D159352: [Driver] Don't default to DWARF 2 on Solaris

2023-09-01 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Looks great! Comment at: clang/test/CodeGen/dwarf-version.c:7 // RUN: %clang -target x86_64-linux-gnu -gdwarf -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER5 +//

[PATCH] D159335: [Object] Change OffloadBinary::write to return SmallString<0>

2023-09-01 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGf93c271d4cc1: [Object] Change OffloadBinary::write to return SmallString<0> (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159335/new/

[PATCH] D85309: [Driver] Support GNU ld on Solaris

2023-09-01 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. LGTM. Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:55 + StringRef UseLinker = A ? A->getValue() : CLANG_DEFAULT_LINKER; + // FIXME: What about -fuse-ld=? + return

[PATCH] D159373: [clang][auto-init] Remove -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang

2023-09-01 Thread Fangrui Song via Phabricator via cfe-commits
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/D159373/new/ https://reviews.llvm.org/D159373 __

[PATCH] D158963: [CodeGen] Function multi-versioning: don't set comdat for internal linkage resolvers

2023-09-05 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/docs/ReleaseNotes.rst:208 (`#61334 `_) +- For function multi-versioning using the ``target`` or ``target_clones`` + attributes, remove comdat for internal linkage functions. --

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

2022-11-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay marked 2 inline comments as done. MaskRay added inline comments. Comment at: clang/lib/Serialization/ASTReader.cpp:1457 + const llvm::compression::Format F = + Blob.size() >= 2 && memcmp(Blob.data(), "\x1f\x8b", 2) == 0 + ? llvm::compression::Fo

[PATCH] D137101: [CodeView] Replace GHASH hasher by BLAKE3

2022-11-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Herald added a subscriber: StephenFan. LGTM, I switched to blake3 for --build-id={md5,sha1} for ELF :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137101/new/ https://reviews.llvm.org/D137101

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

2022-11-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:846 + auto AddStream = [&](size_t Task, + Twine ModuleName) -> std::unique_ptr { int FD = -1; `Twine` should only be used as const refer

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

2022-11-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. Comment at: llvm/lib/Target/Mips/MipsAsmPrinter.cpp:402 MipsTargetStreamer &TS = getTargetStreamer(); + const MipsTargetMachine &MTM = static_cast(TM); delete blank line Comme

[PATCH] D138221: [HIP] Fix lld failure when devie object is empty

2022-11-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay requested changes to this revision. MaskRay added inline comments. This revision now requires changes to proceed. Comment at: lld/ELF/Driver.cpp:179 .Case("msp430elf", {ELF32LEKind, EM_MSP430}) + .Case("elf64_amdgpu", {ELF64LEKind, EM_AMDGPU})

[PATCH] D138221: [HIP] Fix lld failure when devie object is empty

2022-11-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > Some host relocatable objects may not contain device relocatable objects, > where an empty file is passed to lld, which causes lld to fail. How is an empty file (size=0) passed to lld? If a dummy relocatable object file is parsed to lld, lld can infer the machine type

[PATCH] D138276: TableGen: require tablegen cl::opts to be registered explicitly

2022-11-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Herald added a subscriber: StephenFan. This is similar to `mlir::register*Options` and looks good to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D138376: Use None consistently (NFC)

2022-11-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. LGTM. `enum class NoneType { None = 1 };` is from 0cd22f9540c0591132ec991c51103cf800cf4e24 (2017) for MSVC workaround. I assume it is no longer needed.. Repository: rG LLVM Github Monore

[PATCH] D138221: [HIP] Fix lld failure when devie object is empty

2022-11-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138221/new/ https://reviews.llvm.org/D138221 ___ cfe-commits mailing list cfe-commit

[PATCH] D138221: [HIP] Fix lld failure when devie object is empty

2022-11-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D138221#3941173 , @yaxunl wrote: > In D138221#3939384 , @MaskRay wrote: > >>> Some host relocatable objects may not contain device relocatable objects, >>> where an empty file is passe

[PATCH] D138458: [Clang][Driver] Add KCFI to SupportsCoverage

2022-11-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. > with -fsanitize=trace-{pc,cmp}. `-fsanitize-coverage=` modes such as `trace-{pc,cmp}`. (a) It's `-fsanitize-coverage=`, not `-fsanitize-coverage=trace-{pc,cmp}`. (b) ` -fsanitize=trace-{p

[PATCH] D138458: [Clang][Driver] Add KCFI to SupportsCoverage

2022-11-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/CodeGen/sanitize-coverage.c:94 + // CHECK-NOT: call void @__sanitizer_cov_trace_const_cmp + // KCFI-NOT: call void %{{.}}() [ "kcfi"(i32 {{.*}}) ] + f(); samitolvanen wrote: > MaskRay wrote: > > `%c()` > >

[PATCH] D138466: Don't use Optional::getPointer (NFC)

2022-11-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. `getPointer` asserts `hasValue`, so `&*` is safe, Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138466/new/ https://reviews.llvm.org/D138466

[PATCH] D138464: Return None instead of Optional() (NFC)

2022-11-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. We have `return llvm::None;` and `return None;`. Shall we always prefer `llvm::None`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138464/new

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

2022-11-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Hi @nemanjai , I think there is no relevant `__ppc64__` in llvm-project now. (The libc++ change has landed and `third_party/benchmark` has a non-interesting use as that code has `defined(__powerpc64__)` as well). Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[PATCH] D135411: Add generic KCFI operand bundle lowering

2022-11-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:975 // done on PreLink stage. -if (!IsThinLTOPostLink) +if (!IsThinLTOPostLink) { addSanitizers(TargetTriple, CodeGenOpts, LangOpts, PB); -

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

2022-11-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D137217#3945366 , @glandium wrote: > Almost there, but not quite: > > [task 2022-11-22T23:55:36.341Z] > /builds/worker/fetches/llvm-project/llvm/tools/gold/gold-plugin.cpp:1106:6: > error: no matching function for call to o

[PATCH] D138532: [SPARC][clang] Enable frame pointer optimization by default

2022-11-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > Enable frame pointer optimization by default to match it with other targets. I think we usually follow GCC instead of other targets. I am unfamiliar with sparc. Can you show an example where `-fomit-frame-pointer`/`-fno-omit-frame-pointer` demonstrate a codegen diffe

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

2022-11-22 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG987b49395c60: [PPC] Undefine __ppc64__ to match GCC (authored by MaskRay). Changed prior to commit: https://reviews.llvm.org/D137511?vs=473543&id=477348#toc Repository: rG LLVM Github Monorepo CHANG

[PATCH] D138489: [tsan] Add tsan support for loongarch64

2022-11-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp:269 val = FIRST_32_SECOND_64(1152, 1776); +#elif SANITIZER_LOONGARCH64 + val = 1936; We should add new code in this fallback function. The generic code han

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

2022-11-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay marked an inline comment as done. MaskRay added a comment. Ping:) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137885/new/ https://reviews.llvm.org/D137885 ___ cfe-commits mailing list cfe-commi

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

2022-11-23 Thread Fangrui Song via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGfa7bc386ec75: [modules] Support zstd in .pcm file (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D138552: [docs] Document that the modules can improve the linking time

2022-11-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. This seems comparing different things... As mentioned, header files can be refactored to reduce the number of symbols as well. I'd be curious to learn how linking time can be reduced significantly by deploying modules. Repository: rG LLVM Github Monorepo CHANGES SIN

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

2022-11-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:218 + explicit NameLookup(std::nullopt_t) : Data(nullptr, true) {} explicit NameLookup(std::nullptr_t) : Data(nullptr, false) {} NameLookup() : NameLookup(nullptr) {} --

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

2022-11-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D137885#3947532 , @dyung wrote: > @MaskRay your change seems to be causing 1509 failures on the linux PS4 bot. > Most of the errors look similar to this: > > error: 'error' diagnostics seen but not expected: > (frontend

[PATCH] D97854: [RFC][nsan] A Floating-point numerical sanitizer.

2022-11-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. This looks very interesting and I wish that I can help as well (but I'll likely spend very little time in December due to holidays:) ). Hmm, I can't find it in an email archive. Perhaps there hasn't been a discussion so people are unaware of this work... Repository:

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

2022-11-25 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. The name `llvm/lib/TargetParser` LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137838/new/ https://reviews.llvm.org/D137838 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D138854: [AIX][LTO] Enabling Context Sensitive PGO Options

2022-11-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. LGTM. Comment at: llvm/lib/LTO/LTOCodeGenerator.cpp:125 +cl::desc("Perform context sensitive PGO instrumentation"), +cl::init(false)

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

2022-11-30 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. Please check these builds all work: - default - `-DLLVM_LINK_LLVM_DYLIB=on` - `-DBUILD_SHARED_LIBS=on` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137838/new/ https://reviews.llvm.org/D137

[PATCH] D128372: [Clang-Tidy] Empty Check

2022-12-01 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. I just took a glance and the code looks reasonable. Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:131 +bool HasClear = !Candidates.empty(); + +if (HasClear) { delete blank line Com

[PATCH] D138255: [Driver] -p: change from unused warning to error for most targets

2022-12-01 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138255/new/ https://reviews.llvm.org/D138255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi

[PATCH] D138255: [Driver] -p: change from unused warning to error for most targets

2022-12-02 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Thanks! I think this is better landing before AIX -p patches for clarity, so I am landing it now... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138255/new/ https://reviews.llvm.org/D138255 __

[PATCH] D138255: [Driver] -p: change from unused warning to error for most targets

2022-12-02 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG9f07256a5192: [Driver] -p: change from unused warning to error for most targets (authored by MaskRay). Changed prior to commit: https://reviews.llvm.org/D138255?vs=476319&id=479683#toc Repository: rG

[PATCH] D137756: [z/OS][p][pg] Throw Error When Using -p or -pg on z/OS

2022-12-02 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/zos-profiling-error.c:3 + +// RUN: not %clang -target s390x-none-zos -p -S %s 2>&1 | FileCheck -check-prefix=FAIL-P-NAME %s +// FAIL-P-NAME: error: unsupported option '-p' for target 's390x-none-zos' P

[PATCH] D137756: [z/OS][p][pg] Throw Error When Using -p or -pg on z/OS

2022-12-02 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay requested changes to this revision. MaskRay added inline comments. This revision now requires changes to proceed. Herald added a subscriber: StephenFan. Comment at: clang/test/Driver/zos-profiling-error.c:3 + +// RUN: not %clang -target s390x-none-zos -p -S %s 2>&1 | File

[PATCH] D136078: Use-after-return sanitizer binary metadata

2022-12-03 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. ` SanitizerBinaryMetadata::createZeroSizedObjectInSection` creates `__dummy_*`, but why is it needed? (There is no comment.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136078/new/ https://reviews.llvm.org/D136078 _

[PATCH] D76452: Use LLD by default for Android.

2020-03-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Thanks for abandoning this patch. I am glad Android does not need customization on top of the generic Linux behavior. (This makes clangDriver clean.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76452/new/ https://reviews

[PATCH] D73307: Unique Names for Functions with Internal Linkage

2020-03-25 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D73307#1932131 , @rnk wrote: > At a higher level, should this just be an IR pass that clang adds into the > pipeline when the flag is set? It should be safe to rename internal functions > and give private functions internal li

[PATCH] D76416: [WIP][ASan] Apply -ffile-prefix-map mappings to ASan instrumentation

2020-03-26 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > Putting this on hold for now. Although this implementation works for ASan, it > would be have to be repeated for other tools like SourceBasedCoverage or > other sanitizers. (You can click "Add Action..."->"Plan Changes" so that this Differential will disappear from o

[PATCH] D74324: Tools emit the bug report URL on crash

2020-03-26 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/lib/Support/PrettyStackTrace.cpp:145 +static const char* customBugReportMsg; + jhenderson wrote: > jhenderson wrote: > > `static const char *BugReportMsg;` > > > > Why not just have this set to the default in the

[PATCH] D74324: Tools emit the bug report URL on crash

2020-03-26 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/lib/Support/PrettyStackTrace.cpp:145 +static const char* customBugReportMsg; + MaskRay wrote: > jhenderson wrote: > > jhenderson wrote: > > > `static const char *BugReportMsg;` > > > > > > Why not just have this

[PATCH] D76452: Use LLD by default for Android.

2020-03-26 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Another approach will be `-DCLANG_DEFAULT_LINKER=lld`. It provides a default value for `-fuse-ld=`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76452/new/ https://reviews.llvm.org/D76452 __

[PATCH] D76452: Use LLD by default for Android.

2020-03-26 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D76452#1944786 , @srhines wrote: > In D76452#1944733 , @MaskRay wrote: > > > Another approach will be `-DCLANG_DEFAULT_LINKER=lld`. It provides a > > default value for `-fuse-ld=`. > > >

[PATCH] D75153: [ThinLTO] Allow usage of all SMT threads in the system

2020-03-27 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: lld/test/ELF/basic.s:252 -# RUN: not ld.lld %t --thinlto-jobs=0 2>&1 | FileCheck --check-prefix=NOTHREADSTHIN %s -# RUN: not ld.lld %t --plugin-opt=jobs=0 2>&1 | FileCheck --check-prefix=NOTHREADSTHIN %s -# NOTHREADSTHIN: --thinlto-j

[PATCH] D73307: Unique Names for Functions with Internal Linkage

2020-03-30 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D73307#1942838 , @tmsriram wrote: > In D73307#1942805 , @MaskRay wrote: > > > In D73307#1932131 , @rnk wrote: > > > > > At a higher level, should

[PATCH] D73307: Unique Names for Functions with Internal Linkage

2020-03-30 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1165 const auto *ND = cast(GD.getDecl()); + std::string MangledName = getMangledNameImpl(*this, GD, ND); Delete this unrelated cosmetic change. CHANGES SINCE LAST ACTION http

[PATCH] D77168: Add a flag to debug automatic variable initialization

2020-03-31 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/CodeGen/CGDecl.cpp:1814 +if (StopAfter) { + static unsigned Counter = 0; + if (Counter >= StopAfter) I am a bit worried about the static variable. This makes CodeGen not reusable. ==

[PATCH] D77168: Add a flag to debug automatic variable initialization

2020-04-01 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D77168#1955500 , @jfb wrote: > In D77168#1955450 , @jcai19 wrote: > > > In D77168#1955312 , @jfb wrote: > > > > > Do you not think `pragma` is a m

[PATCH] D68049: Propeller: Clang options for basic block sections

2020-04-01 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4858 + if (Arg *A = Args.getLastArg(options::OPT_fbasicblock_sections_EQ)) { +CmdArgs.push_back( If we want to pass the option verbatim, `A->render(Args, CmdArgs);` However,

[PATCH] D77342: Sema: check for null TInfo in ActOnBaseSpecifier

2020-04-02 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. This needs a reduced test case to demonstrate the problem and ensure clang will not regress in the future. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77342/new/ https://reviews.llvm.org/D77342 ___

[PATCH] D74934: [Clang interpreter] Rename Block.{h,cpp} to AllocatedBlock.{h,cpp}

2020-02-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: jfb, gribozavr, nand, RKSimon. Herald added subscribers: llvm-commits, cfe-commits, dexonsmith, mgorny. Herald added projects: clang, LLVM. The Blocks runtime provide a header named Block.h. It is generally preferable to avoid name collision

[PATCH] D74934: [Clang interpreter] Rename Block.{h,cpp} to AllocatedBlock.{h,cpp}

2020-02-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 245755. MaskRay added a comment. Fix a file header Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74934/new/ https://reviews.llvm.org/D74934 Files: clang/lib/AST/CMakeLists.txt clang/lib/AST/Interp/Allocate

[PATCH] D74934: [Clang interpreter] Rename Block.{h,cpp} to InterpBlock.{h,cpp}

2020-02-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 245862. MaskRay retitled this revision from "[Clang interpreter] Rename Block.{h,cpp} to AllocatedBlock.{h,cpp}" to "[Clang interpreter] Rename Block.{h,cpp} to InterpBlock.{h,cpp}". MaskRay added a comment. Rename Block.{h,cpp} to InterpBlock.{h,cpp} Repo

[PATCH] D74934: [Clang interpreter] Rename Block.{h,cpp} to InterpBlock.{h,cpp}

2020-02-21 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGe4df934ca7b4: [Clang interpreter] Rename Block.{h,cpp} to InterpBlock.{h,cpp} (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74934/new/

[PATCH] D75003: [X86] Fix __code_model_*__ predefine names

2020-02-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. LG. The full story: // gcc/config/i386/i386-c.c case CM_SMALL: case CM_SMALL_PIC: def_or_undef (parse_in, "__code_model_small__"); break; case CM_MEDIUM:

[PATCH] D75002: [AArch64] Predefine __AARCH64_CMODEL_*__ as GCC does

2020-02-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. Comment at: clang/lib/Basic/Targets/AArch64.cpp:200 +CodeModel = "small"; + for (auto& c : CodeModel) { +c = toupper(c); This can be simplified. I'll do that. Repository: rG LLVM Github M

[PATCH] D75002: [AArch64] Predefine __AARCH64_CMODEL_*__ as GCC does

2020-02-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 246060. MaskRay added a comment. Add tests Rebase after -mcmodel cleanups (including fc6057e34fb3b1cfbbfcd5d71ae25ba24eb3ffa3 ) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D75002: [AArch64] Predefine __AARCH64_CMODEL_*__ as GCC does

2020-02-21 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGd2e949eed5b4: [AArch64] Predefine __AARCH64_CMODEL_*__ as GCC does (authored by mcgrathr, committed by MaskRay). Changed prior to commit: https://reviews.llvm.org/D75002?vs=246060&id=246061#toc Reposit

[PATCH] D75003: [X86] Fix __code_model_*__ predefine names

2020-02-21 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG271f96477328: [Preprocessor][X86] Fix __code_model_*__ predefine macros (authored by mcgrathr, committed by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D75012: [ReleaseNotes] Mention -fmacro-prefix-map and -ffile-prefix-map.

2020-02-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/docs/ReleaseNotes.rst:142 +- ``-fmacro-prefix-map=OLD=NEW`` allows a directory prefix ``OLD`` in + ``__FILE__`` preprocessor macros to be substituted for ``NEW``. This helps + with reproducible builds that are location independen

[PATCH] D75097: [cc1as] Unset UseNamesOnTempLabels

2020-02-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added a reviewer: efriedma. Herald added a project: clang. Herald added a subscriber: cfe-commits. Similar to rL236642 . This fixes https://reviews.llvm.org/D74006#1890487 Repository: rG LLVM Github Monorepo https://re

[PATCH] D75056: [Driver] Default to -fno-common

2020-02-25 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5651 + // -fno-common is the default, set -fcommon only when that flag is set. + if (Args.hasFlag(options::OPT_fcommon, options::OPT_fno_common, false)) +CmdArgs.push_back("-fcommon");

<    23   24   25   26   27   28   29   30   31   32   >