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

[Sebastian Neubauer via Phabricator via cfe-commits]

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 Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81728/new/

https://reviews.llvm.org/D81728



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer via Phabricator via cfe-commits]

Flakebi marked an inline comment as done.
Flakebi added inline comments.



Comment at: llvm/include/llvm/Analysis/TargetTransformInfo.h:540
+  bool instCombineIntrinsic(InstCombiner &IC, IntrinsicInst &II,
+Instruction **ResultI) const;
+  bool simplifyDemandedUseBitsIntrinsic(InstCombiner &IC, IntrinsicInst &II,

nikic wrote:
> For all three functions, the calling convention seems rather non-idiomatic 
> for InstCombine. Rather than having an `Instruction **` argument and bool 
> result, is there any reason not to have an `Instruction *` return value, with 
> nullptr indicating that the intrinsic couldn't be simplified?
Yes, the function must have the option to return a nullptr and prevent that 
`visitCallBase` is called or other code is executed after 
`instCombineIntrinsic`.
So, somehow the caller must be able to see a difference between 'do nothing, 
just continue execution' and 'return this Instruction*', where the 
`Instruction*` can also be a nullptr.
The return type could be an `optional`.

I’ll take a look at your other comments on Monday.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81728/new/

https://reviews.llvm.org/D81728



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer via Phabricator via cfe-commits]

Flakebi marked an inline comment as done.
Flakebi added inline comments.



Comment at: llvm/include/llvm/Analysis/TargetTransformInfo.h:542
+  bool simplifyDemandedUseBitsIntrinsic(InstCombiner &IC, IntrinsicInst &II,
+APInt DemandedMask, KnownBits &Known,
+bool &KnownBitsComputed,

nikic wrote:
> `const APInt &DemandedMask`?
I tried to change it it to to `const APInt &DemandedMask` but the x86 
simplifyDemandedVectorEltsIntrinsic changes `DemandedMask`, so this function 
would have to copy it or take a non-const reference.
Looking more into it, `SimplifyAndSetOp` takes `DemandedElts` by value too.
An `APInt` consists of a `uint64_t` and an `unsigned`, so it should be 16 Byte 
in most cases. Only if the represented int is larger than 64 bit, it comes with 
an allocation. I guess copying should be fine.
If you think it should be a reference anyway, let me know and I’ll change it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81728/new/

https://reviews.llvm.org/D81728



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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 ${CMAKE_CURRENT_SOURCE_DIR}/cmake ${CMAKE_MODULE_PATH})
+set(LLVM_COMMON_CMAKE_UTILS ${CMAKE_CURRENT_SOURCE_DIR}/../cmake)
+

There are some places that surround this by an `if(NOT DEFINED 
LLVM_COMMON_CMAKE_UTILS)` (e.g. lldb/cmake/modules/LLDBStandalone.cmake, 
clang/CMakeLists.txt, polly/CMakeLists.txt) although other places don’t 
(lld/CMakeLists.txt, libunwind/CMakeLists.txt, etc.), so both seem to be fine.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130545/new/

https://reviews.llvm.org/D130545

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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 `"${LLVM_BINARY_DIR}/include"` here as well?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130553/new/

https://reviews.llvm.org/D130553

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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}")
+set(INCLUDE_DIRS ${LLVM_INCLUDE_DIRS})
 set(LLVM_OBJ_DIR "${LLVM_BINARY_DIR}")

Ericson2314 wrote:
> sebastian-ne wrote:
> > Do we need to add `"${LLVM_BINARY_DIR}/include"` here as well?
> Pretty sure we shouldn't be cause that should be one of the elements of the 
> list when LLVM is also being built in this CMake invocation.
The quotation marks went missing, is this intended? (Same in lld/CMakeLists.txt)



Comment at: clang/CMakeLists.txt:91
 
-  set(LLVM_MAIN_INCLUDE_DIR "${MAIN_INCLUDE_DIR}" CACHE PATH "Path to 
llvm/include")
+  set(LLVM_INCLUDE_DIRS ${INCLUDE_DIRS} CACHE PATH "Path to llvm/include and 
any other header dirs needed")
   set(LLVM_BINARY_DIR "${LLVM_OBJ_ROOT}" CACHE PATH "Path to LLVM build tree")

The quotation marks went missing, is this intended? (Same in lld/CMakeLists.txt)



Comment at: clang/CMakeLists.txt:133
 
-  include_directories("${LLVM_BINARY_DIR}/include" "${LLVM_MAIN_INCLUDE_DIR}")
+  include_directories(${LLVM_INCLUDE_DIRS})
   link_directories("${LLVM_LIBRARY_DIR}")

The quotation marks went missing, is this intended? (Same in lld/CMakeLists.txt)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130553/new/

https://reviews.llvm.org/D130553

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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})
+  set(LLVM_LIBRARY_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/lib)
   if(WIN32 OR CYGWIN)

Just to check if your intention aligns with my understanding, removing the 
suffix here is fine because the destination is in the binary dir and not the 
final install destination?



Comment at: clang/runtime/CMakeLists.txt:90-92
+  -DCMAKE_INSTALL_BINDIR="${CMAKE_INSTALL_BINDIR}"
+  -DCMAKE_INSTALL_LIBDIR="${CMAKE_INSTALL_LIBDIR}"
+  
-DCMAKE_INSTALL_INCLUDEDIR="${CMAKE_INSTALL_INCLUDEDIR}"

nit: The indentation is wrong



Comment at: compiler-rt/cmake/Modules/CompilerRTAIXUtils.cmake:65
   set(output_dir "${LLVM_LIBRARY_OUTPUT_INTDIR}")
-  set(install_dir "${CMAKE_INSTALL_PREFIX}/lib${LLVM_LIBDIR_SUFFIX}")
+  set(install_dir "${CMAKE_INSTALL_PREFIX}/lib")
 else()

This is an install directory, so should this be something like
```
extend_path(install_dir "${CMAKE_INSTALL_PREFIX}" "${CMAKE_INSTALL_LIBDIR}")
```
instead?



Comment at: compiler-rt/cmake/base-config-ix.cmake:48
   set(COMPILER_RT_EXEC_OUTPUT_DIR ${LLVM_RUNTIME_OUTPUT_INTDIR})
-  set(COMPILER_RT_INSTALL_PATH lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION})
+  set(COMPILER_RT_INSTALL_PATH lib/clang/${CLANG_VERSION})
   option(COMPILER_RT_INCLUDE_TESTS "Generate and build compiler-rt unit tests."

This is an install path, so should it use 
`${CMAKE_INSTALL_LIBDIR}/clang/${CLANG_VERSION}` instead?



Comment at: compiler-rt/cmake/base-config-ix.cmake:103
 ${COMPILER_RT_OUTPUT_DIR}/lib)
-  extend_path(default_install_path "${COMPILER_RT_INSTALL_PATH}" lib)
+  extend_path(default_install_path "${COMPILER_RT_INSTALL_PATH}" 
"${CMAKE_INSTALL_LIBDIR}")
   set(COMPILER_RT_INSTALL_LIBRARY_DIR "${default_install_path}" CACHE PATH

What is the result we expect here?
In case that CMAKE_INSTALL_LIBDIR is defined as lib64, this path will change.

In some cases it would have been `lib64/clang/14.0.0/lib`,
but with this patch it would be `lib/clang/14.0.0/lib64` if I understand 
correctly.



Comment at: lldb/source/API/CMakeLists.txt:116
 if(LLDB_ENABLE_PYTHON AND (BUILD_SHARED_LIBS OR LLVM_LINK_LLVM_DYLIB) AND UNIX 
AND NOT APPLE)
-  set_property(TARGET liblldb APPEND PROPERTY INSTALL_RPATH 
"\$ORIGIN/../../../../lib${LLVM_LIBDIR_SUFFIX}")
+  set_property(TARGET liblldb APPEND PROPERTY INSTALL_RPATH 
"\$ORIGIN/../../../../lib")
 endif()

It looks to me like this path is used for installation, so should it have the 
(potential) suffix?
In AddLLVM.cmake, _install_rpath uses `${CMAKE_INSTALL_LIBDIR}`.



Comment at: mlir/cmake/modules/AddMLIRPython.cmake:412
 set_property(TARGET ${target} APPEND PROPERTY
-  INSTALL_RPATH 
"${_origin_prefix}/${ARG_RELATIVE_INSTALL_ROOT}/lib${LLVM_LIBDIR_SUFFIX}")
+  INSTALL_RPATH "${_origin_prefix}/${ARG_RELATIVE_INSTALL_ROOT}/lib")
   endif()

Same here as above, the rpath should probably use `${CMAKE_INSTALL_LIBDIR}`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130586/new/

https://reviews.llvm.org/D130586

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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})
+  set(LLVM_LIBRARY_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/lib)
   if(WIN32 OR CYGWIN)

Ericson2314 wrote:
> sebastian-ne wrote:
> > Just to check if your intention aligns with my understanding, removing the 
> > suffix here is fine because the destination is in the binary dir and not 
> > the final install destination?
> Yes exactly.
> 
> I really should write up the "rules" that I've (a) discovered from reading 
> (b) invented somewhere. Any idea where?
I think the commit message is a good place.



Comment at: llvm/CMakeLists.txt:59-60
+if (NOT DEFINED CMAKE_INSTALL_LIBDIR AND DEFINED LLVM_LIBDIR_SUFFIX)
+  message(WARNING "\"LLVM_LIBDIR_SUFFIX\" is deprecated. "
+  "Please set \"CMAKE_INSTALL_LIBDIR\" directly instead.")
+  set(CMAKE_INSTALL_LIBDIR "lib${LLVM_LIBDIR_SUFFIX}")

There’s also `message(DEPRECATION …)` :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130586/new/

https://reviews.llvm.org/D130586

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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}\")")
 endforeach()

The backslash before `\${outdir}` is wrong here, it must be 
`\"\$ENV{DESTDIR}${outdir}\")")`.

Also, before this patch, there were a lot of backslashes before e.g. 
`\${CMAKE_INSTALL_PREFIX}`, but I hope that shouldn’t matter, the value should 
be the same at configure and install time.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130545/new/

https://reviews.llvm.org/D130545

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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: I’ll be away for a few weeks, please revert my fix if it causes problems.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130545/new/

https://reviews.llvm.org/D130545

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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 a/llvm/cmake/modules/FindZSTD.cmake 
b/llvm/cmake/modules/FindZSTD.cmake
  index 43ccf7232138..8e6ec6e8a988 100644
  --- a/llvm/cmake/modules/FindZSTD.cmake
  +++ b/llvm/cmake/modules/FindZSTD.cmake
  @@ -12,6 +12,10 @@ find_package_handle_standard_args(
   ZSTD_LIBRARY ZSTD_INCLUDE_DIR)
  
   if(ZSTD_FOUND)
  +if(NOT TARGET Zstd::zstd)
  +  add_library(Zstd::zstd UNKNOWN IMPORTED)
  +  set_target_properties(Zstd::zstd PROPERTIES IMPORTED_LOCATION 
"${ZSTD_LIBRARY}")
  +endif()
   set(ZSTD_LIBRARIES ${ZSTD_LIBRARY})
   set(ZSTD_INCLUDE_DIRS ${ZSTD_INCLUDE_DIR})
   endif()
  diff --git a/llvm/lib/Support/CMakeLists.txt b/llvm/lib/Support/CMakeLists.txt
  index 52b95c5377d3..031adfa33ba8 100644
  --- a/llvm/lib/Support/CMakeLists.txt
  +++ b/llvm/lib/Support/CMakeLists.txt
  @@ -26,7 +26,7 @@ if(LLVM_ENABLE_ZLIB)
   endif()
  
   if(LLVM_ENABLE_ZSTD)
  -  list(APPEND imported_libs zstd)
  +  list(APPEND imported_libs Zstd::zstd)
   endif()
  
   if( MSVC OR MINGW )


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128465/new/

https://reviews.llvm.org/D128465

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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 keeps the original permissions by default. This
creates problems when the source folder is read-only, e.g. in a
packaging environment.
Then, the copied folder in the build directory is read-only as well.
Later on, with configure_file, ClangConfig.cmake is copied into that
directory (in the build tree), failing when the directory is read-only.

Fix that problem by copying the folder without keeping the original
permissions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130254

Files:
  clang/cmake/modules/CMakeLists.txt


Index: clang/cmake/modules/CMakeLists.txt
===
--- clang/cmake/modules/CMakeLists.txt
+++ clang/cmake/modules/CMakeLists.txt
@@ -32,8 +32,10 @@
 
 # For compatibility with projects that include(ClangConfig)
 # via CMAKE_MODULE_PATH, place API modules next to it.
+# Copy without source permissions because the source could be read-only
 file(COPY .
   DESTINATION ${clang_cmake_builddir}
+  NO_SOURCE_PERMISSIONS
   FILES_MATCHING PATTERN *.cmake
   PATTERN CMakeFiles EXCLUDE
   )


Index: clang/cmake/modules/CMakeLists.txt
===
--- clang/cmake/modules/CMakeLists.txt
+++ clang/cmake/modules/CMakeLists.txt
@@ -32,8 +32,10 @@
 
 # For compatibility with projects that include(ClangConfig)
 # via CMAKE_MODULE_PATH, place API modules next to it.
+# Copy without source permissions because the source could be read-only
 file(COPY .
   DESTINATION ${clang_cmake_builddir}
+  NO_SOURCE_PERMISSIONS
   FILES_MATCHING PATTERN *.cmake
   PATTERN CMakeFiles EXCLUDE
   )
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer 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 rGf359eac5df06: [CMake][Clang] Copy folder without permissions 
(authored by sebastian-ne).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130254/new/

https://reviews.llvm.org/D130254

Files:
  clang/cmake/modules/CMakeLists.txt


Index: clang/cmake/modules/CMakeLists.txt
===
--- clang/cmake/modules/CMakeLists.txt
+++ clang/cmake/modules/CMakeLists.txt
@@ -32,8 +32,10 @@
 
 # For compatibility with projects that include(ClangConfig)
 # via CMAKE_MODULE_PATH, place API modules next to it.
+# Copy without source permissions because the source could be read-only
 file(COPY .
   DESTINATION ${clang_cmake_builddir}
+  NO_SOURCE_PERMISSIONS
   FILES_MATCHING PATTERN *.cmake
   PATTERN CMakeFiles EXCLUDE
   )


Index: clang/cmake/modules/CMakeLists.txt
===
--- clang/cmake/modules/CMakeLists.txt
+++ clang/cmake/modules/CMakeLists.txt
@@ -32,8 +32,10 @@
 
 # For compatibility with projects that include(ClangConfig)
 # via CMAKE_MODULE_PATH, place API modules next to it.
+# Copy without source permissions because the source could be read-only
 file(COPY .
   DESTINATION ${clang_cmake_builddir}
+  NO_SOURCE_PERMISSIONS
   FILES_MATCHING PATTERN *.cmake
   PATTERN CMakeFiles EXCLUDE
   )
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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, arpith-jacob, antiagainst, shauheen, 
rriddle, mehdi_amini, mgorny.
Herald added a project: All.
sebastian-ne requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, stephenneuendorffer, 
nicolasvasilache.
Herald added projects: clang, MLIR, LLVM.

Copying the folder keeps the original permissions by default. This
creates problems when the source folder is read-only, e.g. in a
packaging environment.
Then, the copied folder in the build directory is read-only as well.
Later on, other files are copied into that directory (in the build
tree), failing when the directory is read-only.

Fix that problem by copying the folder without keeping the original
permissions.

Follow-up to D130254 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130338

Files:
  clang/cmake/modules/CMakeLists.txt
  llvm/cmake/modules/CMakeLists.txt
  mlir/cmake/modules/CMakeLists.txt


Index: mlir/cmake/modules/CMakeLists.txt
===
--- mlir/cmake/modules/CMakeLists.txt
+++ mlir/cmake/modules/CMakeLists.txt
@@ -42,9 +42,12 @@
 
 # For compatibility with projects that include(MLIRConfig)
 # via CMAKE_MODULE_PATH, place API modules next to it.
+# Copy without source permissions because the source could be read-only,
+# but we need to write into the copied folder.
 # This should be removed in the future.
 file(COPY .
   DESTINATION ${mlir_cmake_builddir}
+  NO_SOURCE_PERMISSIONS
   FILES_MATCHING PATTERN *.cmake
   PATTERN CMakeFiles EXCLUDE
   )
Index: llvm/cmake/modules/CMakeLists.txt
===
--- llvm/cmake/modules/CMakeLists.txt
+++ llvm/cmake/modules/CMakeLists.txt
@@ -99,9 +99,12 @@
 
 # For compatibility with projects that include(LLVMConfig)
 # via CMAKE_MODULE_PATH, place API modules next to it.
+# Copy without source permissions because the source could be read-only,
+# but we need to write into the copied folder.
 # This should be removed in the future.
 file(COPY .
   DESTINATION ${llvm_cmake_builddir}
+  NO_SOURCE_PERMISSIONS
   FILES_MATCHING PATTERN *.cmake
   PATTERN CMakeFiles EXCLUDE
   )
Index: clang/cmake/modules/CMakeLists.txt
===
--- clang/cmake/modules/CMakeLists.txt
+++ clang/cmake/modules/CMakeLists.txt
@@ -32,7 +32,8 @@
 
 # For compatibility with projects that include(ClangConfig)
 # via CMAKE_MODULE_PATH, place API modules next to it.
-# Copy without source permissions because the source could be read-only
+# Copy without source permissions because the source could be read-only,
+# but we need to write into the copied folder.
 file(COPY .
   DESTINATION ${clang_cmake_builddir}
   NO_SOURCE_PERMISSIONS


Index: mlir/cmake/modules/CMakeLists.txt
===
--- mlir/cmake/modules/CMakeLists.txt
+++ mlir/cmake/modules/CMakeLists.txt
@@ -42,9 +42,12 @@
 
 # For compatibility with projects that include(MLIRConfig)
 # via CMAKE_MODULE_PATH, place API modules next to it.
+# Copy without source permissions because the source could be read-only,
+# but we need to write into the copied folder.
 # This should be removed in the future.
 file(COPY .
   DESTINATION ${mlir_cmake_builddir}
+  NO_SOURCE_PERMISSIONS
   FILES_MATCHING PATTERN *.cmake
   PATTERN CMakeFiles EXCLUDE
   )
Index: llvm/cmake/modules/CMakeLists.txt
===
--- llvm/cmake/modules/CMakeLists.txt
+++ llvm/cmake/modules/CMakeLists.txt
@@ -99,9 +99,12 @@
 
 # For compatibility with projects that include(LLVMConfig)
 # via CMAKE_MODULE_PATH, place API modules next to it.
+# Copy without source permissions because the source could be read-only,
+# but we need to write into the copied folder.
 # This should be removed in the future.
 file(COPY .
   DESTINATION ${llvm_cmake_builddir}
+  NO_SOURCE_PERMISSIONS
   FILES_MATCHING PATTERN *.cmake
   PATTERN CMakeFiles EXCLUDE
   )
Index: clang/cmake/modules/CMakeLists.txt
===
--- clang/cmake/modules/CMakeLists.txt
+++ clang/cmake/modules/CMakeLists.txt
@@ -32,7 +32,8 @@
 
 # For compatibility with projects that include(ClangConfig)
 # via CMAKE_MODULE_PATH, place API modules next to it.
-# Copy without source permissions because the source could be read-only
+# Copy without source permissions because the source could be read-only,
+# but we need to write into the copied folder.
 file(COPY .
   DESTINATION ${clang_cmake_builddir}
   NO_SOURCE_PERMISSIONS

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

[Sebastian Neubauer 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 rGefe1527e28ca: [CMake] Copy folder without permissions 
(authored by sebastian-ne).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130338/new/

https://reviews.llvm.org/D130338

Files:
  clang/cmake/modules/CMakeLists.txt
  llvm/cmake/modules/CMakeLists.txt
  mlir/cmake/modules/CMakeLists.txt


Index: mlir/cmake/modules/CMakeLists.txt
===
--- mlir/cmake/modules/CMakeLists.txt
+++ mlir/cmake/modules/CMakeLists.txt
@@ -42,9 +42,12 @@
 
 # For compatibility with projects that include(MLIRConfig)
 # via CMAKE_MODULE_PATH, place API modules next to it.
+# Copy without source permissions because the source could be read-only,
+# but we need to write into the copied folder.
 # This should be removed in the future.
 file(COPY .
   DESTINATION ${mlir_cmake_builddir}
+  NO_SOURCE_PERMISSIONS
   FILES_MATCHING PATTERN *.cmake
   PATTERN CMakeFiles EXCLUDE
   )
Index: llvm/cmake/modules/CMakeLists.txt
===
--- llvm/cmake/modules/CMakeLists.txt
+++ llvm/cmake/modules/CMakeLists.txt
@@ -99,9 +99,12 @@
 
 # For compatibility with projects that include(LLVMConfig)
 # via CMAKE_MODULE_PATH, place API modules next to it.
+# Copy without source permissions because the source could be read-only,
+# but we need to write into the copied folder.
 # This should be removed in the future.
 file(COPY .
   DESTINATION ${llvm_cmake_builddir}
+  NO_SOURCE_PERMISSIONS
   FILES_MATCHING PATTERN *.cmake
   PATTERN CMakeFiles EXCLUDE
   )
Index: clang/cmake/modules/CMakeLists.txt
===
--- clang/cmake/modules/CMakeLists.txt
+++ clang/cmake/modules/CMakeLists.txt
@@ -32,7 +32,8 @@
 
 # For compatibility with projects that include(ClangConfig)
 # via CMAKE_MODULE_PATH, place API modules next to it.
-# Copy without source permissions because the source could be read-only
+# Copy without source permissions because the source could be read-only,
+# but we need to write into the copied folder.
 file(COPY .
   DESTINATION ${clang_cmake_builddir}
   NO_SOURCE_PERMISSIONS


Index: mlir/cmake/modules/CMakeLists.txt
===
--- mlir/cmake/modules/CMakeLists.txt
+++ mlir/cmake/modules/CMakeLists.txt
@@ -42,9 +42,12 @@
 
 # For compatibility with projects that include(MLIRConfig)
 # via CMAKE_MODULE_PATH, place API modules next to it.
+# Copy without source permissions because the source could be read-only,
+# but we need to write into the copied folder.
 # This should be removed in the future.
 file(COPY .
   DESTINATION ${mlir_cmake_builddir}
+  NO_SOURCE_PERMISSIONS
   FILES_MATCHING PATTERN *.cmake
   PATTERN CMakeFiles EXCLUDE
   )
Index: llvm/cmake/modules/CMakeLists.txt
===
--- llvm/cmake/modules/CMakeLists.txt
+++ llvm/cmake/modules/CMakeLists.txt
@@ -99,9 +99,12 @@
 
 # For compatibility with projects that include(LLVMConfig)
 # via CMAKE_MODULE_PATH, place API modules next to it.
+# Copy without source permissions because the source could be read-only,
+# but we need to write into the copied folder.
 # This should be removed in the future.
 file(COPY .
   DESTINATION ${llvm_cmake_builddir}
+  NO_SOURCE_PERMISSIONS
   FILES_MATCHING PATTERN *.cmake
   PATTERN CMakeFiles EXCLUDE
   )
Index: clang/cmake/modules/CMakeLists.txt
===
--- clang/cmake/modules/CMakeLists.txt
+++ clang/cmake/modules/CMakeLists.txt
@@ -32,7 +32,8 @@
 
 # For compatibility with projects that include(ClangConfig)
 # via CMAKE_MODULE_PATH, place API modules next to it.
-# Copy without source permissions because the source could be read-only
+# Copy without source permissions because the source could be read-only,
+# but we need to write into the copied folder.
 file(COPY .
   DESTINATION ${clang_cmake_builddir}
   NO_SOURCE_PERMISSIONS
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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
  https://reviews.llvm.org/D101070/new/

https://reviews.llvm.org/D101070

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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"

Shouldn’t this set `config_code` instead of `prefix_var`, which is the output 
variable in the generated code?



Comment at: cmake/Modules/FindPrefixFromConfig.cmake:32
+  "# Installation prefix is fixed absolute path"
+  "set(${prefix_var} \"${CMAKE_INSTALL_PREFIX}\"")
+  else()

This could use a comment to why it ignores `path_to_leave` and why 
`CMAKE_INSTALL_PREFIX` is the correct choice.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117973/new/

https://reviews.llvm.org/D117973

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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 LLVMInstallSymlink.cmake.

Just checked, if I run `ninja install` with this patch, it fails with

  CMake Error at /llvm-project/llvm/cmake/modules/LLVMInstallSymlink.cmake:5 
(include):
include could not find requested file:
  
  ExtendPath
  Call Stack (most recent call first):
tools/llvm-ar/cmake_install.cmake:56 (include)
tools/cmake_install.cmake:49 (include)
cmake_install.cmake:78 (include)
  
  
  FAILED: CMakeFiles/install.util


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101070/new/

https://reviews.llvm.org/D101070

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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)
cmake/modules/AddLLVM.cmake:2154 (llvm_add_tool_symlink)
tools/llvm-ar/CMakeLists.txt:22 (add_llvm_tool_symlink)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101070/new/

https://reviews.llvm.org/D101070

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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/

https://reviews.llvm.org/D117973

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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.

Follow up to D110143 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110612

Files:
  clang/test/utils/update_cc_test_checks/Inputs/exec-all-runlines.c
  clang/test/utils/update_cc_test_checks/Inputs/exec-all-runlines.c.expected
  llvm/utils/update_cc_test_checks.py


Index: llvm/utils/update_cc_test_checks.py
===
--- llvm/utils/update_cc_test_checks.py
+++ llvm/utils/update_cc_test_checks.py
@@ -238,11 +238,8 @@
 run_list = []
 line2func_list = collections.defaultdict(list)
 
-subs = {
-  '%s' : ti.path,
-  '%t' : tempfile.NamedTemporaryFile().name,
-  '%S' : os.getcwd(),
-}
+subs = common.getSubstitutions(ti.path)
+subs.append(('%t', tempfile.NamedTemporaryFile().name))
 
 for l in ti.run_lines:
   commands = [cmd.strip() for cmd in l.split('|')]
@@ -257,16 +254,14 @@
   # Execute non-clang runline.
   if exec_args[0] not in SUBST:
 # Do lit-like substitutions.
-for s in subs:
-  exec_args = [i.replace(s, subs[s]) if s in i else i for i in 
exec_args]
+exec_args = [common.applySubstitutions(a, subs) for a in exec_args]
 run_list.append((None, exec_args, None, None))
 continue
   # This is a clang runline, apply %clang substitution rule, do lit-like 
substitutions,
   # and append args.clang_args
   clang_args = exec_args
   clang_args[0:1] = SUBST[clang_args[0]]
-  for s in subs:
-clang_args = [i.replace(s, subs[s]) if s in i else i for i in 
clang_args]
+  clang_args = [common.applySubstitutions(a, subs) for a in clang_args]
   clang_args += ti.args.clang_args
 
   # Extract -check-prefix in FileCheck args
Index: 
clang/test/utils/update_cc_test_checks/Inputs/exec-all-runlines.c.expected
===
--- clang/test/utils/update_cc_test_checks/Inputs/exec-all-runlines.c.expected
+++ clang/test/utils/update_cc_test_checks/Inputs/exec-all-runlines.c.expected
@@ -1,7 +1,7 @@
 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
 // Check that the non-clang/non-filechecked runlines execute
-// RUN: cp %s %S/Output/execute-all-runlines.copy.c
-// RUN: cp %S/Output/execute-all-runlines.copy.c %s.copy.c
+// RUN: cp %s %S/execute-all-runlines.copy.c
+// RUN: cp %S/execute-all-runlines.copy.c %s.copy.c
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fopenmp %s.copy.c 
-emit-llvm-bc -o %t-host.bc
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fopenmp 
-fopenmp-host-ir-file-path %t-host.bc %s.copy.c -emit-llvm -o - | FileCheck %s 
--check-prefix=CHECK1
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-pch %s -o %t
Index: clang/test/utils/update_cc_test_checks/Inputs/exec-all-runlines.c
===
--- clang/test/utils/update_cc_test_checks/Inputs/exec-all-runlines.c
+++ clang/test/utils/update_cc_test_checks/Inputs/exec-all-runlines.c
@@ -1,6 +1,6 @@
 // Check that the non-clang/non-filechecked runlines execute
-// RUN: cp %s %S/Output/execute-all-runlines.copy.c
-// RUN: cp %S/Output/execute-all-runlines.copy.c %s.copy.c
+// RUN: cp %s %S/execute-all-runlines.copy.c
+// RUN: cp %S/execute-all-runlines.copy.c %s.copy.c
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fopenmp %s.copy.c 
-emit-llvm-bc -o %t-host.bc
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fopenmp 
-fopenmp-host-ir-file-path %t-host.bc %s.copy.c -emit-llvm -o - | FileCheck %s 
--check-prefix=CHECK1
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-pch %s -o %t


Index: llvm/utils/update_cc_test_checks.py
===
--- llvm/utils/update_cc_test_checks.py
+++ llvm/utils/update_cc_test_checks.py
@@ -238,11 +238,8 @@
 run_list = []
 line2func_list = collections.defaultdict(list)
 
-subs = {
-  '%s' : ti.path,
-  '%t' : tempfile.NamedTemporaryFile().name,
-  '%S' : os.getcwd(),
-}
+subs = common.getSubstitutions(ti.path)
+subs.append(('%t', tempfile.NamedTemporaryFile().name))
 
 for l in ti.run_lines:
   commands = [cmd.strip() for cmd in l.split('|')]
@@ -257,16 +254,14 @@
   # Execute non-clang runline.
   if exec_args[0] not in SUBST:
 # Do lit-like substitutions.
-for s in subs:
-  exec_args = [i.replace(s, subs[s]) if s in i else i for i in exec_args]
+exec_args = [common.applySubstitutions(a, subs) for a in exec_args]
 run_list.append((None, exec_args, None, None))
 conti

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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"

This comment is outdated


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130586/new/

https://reviews.llvm.org/D130586

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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 related to debuginfod currently).
The bazel failure seems to be a bug or flake in the build system as well.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130586/new/

https://reviews.llvm.org/D130586

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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 include CMAKE_CFG_INTDIR though (i.e. 
`${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/libllvm-c.exports`), to be consistent 
with other places, though this only affects the ninja multi-config build. I’m 
not sure if that works at all and it’s outside of the scope of this patch 
anyway.



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)

Should this be `set(LIB_DIR ${LLVM_LIBRARY_DIR})`?



Comment at: llvm/tools/llvm-shlib/CMakeLists.txt:139
   foreach(lib ${LIB_NAMES})
 list(APPEND FULL_LIB_NAMES 
${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/lib/${lib}.lib)
   endforeach()

This should probably use LLVM_LIBRARY_DIR as well.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132316/new/

https://reviews.llvm.org/D132316

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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:
> sebastian-ne wrote:
> > Should this be `set(LIB_DIR ${LLVM_LIBRARY_DIR})`?
> I am not sure what is up with `CMAKE_CFG_INTDIR`, so I rather leave that for 
> a later revision.
`LLVM_LIBRARY_DIR` includes `CMAKE_CFG_INTDIR` as it’s set through
```
set(LLVM_LIBRARY_OUTPUT_INTDIR 
${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/lib${LLVM_LIBDIR_SUFFIX})
set(LLVM_LIBRARY_DIR  ${LLVM_LIBRARY_OUTPUT_INTDIR})
```

As far as I understand, `CMAKE_CFG_INTDIR` is set to `.`, unless one does a 
multi-config build, i.e. one build directory for debug and release builds. 
multi-config builds are the default when creating Visual Studio solutions and 
ninja supports multi-config builds since some time (rather recently).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132316/new/

https://reviews.llvm.org/D132316

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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, `CMAKE_CFG_INTDIR` is deprecated and superseded by generator 
expression: https://cmake.org/cmake/help/latest/variable/CMAKE_CFG_INTDIR.html


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132316/new/

https://reviews.llvm.org/D132316

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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 is `LLVM_INCLUDE_DIR` and 
`CMAKE_BINARY_BINDIR` which is `LLVM_TOOLS_BINARY_DIR`.

E.g. llvm/CMakeLists.txt uses

  set( CMAKE_LIBRARY_OUTPUT_DIRECTORY ${LLVM_LIBRARY_DIR} )
  set( CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${LLVM_LIBRARY_DIR} )

while clang uses

  set( CMAKE_LIBRARY_OUTPUT_DIRECTORY 
${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX} )
  set( CMAKE_ARCHIVE_OUTPUT_DIRECTORY 
${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX} )

and this patch replaces the clang code with

  set( CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_LIBDIR} )
  set( CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_LIBDIR} )


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132608/new/

https://reviews.llvm.org/D132608

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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 llvm::Any is 
implemented when RTTI is not available (actually, I think llvm::Any was copied 
from libstdc++ at some point).

> This "compare function pointer" trick is awesome, but it might not always 
> work as explained in this commit.
> This question however, suggest that any_cast doesn't always work with RTTI 
> either, which is weird.
> I don't know of any other potential issues. std::visitor can't be used with 
> std::any in absense of std::any::type, but that's minor, I think.

As you noticed, it is very difficult to implement Any correctly on every 
platform under any circumstances and compiler flags.
That is exactly what this patch aims at: Moving the responsibility to implement 
Any correctly to the standard library.

Unfortunatly, we can’t replace llvm::Any with std::any just yet, because of 
bugs in msvc that were only fixed recently.

> llvm::any_isa<> is kind of convenient and is aligned with llvm::isa<>. Same 
> for llvm::any_cast.

It is, however, there is no std::any_isa or std::any_cast_or_null, so the 
question gets wether we want to keep llvm::Any around as a wrapper of std::any 
once we can use it (in this case this patch would be obsolete)
or if we we want to remove llvm::Any and use std::any only.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139973/new/

https://reviews.llvm.org/D139973

Files:
  clang/unittests/Tooling/RefactoringTest.cpp
  clang/unittests/Tooling/TransformerTest.cpp
  lldb/include/lldb/Core/RichManglingContext.h
  llvm/include/llvm/ADT/Any.h
  llvm/lib/CodeGen/MachinePassManager.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/Passes/StandardInstrumentations.cpp
  llvm/lib/Transforms/IPO/SampleProfileProbe.cpp
  llvm/lib/Transforms/Scalar/LoopPassManager.cpp
  llvm/lib/Transforms/Utils/Debugify.cpp
  llvm/unittests/ADT/AnyTest.cpp
  llvm/unittests/IR/PassBuilderCallbacksTest.cpp

Index: llvm/unittests/IR/PassBuilderCallbacksTest.cpp
===
--- llvm/unittests/IR/PassBuilderCallbacksTest.cpp
+++ llvm/unittests/IR/PassBuilderCallbacksTest.cpp
@@ -290,17 +290,17 @@
   return std::string(name);
 }
 
-template <> std::string getName(const llvm::Any &WrappedIR) {
-  if (any_isa(WrappedIR))
-return any_cast(WrappedIR)->getName().str();
-  if (any_isa(WrappedIR))
-return any_cast(WrappedIR)->getName().str();
-  if (any_isa(WrappedIR))
-return any_cast(WrappedIR)->getName().str();
-  if (any_isa(WrappedIR))
-return any_cast(WrappedIR)->getName().str();
-  if (any_isa(WrappedIR))
-return any_cast(WrappedIR)->getName();
+template <> std::string getName(const Any &WrappedIR) {
+  if (const auto *const *M = any_cast(&WrappedIR))
+return (*M)->getName().str();
+  if (const auto *const *F = any_cast(&WrappedIR))
+return (*F)->getName().str();
+  if (const auto *const *L = any_cast(&WrappedIR))
+return (*L)->getName().str();
+  if (const auto *const *L = any_cast(&WrappedIR))
+return (*L)->getName().str();
+  if (const auto *const *C = any_cast(&WrappedIR))
+return (*C)->getName();
   return "";
 }
 /// Define a custom matcher for objects which support a 'getName' method.
Index: llvm/unittests/ADT/AnyTest.cpp
===
--- llvm/unittests/ADT/AnyTest.cpp
+++ llvm/unittests/ADT/AnyTest.cpp
@@ -24,55 +24,55 @@
 
   // An empty Any is not anything.
   EXPECT_FALSE(A.has_value());
-  EXPECT_FALSE(any_isa(A));
+  EXPECT_FALSE(any_cast(&A));
 
   // An int is an int but not something else.
   EXPECT_TRUE(B.has_value());
-  EXPECT_TRUE(any_isa(B));
-  EXPECT_FALSE(any_isa(B));
+  EXPECT_TRUE(any_cast(&B));
+  EXPECT_FALSE(any_cast(&B));
 
   EXPECT_TRUE(C.has_value());
-  EXPECT_TRUE(any_isa(C));
+  EXPECT_TRUE(any_cast(&C));
 
   // A const char * is a const char * but not an int.
   EXPECT_TRUE(D.has_value());
-  EXPECT_TRUE(any_isa(D));
-  EXPECT_FALSE(any_isa(D));
+  EXPECT_TRUE(any_cast(&D));
+  EXPECT_FALSE(any_cast(&D));
 
   // A double is a double but not a float.
   EXPECT_TRUE(E.has_value());
-  EXPECT_TRUE(any_isa(E));
-  EXPECT_FALSE(any_isa(E));
+  EXPECT_TRUE(any_cast(&E));
+  EXPECT_FALSE(any_cast(&E));
 
   // After copy constructing from an int, the new item and old item are both
   // ints.
   llvm::Any F(B);
   EXPECT_TRUE(B.has_value());
   EXPECT_TRUE(F.has_value());
-  EXPECT_TRUE(any_isa(F));
-  EXPECT_TRUE(any_isa(B));
+  EXPECT_TRUE(any_cast(&F));
+  EXPECT_TRUE(any_cast(&B));
 
   // After move constructing from an int, the new item is an int and the old one
   // isn't.
   llvm::Any G(std::move(C));
   EXPECT_FALSE(C.has_value());
   EXPECT_TRUE(G.has_value());
-  EXPE

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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(&parser));
 return llvm::any_cast(parser);
   }

barannikov88 wrote:
> This is not intuitively clear. In assert you pass an address, but in 'return' 
> below the object is passed by reference.
> 
I changed it to use an address + explicit dereference in the return. I hope 
that makes it clearer.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139973/new/

https://reviews.llvm.org/D139973

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer 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 rGbb7940e25f6c: [llvm] Make llvm::Any similar to std::any 
(authored by sebastian-ne).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139973/new/

https://reviews.llvm.org/D139973

Files:
  clang/unittests/Tooling/RefactoringTest.cpp
  clang/unittests/Tooling/TransformerTest.cpp
  lldb/include/lldb/Core/RichManglingContext.h
  llvm/include/llvm/ADT/Any.h
  llvm/lib/CodeGen/MachinePassManager.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/Passes/StandardInstrumentations.cpp
  llvm/lib/Transforms/IPO/SampleProfileProbe.cpp
  llvm/lib/Transforms/Scalar/LoopPassManager.cpp
  llvm/lib/Transforms/Utils/Debugify.cpp
  llvm/unittests/ADT/AnyTest.cpp
  llvm/unittests/IR/PassBuilderCallbacksTest.cpp

Index: llvm/unittests/IR/PassBuilderCallbacksTest.cpp
===
--- llvm/unittests/IR/PassBuilderCallbacksTest.cpp
+++ llvm/unittests/IR/PassBuilderCallbacksTest.cpp
@@ -290,17 +290,17 @@
   return std::string(name);
 }
 
-template <> std::string getName(const llvm::Any &WrappedIR) {
-  if (any_isa(WrappedIR))
-return any_cast(WrappedIR)->getName().str();
-  if (any_isa(WrappedIR))
-return any_cast(WrappedIR)->getName().str();
-  if (any_isa(WrappedIR))
-return any_cast(WrappedIR)->getName().str();
-  if (any_isa(WrappedIR))
-return any_cast(WrappedIR)->getName().str();
-  if (any_isa(WrappedIR))
-return any_cast(WrappedIR)->getName();
+template <> std::string getName(const Any &WrappedIR) {
+  if (const auto *const *M = any_cast(&WrappedIR))
+return (*M)->getName().str();
+  if (const auto *const *F = any_cast(&WrappedIR))
+return (*F)->getName().str();
+  if (const auto *const *L = any_cast(&WrappedIR))
+return (*L)->getName().str();
+  if (const auto *const *L = any_cast(&WrappedIR))
+return (*L)->getName().str();
+  if (const auto *const *C = any_cast(&WrappedIR))
+return (*C)->getName();
   return "";
 }
 /// Define a custom matcher for objects which support a 'getName' method.
Index: llvm/unittests/ADT/AnyTest.cpp
===
--- llvm/unittests/ADT/AnyTest.cpp
+++ llvm/unittests/ADT/AnyTest.cpp
@@ -24,55 +24,55 @@
 
   // An empty Any is not anything.
   EXPECT_FALSE(A.has_value());
-  EXPECT_FALSE(any_isa(A));
+  EXPECT_FALSE(any_cast(&A));
 
   // An int is an int but not something else.
   EXPECT_TRUE(B.has_value());
-  EXPECT_TRUE(any_isa(B));
-  EXPECT_FALSE(any_isa(B));
+  EXPECT_TRUE(any_cast(&B));
+  EXPECT_FALSE(any_cast(&B));
 
   EXPECT_TRUE(C.has_value());
-  EXPECT_TRUE(any_isa(C));
+  EXPECT_TRUE(any_cast(&C));
 
   // A const char * is a const char * but not an int.
   EXPECT_TRUE(D.has_value());
-  EXPECT_TRUE(any_isa(D));
-  EXPECT_FALSE(any_isa(D));
+  EXPECT_TRUE(any_cast(&D));
+  EXPECT_FALSE(any_cast(&D));
 
   // A double is a double but not a float.
   EXPECT_TRUE(E.has_value());
-  EXPECT_TRUE(any_isa(E));
-  EXPECT_FALSE(any_isa(E));
+  EXPECT_TRUE(any_cast(&E));
+  EXPECT_FALSE(any_cast(&E));
 
   // After copy constructing from an int, the new item and old item are both
   // ints.
   llvm::Any F(B);
   EXPECT_TRUE(B.has_value());
   EXPECT_TRUE(F.has_value());
-  EXPECT_TRUE(any_isa(F));
-  EXPECT_TRUE(any_isa(B));
+  EXPECT_TRUE(any_cast(&F));
+  EXPECT_TRUE(any_cast(&B));
 
   // After move constructing from an int, the new item is an int and the old one
   // isn't.
   llvm::Any G(std::move(C));
   EXPECT_FALSE(C.has_value());
   EXPECT_TRUE(G.has_value());
-  EXPECT_TRUE(any_isa(G));
-  EXPECT_FALSE(any_isa(C));
+  EXPECT_TRUE(any_cast(&G));
+  EXPECT_FALSE(any_cast(&C));
 
   // After copy-assigning from an int, the new item and old item are both ints.
   A = F;
   EXPECT_TRUE(A.has_value());
   EXPECT_TRUE(F.has_value());
-  EXPECT_TRUE(any_isa(A));
-  EXPECT_TRUE(any_isa(F));
+  EXPECT_TRUE(any_cast(&A));
+  EXPECT_TRUE(any_cast(&F));
 
   // After move-assigning from an int, the new item and old item are both ints.
   B = std::move(G);
   EXPECT_TRUE(B.has_value());
   EXPECT_FALSE(G.has_value());
-  EXPECT_TRUE(any_isa(B));
-  EXPECT_FALSE(any_isa(G));
+  EXPECT_TRUE(any_cast(&B));
+  EXPECT_FALSE(any_cast(&G));
 }
 
 TEST(AnyTest, GoodAnyCast) {
Index: llvm/lib/Transforms/Utils/Debugify.cpp
===
--- llvm/lib/Transforms/Utils/Debugify.cpp
+++ llvm/lib/Transforms/Utils/Debugify.cpp
@@ -1031,19 +1031,19 @@
   PIC.registerBeforeNonSkippedPassCallback([this](StringRef P, Any IR) {
 if (isIgnoredPass(P))
   return;
-if (any_isa(IR))
-  applyDebugify(*const_cast(any_cast(IR)),
+if (const auto **F = any_cast(&IR))
+  applyDebugify(*const_cast(*F),
 Mode, DebugInfoBeforePass, P);
-else if (any_isa(IR))
-  applyDebugify(*const_cast(

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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
  https://reviews.llvm.org/D141538/new/

https://reviews.llvm.org/D141538

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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 LAST ACTION
  https://reviews.llvm.org/D137517/new/

https://reviews.llvm.org/D137517

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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
  https://reviews.llvm.org/D141538/new/

https://reviews.llvm.org/D141538

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer 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 rGeb4aa6c7a5f2: [cmake] Fix path to LLVMConfig.cmake for 
multi-config builds (authored by nhat-nguyen, committed by sebastian-ne).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141538/new/

https://reviews.llvm.org/D141538

Files:
  clang/cmake/modules/CMakeLists.txt
  flang/cmake/modules/CMakeLists.txt
  lld/cmake/modules/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/cmake/modules/CMakeLists.txt
  mlir/cmake/modules/CMakeLists.txt
  polly/cmake/CMakeLists.txt


Index: polly/cmake/CMakeLists.txt
===
--- polly/cmake/CMakeLists.txt
+++ polly/cmake/CMakeLists.txt
@@ -12,7 +12,8 @@
 set(LLVM_INSTALL_PACKAGE_DIR "${CMAKE_INSTALL_PACKAGEDIR}/llvm" CACHE STRING
   "Path for CMake subdirectory for LLVM (defaults to 
'${CMAKE_INSTALL_PACKAGEDIR}/llvm')")
 # CMAKE_INSTALL_PACKAGEDIR might be absolute, so don't reuse below.
-set(llvm_cmake_builddir "${LLVM_LIBRARY_DIR}/cmake/llvm")
+string(REPLACE "${CMAKE_CFG_INTDIR}" "." llvm_cmake_builddir 
"${LLVM_LIBRARY_DIR}")
+set(llvm_cmake_builddir "${llvm_cmake_builddir}/cmake/llvm")
 
 if (CMAKE_CONFIGURATION_TYPES)
   set(POLLY_EXPORTS_FILE_NAME "PollyExports-$>.cmake")
Index: mlir/cmake/modules/CMakeLists.txt
===
--- mlir/cmake/modules/CMakeLists.txt
+++ mlir/cmake/modules/CMakeLists.txt
@@ -15,7 +15,8 @@
 set(LLVM_INSTALL_PACKAGE_DIR "${CMAKE_INSTALL_PACKAGEDIR}/llvm" CACHE STRING
   "Path for CMake subdirectory for LLVM (defaults to 
'${CMAKE_INSTALL_PACKAGEDIR}/llvm')")
 # CMAKE_INSTALL_PACKAGEDIR might be absolute, so don't reuse below.
-set(llvm_cmake_builddir "${LLVM_LIBRARY_DIR}/cmake/llvm")
+string(REPLACE "${CMAKE_CFG_INTDIR}" "." llvm_cmake_builddir 
"${LLVM_LIBRARY_DIR}")
+set(llvm_cmake_builddir "${llvm_cmake_builddir}/cmake/llvm")
 
 get_property(MLIR_EXPORTS GLOBAL PROPERTY MLIR_EXPORTS)
 export(TARGETS ${MLIR_EXPORTS} FILE ${mlir_cmake_builddir}/MLIRTargets.cmake)
Index: llvm/cmake/modules/CMakeLists.txt
===
--- llvm/cmake/modules/CMakeLists.txt
+++ llvm/cmake/modules/CMakeLists.txt
@@ -3,7 +3,7 @@
 include(FindPrefixFromConfig)
 
 # CMAKE_INSTALL_PACKAGEDIR might be absolute, so don't reuse below.
-string(REPLACE "${CMAKE_CFG_INTDIR}" "." llvm_cmake_builddir  
"${LLVM_LIBRARY_DIR}")
+string(REPLACE "${CMAKE_CFG_INTDIR}" "." llvm_cmake_builddir 
"${LLVM_LIBRARY_DIR}")
 set(llvm_cmake_builddir "${llvm_cmake_builddir}/cmake/llvm")
 
 # First for users who use an installed LLVM, create the LLVMExports.cmake file.
Index: llvm/cmake/modules/AddLLVM.cmake
===
--- llvm/cmake/modules/AddLLVM.cmake
+++ llvm/cmake/modules/AddLLVM.cmake
@@ -1126,7 +1126,7 @@
   message(FATAL_ERROR "LLVM_INSTALL_PACKAGE_DIR must be defined and 
writable. GEN_CONFIG should only be passe when building LLVM proper.")
   endif()
   # LLVM_INSTALL_PACKAGE_DIR might be absolute, so don't reuse below.
-  string(REPLACE "${CMAKE_CFG_INTDIR}" "." llvm_cmake_builddir  
"${LLVM_LIBRARY_DIR}")
+  string(REPLACE "${CMAKE_CFG_INTDIR}" "." llvm_cmake_builddir 
"${LLVM_LIBRARY_DIR}")
   set(llvm_cmake_builddir "${llvm_cmake_builddir}/cmake/llvm")
   file(WRITE
   "${llvm_cmake_builddir}/LLVMConfigExtensions.cmake"
Index: lld/cmake/modules/CMakeLists.txt
===
--- lld/cmake/modules/CMakeLists.txt
+++ lld/cmake/modules/CMakeLists.txt
@@ -14,7 +14,8 @@
 set(LLVM_INSTALL_PACKAGE_DIR "${CMAKE_INSTALL_PACKAGEDIR}/llvm" CACHE STRING
   "Path for CMake subdirectory for LLVM (defaults to 
'${CMAKE_INSTALL_PACKAGEDIR}/llvm')")
 # CMAKE_INSTALL_PACKAGEDIR might be absolute, so don't reuse below.
-set(llvm_cmake_builddir "${LLVM_LIBRARY_DIR}/cmake/llvm")
+string(REPLACE "${CMAKE_CFG_INTDIR}" "." llvm_cmake_builddir 
"${LLVM_LIBRARY_DIR}")
+set(llvm_cmake_builddir "${llvm_cmake_builddir}/cmake/llvm")
 
 get_property(LLD_EXPORTS GLOBAL PROPERTY LLD_EXPORTS)
 export(TARGETS ${LLD_EXPORTS} FILE ${lld_cmake_builddir}/LLDTargets.cmake)
Index: flang/cmake/modules/CMakeLists.txt
===
--- flang/cmake/modules/CMakeLists.txt
+++ flang/cmake/modules/CMakeLists.txt
@@ -14,7 +14,8 @@
 set(LLVM_INSTALL_PACKAGE_DIR "${CMAKE_INSTALL_PACKAGEDIR}/llvm" CACHE STRING
   "Path for CMake subdirectory for LLVM (defaults to 
'${CMAKE_INSTALL_PACKAGEDIR}/llvm')")
 # CMAKE_INSTALL_PACKAGEDIR might be absolute, so don't reuse below.
-set(llvm_cmake_builddir "${LLVM_LIBRARY_DIR}/cmake/llvm")
+string(REPLACE "${CMAKE_CFG_INTDIR}" "." llvm_cmake_builddir 
"${LLVM_LIBRARY_DIR}")
+set(llvm_cmake_builddir "${llvm_cmake_builddir}/

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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 FileCheck failed 
previously.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139006/new/

https://reviews.llvm.org/D139006

Files:
  clang/test/utils/update_cc_test_checks/Inputs/basic-cplusplus.cpp.expected
  
clang/test/utils/update_cc_test_checks/Inputs/check-attributes.cpp.funcattrs.expected
  
clang/test/utils/update_cc_test_checks/Inputs/check-attributes.cpp.plain.expected
  clang/test/utils/update_cc_test_checks/Inputs/def-and-decl.c.expected
  clang/test/utils/update_cc_test_checks/Inputs/exec-all-runlines.c.expected
  
clang/test/utils/update_cc_test_checks/Inputs/explicit-template-instantiation.cpp.expected
  clang/test/utils/update_cc_test_checks/Inputs/generated-funcs-regex.c.expected
  
clang/test/utils/update_cc_test_checks/Inputs/generated-funcs.c.generated.expected
  
clang/test/utils/update_cc_test_checks/Inputs/generated-funcs.c.no-generated.expected
  
clang/test/utils/update_cc_test_checks/Inputs/global-hex-value-regex.c.expected
  clang/test/utils/update_cc_test_checks/Inputs/global-value-regex.c.expected
  clang/test/utils/update_cc_test_checks/Inputs/ifdef.c.expected
  clang/test/utils/update_cc_test_checks/Inputs/mangled_names.c.expected
  clang/test/utils/update_cc_test_checks/Inputs/on_the_fly_arg_change.c.expected
  
clang/test/utils/update_cc_test_checks/Inputs/replace-value-regex-across-runs.c.expected
  clang/test/utils/update_cc_test_checks/check-globals.test
  
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/argument_name_reuse.ll.plain.expected
  llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/basic.ll.expected
  
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/custom-tool.ll.expected
  
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/define_after_call.ll
  
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/define_after_call.ll.expected
  
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/generated_funcs.ll.generated.expected
  
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/generated_funcs.ll.generated.globals.expected
  
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/generated_funcs.ll.nogenerated.expected
  
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/generated_funcs.ll.nogenerated.globals.expected
  
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/generated_funcs_prefix_reuse.ll.generated.expected
  
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/generated_funcs_prefix_reuse.ll.generated.globals.expected
  
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/generated_funcs_prefix_reuse.ll.nogenerated.expected
  
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/generated_funcs_prefix_reuse.ll.nogenerated.globals.expected
  
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/scrub_attrs.ll.plain.expected
  
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/scrub_attrs.ll.scrub.expected
  
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/sometimes_deleted_function.ll.expected
  
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/various_ir_values.ll.expected
  llvm/test/tools/UpdateTestChecks/update_test_checks/define_after_call.test
  llvm/utils/UpdateTestChecks/common.py

Index: llvm/utils/UpdateTestChecks/common.py
===
--- llvm/utils/UpdateTestChecks/common.py
+++ llvm/utils/UpdateTestChecks/common.py
@@ -1028,7 +1028,7 @@
   func_name, preserve_names, function_sig,
   global_vars_seen_dict, is_filtered):
   # Label format is based on IR string.
-  function_def_regex = 'define {{[^@]+}}' if function_sig else ''
+  function_def_regex = 'define {{[^@]+}}'
   check_label_format = '{} %s-LABEL: {}@%s%s%s'.format(comment_marker, function_def_regex)
   return add_checks(output_lines, comment_marker, prefix_list, func_dict, func_name,
 check_label_format, False, preserve_names, global_vars_seen_dict,
Index: llvm/test/tools/UpdateTestChecks/update_test_checks/define_after_call.test
===
--- /dev/null
+++ llvm/test/tools/UpdateTestChecks/update_test_checks/define_after_call.test
@@ -0,0 +1,4 @@
+# Verify that calls and defines of the same function can be mixed
+# RUN: cp -f %S/Inputs/define_after_call.ll %t.ll && %update_test_checks %t.ll
+# RUN: diff -u %t.ll %S/Inputs/define_after_call.ll.expected
+# RUN: opt < %t.ll -S | FileCheck %t.ll
Index: llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/various_ir_values.ll.expected

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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:
> Does this actually fail without the define match? I wouldn't expect it to?
Yes, it fails (which is why I put the opt | FileCheck line into the .test 
script as well).
`CHECK-LABEL: @b(` matches the `call i32 @b(i32 0)` line and and as labels are 
matched before check-lines, then the `CHECK-NEXT: [[VAL:%.*]] = call i32 @b(i32 
0)` line doesn’t find its match anymore (or the ret i32 match, I’m not quite 
sure, but it definitely fails).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139006/new/

https://reviews.llvm.org/D139006

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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 our downstream project.
We use add_subdirectory() to include LLVM and our parent project already sets 
PACKAGE_VERSION to something that is not the LLVM version.

Parsing PACKAGE_VERSION only if CLANG_VERSION_MAJOR is not already known seems 
to work:
```
if (NOT CLANG_VERSION_MAJOR)
  string(REGEX MATCH "^[0-9]+" CLANG_VERSION_MAJOR ${PACKAGE_VERSION})
endif()
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141907/new/

https://reviews.llvm.org/D141907

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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/

https://reviews.llvm.org/D139006

Files:
  clang/test/utils/update_cc_test_checks/Inputs/basic-cplusplus.cpp.expected
  
clang/test/utils/update_cc_test_checks/Inputs/check-attributes.cpp.funcattrs.expected
  
clang/test/utils/update_cc_test_checks/Inputs/check-attributes.cpp.plain.expected
  clang/test/utils/update_cc_test_checks/Inputs/def-and-decl.c.expected
  clang/test/utils/update_cc_test_checks/Inputs/exec-all-runlines.c.expected
  
clang/test/utils/update_cc_test_checks/Inputs/explicit-template-instantiation.cpp.expected
  clang/test/utils/update_cc_test_checks/Inputs/generated-funcs-regex.c.expected
  
clang/test/utils/update_cc_test_checks/Inputs/generated-funcs.c.generated.expected
  
clang/test/utils/update_cc_test_checks/Inputs/generated-funcs.c.no-generated.expected
  
clang/test/utils/update_cc_test_checks/Inputs/global-hex-value-regex.c.expected
  clang/test/utils/update_cc_test_checks/Inputs/global-value-regex.c.expected
  clang/test/utils/update_cc_test_checks/Inputs/ifdef.c.expected
  clang/test/utils/update_cc_test_checks/Inputs/mangled_names.c.expected
  clang/test/utils/update_cc_test_checks/Inputs/on_the_fly_arg_change.c.expected
  
clang/test/utils/update_cc_test_checks/Inputs/replace-value-regex-across-runs.c.expected
  clang/test/utils/update_cc_test_checks/check-globals.test
  
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/argument_name_reuse.ll.plain.expected
  llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/basic.ll.expected
  
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/custom-tool.ll.expected
  
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/define_after_call.ll
  
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/define_after_call.ll.expected
  
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/generated_funcs.ll.generated.expected
  
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/generated_funcs.ll.generated.globals.expected
  
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/generated_funcs.ll.nogenerated.expected
  
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/generated_funcs.ll.nogenerated.globals.expected
  
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/generated_funcs_prefix_reuse.ll.generated.expected
  
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/generated_funcs_prefix_reuse.ll.generated.globals.expected
  
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/generated_funcs_prefix_reuse.ll.nogenerated.expected
  
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/generated_funcs_prefix_reuse.ll.nogenerated.globals.expected
  
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/scrub_attrs.ll.plain.expected
  
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/scrub_attrs.ll.scrub.expected
  
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/sometimes_deleted_function.ll.expected
  
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/various_ir_values.ll.expected
  llvm/test/tools/UpdateTestChecks/update_test_checks/define_after_call.test
  llvm/utils/UpdateTestChecks/common.py

Index: llvm/utils/UpdateTestChecks/common.py
===
--- llvm/utils/UpdateTestChecks/common.py
+++ llvm/utils/UpdateTestChecks/common.py
@@ -1028,7 +1028,7 @@
   func_name, preserve_names, function_sig,
   global_vars_seen_dict, is_filtered):
   # Label format is based on IR string.
-  function_def_regex = 'define {{[^@]+}}' if function_sig else ''
+  function_def_regex = 'define {{[^@]+}}'
   check_label_format = '{} %s-LABEL: {}@%s%s%s'.format(comment_marker, function_def_regex)
   return add_checks(output_lines, comment_marker, prefix_list, func_dict, func_name,
 check_label_format, False, preserve_names, global_vars_seen_dict,
Index: llvm/test/tools/UpdateTestChecks/update_test_checks/define_after_call.test
===
--- /dev/null
+++ llvm/test/tools/UpdateTestChecks/update_test_checks/define_after_call.test
@@ -0,0 +1,4 @@
+# Verify that calls and defines of the same function can be mixed
+# RUN: cp -f %S/Inputs/define_after_call.ll %t.ll && %update_test_checks %t.ll
+# RUN: diff -u %t.ll %S/Inputs/define_after_call.ll.expected
+# RUN: opt < %t.ll -S | FileCheck %t.ll
Index: llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/various_ir_values.ll.expected
===
--- llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/various_ir_values.ll.expected
+++ llvm/test/tools/UpdateTestChecks/update_te

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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 after a change in clang-format, we 
can just do that when regenerating tests when they have changes anyway.
So, the churn is that we have extra changes when regenerating a test for a 
patch.
(In my personal experience there are often extra changes anyway, because 
metadata was added or similar changes that pass the old CHECKs but show up on a 
freshly regenerated test.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139006/new/

https://reviews.llvm.org/D139006

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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 added subscribers: llvm-commits, lldb-commits, cfe-commits.

This facilitates replacing llvm::Any with std::any.

- Deprecate any_isa in favor of using any_cast(Any*) and checking for nullptr 
because C++17 has no any_isa.
- Remove the assert from any_cast(Any*), so it returns nullptr if the type is 
not correct. This aligns it with std::any_cast(any*).

Use any_cast(Any*) throughout LLVM instead of checks with any_isa.

This is the first part outlined in
https://discourse.llvm.org/t/rfc-switching-from-llvm-any-to-std-any/67176


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139973

Files:
  clang/unittests/Tooling/RefactoringTest.cpp
  clang/unittests/Tooling/TransformerTest.cpp
  lldb/include/lldb/Core/RichManglingContext.h
  llvm/include/llvm/ADT/Any.h
  llvm/lib/CodeGen/MachinePassManager.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/Passes/StandardInstrumentations.cpp
  llvm/lib/Transforms/IPO/SampleProfileProbe.cpp
  llvm/lib/Transforms/Scalar/LoopPassManager.cpp
  llvm/lib/Transforms/Utils/Debugify.cpp
  llvm/unittests/ADT/AnyTest.cpp
  llvm/unittests/IR/PassBuilderCallbacksTest.cpp

Index: llvm/unittests/IR/PassBuilderCallbacksTest.cpp
===
--- llvm/unittests/IR/PassBuilderCallbacksTest.cpp
+++ llvm/unittests/IR/PassBuilderCallbacksTest.cpp
@@ -290,17 +290,17 @@
   return std::string(name);
 }
 
-template <> std::string getName(const llvm::Any &WrappedIR) {
-  if (any_isa(WrappedIR))
-return any_cast(WrappedIR)->getName().str();
-  if (any_isa(WrappedIR))
-return any_cast(WrappedIR)->getName().str();
-  if (any_isa(WrappedIR))
-return any_cast(WrappedIR)->getName().str();
-  if (any_isa(WrappedIR))
-return any_cast(WrappedIR)->getName().str();
-  if (any_isa(WrappedIR))
-return any_cast(WrappedIR)->getName();
+template <> std::string getName(const Any &WrappedIR) {
+  if (const auto *const *M = any_cast(&WrappedIR))
+return (*M)->getName().str();
+  if (const auto *const *F = any_cast(&WrappedIR))
+return (*F)->getName().str();
+  if (const auto *const *L = any_cast(&WrappedIR))
+return (*L)->getName().str();
+  if (const auto *const *L = any_cast(&WrappedIR))
+return (*L)->getName().str();
+  if (const auto *const *C = any_cast(&WrappedIR))
+return (*C)->getName();
   return "";
 }
 /// Define a custom matcher for objects which support a 'getName' method.
Index: llvm/unittests/ADT/AnyTest.cpp
===
--- llvm/unittests/ADT/AnyTest.cpp
+++ llvm/unittests/ADT/AnyTest.cpp
@@ -24,55 +24,55 @@
 
   // An empty Any is not anything.
   EXPECT_FALSE(A.has_value());
-  EXPECT_FALSE(any_isa(A));
+  EXPECT_FALSE(any_cast(&A));
 
   // An int is an int but not something else.
   EXPECT_TRUE(B.has_value());
-  EXPECT_TRUE(any_isa(B));
-  EXPECT_FALSE(any_isa(B));
+  EXPECT_TRUE(any_cast(&B));
+  EXPECT_FALSE(any_cast(&B));
 
   EXPECT_TRUE(C.has_value());
-  EXPECT_TRUE(any_isa(C));
+  EXPECT_TRUE(any_cast(&C));
 
   // A const char * is a const char * but not an int.
   EXPECT_TRUE(D.has_value());
-  EXPECT_TRUE(any_isa(D));
-  EXPECT_FALSE(any_isa(D));
+  EXPECT_TRUE(any_cast(&D));
+  EXPECT_FALSE(any_cast(&D));
 
   // A double is a double but not a float.
   EXPECT_TRUE(E.has_value());
-  EXPECT_TRUE(any_isa(E));
-  EXPECT_FALSE(any_isa(E));
+  EXPECT_TRUE(any_cast(&E));
+  EXPECT_FALSE(any_cast(&E));
 
   // After copy constructing from an int, the new item and old item are both
   // ints.
   llvm::Any F(B);
   EXPECT_TRUE(B.has_value());
   EXPECT_TRUE(F.has_value());
-  EXPECT_TRUE(any_isa(F));
-  EXPECT_TRUE(any_isa(B));
+  EXPECT_TRUE(any_cast(&F));
+  EXPECT_TRUE(any_cast(&B));
 
   // After move constructing from an int, the new item is an int and the old one
   // isn't.
   llvm::Any G(std::move(C));
   EXPECT_FALSE(C.has_value());
   EXPECT_TRUE(G.has_value());
-  EXPECT_TRUE(any_isa(G));
-  EXPECT_FALSE(any_isa(C));
+  EXPECT_TRUE(any_cast(&G));
+  EXPECT_FALSE(any_cast(&C));
 
   // After copy-assigning from an int, the new item and old item are both ints.
   A = F;
   EXPECT_TRUE(A.has_value());
   EXPECT_TRUE(F.has_value());
-  EXPECT_TRUE(any_isa(A));
-  EXPECT_TRUE(any_isa(F));
+  EXPECT_TRUE(any_cast(&A));
+  EXPECT_TRUE(any_cast(&F));
 
   // After move-assigning from an int, the new item and old item are both ints.
   B = std::move(G);
   EXPECT_TRUE(B.has_value());
   EXPECT_FALSE(G.has_value());
-  EXPECT_TRUE(any_isa(B));
-  EXPECT_FALSE(any_isa(G));
+  EXPECT_TRUE(any_cast(&B));
+  EXPECT_FALSE(any_cast(&G));
 }
 
 TEST(AnyTest, GoodAnyCast) {
Index: llvm/lib/Transforms/Utils/Debugify.cpp
==

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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 already reverted).

> That's why I'm suggesting to change the default behavior for new tests

Adding `--function-signature` by default sounds like a good idea to me.
Is there any reason why we wouldn’t want to enable this by default (for new 
tests)?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139006/new/

https://reviews.llvm.org/D139006

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sebastian Neubauer via Phabricator via cfe-commits]

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 
`llvm/cmake/modules/LLVMCheckLinkerFlag.cmake` with the one defined in 
`cmake/Modules/CheckLinkerFlag.cmake`, which does not check the linker but the 
compiler.
This breaks our gcc+ld build because ld does not support `--color-diagnostics`.

Our downstream issue is here: 
https://github.com/GPUOpen-Drivers/llpc/issues/1645


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116521/new/

https://reviews.llvm.org/D116521

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits