[PATCH] D155670: [c-index-test] Suppress -Wcast-qual after D153911

2023-07-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay abandoned this revision. MaskRay added a comment. commit 0f0c3d45d7d75ba82a955246da654146a7d57a0d did this first Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155670/new

[PATCH] D155756: -fsanitize={function,kcfi}: Switch to xxh3_64bits

2023-07-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added a reviewer: samitolvanen. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Following recent changes switching from xxh64 to xxh32 for better hashing performanc

[PATCH] D155756: -fsanitize={function,kcfi}: Switch to xxh3_64bits

2023-07-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D155756#4516501 , @samitolvanen wrote: > Changing the hash function breaks compatibility with the Rust KCFI > implementation >

[PATCH] D155729: [OptTable] Make explicitly included options override excluded ones

2023-07-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Thank you! The clean up is nice. I missed the original change that added the DXCOption complexity.. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155729/new/ https://reviews.llvm.org/D155729 __

[PATCH] D153906: [clang] Allow disassembly of multi-module bitcode files

2023-07-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Is a simpler change like D154923 sufficient? It also handles the case when we don't emit `.ll`, but `.o` or `.s`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153906/new/ https://reviews.llvm.org/D153906

[PATCH] D150803: [WebAssembly] Support `annotate` clang attributes for marking functions.

2023-07-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp:568 + // Group all the custom attributes by name. + StringMap> CustomSections; + const ConstantArray *CA = cast(V->getOperand(0)); "StringMap iteration order, howe

[PATCH] D149162: [Clang][OpenMP][IRBuilder] Move registerTargetGlobalVariable & getAddrOfDeclareTargetVar into the OMPIRBuilder

2023-07-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. `registerTargetGlobalVariable` has a iteration order issue using StringMap, caught by D155789 curl -L 'https://reviews.llvm.org/D155789?download=1' | patch -p1 cmake ... -DLLVM_ENABLE_REVERSE_ITERATION=on ninja -C ... check-llvm-u

[PATCH] D155857: [clang] fix nonnull warnings during build

2023-07-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. I agree with the analysis. My understanding is that we don't worsen the code quality for Clang to work around a GCC warning. And generally we care strongly about Clang warnings, but less about GCC warnings, especially in `.cpp` files. However, this is in a header and per

[PATCH] D154688: [clang] Show verify prefix in error messages

2023-07-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. The change looks great: (from `error: 'error' diagnostics seen but not expected:` to `error: 'expected-error' diagnostics seen but not expected`). When I read Clang tests, I was confused by what `error` meant. I eventually figured out `-verify` is special. The new diagno

[PATCH] D155857: [clang] fix nonnull warnings during build

2023-07-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Perhaps `-Wno-nonnull` should be GCC specific (i.e. not apply to Clang). This option will disable certain warnings related to Clang `_Nonnull` (seems unused in llvm-project) and `__attribute__((nonnull))` (used in a few places). Repository: rG LLVM Github Monorepo C

[PATCH] D155857: [clang] fix nonnull warnings during build

2023-07-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Perhaps this can be placed beside `llvm/cmake/config-ix.cmake` `-Wmaybe-uninitialized`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155857/new/ https://reviews.llvm.org/D155857 __

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/baremetal.cpp:348 +// RUN: %clang %s -### --target=powerpc-unknown-eabi 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-PPCEABI %s MaskRay wrote: > Without a sysroot, we may pick up powerpc-unknown

[PATCH] D142702: [Clang][AArch64][SME] Generate target features from +(no)sme.* options

2023-07-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/aarch64-implied-sme-features.c:49 +// RUN: %clang -target aarch64-linux-gnu -march=armv8-a+nosme+sme-i16i64 %s -### 2>&1 | FileCheck %s --check-prefix=SME-SUBFEATURE-CONFLICT-REV +// SME-SUBFEATURE-CONFLICT-REV: "-targ

[PATCH] D149162: [Clang][OpenMP][IRBuilder] Move registerTargetGlobalVariable & getAddrOfDeclareTargetVar into the OMPIRBuilder

2023-07-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D149162#4517500 , @MaskRay wrote: > `registerTargetGlobalVariable` relies on the iteration order of StringMap, > which is not guaranteed. The bug is caught by D155789 > > > curl -L 'https:

[PATCH] D149162: [Clang][OpenMP][IRBuilder] Move registerTargetGlobalVariable & getAddrOfDeclareTargetVar into the OMPIRBuilder

2023-07-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D149162#4520497 , @agozillon wrote: > In D149162#4520395 , @MaskRay wrote: > >> In D149162#4517500 , @MaskRay >> wrote: >> >>> `registerTarget

[PATCH] D155992: [clangd] Use xxh3_64bits for background index file digests

2023-07-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: kadircet, sammccall. Herald added a subscriber: arphaman. Herald added a project: All. MaskRay requested review of this revision. Herald added subscribers: cfe-commits, ilya-biryukov. Herald added a project: clang-tools-extra. Many sources sh

[PATCH] D154923: [CodeGen] Support bitcode input containing multiple modules

2023-07-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 543083. MaskRay edited the summary of this revision. MaskRay added a comment. rename a variable Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154923/new/ https://reviews.llvm.org/D154923 Files: clang/lib/Cod

[PATCH] D154923: [CodeGen] Support bitcode input containing multiple modules

2023-07-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 543084. MaskRay added a comment. rename a variable Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154923/new/ https://reviews.llvm.org/D154923 Files: clang/lib/CodeGen/CodeGenAction.cpp clang/test/CodeGen/s

[PATCH] D155540: [clangd] Remove extra dependancies for clangd

2023-07-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. `llvm/cmake/modules/HandleLLVMOptions.cmake` passes `-Wl,-z,defs`. `-DBUILD_SHARED_LIBS=on` builds get checking from the linker option (https://maskray.me/blog/2021-06-13-dependency-related-linker-options#z-defs). This is similar Bazel's layering_check. For a dependenc

[PATCH] D155540: [clangd] Remove extra dependancies for clangd

2023-07-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D155540#4524326 , @sammccall wrote: > In D155540#4524219 , @MaskRay wrote: > >> `llvm/cmake/modules/HandleLLVMOptions.cmake` passes `-Wl,-z,defs`. >> `-DBUILD_SHARED_LIBS=on` builds ge

[PATCH] D155857: [clang] fix nonnull warnings during build

2023-07-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/cmake/modules/HandleLLVMOptions.cmake:773 + # Disabling this GCC warning as it is emitting a lot of false positives + append_if(CMAKE_COMPILER_IS_GNUCXX "-Wno-nonnull" CMAKE_CXX_FLAGS) Use an imperative sentence

[PATCH] D154923: [CodeGen] Support bitcode input containing multiple modules

2023-07-21 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 rGb2f7b5dbaefe: [CodeGen] Support bitcode input containing multiple modules (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D155992: [clangd] Use xxh3_64bits for background index file digests

2023-07-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 543200. MaskRay added a comment. Herald added a subscriber: wenlei. bump version Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155992/new/ https://reviews.llvm.org/D155992 Files: clang-tools-extra/clangd/Sou

[PATCH] D155857: [clang] fix nonnull warnings during build

2023-07-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D155857#4525421 , @fzakaria wrote: > Address feedback from maskray@ > > - imperative comment > - switch to CMAKE_GNU_COMPILER_ID for variable Does `append_if(CMAKE_GNU_COMPILER_ID "-Wno-nonnull" CMAKE_CXX_FLAGS)` work? `build

[PATCH] D154688: [clang] Show verify prefix in error messages

2023-07-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Frontend/VerifyDiagnosticConsumer.cpp:881 + std::string KindStr = Prefix + "-" + Kind; + Diags.Report(diag::err_verify_inconsistent_diags).setForceEmit() The variables are immediately used. I think in this

[PATCH] D155857: [CMake] Disable GCC -Wnonnull

2023-07-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 rG1f8f8760b586: [CMake] Disable GCC -Wnonnull (authored by fzakaria, committed by MaskRay). Changed prior to commit: https://reviews.llvm.org/D15585

[PATCH] D155992: [clangd] Use xxh3_64bits for background index file digests

2023-07-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D155992#4527081 , @kadircet wrote: > thanks for doing this! > > as Sam pointed out this will result in invalidation of all the index shards, > but that's not something new. we already don't clean up non-relevant index > shard

[PATCH] D155808: [clang][driver] Call IsARMBigEndain function only for isARM and isThumb.

2023-07-24 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. Sounds good to make `arm::isARMBigEndian` focus on AArch32 :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155808/new/ https://reviews.llvm.o

[PATCH] D155808: [clang][driver] Call IsARMBigEndain function only for isARM and isThumb.

2023-07-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Add NFC if this is NFC? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155808/new/ https://reviews.llvm.org/D155808 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:/

[PATCH] D156089: [Driver][XRay][test] Convert more tests to check 'target=...' after 016785d9316d8c5abc5fdf3cdb86479095bbb677

2023-07-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/XRay/xray-ignore-loops-flags.cpp:1 // This test ensures that when we invoke the clang compiler, that the -cc1 // options include the -fxray-ignore-loops flag we provide in the I have cleaned up a few

[PATCH] D154774: [Sema] Respect instantiated-from template's VisibilityAttr for two implicit/explicit instantiation cases

2023-07-24 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/D154774/new/ https://reviews.llvm.org/D154774 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/

[PATCH] D156090: [Driver][XRay][test] Mark these tests as supported on loongarch64

2023-07-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Thank you for improving the XRay tests. I have cleaned up some xray tests in June but did not finish the remaining ones. This motivated me to properly fix them. I'll soon remove `clang/test/Driver/XRay/lit.cfg.py` after we no longer rely on the default target triple.

[PATCH] D155992: [clangd] Use xxh3_64bits for background index file digests

2023-07-25 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG16d79d26b3b6: [clangd] Use xxh3_64bits for background index file digests (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155992/new/ ht

[PATCH] D156330: [hexagon] restore library path arguments

2023-07-26 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. Tip: use `%clang -fdriver-only -Werror -v` to test that a command produces no warning or error and has an exit code of 0. Without it your `Args.ClaimAllArgs(options::OPT_L);` change is untes

[PATCH] D156363: [Driver] -###: exit with code 1 if hasErrorOccurred

2023-07-26 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. @jhuber6 @yaxunl you may want to revise some AMDGPU related tests, e.g. `amdgcn-gz-options.cl`. Some `RUN: %clang` lines currently fail with `error: cannot find ROCm device library; provide its path via '--rocm-path' or '--rocm-device-lib-path', or pass '-nogpulib' to b

[PATCH] D156330: [hexagon] restore library path arguments

2023-07-26 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D156330#4536827 , @bcain wrote: > Fixed target: `--target=hexagon-unknown-linux` was not correct for testing > this bug, it should have been `--target=hexagon-unknown-linux-musl`. > > Used `-fdriver-only -Werror` as suggested

[PATCH] D156351: clang driver throws error for -mabi=elfv2 or elfv2

2023-07-26 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > After clang release/16.x there is a regression that -mabi=elfv1 or > -mabi=elfv2 are being unused and throws warning. Please just give the full clang command line and add a test case under `test/Driver`. FWIW: `clang --target=powerpc64le-linux-gnu -mabi=elfv1 -mabi=e

[PATCH] D156325: [Driver] Link shared asan runtime lib with -z now on Solaris/x86

2023-07-26 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. This needs a clang/test/Driver test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156325/new/ https://reviews.llvm.org/D156325 ___ cfe-commits mailing list cfe-commits@lists.llv

[PATCH] D142660: [AIX] supporting -X options for llvm-ranlib in AIX OS

2023-07-27 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/lit.cfg.py:391 +if 'system-aix' in config.available_features: + config.environment['OBJECT_MODE'] = '32_64' Recent Python style prefers double quotes Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D156363: [Driver] -###: exit with code 1 if hasErrorOccurred

2023-07-27 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D156363#4539877 , @dblaikie wrote: > FWIW this sounds good to me (though given how wide the patch is, might be > worth waiting a few days to a week in case anyone else has thoughts). > > I only looked at a sample of the test c

[PATCH] D156351: clang driver throws error for -mabi=elfv2 or elfv2

2023-07-27 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/ppc-abi.c:18 // RUN: -mabi=altivec | FileCheck -check-prefix=CHECK-ELFv2 %s +// RUN: %clang -target powerpc64le-unknown-unknown-gnu %s -### -o %t.o 2>&1 \ +// RUN: -mabi=elfv1 | FileCheck -check-prefix=CHECK-UNKNOW

[PATCH] D157663: [Driver] Default riscv*-linux* to -fdebug-default-version=4

2023-08-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: asb, dblaikie, jrtc27, kito-cheng. Herald added subscribers: VincentWu, vkmr, luismarques, sameer.abuasal, s.egerton, Jim, benna, psnobl, PkmX, rogfer01, shiva0217, simoncook, arichardson. Herald added a project: All. MaskRay requested revie

[PATCH] D157663: [Driver] Default riscv*-linux* to -fdebug-default-version=4

2023-08-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 549192. MaskRay added a comment. add a comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157663/new/ https://reviews.llvm.org/D157663 Files: clang/lib/Driver/ToolChains/Linux.cpp clang/test/Driver/clan

[PATCH] D157663: [Driver] Default riscv*- triples to -fdebug-default-version=4

2023-08-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 549233. MaskRay retitled this revision from "[Driver] Default riscv*-linux* to -fdebug-default-version=4" to "[Driver] Default riscv*- triples to -fdebug-default-version=4". MaskRay edited the summary of this revision. MaskRay added a comment. Thanks for men

[PATCH] D157680: [X86]Support options -mno-gather -mno-scatter

2023-08-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/include/clang/Driver/Options.td:903-906 +def mno_gather : Flag<["-"], "mno-gather">, Flags<[NoXarchOption]>, + HelpText<"Disable generation of gather instructions in auto-vectorization(x86 only)">; +def mno_scatter

[PATCH] D155290: [PGO] Use Unique Profile Files when New Processes are Forked

2023-08-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: compiler-rt/lib/profile/InstrProfilingFile.c:94 static lprofFilename lprofCurFilename = {0, 0, 0, {0}, NULL, {0}, 0, 0, 0, PNS_unknown}; static int ProfileMergeRequested = 0; --

[PATCH] D155290: [PGO] Use Unique Profile Files when New Processes are Forked

2023-08-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D155290#4576643 , @qiongsiwu1 wrote: > In D155290#4574797 , @MaskRay wrote: > >> In D155290#4572965 , @qiongsiwu1 >> wrote: >> >>> Ping for r

[PATCH] D155290: [PGO] Use Unique Profile Files when New Processes are Forked

2023-08-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Actually, I am not sure changing `%p` to create multiple raw profile files should be the default. Currently, thanks to online profile merging we get just one raw profile file (though counters before fork may be doubly incremented). Using `%p` should not magically change

[PATCH] D157146: [Clang][Docs] Consolidate option hiding in ClangOptionDocEmitter

2023-08-12 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/utils/TableGen/ClangOptionDocEmitter.cpp:43 +bool hasFlag(const Record *Option, StringRef OptionFlag) { + for (const Record *Flag : Option->getValu

[PATCH] D157146: [Clang][Docs] Consolidate option hiding in ClangOptionDocEmitter

2023-08-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. The diff is preferable:) --- tools/clang/docs/html/ClangCommandLineReference.html2023-08-12 22:10:23.876913374 -0700 +++ tools/clang/docs/html/ClangCommandLineReference.html.old2023-08-12 22:09:58.900516809 -0700 @@ -94,8 +94,18 @@ Static analyzer o

[PATCH] D157793: [Headers] Add missing __need_ macros to stdarg.h

2023-08-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > Apple needs a __need_ macro for va_list. Add one, and also ones for > va_start/va_arg/va_end, __va_copy, va_copy. Do you have a link to the specification or the source that this is needed? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

[PATCH] D157150: [Driver] Update BoolOption to handle Visibility. NFC

2023-08-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. LGTM once the https://reviews.llvm.org/D157151#4582109 "Default" naming discussion is agreed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D15

[PATCH] D157275: [Driver] Select newest GCC installation on Solaris

2023-08-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Thanks for the update. Regarding testing, it seems that unittests will be more convenience than creating so many placeholder files in `Inputs/`. `clang/unittests/Driver/ToolChainTest.cpp` `TEST(ToolChainTest, VFSGCCInstallation)` has an example for Linux. You can add an

[PATCH] D157663: [Driver] Default riscv*- triples to -fdebug-default-version=4

2023-08-14 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGbbc0f99f3bc9: [Driver] Default riscv*- triples to -fdebug-default-version=4 (authored by MaskRay). Changed prior to commit: https://reviews.llvm.org/D157663?vs=549233&id=549956#toc Repository: rG LLV

[PATCH] D157794: [Driver] Make /Zi and /Z7 aliases of -g rather than handling them specially

2023-08-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4522 RenderDebugInfoCompressionArgs(Args, CmdArgs, D, TC); + + Keep just one blank line. Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[PATCH] D157280: [PGO] Enable `-fprofile-update` for `-fprofile-generate`

2023-08-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. Thanks! Comment at: llvm/lib/Passes/PassBuilderPipelines.cpp:1797 addPGOInstrPasses(MPM, Level, /* RunProfileGen */ true, -/* IsCS */ true, P

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

2023-08-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. I think the best place to test `RISCVISAInfo.cpp` is `llvm/unittests/Support/RISCVISAInfoTest.cpp`. `clang/test/Driver/print-supported-extensions.c` can test just a few lines, so that changes to RISC-V extensions will generally not require updates to `clang/test/Driver

[PATCH] D157280: [PGO] Enable `-fprofile-update` for `-fprofile-generate`

2023-08-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Please add a link to https://reviews.llvm.org/D87737 in the summary/commit message. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157280/new/ https://reviews.llvm.org/D157280 __

[PATCH] D157280: [PGO] Enable `-fprofile-update` for `-fprofile-generate`

2023-08-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/lib/Passes/PassBuilderPipelines.cpp:1797 addPGOInstrPasses(MPM, Level, /* RunProfileGen */ true, -/* IsCS */ true, PGOOpt->CSProfileGenFile, -PGOOpt->ProfileRemappingFile, +

[PATCH] D157149: [Option] Add "Visibility" field and clone the OptTable APIs to use it

2023-08-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added subscribers: python3kgae, beanz. MaskRay added a comment. This revision is now accepted and ready to land. > ... but it really doesn't scale well, as can be seen by things like the > somewhat recently introduced CLDXCOption. FYI @beanz @python3kgae D

[PATCH] D157497: feat: Migrate isArch16Bit

2023-08-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D157497#4584621 , @Pivnoy wrote: > This discussion was the main motivation for this change. > https://discourse.llvm.org/t/rfc-refactor-triple-related-classes/70410/11 > As a result, Triple should become a data class, and its d

[PATCH] D157813: [VE][Clang] Change to enable VPU flag by default

2023-08-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/include/clang/Driver/Options.td:5166 +// VE feature flags +let Flags = [TargetSpecific, CC1Option] in { +def mvevpu : Flag<["-"], "mvevpu">, Group, Other feature group options don't set CC1Option: we do not need t

[PATCH] D157813: [VE][Clang] Change to enable VPU flag by default

2023-08-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/include/clang/Driver/Options.td:5169 + HelpText<"Emit VPU instructions for VE">; +def mno_vevpu : Flag<["-"], "mno-vevpu">, Group, + HelpText<"Do not emit VPU instructions for VE">; In general, we just need docum

[PATCH] D157275: [Driver] Select newest GCC installation on Solaris

2023-08-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. Thanks! Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:2258 - Prefixes.push_back(CandidatePrefix); + SolarisPrefixes.push_back(CandidatePrefix); } ---

[PATCH] D151567: [LLVM][Support] Report EISDIR when opening a directory on AIX

2023-08-15 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. Thanks for the patch, but there are two issues that should be fixed: (1) stat => fstat (2) change the subject to mean that this is not AIX specific (`[LLVM][Support] Report EISDIR

[PATCH] D157445: [CodeGen][UBSan] Add support for handling attributed functions in getUBSanFunctionTypeHash.

2023-08-15 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. Comment at: clang/test/CodeGen/ubsan-function-attributed.c:3 + +// CHECK: .long248076293 +void __attribute__((ms_abi)) f(void) {} It's useful to add `// CHECK-LABEL: f:` interleaving with `.lo

[PATCH] D142660: [AIX] supporting -X options for llvm-ranlib in AIX OS

2023-08-15 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. `# REQUIRES: system-aix` in llvm/test/tools/llvm-ranlib/aix-X-option.test unfortunately makes most -X tests only work on AIX, which most contributors don't have access to. Ideally `-X` is usable when the user specifies `--format=bigarchive`. The environment variable `O

[PATCH] D142660: [AIX] supporting -X options for llvm-ranlib in AIX OS

2023-08-15 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/include/llvm/Object/ArchiveWriter.h:43 +enum SymtabWritingMode { + NoSymtab, // Do not write symbol table. Below you use `SymtabWritingMode::` for all members, so just make this enum scoped. ==

[PATCH] D78441: Delete NaCl support

2023-08-15 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D78441#4074689 , @MaskRay wrote: > In D78441#4067440 , @dschuff wrote: > >> Sorry, no :) > > Is there a timeline for the removal? @dschuff :) Repository: rG LLVM Github Monorepo CHA

[PATCH] D158006: [Clang][WIP]Experimental implementation of data member packs in dependent context.

2023-08-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:3292 + std::string NewFieldName = + PackedField->getName().str() + "@" + std::to_string(Arg); + PackedField->setDeclName(&Context.Idents.get(NewFieldName)); --

[PATCH] D158006: [Clang][WIP]Experimental implementation of data member packs in dependent context.

2023-08-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Sema/TreeTransform.h:4218 + Outputs.push_back(Out.get()); + Arg++; +} https://llvm.org/docs/CodingStandards.html#prefer-preincrement Repository: rG LLVM Github Monorepo CHANGES SI

[PATCH] D157680: [X86]Support options -mno-gather -mno-scatter

2023-08-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/x86-no-gather-no-scatter.cpp:4 +// RUN: | FileCheck --check-prefix=NOGATHER %s +// RUN: %clangxx -c -mno-gather -### %s 2>&1 \ +// RUN: | FileCheck --check-prefix=NOGATHER %s Each RUN line makes tests

[PATCH] D129061: [Lex] Diagnose macro in command lines

2023-08-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/include/clang/Driver/Options.td:664 HelpText<"Define to (or 1 if omitted)">; +def DriverDefine : JoinedOrSeparate<["-"], "driver-define">, Group, +Flags<[CC1Option, FlangOption, FC1Option]>, MetaVarName<"=">, -

[PATCH] D157485: [X86][RFC] Support new feature AVX10

2023-08-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Basic/Targets/X86.cpp:739 + if (HasAVX10_512BIT) +Builder.defineMacro("__AVX10_512BIT__"); + This is untested? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D158137: Change -ffp-model= related warn_drv_overriding_flag_option to warn_drv_overriding_option

2023-08-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: clang, aaron.ballman, andrew.w.kaylor, hans, skan, zahiraam. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. warn_drv_overriding_flag_option was i

[PATCH] D41425: [darwin][driver] Warn about mismatching --version-min rather than superfluous --version-min compiler option

2023-08-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Herald added a subscriber: ributzka. Herald added a project: All. Comment at: cfe/trunk/lib/Driver/ToolChains/Darwin.cpp:1545 +std::string TargetArg = OSTarget->getAsString(Args, Opts); +getDriver().Diag(clang::diag::warn_drv_overrid

[PATCH] D157485: [X86][RFC] Support new feature AVX10

2023-08-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Arch/X86.cpp:261 +if (AVXVecSize == 256) + D.Diag(diag::warn_drv_overriding_flag_option) << "AVX10-256" +<< "AVX10-512"; ---

[PATCH] D154774: [Sema] Respect instantiated-from template's VisibilityAttr for two implicit/explicit instantiation cases

2023-08-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 550961. MaskRay added a comment. rebase. this does not address rjmccall's comment, but just to show the difference to other tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154774/new/ https://reviews.llvm.

[PATCH] D153835: [Sema] Clone VisibilityAttr for functions in template instantiations

2023-08-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 550963. MaskRay added a comment. rebase we may not go with this approach, but update to show the test difference Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153835/new/ https://reviews.llvm.org/D153835 Files

[PATCH] D158137: Change -ffp-model= related warn_drv_overriding_flag_option to warn_drv_overriding_option

2023-08-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D158137#4594181 , @dexonsmith wrote: > LGTM. > > Perhaps as a follow-up, rename warn_drv_overriding_flag_option to have “t” in > it? Thanks! I agree. d9ad0681fad9a98f43d9baddb95d505b37153c48 (2013) renamed `warn_drv_overrid

[PATCH] D158137: Change -ffp-model= related warn_drv_overriding_flag_option to warn_drv_overriding_option

2023-08-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D158137#4595234 , @hans wrote: >> Thanks! I agree. d9ad0681fad9a98f43d9baddb95d505b37153c48 (2013) renamed >> `warn_drv_overriding_t_option` to `warn_drv_overriding_flag_option`. >> Perhaps the original name `warn_drv_overridi

[PATCH] D158137: Rename warn_drv_overriding_flag_option (-Woverriding-t-option) to warn_drv_overriding_flag_option (-Woverriding-option)

2023-08-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 551156. MaskRay retitled this revision from "Change -ffp-model= related warn_drv_overriding_flag_option to warn_drv_overriding_option" to "Rename warn_drv_overriding_flag_option (-Woverriding-t-option) to warn_drv_overriding_flag_option (-Woverriding-option)

[PATCH] D158137: Rename warn_drv_overriding_flag_option (-Woverriding-t-option) to warn_drv_overriding_flag_option (-Woverriding-option)

2023-08-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D158137#4595927 , @dexonsmith wrote: > This seems to drop `-Woverriding-t-option` entirely. Could that break builds > if someone has (e.g.) `-Werror -Wno-overriding-t-option` in their build > settings? Yes. We could also ad

[PATCH] D158137: Rename warn_drv_overriding_flag_option (-Woverriding-t-option) to warn_drv_overriding_flag_option (-Woverriding-option)

2023-08-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D158137#4596111 , @MaskRay wrote: > In D158137#4595927 , @dexonsmith > wrote: > >> This seems to drop `-Woverriding-t-option` entirely. Could that break builds >> if someone has (e.g.

[PATCH] D158206: [Driver] Add PIE support on Solaris

2023-08-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:55 const char *LinkingOutput) const { + const bool IsPIE = + !Args.hasArg(options::OPT_shared) && On Linux, `clang -r` `-static` also disa

[PATCH] D158206: [Driver] Add PIE support on Solaris

2023-08-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > I'll submit a follow-up patch to make use of crtbeginS.o and crtendS.o > shortly. I think this patch should make this change, so that the change is correct on itself. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158206

[PATCH] D157750: Properly handle -fsplit-machine-functions for fatbinary compilation

2023-08-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c:16 +// causes a warning. +// RUN: %clang --target=x86_64-unknown-linux-gnu -nogpulib -nogpuinc \ +// RUN: --cuda-gpu-arch=sm_70 -x cuda -fsplit-machine-functions -S %s 2>

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

2023-08-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Herald added a subscriber: sunshaoce. In D146054#4587210 , @4vtomat wrote: > In D146054#4586067 , @MaskRay wrote: > >> I think the best place to test `RISCVISAInfo.cpp` is >> `llvm/unitte

[PATCH] D158218: [CMake] Deprecate DEFAULT_SYSROOT and GCC_INSTALL_PREFIX

2023-08-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: DavidSpickett, phosek, smeenai, tra, tstellar. Herald added subscribers: ekilmer, s.egerton, abidh, PkmX, atanasyan, simoncook, fedor.sergeev, kristof.beyls, krytarowski, arichardson, sdardis, emaste. Herald added a project: All. MaskRay req

[PATCH] D158137: Rename warn_drv_overriding_flag_option (-Woverriding-t-option) to warn_drv_overriding_flag_option (-Woverriding-option)

2023-08-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D158137#4596948 , @dexonsmith wrote: > Can you explain the downside of leaving behind an alias? Two minor ones. (a) Existing `-Wno-overriding-t-option` will not notice that they need to migrate and (b) Clang has accrued tiny

[PATCH] D158231: [clang][test] Fix clang machine-function-split tests

2023-08-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/CodeGen/fsplit-machine-functions.c:41 + +// RUN: %clang -c -target arm-unknown-linux-gnueabi -fsplit-machine-functions %s \ +// RUN: 2>&1 | FileCheck -check-prefix=MFS5 %s `-target ` has been deprecated s

[PATCH] D158231: [clang][test] Fix clang machine-function-split tests

2023-08-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/CodeGen/fsplit-machine-functions.c:41 + +// RUN: %clang -c -target arm-unknown-linux-gnueabi -fsplit-machine-functions %s \ +// RUN: 2>&1 | FileCheck -check-prefix=MFS5 %s shenhan wrote: > MaskRay wrote:

[PATCH] D158137: Rename warn_drv_overriding_flag_option (-Woverriding-t-option) to warn_drv_overriding_flag_option (-Woverriding-option)

2023-08-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D158137#4597491 , @dexonsmith wrote: > In D158137#4597009 , @MaskRay wrote: > >> In D158137#4596948 , @dexonsmith >> wrote: >> >>> Can you ex

[PATCH] D158137: Rename warn_drv_overriding_flag_option (-Woverriding-t-option) to warn_drv_overriding_flag_option (-Woverriding-option)

2023-08-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 551542. MaskRay added a comment. add a release note about the renamed -W option Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158137/new/ https://reviews.llvm.org/D158137 Files: clang/docs/ReleaseNotes.rst

[PATCH] D158137: Rename warn_drv_overriding_flag_option (-Woverriding-t-option) to warn_drv_overriding_flag_option (-Woverriding-option)

2023-08-18 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 rG1c66d08b0137: Rename warn_drv_overriding_flag_option (-Woverriding-t-option) to… (authored by MaskRay). Changed prior to commit: https://reviews.l

[PATCH] D158206: [Driver] Add PIE support on Solaris

2023-08-18 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/lib/Driver/ToolChains/Solaris.cpp:136 +const char *crtbegin = nullptr; +if (Args.hasArg(options::OPT_shared) || IsPIE) + crtbegin = "crtb

[PATCH] D158137: Rename warn_drv_overriding_flag_option (-Woverriding-t-option) to warn_drv_overriding_flag_option (-Woverriding-option)

2023-08-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D158137#4599461 , @dblaikie wrote: > In D158137#4597491 , @dexonsmith > wrote: > >> In D158137#4597009 , @MaskRay >> wrote: >> >>> In D158137

[PATCH] D158301: Add back overriding-t-options for -m-version-min diagnostic

2023-08-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: clang, arphaman, aaron.ballman, andrew.w.kaylor, hans, skan, zahiraam, dexonsmith. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This restores t

[PATCH] D158218: [CMake] Deprecate DEFAULT_SYSROOT and GCC_INSTALL_PREFIX

2023-08-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/CMakeLists.txt:179-183 +if(DEFAULT_SYSROOT) + message(WARNING "DEFAULT_SYSROOT is deprecated and will be removed. Use " +"configuration files (https://clang.llvm.org/docs/UsersManual.html#configuration-files)" +"to specif

[PATCH] D158231: [clang][test] Fix clang machine-function-split tests

2023-08-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/CodeGen/fsplit-machine-functions.c:1 +// REQUIRES: system-linux +// REQUIRES: x86-registered-target Any reason `system-linux` is needed? Comment at: clang/test/CodeGen/fsplit-machine-functio

<    20   21   22   23   24   25   26   27   28   29   >