[PATCH] D139723: [OpenMP][AMDGPU] Enable use of abs labs and llabs math functions in C code

mgorny closed this revision. mgorny added a comment. I've pushed a fix in dab67c66932b9149842f7c8431e951f952125fc0 , based on @jhuber6's fix from the other diff. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D140155: [Clang][OpenMP] Allow host call to nohost function with host variant

mgorny closed this revision. mgorny added a comment. Thank you. I'm going to try if the same solution helps with D139723 too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140155/new/

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

mgorny accepted this revision. mgorny 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/D138255/new/ https://reviews.llvm.org/D138255

[PATCH] D138792: [AArch64] Improve TargetParser API

mgorny added a comment. In D138792#3966920 , @Hahnfeld wrote: > Hi, I bisected this change to lead to a couple of test failures when building > with `LLVM_LINK_LLVM_DYLIB`. In the past, this had to do with global variable > initialization order, but

[PATCH] D138258: clang/cmake: Fix incorrectly disabling tests when LLVM_EXTERNAL_LIT is used

mgorny added inline comments. Comment at: clang/CMakeLists.txt:97 # Seek installed Lit. -find_program(LLVM_LIT - NAMES llvm-lit lit.py lit - PATHS "${LLVM_MAIN_SRC_DIR}/utils/lit" - DOC "Path to lit.py") +if (NOT

[PATCH] D138247: PR58819: Correct linkage and mangling of lambdas in inline static member initializers

mgorny added a comment. From Gentoo perspective, correct is better, correct and matching GCC is the best (we support mixing compilers to some degree). I think it'd be best to wait for confirmation on GCC end. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D137024: [compiler-rt] Switch from llvm-config to find_package(LLVM)

mgorny added a comment. In D137024#3931126 , @thetruestblue wrote: > This patch removes the logic that sets the binary tools dir using > llvm-config. We don't have llvmConfig.cmake in our toolchain or our build > tree and relied on llvm-config to >

[PATCH] D136940: [clang][Driver] allow tilde in user config dir

mgorny added a comment. Ah, I'm sorry for the delay. I have time right now, so I'll test and push it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136940/new/ https://reviews.llvm.org/D136940 ___

[PATCH] D136940: [clang][Driver] allow tilde in user config dir

This revision was automatically updated to reflect the committed changes. Closed by commit rGb3f94fe1c3c4: [clang][Driver] allow tilde in user config dir (authored by mgorny). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136940/new/

[PATCH] D137024: [compiler-rt] Switch from llvm-config to find_package(LLVM)

mgorny added a comment. Well, I'm certainly not opposed to making all the paths configurable. However, I'm not sure if having CMake file accessible one way or another wouldn't eventually be a necessity. For one thing, I would like to move more common code from standalone build codepaths of

[PATCH] D137043: [clang] add implicit include for Linux/gnu compatibility

mgorny resigned from this revision. mgorny added a reviewer: MaskRay. mgorny added a comment. Herald added a subscriber: StephenFan. I'm afraid this is above my skills. Maybe @MaskRay? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137043/new/

[PATCH] D138520: [clangd] Make decision forest model optional

mgorny added inline comments. Comment at: clang-tools-extra/clangd/CodeComplete.h:129 +#if defined(CLANGD_DECISION_FOREST) +# define DEFAULT_RANKING_MODEL DecisionForest sammccall wrote: > this approach feels a bit heavy on the preprocessor, especially for a

[PATCH] D138520: [clangd] Make decision forest model optional

mgorny abandoned this revision. mgorny added a comment. Closing in favor of D139107 . CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138520/new/ https://reviews.llvm.org/D138520 ___ cfe-commits mailing list

[PATCH] D139107: [clangd] Allow to build Clangd without decision forest

mgorny added a comment. Thanks a lot for doing this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139107/new/ https://reviews.llvm.org/D139107 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D138520: [clangd] Make decision forest model optional

mgorny created this revision. mgorny added reviewers: thesamesam, sammccall, thakis, MaskRay. Herald added subscribers: StephenFan, kadircet, arphaman. Herald added a project: All. mgorny requested review of this revision. Herald added a subscriber: ilya-biryukov. Herald added a project:

[PATCH] D138520: [clangd] Make decision forest model optional

mgorny added a comment. @thesamesam, could you test it on PPC32, please? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138520/new/ https://reviews.llvm.org/D138520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D138062: [clang] Don't include C++ Standard Library headers when -nostdinc is used

mgorny accepted this revision as: mgorny. mgorny added a comment. I'd say this is fine to land as 16.x. I'll be doing another snapshot for Gentoo next weekend, so if it lands by then, we're going to do some crash testing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D142449: [clang] Fix linking to LLVMTestingAnnotations in standalone build

mgorny created this revision. mgorny added reviewers: rupprecht, tstellar, MaskRay, thesamesam. Herald added a project: All. mgorny requested review of this revision. The LLVMTestingAnnotations library that is now used by unittests is not installed as part of LLVM. In order to make it possible

[PATCH] D140315: [AMDGCN] Update search path for device libraries

mgorny added a comment. I'm sorry for reporting it this late (clang's been broken for over two weeks, so we couldn't periodically test it) but this change is causing test regressions on Gentoo/amd64: FAIL: Clang :: Driver/hip-device-libs.hip (9304 of 17486) TEST 'Clang

[PATCH] D142449: [clang] Fix linking to LLVMTestingAnnotations in standalone build

mgorny added a comment. Thanks for the superfast review! I'll push it later today. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142449/new/ https://reviews.llvm.org/D142449 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D142449: [clang] Fix linking to LLVMTestingAnnotations in standalone build

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0f8b995b6362: [clang] Fix linking to LLVMTestingAnnotations in standalone build (authored by mgorny). Herald added a project: clang. Repository:

[PATCH] D141581: [clang] Make clangBasic and clangDriver depend on LLVMTargetParser.

mgorny added a comment. In D141581#4072692 , @kwk wrote: > In D141581#4071617 , @thesamesam > wrote: > >> This is currently holding back further testing on our end which is >> concerning me a bit, especially as

[PATCH] D140315: [AMDGCN] Update search path for device libraries

mgorny added a comment. In D140315#4077193 , @yaxunl wrote: > @scchan the test expects CLANG_INSTALL_LIBDIR_BASENAME to be `lib`. Maybe in > the failed test it was not true. `CMAKE_INSTALL_LIBDIR=lib64`, so I guess `_BASENAME` is that as well.

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

mgorny added a comment. While the general direction seems sound, I suspect this may be a real PITA given that for us: -DCLANG_RESOURCE_DIR="../../../../lib/clang/${LLVM_MAJOR}" I'm going to try and see later throughout the week if it's possible to keep that working with this patch.

[PATCH] D141248: [Clang] [Python] Fix tests when default config file contains -include

mgorny added reviewers: jbcoe, bkramer. mgorny added a comment. I'm wondering if we shouldn't just by setting `CLANG_NO_DEFAULT_CONFIG=1` like in clang's test suite rather than tying to predict how different config files will affect these tests. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D137024: [compiler-rt] Switch from llvm-config to find_package(LLVM)

mgorny added a comment. > These workarounds are fine for me, but I wonder if it would be useful with a > more direct cmake option to tell it not look for these files at all. CMake has something like that built-in. I think it's `-DCMAKE_DISABLE_FIND_PACKAGE_LLVM=ON`. Repository: rG LLVM

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

mgorny added reviewers: serge-sans-paille, sylvestre.ledru, phosek, MaskRay. mgorny added a comment. Herald added subscribers: Michael137, StephenFan. To be honest, I've never seen much purpose in the x.y.z directory naming and I feel like it's making packaging unnecessarily more complex, at

[PATCH] D137669: clang/cmake: Require pre-built test dependencies for stand-alone builds

mgorny added a comment. Could you issue a warning though? It's really annoying when targets disappear like that and you have to guess which of the checks failed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137669/new/

[PATCH] D137669: clang/cmake: Require pre-built test dependencies for stand-alone builds

mgorny added a comment. I don't have a strong opinion. I'm happy as long as there is *anything* telling me "tests were disabled because ...". Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137669/new/ https://reviews.llvm.org/D137669

[PATCH] D137337: Replace LLVM_LIBDIR_SUFFIX by CMAKE_INSTALL_LIBDIR

mgorny added inline comments. Comment at: bolt/lib/RuntimeLibs/RuntimeLibrary.cpp:32 SmallString<128> LibPath = llvm::sys::path::parent_path(Dir); - llvm::sys::path::append(LibPath, "lib" LLVM_LIBDIR_SUFFIX); + llvm::sys::path::append(LibPath, CMAKE_INSTALL_LIBDIR); if

[PATCH] D136282: [clang] [CMake] Link libclangBasic against libatomic when necessary.

This revision was automatically updated to reflect the committed changes. Closed by commit rG20132d8eaa68: Link libclangBasic against libatomic when necessary. (authored by thesamesam, committed by mgorny). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D137024: [compiler-rt] Switch from llvm-config to find_package(LLVM)

This revision was automatically updated to reflect the committed changes. Closed by commit rG37acf9bdd468: [compiler-rt] Switch from llvm-config to find_package(LLVM) (authored by mgorny). Herald added a project: Sanitizers. Herald added a subscriber: Sanitizers. Repository: rG LLVM Github

[PATCH] D137024: [compiler-rt] Switch from llvm-config to find_package(LLVM)

mgorny reopened this revision. mgorny added a comment. This revision is now accepted and ready to land. Gotta figure the buildbot regression out. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137024/new/ https://reviews.llvm.org/D137024

[PATCH] D137024: [compiler-rt] Switch from llvm-config to find_package(LLVM)

mgorny added a comment. I'd really use some help with the buildbot failures because I can't even figure out how to reproduce them: https://lab.llvm.org/buildbot#builders/169/builds/13146 https://lab.llvm.org/buildbot#builders/70/builds/29330

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

mgorny added inline comments. Comment at: clang/CMakeLists.txt:14 set(CLANG_BUILT_STANDALONE TRUE) + if ("${CMAKE_VERSION}" VERSION_LESS "3.20.0") +message(WARNING I wonder if we could move this to `CMakePolicy.cmake`, though admittedly you'd have to

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

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

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

mgorny accepted this revision. mgorny added a comment. This revision is now accepted and ready to land. LGTM (without testing, just eyeball review). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136354/new/ https://reviews.llvm.org/D136354

[PATCH] D137024: [compiler-rt] Switch from llvm-config to find_package(LLVM)

mgorny added a comment. Ok, I think I know what the problem is — these buildbots rely on being able to override `LLVM_CONFIG_PATH`. I'm going to see if I can provide backwards compatibility with it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D137024: [compiler-rt] Switch from llvm-config to find_package(LLVM)

mgorny updated this revision to Diff 472285. mgorny added a comment. Add a backwards compatibility path that uses `LLVM_CONFIG_PATH` to locate LLVM CMake files. I think it's sufficient to use path relative to the specified location, and we don't want to actually call it. Restore the explicit

[PATCH] D137024: [compiler-rt] Switch from llvm-config to find_package(LLVM)

mgorny added a comment. @phosek, could you take another look? The only changes are on top of `load_llvm_config`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137024/new/ https://reviews.llvm.org/D137024 ___ cfe-commits mailing list

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

mgorny added reviewers: MaskRay, thesamesam, tstellar. mgorny added a comment. Ok, this turned out to be surprisingly painless, at least within our packaging. However, I'd prefer if someone more experienced looked at the CMake changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D146427: [clang-tools-extra] Fix linking ClangdTests when using libclang-cpp

mgorny added a reviewer: thesamesam. mgorny added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146427/new/ https://reviews.llvm.org/D146427 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D143436: [clangd] Move standard options adaptor to CommandMangler

mgorny added a comment. This commit broke building against `libclang-cpp.so` dylib. I've submitted a fix as D146427 but it haven't received any attention so far. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D146427: [clang-tools-extra] Fix linking ClangdTests when using libclang-cpp

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3bff90e7e84a: [clang-tools-extra] Fix linking ClangdTests when using libclang-cpp (authored by mgorny). Repository: rG LLVM Github Monorepo

[PATCH] D146427: [clang-tools-extra] Fix linking ClangdTests when using libclang-cpp

mgorny added a comment. Thank you. 16.x isn't affected. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146427/new/ https://reviews.llvm.org/D146427 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D146891: [Driver][NetBSD] Simplify NetBSD version handling

mgorny accepted this revision. mgorny added a comment. This revision is now accepted and ready to land. WFM. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146891/new/ https://reviews.llvm.org/D146891

[PATCH] D148496: [compiler-rt] [test] Mark dfsan tests XFAIL on glibc-2.37

mgorny created this revision. mgorny added reviewers: browneee, vitalybuka, MaskRay, thesamesam, jianzhou.zh. Herald added subscribers: Enna1, dberris. Herald added a project: All. mgorny requested review of this revision. Mark the two dfsan tests that are known to be broken on glibc-2.37 as

[PATCH] D148496: [compiler-rt] [test] Mark dfsan tests XFAIL on glibc-2.37

This revision was automatically updated to reflect the committed changes. Closed by commit rG488a4b24817a: [compiler-rt] [test] Mark dfsan tests XFAIL on glibc-2.37 (authored by mgorny). Herald added a project: Sanitizers. Herald added a subscriber: Sanitizers. Repository: rG LLVM Github

[PATCH] D148496: [compiler-rt] [test] Mark dfsan tests XFAIL on glibc-2.37

mgorny added a comment. In D148496#4274352 , @browneee wrote: > Issue appears to be with the program rather than the instrumentation. Did the > behavior of the the libc functions used in the test change? If the question was directed at me, then I'm

[PATCH] D146427: [clang-tools-extra] Fix linking ClangdTests when using libclang-cpp

mgorny created this revision. mgorny added reviewers: DmitryPolukhin, kadircet, tstellar, MaskRay. Herald added subscribers: mikhail.ramalho, arphaman. Herald added a project: All. mgorny requested review of this revision. Herald added a subscriber: ilya-biryukov. Herald added a project:

[PATCH] D155380: [clang] Fix delayed template parsing

mgorny accepted this revision. mgorny added a comment. This revision is now accepted and ready to land. Thanks. I can confirm that the tests pass for me with this applied. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155380/new/

[PATCH] D148793: [clang-tidy] Implement an include-cleaner check.

mgorny added a comment. I'm getting completely incomprehensible build errors on Gentoo from this, as of aa7eace8431ba213c5431638b894b0e1b4b481c7 : samu: job failed: : && /usr/lib/ccache/bin/x86_64-pc-linux-gnu-g++

[PATCH] D148793: [clang-tidy] Implement an include-cleaner check.

mgorny added a comment. My educated guess would be that `clangIncludeCleaner` is being linked via `clang_target_link_libraries` while it's not part of `libclang-cpp`, so it should go into regular `target_link_libraries`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D153590: Don't use float_t and double_t with #pragma clang fp eval_method.

mgorny added a comment. This change causes test regressions on 32-bit x86: FAIL: Clang :: Sema/fp-eval-pragma-with-float-double_t-3.c (14665 of 17845) TEST 'Clang :: Sema/fp-eval-pragma-with-float-double_t-3.c' FAILED Script: -- : 'RUN: at

[PATCH] D153770: Fix test regression on 32-bit x86.

mgorny accepted this revision. mgorny added a comment. This revision is now accepted and ready to land. Thanks. I can confirm that this fixes the test failures for me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153770/new/

[PATCH] D153770: Fix test regression on 32-bit x86.

mgorny added a comment. I think you also need to require `X86` target but I'm not 100% sure. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153770/new/ https://reviews.llvm.org/D153770 ___ cfe-commits

[PATCH] D153770: Fix test regression on 32-bit x86.

mgorny added a comment. In D153770#4449154 , @zahiraam wrote: > In D153770#4449078 , @mgorny wrote: > >> I think you also need to require `X86` target but I'm not 100% sure. > > I don't think so. That warning was

[PATCH] D152696: Prevent deadlocks in death tests.

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

[PATCH] D152696: Prevent deadlocks in death tests.

mgorny added a comment. This change is causing a lot of unittests to fail on Gentoo. I've tested both on amd64 and arm64; on amd64 additionally the test suite seems to hang. Failed Tests (75): LLVM-Unit :: ADT/./ADTTests/APFloatTest/SemanticsDeath LLVM-Unit ::

<    4   5   6   7   8   9