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

sebastian-ne added inline comments. Comment at: cmake/Modules/GetClangResourceDir.cmake:15 + else() +string(REGEX MATCH "^[0-9]+" CLANG_VERSION_MAJOR ${PACKAGE_VERSION}) +set(ret_dir lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION_MAJOR}) This fails in

[PATCH] D141538: [cmake] Fix path to LLVMConfig.cmake for multi-config builds

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGeb4aa6c7a5f2: [cmake] Fix path to LLVMConfig.cmake for multi-config builds (authored by nhat-nguyen, committed by sebastian-ne). Repository: rG

[PATCH] D141538: [cmake] Fix path to LLVMConfig.cmake for multi-config builds

sebastian-ne added a comment. The debian pre-checkin test is unfortunately quite unstable. I see the same failures in D141469 for example. This looks good to go. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D137517: [TargetParser] Generate the defs for RISCV CPUs using llvm-tblgen.

sebastian-ne added a comment. FYI, the CMake file should use `PROJECT_SOURCE_DIR` instead of `CMAKE_SOURCE_DIR`, otherwise it breaks builds that use CMake’s add_subdirectory. I put up D141521 to fix that. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D141538: [cmake] Fix path to LLVMConfig.cmake for multi-config builds

sebastian-ne accepted this revision. sebastian-ne added a comment. This revision is now accepted and ready to land. Herald added a subscriber: jdoerfert. Thanks for fixing the whitespace as well! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D139973: [llvm] Make llvm::Any similar to std::any

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGbb7940e25f6c: [llvm] Make llvm::Any similar to std::any (authored by sebastian-ne). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D139973: [llvm] Make llvm::Any similar to std::any

sebastian-ne added inline comments. Comment at: lldb/include/lldb/Core/RichManglingContext.h:90-91 assert(parser.has_value()); -assert(llvm::any_isa(parser)); +assert(llvm::any_cast()); return llvm::any_cast(parser); } barannikov88 wrote: >

[PATCH] D139973: [llvm] Make llvm::Any similar to std::any

sebastian-ne updated this revision to Diff 483975. sebastian-ne marked 2 inline comments as done. sebastian-ne added a comment. > It is surprising to me that std::any can work without RTTI. Never thought it > could be implemented. It seems like libstdc++ and libc++ both implement it the way

[PATCH] D139006: [UpdateTestChecks] Match define for labels

sebastian-ne added a comment. > I believe the motivation here is the default behavior Correct, update_test_checks produces a broken test for some input and I think this is a bug that we should fix. Sorry for the test churn and thanks for the revert (I only got to work a few hours after nikic

[PATCH] D139973: [llvm] Make llvm::Any similar to std::any

sebastian-ne created this revision. sebastian-ne added reviewers: jloser, MaskRay, dblaikie, jsilvanus. Herald added subscribers: ormris, StephenFan, wenlei, hiraditya. Herald added a project: All. sebastian-ne requested review of this revision. Herald added projects: clang, LLDB, LLVM. Herald

[PATCH] D139006: [UpdateTestChecks] Match define for labels

sebastian-ne added a comment. In D139006#3988215 , @mkazantsev wrote: > So now every single test needs to be regenerated? It'll create straw diff > from nowhere... We don’t need to regenerate every test. Similar to how we don’t reformat all of LLVM

[PATCH] D139006: [UpdateTestChecks] Match define for labels

This revision was automatically updated to reflect the committed changes. Closed by commit rGa25aeef8: [UpdateTestChecks] Match define for labels (authored by sebastian-ne). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139006/new/

[PATCH] D139006: [UpdateTestChecks] Match define for labels

sebastian-ne added a comment. I guess this is fine to merge. I’ll leave it for a day in case someone has more comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139006/new/ https://reviews.llvm.org/D139006

[PATCH] D139006: [UpdateTestChecks] Match define for labels

sebastian-ne added inline comments. Comment at: llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/define_after_call.ll.expected:14 +define i32 @b(i32 %arg) { +; CHECK-LABEL: define {{[^@]+}}@b( +; CHECK-NEXT:ret i32 [[ARG:%.*]] arichardson wrote: >

[PATCH] D139006: [UpdateTestChecks] Match define for labels

sebastian-ne updated this revision to Diff 478933. sebastian-ne added a comment. Herald added a reviewer: jdoerfert. Herald added subscribers: cfe-commits, jdoerfert, sstefan1. Herald added a project: clang. Thanks for the review! I updated the update_cc_tests tests and added a test where the

[PATCH] D132316: [CMake] Avoid `LLVM_BINARY_DIR` when other more specific variable are better-suited, part 2

sebastian-ne accepted this revision. sebastian-ne added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132316/new/ https://reviews.llvm.org/D132316 ___ cfe-commits mailing list

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

sebastian-ne added a comment. I’m not sure if it’s the case for all places (as `CMAKE_CFG_INTDIR` is not defined at install time), but I think the `CMAKE_BINARY_LIBDIR` introduced here serves the same purpose as the already existing `LLVM_LIBRARY_DIR`. Same for `CMAKE_BINARY_INCLUDEDIR`, which

[PATCH] D132316: [CMake] Avoid `LLVM_BINARY_DIR` when other more specific variable are better-suited

sebastian-ne added a comment. This is probably caused by using `CMAKE_CFG_INTDIR` (indirectly) in more places. Seems like it expands to `$(Configuration)` for Visual Studio. Searching more about it, it’s only set at build time, but not at install time, which is a problem as well. On top of all,

[PATCH] D132316: [CMake] Avoid `LLVM_BINARY_DIR` when other more specific variable are better-suited

sebastian-ne added a comment. The build afterwards succeeded again. Seems like every ~20th build on that windows machine fails. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132316/new/ https://reviews.llvm.org/D132316

[PATCH] D132316: [CMake] `${LLVM_BINARY_DIR}/lib(${LLVM_LIBDIR_SUFFIX})?` -> `${LLVM_LIBRARY_DIR}`

sebastian-ne added inline comments. Comment at: llvm/tools/llvm-shlib/CMakeLists.txt:91 set(LIB_DIR ${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/lib${LLVM_LIBDIR_SUFFIX}) set(LIB_NAME ${LIB_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}LLVM) Ericson2314 wrote: >

[PATCH] D132316: [CMake] `${LLVM_BINARY_DIR}/lib(${LLVM_LIBDIR_SUFFIX})?` -> `${LLVM_LIBRARY_DIR}`

sebastian-ne added inline comments. Comment at: llvm/tools/llvm-shlib/CMakeLists.txt:89 - set(LLVM_EXPORTED_SYMBOL_FILE ${LLVM_BINARY_DIR}/libllvm-c.exports) The lib here is not used as a folder, so this should probably not be replaced? It should probably

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

sebastian-ne added a comment. In D130586#3731021 , @Ericson2314 wrote: > Anyone have any idea what this Debian test failure is about? I haven’t seen a passing debian pre-merge check for months now, so I usually ignore it (the failures seems to be

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

sebastian-ne accepted this revision. sebastian-ne added a comment. Looks good to me, thanks! Comment at: utils/bazel/llvm-project-overlay/clang/include/clang/Config/config.h:70 /* Multilib suffix for libdir. */ +#define CLANG_INSTALL_LIBDIR_BASENAME "lib"

[PATCH] D130545: [cmake] Slight fix ups to make robust to the full range of GNUInstallDirs

sebastian-ne added a comment. I pushed a potential fix (removing the backslash) in 50716ba2b337afe46ac256cc91673dc27356a776 . I don’t know which buildbots to look at though, it looks like the OpenMP ones all succeed. PS:

[PATCH] D130545: [cmake] Slight fix ups to make robust to the full range of GNUInstallDirs

sebastian-ne added inline comments. Comment at: openmp/runtime/src/CMakeLists.txt:383 \"${alias}${LIBOMP_LIBRARY_SUFFIX}\" WORKING_DIRECTORY -\"\$ENV{DESTDIR}\${CMAKE_INSTALL_PREFIX}/${OPENMP_INSTALL_LIBDIR}\")") +\"\$ENV{DESTDIR}\${outdir}\")")

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

sebastian-ne added inline comments. Comment at: clang/CMakeLists.txt:100 set(LLVM_RUNTIME_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin) - set(LLVM_LIBRARY_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/lib${LLVM_LIBDIR_SUFFIX}) +

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

sebastian-ne added inline comments. Comment at: clang/CMakeLists.txt:100 set(LLVM_RUNTIME_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin) - set(LLVM_LIBRARY_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/lib${LLVM_LIBDIR_SUFFIX}) +

[PATCH] D130553: [clang][lld][cmake] Simplify header dirs

sebastian-ne accepted this revision. sebastian-ne added a comment. This revision is now accepted and ready to land. Looks good to me, I left three questions inline. Comment at: clang/CMakeLists.txt:81 # path is removed. -set(MAIN_INCLUDE_DIR "${LLVM_INCLUDE_DIR}") +

[PATCH] D130553: [clang][lld][cmake] Simplify header dirs

sebastian-ne added inline comments. Comment at: clang/CMakeLists.txt:81 # path is removed. -set(MAIN_INCLUDE_DIR "${LLVM_INCLUDE_DIR}") +set(INCLUDE_DIRS ${LLVM_INCLUDE_DIRS}) set(LLVM_OBJ_DIR "${LLVM_BINARY_DIR}") Do we need to add

[PATCH] D130545: [cmake] Slight fix ups to make robust to the full range of GNUInstallDirs

sebastian-ne accepted this revision. sebastian-ne added a comment. This revision is now accepted and ready to land. Looks good to me Comment at: openmp/CMakeLists.txt:3 -# Add cmake directory to search for custom cmake functions. -set(CMAKE_MODULE_PATH

[PATCH] D117973: [cmake] Support custom package install paths

sebastian-ne added a comment. I found a regression when llvm is added with CMake’s add_subdirectory. D130555 has a fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117973/new/ https://reviews.llvm.org/D117973

[PATCH] D101070: [llvm][cmake] Make `install_symlink` workflow work with absolute install dirs

sebastian-ne accepted this revision. sebastian-ne added a comment. This revision is now accepted and ready to land. No problem, thanks for the fixes! LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101070/new/ https://reviews.llvm.org/D101070

[PATCH] D117973: [cmake] Support custom package install paths

sebastian-ne accepted this revision. sebastian-ne added a comment. This revision is now accepted and ready to land. Thank you, the comments make it clearer what’s happening. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117973/new/

[PATCH] D101070: [llvm][cmake] Make `install_symlink` workflow work with absolute install dirs

sebastian-ne added a comment. I get a configure error with this version: CMake Error at cmake/modules/AddLLVM.cmake:2052 (extend_path): Unknown CMake command "extend_path". Call Stack (most recent call first): cmake/modules/AddLLVM.cmake:2148 (llvm_install_symlink)

[PATCH] D101070: [llvm][cmake] Make `install_symlink` robust to absolute dirs.

sebastian-ne added a comment. In D101070#3675462 , @sebastian-ne wrote: > Hi, not sure if you saw D130256 , but I > needed to extend CMAKE_MODULE_PATH, otherwise the ExtendPath module was not > found when running

[PATCH] D117973: [cmake] Support custom package install paths

sebastian-ne added a comment. Two comments inline, apart from that it looks good to me. Comment at: cmake/Modules/FindPrefixFromConfig.cmake:30 + if(IS_ABSOLUTE "${path_to_leave}") +set(prefix_var + "# Installation prefix is fixed absolute path"

[PATCH] D101070: [llvm][cmake] Make `install_symlink` robust to absolute dirs.

sebastian-ne added a comment. Hi, not sure if you saw D130256 , but I needed to extend CMAKE_MODULE_PATH, otherwise the ExtendPath module was not found when running LLVMInstallSymlink.cmake. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D130338: [CMake] Copy folder without permissions

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGefe1527e28ca: [CMake] Copy folder without permissions (authored by sebastian-ne). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D130338: [CMake] Copy folder without permissions

sebastian-ne created this revision. sebastian-ne added reviewers: awarzynski, Ericson2314, tstellar. Herald added subscribers: bzcheeseman, sdasgup3, wenzhicui, wrengr, cota, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh, grosul1, Joonsoo, liufengdb, aartbik, mgester,

[PATCH] D130254: [CMake][Clang] Copy folder without permissions

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf359eac5df06: [CMake][Clang] Copy folder without permissions (authored by sebastian-ne). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D130254: [CMake][Clang] Copy folder without permissions

sebastian-ne created this revision. sebastian-ne added reviewers: awarzynski, PeteSteinfeld. Herald added a subscriber: mgorny. Herald added a project: All. sebastian-ne requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Copying the folder

[PATCH] D128465: [llvm] add zstd to `llvm::compression` namespace

sebastian-ne added a comment. This breaks builds when LLVM is included with CMake’s `add_subdirectory`. I think the zstd target needs to be marked as imported. The following patch fixes the problem, although I’m not familiar enough with CMake to know if this is the right way to go: diff --git

[PATCH] D116521: [CMake] Factor out config prefix finding logic

sebastian-ne added inline comments. Comment at: llvm/CMakeLists.txt:209 "${CMAKE_CURRENT_SOURCE_DIR}/cmake/modules" + "${LLVM_COMMON_CMAKE_UTILS}/Modules" ) Hi, adding this module path overwrites the `llvm_check_linker_flag` from

[PATCH] D110612: [Utils] Use common substs in update_cc_test_checks

sebastian-ne created this revision. sebastian-ne added reviewers: foad, arichardson. sebastian-ne requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. Use substitution methods from common.py in update_cc_test_checks.py.

[PATCH] D81728: [InstCombine] Add target-specific inst combining

Flakebi added a comment. Thanks for the notification @davezarzycki, an auto-bisecting bot is cool! This failure should be fixed in b99898c1e9c5d8bade1d898e84604d3241b0087c . Repository: rG LLVM Github

[PATCH] D81728: [InstCombine] Add target-specific inst combining

Flakebi marked an inline comment as done. Flakebi added inline comments. Comment at: llvm/include/llvm/Analysis/TargetTransformInfo.h:542 + bool simplifyDemandedUseBitsIntrinsic(InstCombiner , IntrinsicInst , +APInt DemandedMask,

[PATCH] D81728: [InstCombine] Add target-specific inst combining

Flakebi marked an inline comment as done. Flakebi added inline comments. Comment at: llvm/include/llvm/Analysis/TargetTransformInfo.h:540 + bool instCombineIntrinsic(InstCombiner , IntrinsicInst , +Instruction **ResultI) const; + bool