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

2022-11-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/freebsd.cpp:48 +// DRIVER-PASS-INCLUDES: "-cc1" {{.*}}"-resource-dir" "[[RESOURCE:[^"]+]]" +// DRIVER-PASS-INCLUDES: "-internal-isystem" "/usr/include/c++/v1" +// DRIVER-PASS-INCLUDES: "-internal-isystem" "[[RESOURCE]]/

[PATCH] D138081: [IR] Split out IR printing passes into IRPrinter

2022-11-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > This diff splits out (from LLVMCore) IR printing passes into IRPrinter. I haven't spent too much time reading this patch. Is this to fix layering check for `-module-summary/-flto=thin with -S/-emit-llvm`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

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

2022-11-17 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. Rubber stamp. I try to find some mips folks who can verify, so maybe wait a day or so. > When we use llc or lld to compiler IR files, the features +nan2008 and > +fpxx/+fp64 are not used. T

[PATCH] D137753: [Clang][GNU][AIX][p]Enable -p Functionality

2022-11-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Please make `-p` accepted for AIX only and don't change the semantics for other targets. For FreeBSD and Linux (musl and gnu) we can try rejecting `-p`. If OpenBSD wants to make `-p` an alias for `-pg`, that's fine. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D135411: Add generic KCFI operand bundle lowering

2022-11-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/KCFI.cpp:16 +#include "llvm/ADT/Statistic.h" +#include "llvm/CodeGen/TargetLowering.h" +#include "llvm/CodeGen/TargetSubtargetInfo.h" Sorry, I just noticed this include. lib/Transforms

[PATCH] D137753: [Clang][GNU][AIX][p]Enable -p Functionality

2022-11-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D137753#3935126 , @francii wrote: > In D137753#3934932 , @MaskRay wrote: > >> Please make `-p` accepted for AIX only and don't change the semantics for >> other targets in this patch.

[PATCH] D137753: [Clang][GNU][AIX][p]Enable -p Functionality

2022-11-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D137753#3935305 , @francii wrote: > Recall that the goal with `-p` is to create parity with GCC (at least with > Linux and AIX), as per the RFC discussion. > > In D137753#3935138 , @Ma

[PATCH] D135411: Add generic KCFI operand bundle lowering

2022-11-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/include/llvm/Transforms/Instrumentation/KCFI.h:24 +public: + KCFIPass() {} + static bool isRequired() { return true; } remove default constructor? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

2022-11-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: francii, hubert.reinterpretcast, cebowleratibm, thesamesam, brad. Herald added subscribers: StephenFan, krytarowski, arichardson, emaste. Herald added a reviewer: ctetreau. Herald added a project: All. MaskRay requested review of this revisio

[PATCH] D137753: [Clang][GNU][AIX][p]Enable -p Functionality

2022-11-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D137753#3935617 , @francii wrote: > In D137753#3935391 , @MaskRay wrote: > >> In D137753#3935305 , @francii >> wrote: >> >>> Recall that the g

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

2022-11-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > This matches OpenBSD, and it supports Swift's use of clang for its C interop > functionality. Recent changes to Swift use AddClangSystemIncludeArgs() to > inspect the cc1 args; this doesn't work for platforms where cc1 adds standard > include paths implicitly. Use a

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

2022-11-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 476360. MaskRay marked an inline comment as done. MaskRay added a comment. add a comment about the zlib header Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137885/new/ https://reviews.llvm.org/D137885 Files:

[PATCH] D118493: Set rpath on openmp executables

2023-01-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D118493#4075970 , @JonChesterfield wrote: > That works if you have one toolchain installed globally or you are happy to > cobble together a working system using environment variables. If you have > multiple toolchains, they

[PATCH] D142757: [clang][driver] Do not warn about position of `/clang:-xc` in cl mode

2023-01-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. See https://github.com/llvm/llvm-project/issues/59307#issuecomment-1407529807 I think we should just report an error for `/clang:-xc` Comment at: clang/lib/Driver/Driver.cpp:2571 // No driver mode exposes -x and /TC or /TP; we don't support mixing

[PATCH] D142595: [Driver][AVR] Don't emit default '-Tdata' when a linker script is specified

2023-01-28 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. This is probably not a good idea. See https://github.com/llvm/llvm-project/issues/60203#issuecomment-1407532083 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D142688: [Clang][Driver] Handle LoongArch multiarch tuples

2023-01-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Linux.cpp:116 + +return std::string("loongarch64-linux-") + Libc + FPFlavor; + } `return (... + ...).str();` if an operand of `+` is a `Twine` Repository: rG LLVM Github Monorepo CHA

[PATCH] D141705: [HLSL] [Dirver] add dxv as a Driver Action Job

2023-01-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:687 +def warn_drv_dxc_missing_dxv : Warning<"dxv not found." +" Resulting DXIL will not be signed for use in release environments.">; python3kgae wrote: > beanz wr

[PATCH] D142595: [Driver][AVR] Don't emit default '-Tdata' when a linker script is specified

2023-01-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. OK. If this doesn't add `-Tdata=` driver options, I'm fine with it. But why is default `-Tdata` added in the first place? Most linker scripts are added as `-Wl,-T,a.lds` (`-Wl,` values are opaque to the driver), so the driver cannot really know whether a linker script is

[PATCH] D142857: [clang] Remove clang::Optional

2023-01-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! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142857/new/ https://reviews.llvm.org/D142857 __

[PATCH] D142861: [Clang] avoid relying on StringMap iteration order when roundtripping -analyzer-config

2023-01-30 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:886 + llvm::sort(SortedConfigOpts, [](const auto &A, const auto &B) { +return std::get<0>(A) < std::get<0>(B); + }); If you use `pair` inste

[PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

2023-01-30 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. To enable smooth hash function migration in the future, we can explore the idea of adding `#ifdef EXPENSIVE_CHECKS\nshuffle` (see https://github.com/llvm/llvm-project/issues/34483). That means we should fix these tests properly to be non-dependent on the iteration order

[PATCH] D142595: [Driver][AVR] Don't emit default '-Tdata' when '-T' is specified

2023-01-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. In D142595#4089391 , @benshi001 wrote: > In D142595#4089124 , @MaskRay wrote: > >> OK. If this do

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

2023-01-30 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D140270#4089857 , @wzssyqa wrote: > ping I don't know how to test this, so this will be up to @nathanchance ... (Thanks for improving the mips port, though) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

2023-01-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. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140270/new/ https://reviews.llvm.org/D140270 __

[PATCH] D142757: [clang][driver] Emit an error for `/clang:-x`

2023-01-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. LGTM. Comment at: clang/docs/ReleaseNotes.rst:60 `Issue 59446 `_. +- Fix confusing warning message when `/clang:-x` is

[PATCH] D142757: [clang][driver] Emit an error for `/clang:-x`

2023-01-30 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > In CL mode values of /clang: arguments end up at the end of arguments list so > passing /clang:-x doesn't make sense and produces confusing warning. The description needs updating. `clang:-x` emits an error instead of a warning. And if the error is suppressed, `/clan

[PATCH] D142757: [clang][driver] Emit an error for `/clang:-x`

2023-01-30 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/Driver.cpp:2571 // No driver mode exposes -x and /TC or /TP; we don't support mixing them. assert(!Args.hasArg(options::OPT_x) && "-x and /TC or /TP is not allowed"); } Fznamznon wrote: > Ma

[PATCH] D139547: [NFC] Correct argument comment typo

2023-01-30 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG9f5d881206ce: [NFC] Correct argument comment typo (authored by gAlfonso-bit, committed by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139547/new/

[PATCH] D142595: [Driver][AVR] Don't emit default '-Tdata' when '-T' is specified

2023-01-30 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. How does avr-gcc know whether the user provides a linker script? I really don't like adding a default `-Tdata=` when it can interfere with common uses of specifying a linker script. In these situations drivers not adding default options is usually better than guessing t

[PATCH] D142757: [clang][driver] Emit an error for `/clang:-x`

2023-01-31 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. Comment at: clang/test/Driver/x-args.c:12 +// RUN: not %clang_cl /TC /WX /clang:-xc /clang:-E /clang:-dM %s 2>&1 | FileCheck -check-prefix=CL %s +// CL-NOT: '-x c' after last input file has no effect +// CL: error: u

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

2023-01-31 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. edeaf16f2c2f02d6e43312d48d26d354d87913f3 (2011) added the CMake variable `CLANG_RESOURCE_DIR` but did not explain why. The patch introduced conditions in C++ code like std::string path_to_clang_dir = std::string(CLANG_RESOURCE_DIR).empty()

[PATCH] D142757: [clang][driver] Emit an error for `/clang:-x`

2023-02-01 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/x-args.c:12 +// RUN: not %clang_cl /TC /WX /clang:-xc /clang:-E /clang:-dM %s 2>&1 | FileCheck -check-prefix=CL %s +// CL-NOT: '-x c' after last input file has no effect +// CL: error: unsupported option '-x c'; did yo

[PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

2023-02-02 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp:638 - EXPECT_THAT(Storage.keys(), ElementsAre(testPath("A.cc"), testPath("A.h"))); + EXPECT_THAT(Storage.keys(), ElementsAre(testPath("A.h"), testPath("A.cc"))); // Make

[PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

2023-02-02 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: mlir/test/Transforms/print-op-graph.mlir:1 // RUN: mlir-opt -allow-unregistered-dialect -mlir-elide-elementsattrs-if-larger=2 -view-op-graph %s -o %t 2>&1

[PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

2023-02-02 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > `For posterity, the tests that fail on main are:` > > `Skipped : 43` > [...] You can remove these from the description. They are flaky tests unrelated to this patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D142499: [Clang][AMDGPU] Set LTO CG opt level based on Clang option

2023-02-03 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. Fine with me, but you may need an AMDGPU reviewer. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142499/new/ https://reviews.llvm.org/D142499

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

2023-02-03 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. The test appears to belong to test/CodeGen instead of test/Sema ? Mixed 8-column and 2-column indentation probably should be unified. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143300/new/ https://reviews.llvm.org/D1433

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

2023-02-03 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Sema/init-randomized-struct-fwd-decl.c:23 + +// CHECK-LABEL: t1 +// CHECK: getelementptr inbounds %struct.foo, ptr %0, i32 0, i32 2 `// CHECK-LABEL: define {{.*}} @t1(` is less likely to cause a conflict with

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

2023-02-03 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: jdoerfert, jhuber6, JonChesterfield, tstellar. Herald added subscribers: guansong, yaxunl. Herald added a project: All. MaskRay requested review of this revision. Herald added subscribers: cfe-commits, sstefan1. Herald added a project: clang.

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

2023-02-03 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. No test probably makes this easy to back port. Eventually there needs to be a test but the original author can do it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143306/new/ https://reviews.llvm.org/D143306

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

2023-02-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. `ItaniumManglingCanonicalizer.cpp` doesn't have many includes. I think it is fine to remain in llvm/lib/Support ... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143318/new/ https://reviews.llvm.org/D143318 __

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

2023-02-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D143306#4104208 , @JonChesterfield wrote: > Added some people I can remember from the original discussion. > > The effect of this patch will be that clang -fopenmp foo.cpp will build an > executable that doesn't know where it

[PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

2023-02-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. Comment at: llvm/test/ObjectYAML/Offload/multiple_members.yaml:21 Value:"gfx908" Content: "cafefeed" Rebase:) I fixed obj2yaml Comment at: llvm/

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

2023-02-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: jdoerfert, jhuber6, tra, yaxunl. Herald added a reviewer: sscalpone. Herald added a subscriber: jeroen.dobbelaere. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber

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

2023-02-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. An option value treating a space differently (as a delimiter in this case) is uncommon. I suggest an alias `-mllvm=` (D143325 ) which is more inline with the convention of the majority of options that take a value (`=` instead of ` `).

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

2023-02-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 494844. MaskRay added a comment. fix test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143325/new/ https://reviews.llvm.org/D143325 Files: clang/include/clang/Driver/Options.td clang/test/Driver/hip-optio

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

2023-02-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. `clang/test/CodeGen/init-randomized-struct-fwd-decl.c` passes without this patch. Is it correct? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143300/new/ https://reviews.llvm.org/D143300 _

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

2023-02-05 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 rG90094ab8850e: [Driver] Add -mllvm= as an alias for -mllvm (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

2023-02-05 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D143325#4105356 , @Meinersbur wrote: > This patch broke the Flang buildbot, e.g. > https://lab.llvm.org/buildbot/#/builders/172/builds/23305 > > The tests > https://github.com/llvm/llvm-project/blob/main/flang/test/Driver/dri

[PATCH] D142606: Lazyly initialize uncommon toolchain detector

2023-02-05 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. LGTM Comment at: clang/lib/Driver/ToolChains/LazyDetector.h:36 +} +return &Detector.value(); + } tbaeder wrote: > Is the `.value()` here permitted? AFAIK the convention is to always use > `operator*`. > > And I think @aaron.b

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

2023-02-05 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/lib/Target/RISCV/RISCVSubtarget.cpp:16 +#include "GISel/RISCVLegalizerInfo.h" +#include "GISel/RISCVRegisterBankInfo.h" #include "RISCV.h" mcgrathr wrote: > jrtc27 wrote: > > Unrelated change > arcanist requested i

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

2023-02-06 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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143318/new/ https://reviews.llvm.org/D143318

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

2023-02-06 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/test/CodeGen/init-randomized-struct-fwd-decl.c:23 + +// CHECK-LABEL: define {{.*}}@t1 +// CHECK-NOT: getelementptr inbounds %struct.foo, ptr %3, i32 0

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

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

[PATCH] D142606: Lazyly initialize uncommon toolchain detector

2023-02-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. (The first line of 0ffaffcaac97de31e1b0e7e80c4f7cab724eda20 should have mentioned `Lazyly initialize uncommon toolchain detector`. You can add `Reland` prefix or add the message in the body of the com

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

2023-02-06 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/test/Driver/zos-profiling-error.c:1 +// Check failed cases + Delete this comment. It does not help reading the test. ==

[PATCH] D141310: [clang] add -Wcompare-function-pointers

2023-01-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. I have some notes about `--icf={safe,all}`: https://maskray.me/blog/2020-11-15-explain-gnu-linker-options#icfall-and---icfsafe I think this diagnostic has some value (and I appreciate that it can be implemented so little code!). Some `--icf=all` issues like using functi

[PATCH] D139774: [libclang] Add API to set temporary directory location

2023-01-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. libclang functions like `clang_reparseTranslationUnit_Impl` call `clang/lib/Frontend/ASTUnit.cpp:1397` and build the preamble with `/*StoreInMemory=*/false`. If `StoreInMemory` is configurable (true), then you can avoid the temporary file `preamble-*.pch`. Repository:

[PATCH] D141182: [mips][clang] Do not define __GCC_HAVE_SYNC_COMPARE_AND_SWAP macros for MIPS-I

2023-01-11 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! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141182/new/ https://reviews.llvm.org/D141182 _

[PATCH] D139686: [lsan] Add lsan support for loongarch64

2023-01-12 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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139686/new/ https://reviews.llvm.org/D139686

[PATCH] D141596: [Driver] Add crtfastmath.o on Solaris if appropriate

2023-01-12 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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141596/new/ https://reviews.llvm.org/D141596

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

2023-01-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:47 + Features, [&Args](const Twine &Str) { return Args.MakeArgString(Str); }, + true); return true; craig.topper wrote: > `true` -> `/*AddAllExtensions*/true` The c

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

2023-01-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. Comment at: clang/test/Driver/riscv-march-mcpu-mtune.c:30 +// MARCH-RV32IMAFDC: "-target-feature" "+c" +// MARCH-RV32IMAFDC-NOT: "-target-cpu" + This NOT pattern isn't useful as `-target-cpu` has occu

[PATCH] D141647: [clang][test] Remove unnecessary 'REQUIRES'

2023-01-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Unless there is something weird, `-E` does not need llvm/lib/Target code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141647/new/ https://reviews.llvm.org/D141647 ___ cfe-commi

[PATCH] D92001: [ubsan] Fix crash on __builtin_assume_aligned

2023-01-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. `-fsanitize=alignment -c a.c -O[012]` no longer crashes for the test. From the comment that the problem was up the call chain, I think we can close this revision now. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92001/new/ https://reviews.llvm.org/D92001 ___

[PATCH] D141723: [Clang] Remove `CLANG_OPENMP_NVPTX_DEFAULT_ARCH` CMake option.

2023-01-13 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. Removing a `CMAKE_*_DEFAULT_*` makes me happy:) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141723/new/ https://reviews.llvm.org/D141723 __

[PATCH] D141310: [clang] add -Wcompare-function-pointers

2023-01-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7007 + "comparison of function pointers (%0 and %1)">, + InGroup, DefaultIgnore; def warn_typecheck_ordered_comparison_of_function_pointers : Warning< aaron.ballman wro

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

2023-01-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. This will cause warnings for the common usage related to bitmask macros/enumerators. #include int main(void) { unsigned char other = 0x81; other &= ~STO_AARCH64_VARIANT_PCS; return other; } (enumerator) `llvm/tools/llvm-readobj/ELFDumper.cpp:3869:1

[PATCH] D141750: [docs] Add llvm & clang release notes for LoongArch

2023-01-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. pip3 install --user recommonmark sphinx # sphinx-automodapi for lldb cmake ... ninja ... docs-llvm-html Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141750/new/ https://reviews.llvm.org/D141750 ___

[PATCH] D141788: [NFC] Use `llvm::enumerate` in llvm/unittests/Object

2023-01-15 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. non-reference `auto` shall work better in these cases. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141788/new/ https://reviews.llvm.org/D141

[PATCH] D141798: Drop the ZeroBehavior parameter from countLeadingZeros and the like (NFC)

2023-01-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. `ZB_Max` is the strange mode that should be dropped, perhaps also `ZB_Undefined`. In D141798#4064114 , @arsenm wrote: >> If you care about compilation speed, you should build LLVM with an >> appropriate -march= to take advantag

[PATCH] D142346: [docs] Add release notes for news in 16.x done by me, or otherwise relating to MinGW targets

2023-01-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. Comment at: lld/docs/ReleaseNotes.rst:78 + if ``-static`` has been specified. This fixes conformance to what + GNU ld dpes. (`D135651 `_) + `s/dpes/does/` Reposit

[PATCH] D142144: [RISCV][Driver] Add -rvv-vector-bits= option similar to -sve-vector-bits=

2023-01-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/docs/ReleaseNotes.rst:844 take architecture extensions from ``-march`` if both are given. +- Added -rvv-vector-bits= option to give an upper bound on vector length. Valid + values are powers of 2 between 64 and 65536. We also a

[PATCH] D78441: Delete NaCl support

2023-01-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D78441#4067440 , @dschuff wrote: > Sorry, no :) Is there a timeline for the removal? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78441/new/ https://reviews.llvm.org/D78441 __

[PATCH] D118493: Set rpath on openmp executables

2023-01-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Enabled-by-default `-fopenmp-implicit-rpath` is odd. For other shared objects (e.g. `libclang_rt.*.so`) we don't do this. For libraries in a well-known system directory, we just rely on the default `/etc/ld.so.conf*`. For libraries in other directories or when `--sysroot

[PATCH] D127310: [clang][driver] fix to correctly set devtoolset on RHEL

2022-06-10 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! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127310/new/ https://reviews.llvm.org/D127310 _

[PATCH] D127393: [Driver] Don't add -lresolv on FreeBSD

2022-06-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. Perhaps change to `isOSLinux() && isGNUEnvironment()` now. I suspect other *BSD don't need this, either. musl definitely doesn't need this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D12739

[PATCH] D112916: Confusable identifiers detection

2022-06-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > Subject: Confusable identifiers detection Add `[clang-tidy] ` Comment at: clang-tools-extra/clang-tidy/misc/ConfusableTable/build_confusable_table.cpp:30 + +SmallVector Values; +Line.split(Values, ';'); Place SmallVector out

[PATCH] D112916: [clang-tidy] Confusable identifiers detection

2022-06-13 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. LGTM. Added some clang-tidy reviewers. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112916/new/ https://reviews.llvm.org/D112916 ___

[PATCH] D126192: [Driver] Improve linking options for target AVR

2022-06-13 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. It is fine if you don't add a magic linker script for -fuse-ld=lld. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126192/new/ https://reviews.llvm.org/D126192 _

[PATCH] D127393: [Driver] Don't add -lresolv on FreeBSD

2022-06-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. I pushed 0ba43f4c2b263268f6fbc56bb3f6d43936781957 with an elaborated comment. This patch is not needed now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D127812: [AArch64] Function multiversioning support added.

2022-06-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/docs/ClangCommandLineReference.rst:1086 +.. option:: -mno-fmv + This file is auto-generated. Don't touch it. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:1173 return None; - case ARM::BI_B

[PATCH] D127589: [Driver][test] Make RISCV tests robust with PATH=

2022-06-14 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. > unknown-linux-gnu-ld riscv64-unknown-linux-gnu-ld. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127589/new/ https://reviews.llvm.org/D1275

[PATCH] D127826: [Driver] Pass -X to ld for riscv*-{elf,freebsd,linux}

2022-06-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. Herald added subscribers: sunshaoce, VincentWu, luke957, StephenFan, vkmr, frasercrmck, evandro, luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, shiva0217,

[PATCH] D127890: [Docs] Update clang & llvm release notes for HLSL

2022-06-15 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. Comment at: clang/docs/ReleaseNotes.rst:72 +- Clang is gaining support for HLSL. Basic features for the HLSL language have + started being merged during Clang-15 development, but the implementation is + incomplete a

[PATCH] D127798: [AArch64] Define __ARM_FEATURE_RCPC

2022-06-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Preprocessor/aarch64-target-features.c:22 // CHECK-NOT: __ARM_FEATURE_CRC32 1 +// CHECK-NOT: __ARM_FEATURE_RCPC 1 // CHECK-NOT: __ARM_FEATURE_CRYPTO 1 The list is alphabetical. RCPC should be moved below.

[PATCH] D127890: [Docs] Update clang & llvm release notes for HLSL

2022-06-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/docs/ReleaseNotes.rst:117 +* DirectX has been added as an experimental target. Specify +`-DLLVM_EXPERIMENTAL_TARGETS_TO_BUILD=DirectX` in your CMake configuration to + enable it. The target is not packaged in pre-built binaries. --

[PATCH] D127826: [Driver] Pass -X to ld for riscv*-{elf,freebsd,linux}

2022-06-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Thanks for the archaeology! > ld will treat ld -r -s as ld -r -S -x, This seems really weird... I agree that using driver-passed -X instead of the implied -x is safe. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127826/n

[PATCH] D127826: [Driver] Pass -X to ld for riscv*-{elf,freebsd,linux}

2022-06-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a subscriber: dim. MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/FreeBSD.cpp:226 CmdArgs.push_back("elf32lriscv"); +CmdArgs.push_back("-X"); break; @dim FYI Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D127826: [Driver] Pass -X to ld for riscv*-{elf,freebsd,linux}

2022-06-16 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 rGc324c938becd: [Driver] Pass -X to ld for riscv*-{elf,freebsd,linux} (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D128098: [clang][driver] Ensure we don't accumulate entries in -MJ files

2022-06-17 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. LGTM. Comment at: clang/test/Driver/compilation_database_multiarch.c:15 // CHECK-NEXT: { "directory": "{{.*}}", "file": "{{.*}}", "o

[PATCH] D127142: [HIP] Link with clang_rt.builtins

2022-06-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D127142#3592207 , @yaxunl wrote: > In D127142#3590874 , @yaxunl wrote: > >> In D127142#3571260 , @MaskRay >> wrote: >> >>> In D127142#3570290

[PATCH] D128098: [clang][driver] Ensure we don't accumulate entries in -MJ files

2022-06-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/compilation_database_multiarch.c:15 // CHECK-NEXT: { "directory": "{{.*}}", "file": "{{.*}}", "output": "[[OUTPUT_ARM64:.*]]", "arguments": [{{.*}}, "-o", "[[OUTPUT_ARM64]]", {{.*}} "--target=arm64-apple-macosx12.0.0

[PATCH] D128109: [Driver] Pass -X to ld for riscv64-openbsd

2022-06-17 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. Please add a test. Comment at: clang/lib/Driver/ToolChains/OpenBSD.cpp:159 + if (ToolChain.getArch() == llvm::Triple::riscv64) +CmdArgs.push_back("-X");

[PATCH] D128109: [Driver] Pass -X to ld for riscv64-openbsd

2022-06-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/openbsd.c:133 +// RUN: | FileCheck -check-prefix=CHECK-RISCV64-FLAGS %s +// CHECK-RISCV64-FLAGS: "-X" It's useful to check other values like the `-m` emulation and the dynamic linker. Repository:

[PATCH] D128127: [Driver][OpenBSD] Specify linker emulations

2022-06-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Is the `-m` value useful for OpenBSD? If not, it's perhaps not useful to have this logic. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128127/new/ https://reviews.llvm.org/D128127

[PATCH] D127186: [Driver] Support linking to compiler-rt for target AVR

2022-06-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/avr-toolchain.c:83 +// RUN: %clang %s -### --target=avr -mmcu=atmega328 --sysroot=%S/Inputs/basic_avr_tree/ -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir -fuse-ld=%S/Inputs/basic_avr_tree/usr/bin/ld.lld

[PATCH] D128134: [Driver] Pass -X to ld for riscv64-fuchsia

2022-06-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Looks good if Fuchsia wants to match Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128134/new/ https://reviews.llvm.org/D128134 ___ cfe-commits mailing list cfe-commits@lists.llv

[PATCH] D128127: [Driver][OpenBSD] Specify linker emulations

2022-06-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Then maybe this is not needed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128127/new/ https://reviews.llvm.org/D128127 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D128235: [RISCV] Add support for the Zawrs extension

2022-06-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > Context not available. See `-U99` on https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface or use `arc diff` (PHP 8 may need https://discourse.llvm.org/t/arcanist-deprecation-errors-on-php-8/63231) Comment at: llvm/t

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