[PATCH] D86154: AMDGPU: Add llvm.amdgcn.{read,readfirst,write}lane2 intrinsics with type overloads

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle abandoned this revision.
nhaehnle added a comment.

Indeed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86154

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

[PATCH] D138278: TableGen: honor LLVM_LINK_LLVM_DYLIB by default

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.
Herald added a subscriber: wangpc.

I haven't looked at this in a while, sorry. I do plan to cycle back to it 
eventually, but right now my work scheduling is more stack-based than FIFO and 
this is fairly low down...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138278

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

[PATCH] D145441: [AMDGPU] Define data layout entries for buffers

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

In D145441#4175428 , @krzysz00 wrote:

> @foad I was trying to avoid sending in one mega-patch that has the entire 
> prototype.

The best way to do this is to work on a branch that you rebase regularly. You 
can push the branch to a personal clone of llvm-project on GitHub and point 
folks there from e.g. Discourse, and for the purpose of Phabricator you can set 
up a "Stack" via the "Edit Related Revisions" option.

If you're using `arc`, then `git rebase -i -x "arc diff @^"` is one way to 
update a bunch of patches at once. There are probably other tools that people 
have written.

> These [addrspace(8) pointers] are, however, integral, since inttoptr and 
> ptrtoint behave deterministically and reasonably.

That doesn't make sense to me. I though "integral" means that

  inttoptr(ptrtoint(p) + offset)

is the same (in terms of address, and potentially modulo provenance questions) 
as

  gep i8, p, offset

But that is rather untrue for addrspace(8), isn't it?




Comment at: llvm/docs/AMDGPUUsage.rst:794
+**Buffer Resource**
+  The buffer resource is an experemntal address space that is currently 
unsupported
+  in the backend. It exposes a non-integral pointer that will represent a 
128-bit

Typo: experimental



Comment at: llvm/docs/AMDGPUUsage.rst:804-805
+
+  Casting a buffer resource to a bufer fat pointer is permitted and adds an 
offset
+  of 0.
+

Presumably it is only permitted if the underlying descriptor satisfies all the 
rules mentioned for addrspace(7).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145441

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

[PATCH] D143522: [AMDGPU] Set a data layout entry for buffer descriptors (addrspace 7)

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

This really needs to be a 160-bit type, not a 128-bit type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143522

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

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

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

This (moving to ProfileData) seems like a simple and pragmatic solution for a 
(perhaps not gigantic but) real problem. It feels fine to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143318

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

[PATCH] D138278: TableGen: honor LLVM_LINK_LLVM_DYLIB by default

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle created this revision.
Herald added subscribers: Moerafaat, zero9178, bzcheeseman, sdasgup3, 
wenzhicui, wrengr, cota, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, 
Kayjukh, grosul1, Joonsoo, liufengdb, aartbik, mgester, arpith-jacob, 
antiagainst, shauheen, rriddle, mehdi_amini, s.egerton, simoncook.
Herald added a reviewer: rriddle.
Herald added a reviewer: antiagainst.
Herald added a project: All.
nhaehnle requested review of this revision.
Herald added subscribers: cfe-commits, pcwang-thead, stephenneuendorffer, 
nicolasvasilache.
Herald added projects: clang, MLIR, LLVM.

Most TableGen tools have link dependency chains of the form

  ${project}-tblgen -> ${project}Support -> LLVMSupport

In LLVM_LINK_LLVM_DYLIB=ON builds, ${project}Support naturally links
aginst LLVMSupport implicitly by linking against libLLVM-*.so, and so
${project}-tblgen ends up with a dependency on the shared library.

Configuring the tool itself with DISABLE_LLVM_LINK_LLVM_DYLIB then
typically leads to LLVMSupport to be also included statically, leading
to duplicate definitions of symbols from LLVMSupport after the dynamic
linker has done its thing.

TableGen tools simply aren't that special: they can be linked dynamically
just fine, and so we do that to simplify the build setup.

The only exception to this rule is llvm-tblgen itself, which must be
statically linked to avoid circular dependencies of the form

  llvm-tblgen -> llvm-shlib -> tablegen-generated files -> llvm-tblgen


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138278

Files:
  clang/lib/Support/CMakeLists.txt
  clang/utils/TableGen/CMakeLists.txt
  llvm/cmake/modules/CrossCompile.cmake
  llvm/cmake/modules/TableGen.cmake
  llvm/utils/TableGen/CMakeLists.txt
  mlir/lib/Support/CMakeLists.txt
  mlir/lib/TableGen/CMakeLists.txt
  mlir/lib/Tools/PDLL/Parser/CMakeLists.txt
  mlir/lib/Tools/mlir-tblgen/CMakeLists.txt
  mlir/lib/Tools/tblgen-lsp-server/CMakeLists.txt

Index: mlir/lib/Tools/tblgen-lsp-server/CMakeLists.txt
===
--- mlir/lib/Tools/tblgen-lsp-server/CMakeLists.txt
+++ mlir/lib/Tools/tblgen-lsp-server/CMakeLists.txt
@@ -12,8 +12,6 @@
   ADDITIONAL_HEADER_DIRS
   ${MLIR_MAIN_INCLUDE_DIR}/mlir/Tools/tblgen-lsp-server
 
-  DISABLE_LLVM_LINK_LLVM_DYLIB
-
   LINK_LIBS PUBLIC
   MLIRLspServerSupportLib
   MLIRSupport
Index: mlir/lib/Tools/mlir-tblgen/CMakeLists.txt
===
--- mlir/lib/Tools/mlir-tblgen/CMakeLists.txt
+++ mlir/lib/Tools/mlir-tblgen/CMakeLists.txt
@@ -4,8 +4,6 @@
   ADDITIONAL_HEADER_DIRS
   ${MLIR_MAIN_INCLUDE_DIR}/mlir/Tools/mlir-tblgen
 
-  DISABLE_LLVM_LINK_LLVM_DYLIB
-
   LINK_COMPONENTS
   TableGen
 
Index: mlir/lib/Tools/PDLL/Parser/CMakeLists.txt
===
--- mlir/lib/Tools/PDLL/Parser/CMakeLists.txt
+++ mlir/lib/Tools/PDLL/Parser/CMakeLists.txt
@@ -4,8 +4,6 @@
   Lexer.cpp
   Parser.cpp
   
-  DISABLE_LLVM_LINK_LLVM_DYLIB
-
   LINK_COMPONENTS
   Support
   TableGen
Index: mlir/lib/TableGen/CMakeLists.txt
===
--- mlir/lib/TableGen/CMakeLists.txt
+++ mlir/lib/TableGen/CMakeLists.txt
@@ -1,14 +1,5 @@
-# This library is unusual, since mlir-tblgen depends on it, which is
-# built with DISABLE_LLVM_LINK_LLVM_DYLIB, this must also be built
-# with that option.  Otherwise builds with LLVM_BUILD_LLVM_DYLIB and
-# LLVM_LINK_LLVM_DYLIB fail.  (Note that even if this has no llvm
-# component dependencies, LLVM_LINK_LLVM_DYLIB tends to introduce a
-# dependence on libLLVM.so)  However, it must also be linkable against
-# libMLIR.so in some contexts (see unittests/Tablegen, for instance, which
-# has a dependence on MLIRIR, which must depend on libLLVM.so).  This works
-# in this special case because this library is static.
 
-llvm_add_library(MLIRTableGen STATIC
+llvm_add_library(MLIRTableGen
   Argument.cpp
   Attribute.cpp
   AttrOrTypeDef.cpp
@@ -30,8 +21,6 @@
   Trait.cpp
   Type.cpp
 
-  DISABLE_LLVM_LINK_LLVM_DYLIB
-
   ADDITIONAL_HEADER_DIRS
   ${MLIR_MAIN_INCLUDE_DIR}/mlir/TableGen
 )
Index: mlir/lib/Support/CMakeLists.txt
===
--- mlir/lib/Support/CMakeLists.txt
+++ mlir/lib/Support/CMakeLists.txt
@@ -30,8 +30,6 @@
 add_llvm_library(MLIRSupportIndentedOstream
   IndentedOstream.cpp
 
-  DISABLE_LLVM_LINK_LLVM_DYLIB
-
   LINK_COMPONENTS
   Support
   )
Index: llvm/utils/TableGen/CMakeLists.txt
===
--- llvm/utils/TableGen/CMakeLists.txt
+++ llvm/utils/TableGen/CMakeLists.txt
@@ -3,6 +3,7 @@
 set(LLVM_LINK_COMPONENTS Support)
 
 add_tablegen(llvm-tblgen LLVM
+  DISABLE_LLVM_LINK_LLVM_DYLIB
   DESTINATION "${LLVM_TOOLS_INSTALL_DIR}"
   EXPORT LLVM
   AsmMatcherEmitter.cpp
Index: llvm/cmake/modules/TableGen.cmake

[PATCH] D138276: TableGen: require tablegen cl::opts to be registered explicitly

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle created this revision.
Herald added subscribers: libc-commits, Moerafaat, zero9178, bzcheeseman, 
sdasgup3, wenzhicui, wrengr, cota, teijeong, rdzhabarov, tatianashp, msifontes, 
jurahul, Kayjukh, grosul1, Joonsoo, liufengdb, aartbik, mgester, arpith-jacob, 
antiagainst, shauheen, rriddle, mehdi_amini, hiraditya.
Herald added projects: libc-project, All.
nhaehnle requested review of this revision.
Herald added subscribers: lldb-commits, cfe-commits, stephenneuendorffer, 
nicolasvasilache.
Herald added projects: clang, LLDB, MLIR, LLVM.

We plan to include the TableGen library into the LLVM shared library.
This is a preparatory change to avoid polluting the command-line options
space of non-tablegen tools.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138276

Files:
  clang/utils/TableGen/TableGen.cpp
  libc/utils/HdrGen/Main.cpp
  libc/utils/HdrGen/PrototypeTestGen/PrototypeTestGen.cpp
  libc/utils/tools/WrapperGen/Main.cpp
  lldb/utils/TableGen/LLDBTableGen.cpp
  llvm/include/llvm/TableGen/Main.h
  llvm/lib/TableGen/Main.cpp
  llvm/tools/llvm-shlib/CMakeLists.txt
  llvm/utils/TableGen/TableGen.cpp
  mlir/lib/Tools/mlir-tblgen/MlirTblgenMain.cpp

Index: mlir/lib/Tools/mlir-tblgen/MlirTblgenMain.cpp
===
--- mlir/lib/Tools/mlir-tblgen/MlirTblgenMain.cpp
+++ mlir/lib/Tools/mlir-tblgen/MlirTblgenMain.cpp
@@ -129,6 +129,7 @@
   llvm::cl::opt generator(
   "", llvm::cl::desc("Generator to run"), cl::location(::generator));
 
+  llvm::registerTableGenOptions();
   cl::ParseCommandLineOptions(argc, argv);
 
   return TableGenMain(argv[0], );
Index: llvm/utils/TableGen/TableGen.cpp
===
--- llvm/utils/TableGen/TableGen.cpp
+++ llvm/utils/TableGen/TableGen.cpp
@@ -286,6 +286,7 @@
 
 int main(int argc, char **argv) {
   InitLLVM X(argc, argv);
+  registerTableGenOptions();
   cl::ParseCommandLineOptions(argc, argv);
 
   return TableGenMain(argv[0], );
Index: llvm/tools/llvm-shlib/CMakeLists.txt
===
--- llvm/tools/llvm-shlib/CMakeLists.txt
+++ llvm/tools/llvm-shlib/CMakeLists.txt
@@ -19,7 +19,6 @@
 
   # Exclude libLLVMTableGen for the following reasons:
   #  - it is only used by internal *-tblgen utilities;
-  #  - it pollutes the global options space.
   list(REMOVE_ITEM LIB_NAMES "LLVMTableGen")
 
   if(LLVM_DYLIB_EXPORTED_SYMBOL_FILE)
Index: llvm/lib/TableGen/Main.cpp
===
--- llvm/lib/TableGen/Main.cpp
+++ llvm/lib/TableGen/Main.cpp
@@ -26,36 +26,45 @@
 #include 
 using namespace llvm;
 
-static cl::opt
-OutputFilename("o", cl::desc("Output filename"), cl::value_desc("filename"),
-   cl::init("-"));
+namespace {
 
-static cl::opt
-DependFilename("d",
-   cl::desc("Dependency filename"),
-   cl::value_desc("filename"),
-   cl::init(""));
+struct TableGenOptions {
+  cl::opt OutputFilename{"o", cl::desc("Output filename"),
+  cl::value_desc("filename"),
+  cl::init("-")};
 
-static cl::opt
-InputFilename(cl::Positional, cl::desc(""), cl::init("-"));
+  cl::opt DependFilename{"d", cl::desc("Dependency filename"),
+  cl::value_desc("filename"), cl::init("")};
 
-static cl::list
-IncludeDirs("I", cl::desc("Directory of include files"),
-cl::value_desc("directory"), cl::Prefix);
+  cl::opt InputFilename{cl::Positional, cl::desc(""),
+ cl::init("-")};
 
-static cl::list
-MacroNames("D", cl::desc("Name of the macro to be defined"),
-cl::value_desc("macro name"), cl::Prefix);
+  cl::list IncludeDirs{"I", cl::desc("Directory of include files"),
+cl::value_desc("directory"), cl::Prefix};
 
-static cl::opt
-WriteIfChanged("write-if-changed", cl::desc("Only write output if it changed"));
+  cl::list MacroNames{"D",
+   cl::desc("Name of the macro to be defined"),
+   cl::value_desc("macro name"), cl::Prefix};
 
-static cl::opt
-TimePhases("time-phases", cl::desc("Time phases of parser and backend"));
+  cl::opt WriteIfChanged{"write-if-changed",
+   cl::desc("Only write output if it changed")};
 
-static cl::opt NoWarnOnUnusedTemplateArgs(
-"no-warn-on-unused-template-args",
-cl::desc("Disable unused template argument warnings."));
+  cl::opt TimePhases{"time-phases",
+   cl::desc("Time phases of parser and backend")};
+
+  cl::opt NoWarnOnUnusedTemplateArgs{
+  "no-warn-on-unused-template-args",
+  cl::desc("Disable unused template argument warnings.")};
+};
+
+TableGenOptions () {
+  static TableGenOptions Opts;
+  return Opts;
+}
+
+} // anonymous namespace
+

[PATCH] D134637: clang-tblgen build: avoid duplicate inclusion of libLLVMSupport

[Nicolai Hähnle via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGdce78646f07f: clang-tblgen build: avoid duplicate inclusion 
of libLLVMSupport (authored by nhaehnle).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134637

Files:
  clang/lib/Support/CMakeLists.txt
  clang/utils/TableGen/CMakeLists.txt


Index: clang/utils/TableGen/CMakeLists.txt
===
--- clang/utils/TableGen/CMakeLists.txt
+++ clang/utils/TableGen/CMakeLists.txt
@@ -25,6 +25,6 @@
   TableGen.cpp
   )
 
-target_link_libraries(clang-tblgen PRIVATE clangSupport)
+target_link_libraries(clang-tblgen PRIVATE clangSupport_tablegen)
 
 set_target_properties(clang-tblgen PROPERTIES FOLDER "Clang tablegenning")
Index: clang/lib/Support/CMakeLists.txt
===
--- clang/lib/Support/CMakeLists.txt
+++ clang/lib/Support/CMakeLists.txt
@@ -9,8 +9,24 @@
   Support
   )
 
-add_clang_library(clangSupport
+set(clangSupport_sources
   RISCVVIntrinsicUtils.cpp
   )
 
+add_clang_library(clangSupport ${clangSupport_sources})
+
+if (NOT XCODE)
+  add_library(clangSupport_tablegen ALIAS obj.clangSupport)
+elseif (NOT LLVM_LINK_LLVM_DYLIB)
+  add_library(clangSupport_tablegen ALIAS clangSupport)
+else()
+  # Build a version of the support library that does not link against
+  # libLLVM-*.so, to be used by clang-tblgen. This is so clang-tblgen doesn't
+  # link against libLLVMSupport twice (once statically and once via
+  # libLLVM-*.so).
+  add_llvm_library(clangSupport_tablegen
+BUILDTREE_ONLY STATIC DISABLE_LLVM_LINK_LLVM_DYLIB
+${clangSupport_sources})
+endif()
+
 set(LLVM_COMMON_DEPENDS ${LLVM_COMMON_DEPENDS_OLD})


Index: clang/utils/TableGen/CMakeLists.txt
===
--- clang/utils/TableGen/CMakeLists.txt
+++ clang/utils/TableGen/CMakeLists.txt
@@ -25,6 +25,6 @@
   TableGen.cpp
   )
 
-target_link_libraries(clang-tblgen PRIVATE clangSupport)
+target_link_libraries(clang-tblgen PRIVATE clangSupport_tablegen)
 
 set_target_properties(clang-tblgen PROPERTIES FOLDER "Clang tablegenning")
Index: clang/lib/Support/CMakeLists.txt
===
--- clang/lib/Support/CMakeLists.txt
+++ clang/lib/Support/CMakeLists.txt
@@ -9,8 +9,24 @@
   Support
   )
 
-add_clang_library(clangSupport
+set(clangSupport_sources
   RISCVVIntrinsicUtils.cpp
   )
 
+add_clang_library(clangSupport ${clangSupport_sources})
+
+if (NOT XCODE)
+  add_library(clangSupport_tablegen ALIAS obj.clangSupport)
+elseif (NOT LLVM_LINK_LLVM_DYLIB)
+  add_library(clangSupport_tablegen ALIAS clangSupport)
+else()
+  # Build a version of the support library that does not link against
+  # libLLVM-*.so, to be used by clang-tblgen. This is so clang-tblgen doesn't
+  # link against libLLVMSupport twice (once statically and once via
+  # libLLVM-*.so).
+  add_llvm_library(clangSupport_tablegen
+BUILDTREE_ONLY STATIC DISABLE_LLVM_LINK_LLVM_DYLIB
+${clangSupport_sources})
+endif()
+
 set(LLVM_COMMON_DEPENDS ${LLVM_COMMON_DEPENDS_OLD})
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134637: clang-tblgen build: avoid duplicate inclusion of libLLVMSupport

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

Ping^2 after ~a week.

The previous discussion seemed quite favorable, so is this just a case of 
everybody being too afraid to say "Yes" because this is a dark corner that few 
people ever look at? ;)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134637

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

[PATCH] D134637: clang-tblgen build: avoid duplicate inclusion of libLLVMSupport

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

Gentle ping :)

If there are no objections, I'd like to commit this in a few days.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134637

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

[PATCH] D134637: clang-tblgen build: avoid duplicate inclusion of libLLVMSupport

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle updated this revision to Diff 467550.
nhaehnle added a comment.

- use the object library version of clangSupport if available

Thank you for the pointers. Using this object library makes a lot of
sense to me and it does appear to work. Judging by AddClang.cmake though,
the object library is only available if (NOT XCODE), so I kept the
previous way of doing things as an alternative. In all cases, I'm using an
alias so that the complexity is contained within the clangSupport CMakeLists.txt


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134637

Files:
  clang/lib/Support/CMakeLists.txt
  clang/utils/TableGen/CMakeLists.txt


Index: clang/utils/TableGen/CMakeLists.txt
===
--- clang/utils/TableGen/CMakeLists.txt
+++ clang/utils/TableGen/CMakeLists.txt
@@ -25,6 +25,6 @@
   TableGen.cpp
   )
 
-target_link_libraries(clang-tblgen PRIVATE clangSupport)
+target_link_libraries(clang-tblgen PRIVATE clangSupport_tablegen)
 
 set_target_properties(clang-tblgen PROPERTIES FOLDER "Clang tablegenning")
Index: clang/lib/Support/CMakeLists.txt
===
--- clang/lib/Support/CMakeLists.txt
+++ clang/lib/Support/CMakeLists.txt
@@ -9,8 +9,24 @@
   Support
   )
 
-add_clang_library(clangSupport
+set(clangSupport_sources
   RISCVVIntrinsicUtils.cpp
   )
 
+add_clang_library(clangSupport ${clangSupport_sources})
+
+if (NOT XCODE)
+  add_library(clangSupport_tablegen ALIAS obj.clangSupport)
+elseif (NOT LLVM_LINK_LLVM_DYLIB)
+  add_library(clangSupport_tablegen ALIAS clangSupport)
+else()
+  # Build a version of the support library that does not link against
+  # libLLVM-*.so, to be used by clang-tblgen. This is so clang-tblgen doesn't
+  # link against libLLVMSupport twice (once statically and once via
+  # libLLVM-*.so).
+  add_llvm_library(clangSupport_tablegen
+BUILDTREE_ONLY STATIC DISABLE_LLVM_LINK_LLVM_DYLIB
+${clangSupport_sources})
+endif()
+
 set(LLVM_COMMON_DEPENDS ${LLVM_COMMON_DEPENDS_OLD})


Index: clang/utils/TableGen/CMakeLists.txt
===
--- clang/utils/TableGen/CMakeLists.txt
+++ clang/utils/TableGen/CMakeLists.txt
@@ -25,6 +25,6 @@
   TableGen.cpp
   )
 
-target_link_libraries(clang-tblgen PRIVATE clangSupport)
+target_link_libraries(clang-tblgen PRIVATE clangSupport_tablegen)
 
 set_target_properties(clang-tblgen PROPERTIES FOLDER "Clang tablegenning")
Index: clang/lib/Support/CMakeLists.txt
===
--- clang/lib/Support/CMakeLists.txt
+++ clang/lib/Support/CMakeLists.txt
@@ -9,8 +9,24 @@
   Support
   )
 
-add_clang_library(clangSupport
+set(clangSupport_sources
   RISCVVIntrinsicUtils.cpp
   )
 
+add_clang_library(clangSupport ${clangSupport_sources})
+
+if (NOT XCODE)
+  add_library(clangSupport_tablegen ALIAS obj.clangSupport)
+elseif (NOT LLVM_LINK_LLVM_DYLIB)
+  add_library(clangSupport_tablegen ALIAS clangSupport)
+else()
+  # Build a version of the support library that does not link against
+  # libLLVM-*.so, to be used by clang-tblgen. This is so clang-tblgen doesn't
+  # link against libLLVMSupport twice (once statically and once via
+  # libLLVM-*.so).
+  add_llvm_library(clangSupport_tablegen
+BUILDTREE_ONLY STATIC DISABLE_LLVM_LINK_LLVM_DYLIB
+${clangSupport_sources})
+endif()
+
 set(LLVM_COMMON_DEPENDS ${LLVM_COMMON_DEPENDS_OLD})
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134637: clang-tblgen build: avoid duplicate inclusion of libLLVMSupport

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

Thank you all for the reviews. I've integrated the suggestions except for:

> A possible alternative solution would be to build clangSupport_sources as an 
> object library, and then link that library into clangSupport and clang-tblgen 
> which could be done unconditionally; the advantage is that you don't need to 
> compile clangSupport_sources twice.

I'm not sure how this would work. It doesn't seem to be something with 
precedent in the LLVM tree, and seems to require using raw CMake `add_library`, 
though it's quite likely that I missed something?




Comment at: clang/lib/Support/CMakeLists.txt:23
+  # libLLVM-*.so).
+  llvm_add_library(clangSupport_tablegen
+STATIC

beanz wrote:
> Unless there is a reason not to you should probably use `add_llvm_library` 
> here probably with the `BUILDTREE_ONLY` option.
That seems to work, thank you.



Comment at: clang/lib/Support/CMakeLists.txt:27
+${clangSupport_sources})
+endif()
+

beanz wrote:
> We could add an `else` here that creates the clangSupport_tablegen target as 
> an alias of clangSupport
> ```
> add_library(clangSupport_tablegen ALIAS clangSupport)
> ```
> 
> See: 
> https://cmake.org/cmake/help/v3.13/command/add_library.html#alias-libraries
> 
> This would allow clang-tablegen to always depend on clangSupport_tablegen 
> simplifying the code on that end.
Good idea, done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134637

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

[PATCH] D134637: clang-tblgen build: avoid duplicate inclusion of libLLVMSupport

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle updated this revision to Diff 465284.
nhaehnle added a comment.

Remove the confusing "accidentally"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134637

Files:
  clang/lib/Support/CMakeLists.txt
  clang/utils/TableGen/CMakeLists.txt


Index: clang/utils/TableGen/CMakeLists.txt
===
--- clang/utils/TableGen/CMakeLists.txt
+++ clang/utils/TableGen/CMakeLists.txt
@@ -25,6 +25,6 @@
   TableGen.cpp
   )
 
-target_link_libraries(clang-tblgen PRIVATE clangSupport)
+target_link_libraries(clang-tblgen PRIVATE clangSupport_tablegen)
 
 set_target_properties(clang-tblgen PROPERTIES FOLDER "Clang tablegenning")
Index: clang/lib/Support/CMakeLists.txt
===
--- clang/lib/Support/CMakeLists.txt
+++ clang/lib/Support/CMakeLists.txt
@@ -9,8 +9,22 @@
   Support
   )
 
-add_clang_library(clangSupport
+set(clangSupport_sources
   RISCVVIntrinsicUtils.cpp
   )
 
+add_clang_library(clangSupport ${clangSupport_sources})
+
+if (LLVM_LINK_LLVM_DYLIB)
+  # Build a version of the support library that does not link against
+  # libLLVM-*.so, to be used by clang-tblgen. This is so clang-tblgen doesn't
+  # link against libLLVMSupport twice (once statically and once via
+  # libLLVM-*.so).
+  add_llvm_library(clangSupport_tablegen
+BUILDTREE_ONLY STATIC DISABLE_LLVM_LINK_LLVM_DYLIB
+${clangSupport_sources})
+else()
+  add_library(clangSupport_tablegen ALIAS clangSupport)
+endif()
+
 set(LLVM_COMMON_DEPENDS ${LLVM_COMMON_DEPENDS_OLD})


Index: clang/utils/TableGen/CMakeLists.txt
===
--- clang/utils/TableGen/CMakeLists.txt
+++ clang/utils/TableGen/CMakeLists.txt
@@ -25,6 +25,6 @@
   TableGen.cpp
   )
 
-target_link_libraries(clang-tblgen PRIVATE clangSupport)
+target_link_libraries(clang-tblgen PRIVATE clangSupport_tablegen)
 
 set_target_properties(clang-tblgen PROPERTIES FOLDER "Clang tablegenning")
Index: clang/lib/Support/CMakeLists.txt
===
--- clang/lib/Support/CMakeLists.txt
+++ clang/lib/Support/CMakeLists.txt
@@ -9,8 +9,22 @@
   Support
   )
 
-add_clang_library(clangSupport
+set(clangSupport_sources
   RISCVVIntrinsicUtils.cpp
   )
 
+add_clang_library(clangSupport ${clangSupport_sources})
+
+if (LLVM_LINK_LLVM_DYLIB)
+  # Build a version of the support library that does not link against
+  # libLLVM-*.so, to be used by clang-tblgen. This is so clang-tblgen doesn't
+  # link against libLLVMSupport twice (once statically and once via
+  # libLLVM-*.so).
+  add_llvm_library(clangSupport_tablegen
+BUILDTREE_ONLY STATIC DISABLE_LLVM_LINK_LLVM_DYLIB
+${clangSupport_sources})
+else()
+  add_library(clangSupport_tablegen ALIAS clangSupport)
+endif()
+
 set(LLVM_COMMON_DEPENDS ${LLVM_COMMON_DEPENDS_OLD})
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134637: clang-tblgen build: avoid duplicate inclusion of libLLVMSupport

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle updated this revision to Diff 465283.
nhaehnle added a comment.

- define an alias so that clang-tblgen can always link against 
"clangSupport_tablegen"
- use add_llvm_library(... BUILDTREE_ONLY ...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134637

Files:
  clang/lib/Support/CMakeLists.txt
  clang/utils/TableGen/CMakeLists.txt


Index: clang/utils/TableGen/CMakeLists.txt
===
--- clang/utils/TableGen/CMakeLists.txt
+++ clang/utils/TableGen/CMakeLists.txt
@@ -25,6 +25,6 @@
   TableGen.cpp
   )
 
-target_link_libraries(clang-tblgen PRIVATE clangSupport)
+target_link_libraries(clang-tblgen PRIVATE clangSupport_tablegen)
 
 set_target_properties(clang-tblgen PROPERTIES FOLDER "Clang tablegenning")
Index: clang/lib/Support/CMakeLists.txt
===
--- clang/lib/Support/CMakeLists.txt
+++ clang/lib/Support/CMakeLists.txt
@@ -9,8 +9,22 @@
   Support
   )
 
-add_clang_library(clangSupport
+set(clangSupport_sources
   RISCVVIntrinsicUtils.cpp
   )
 
+add_clang_library(clangSupport ${clangSupport_sources})
+
+if (LLVM_LINK_LLVM_DYLIB)
+  # Build a version of the support library that does not link against
+  # libLLVM-*.so, to be used by clang-tblgen. This is so clang-tblgen doesn't
+  # accidentally link against libLLVMSupport twice (once statically and once 
via
+  # libLLVM-*.so).
+  add_llvm_library(clangSupport_tablegen
+BUILDTREE_ONLY STATIC DISABLE_LLVM_LINK_LLVM_DYLIB
+${clangSupport_sources})
+else()
+  add_library(clangSupport_tablegen ALIAS clangSupport)
+endif()
+
 set(LLVM_COMMON_DEPENDS ${LLVM_COMMON_DEPENDS_OLD})


Index: clang/utils/TableGen/CMakeLists.txt
===
--- clang/utils/TableGen/CMakeLists.txt
+++ clang/utils/TableGen/CMakeLists.txt
@@ -25,6 +25,6 @@
   TableGen.cpp
   )
 
-target_link_libraries(clang-tblgen PRIVATE clangSupport)
+target_link_libraries(clang-tblgen PRIVATE clangSupport_tablegen)
 
 set_target_properties(clang-tblgen PROPERTIES FOLDER "Clang tablegenning")
Index: clang/lib/Support/CMakeLists.txt
===
--- clang/lib/Support/CMakeLists.txt
+++ clang/lib/Support/CMakeLists.txt
@@ -9,8 +9,22 @@
   Support
   )
 
-add_clang_library(clangSupport
+set(clangSupport_sources
   RISCVVIntrinsicUtils.cpp
   )
 
+add_clang_library(clangSupport ${clangSupport_sources})
+
+if (LLVM_LINK_LLVM_DYLIB)
+  # Build a version of the support library that does not link against
+  # libLLVM-*.so, to be used by clang-tblgen. This is so clang-tblgen doesn't
+  # accidentally link against libLLVMSupport twice (once statically and once via
+  # libLLVM-*.so).
+  add_llvm_library(clangSupport_tablegen
+BUILDTREE_ONLY STATIC DISABLE_LLVM_LINK_LLVM_DYLIB
+${clangSupport_sources})
+else()
+  add_library(clangSupport_tablegen ALIAS clangSupport)
+endif()
+
 set(LLVM_COMMON_DEPENDS ${LLVM_COMMON_DEPENDS_OLD})
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134637: clang-tblgen build: avoid duplicate inclusion of libLLVMSupport

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added inline comments.



Comment at: clang/lib/Support/CMakeLists.txt:21
+  # libLLVM-*.so, to be used by clang-tblgen. This is so clang-tblgen doesn't
+  # accidentally link against libLLVMSupport twice (once statically and once 
via
+  # libLLVM-*.so).

DavidSpickett wrote:
> Without this change, is it the case that it will always link against 
> libLLVMSupport twice with the DYLIB conifg?
> 
> "accidentally" sounds like you could stumble into it but from what I see, 
> it's always been doing this and once your other change lands, it would always 
> result in an error.
> ```
> This is so clang-tblgen doesn't link against libLLVMSupport twice (once 
> statically and once via libLLVM-*.so).
> ```
I meant "accidentally" in the sense that *-tblgen isn't supposed to link 
against libLLVM-*.so, but ended up doing so after clangSupport was added 
earlier this year. Perhaps I should just remover the adverb?



Comment at: clang/lib/Support/CMakeLists.txt:26
+DISABLE_LLVM_LINK_LLVM_DYLIB
+${clangSupport_sources})
+endif()

DavidSpickett wrote:
> Can you detail which targets link to/include what and how the problem 
> happens? I'm trying to understand why we can't just use 
> `DISABLE_LLVM_LINK_LLVM_DYLIB` on its own here.
clangSupport is included by clang-tblgen but also by libclangcpp. The 
underlying idea is that of all the users of clangSupport, clang-tblgen is 
special because it uses the DISABLE_LLVM_LINK_LLVM_DYLIB override.

clangSupport links against Support, which becomes a link against libLLVM-*.so 
with LLVM_LINK_LLVM_DYLIB=ON. So, in an LLVM_LINK_LLVM_DYLIB=ON build, we get 
with this change:

- clangSupport links against Support, which becomes dynamically linking against 
libLLVM-*.so (this is unchanged)
- clangSupport_tablegen links against Support statically
- clang-tblgen links against clangSupport_tablegen (and also directly against 
Support) statically
- other users of clangSupport link against clangSupport somehow, and then 
transitively dynamically against libLLVM-*.so

Does that answer your questions?

Specifically, if we were to just add DISABLE_LLVM_LINK_LLVM_DYLIB to 
clangSupport, then we risk a situation where some other user of clangSupport 
links against Support twice; once via the copy of Support that is statically 
linked to from clangSupport; and once via libLLVM-*.so that gets pulled in via 
other dependencies. To be honest, I don't know for certain whether that is a 
problem that would happen, but it seemed likely enough to me that I wouldn't 
want to risk it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134637

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

[PATCH] D134637: clang-tblgen build: avoid duplicate inclusion of libLLVMSupport

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle created this revision.
nhaehnle added reviewers: kito-cheng, khchen, MaskRay, aaron.ballman, 
DavidSpickett.
Herald added subscribers: StephenFan, kristof.beyls.
Herald added a project: All.
nhaehnle requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

TableGen executables are supposed to never be linked against libLLVM-*.so,
even when LLVM_LINK_LLVM_DYLIB=ON, presumably for cross-compilation.

It turns out that clang-tblgen *did* link against libLLVM-*.so,
indirectly so via the clangSupport.

This lead to a regression in what should have been unrelated work of
cleaning up ManagedStatics in LLVMSupport. A running clang-tblgen
process ended up with two copies of a cl::opt static global:

- one from libLLVMSupport linked statically into clang-tblgen as a direct 
dependency
- one from libLLVMSupport linked into libLLVM-*.so, which clang-tblgen linked 
against due to the clangSupport dependency

For a bit more context, see the discussion at
https://discourse.llvm.org/t/flang-aarch64-dylib-buildbot-need-help-understanding-a-regression-in-clang-tblgen/64871/

None of the potential solutions I could find are perfect. Presumably one
possible solution would be to remove "Support" from the explicit
dependencies of clang-tblgen. However, relying on the transitive
inclusion via clangSupport seems risky, and in any case this wouldn't
address the issue of clang-tblgen surprisingly linking against libLLVM-*.so.

This change instead creates a second version of the clangSupport library
that is explicitly linked statically, to be used by clang-tblgen.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134637

Files:
  clang/lib/Support/CMakeLists.txt
  clang/utils/TableGen/CMakeLists.txt


Index: clang/utils/TableGen/CMakeLists.txt
===
--- clang/utils/TableGen/CMakeLists.txt
+++ clang/utils/TableGen/CMakeLists.txt
@@ -25,6 +25,10 @@
   TableGen.cpp
   )
 
-target_link_libraries(clang-tblgen PRIVATE clangSupport)
+if(LLVM_LINK_LLVM_DYLIB)
+  target_link_libraries(clang-tblgen PRIVATE clangSupport_tablegen)
+else()
+  target_link_libraries(clang-tblgen PRIVATE clangSupport)
+endif()
 
 set_target_properties(clang-tblgen PROPERTIES FOLDER "Clang tablegenning")
Index: clang/lib/Support/CMakeLists.txt
===
--- clang/lib/Support/CMakeLists.txt
+++ clang/lib/Support/CMakeLists.txt
@@ -9,8 +9,22 @@
   Support
   )
 
-add_clang_library(clangSupport
+set(clangSupport_sources
   RISCVVIntrinsicUtils.cpp
   )
 
+add_clang_library(clangSupport ${clangSupport_sources})
+
+if (LLVM_LINK_LLVM_DYLIB)
+  # Build a version of the support library that does not link against
+  # libLLVM-*.so, to be used by clang-tblgen. This is so clang-tblgen doesn't
+  # accidentally link against libLLVMSupport twice (once statically and once 
via
+  # libLLVM-*.so).
+  llvm_add_library(clangSupport_tablegen
+STATIC
+DISABLE_LLVM_LINK_LLVM_DYLIB
+${clangSupport_sources})
+endif()
+
+
 set(LLVM_COMMON_DEPENDS ${LLVM_COMMON_DEPENDS_OLD})


Index: clang/utils/TableGen/CMakeLists.txt
===
--- clang/utils/TableGen/CMakeLists.txt
+++ clang/utils/TableGen/CMakeLists.txt
@@ -25,6 +25,10 @@
   TableGen.cpp
   )
 
-target_link_libraries(clang-tblgen PRIVATE clangSupport)
+if(LLVM_LINK_LLVM_DYLIB)
+  target_link_libraries(clang-tblgen PRIVATE clangSupport_tablegen)
+else()
+  target_link_libraries(clang-tblgen PRIVATE clangSupport)
+endif()
 
 set_target_properties(clang-tblgen PROPERTIES FOLDER "Clang tablegenning")
Index: clang/lib/Support/CMakeLists.txt
===
--- clang/lib/Support/CMakeLists.txt
+++ clang/lib/Support/CMakeLists.txt
@@ -9,8 +9,22 @@
   Support
   )
 
-add_clang_library(clangSupport
+set(clangSupport_sources
   RISCVVIntrinsicUtils.cpp
   )
 
+add_clang_library(clangSupport ${clangSupport_sources})
+
+if (LLVM_LINK_LLVM_DYLIB)
+  # Build a version of the support library that does not link against
+  # libLLVM-*.so, to be used by clang-tblgen. This is so clang-tblgen doesn't
+  # accidentally link against libLLVMSupport twice (once statically and once via
+  # libLLVM-*.so).
+  llvm_add_library(clangSupport_tablegen
+STATIC
+DISABLE_LLVM_LINK_LLVM_DYLIB
+${clangSupport_sources})
+endif()
+
+
 set(LLVM_COMMON_DEPENDS ${LLVM_COMMON_DEPENDS_OLD})
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129131: Remove uses of llvm_shutdown

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle updated this revision to Diff 449609.
nhaehnle added a comment.

Address review comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129131

Files:
  bolt/tools/driver/llvm-bolt.cpp
  bolt/tools/merge-fdata/merge-fdata.cpp
  clang/include/clang/Frontend/CompilerInstance.h
  clang/tools/clang-repl/ClangRepl.cpp
  clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
  clang/unittests/Interpreter/InterpreterTest.cpp
  clang/utils/TableGen/TableGen.cpp
  libclc/utils/prepare-builtins.cpp
  lldb/tools/driver/Driver.cpp
  lldb/tools/lldb-test/lldb-test.cpp
  lldb/unittests/Utility/LogTest.cpp
  lldb/utils/TableGen/LLDBTableGen.cpp
  llvm/examples/BrainF/BrainFDriver.cpp
  llvm/examples/HowToUseJIT/HowToUseJIT.cpp
  llvm/include/llvm/PassRegistry.h
  llvm/include/llvm/Support/DynamicLibrary.h
  llvm/include/llvm/Support/InitLLVM.h
  llvm/lib/IR/Pass.cpp
  llvm/lib/Support/InitLLVM.cpp
  llvm/lib/Support/Unix/DynamicLibrary.inc
  llvm/lib/Support/Unix/Signals.inc
  llvm/lib/Support/Windows/DynamicLibrary.inc
  llvm/tools/gold/gold-plugin.cpp
  llvm/tools/llvm-gsymutil/llvm-gsymutil.cpp
  llvm/unittests/ExecutionEngine/ExecutionEngineTest.cpp
  llvm/unittests/Support/DynamicLibrary/DynamicLibraryTest.cpp
  llvm/utils/KillTheDoctor/KillTheDoctor.cpp
  mlir/tools/mlir-vulkan-runner/mlir-vulkan-runner.cpp
  polly/lib/External/isl/interface/extract_interface.cc

Index: polly/lib/External/isl/interface/extract_interface.cc
===
--- polly/lib/External/isl/interface/extract_interface.cc
+++ polly/lib/External/isl/interface/extract_interface.cc
@@ -48,7 +48,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
Index: mlir/tools/mlir-vulkan-runner/mlir-vulkan-runner.cpp
===
--- mlir/tools/mlir-vulkan-runner/mlir-vulkan-runner.cpp
+++ mlir/tools/mlir-vulkan-runner/mlir-vulkan-runner.cpp
@@ -62,7 +62,6 @@
 }
 
 int main(int argc, char **argv) {
-  llvm::llvm_shutdown_obj x;
   registerPassManagerCLOptions();
 
   llvm::InitLLVM y(argc, argv);
Index: llvm/utils/KillTheDoctor/KillTheDoctor.cpp
===
--- llvm/utils/KillTheDoctor/KillTheDoctor.cpp
+++ llvm/utils/KillTheDoctor/KillTheDoctor.cpp
@@ -36,7 +36,6 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/CommandLine.h"
-#include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Signals.h"
@@ -297,7 +296,6 @@
   // Print a stack trace if we signal out.
   sys::PrintStackTraceOnErrorSignal(argv[0]);
   PrettyStackTraceProgram X(argc, argv);
-  llvm_shutdown_obj Y;  // Call llvm_shutdown() on exit.
 
   ToolName = argv[0];
 
Index: llvm/unittests/Support/DynamicLibrary/DynamicLibraryTest.cpp
===
--- llvm/unittests/Support/DynamicLibrary/DynamicLibraryTest.cpp
+++ llvm/unittests/Support/DynamicLibrary/DynamicLibraryTest.cpp
@@ -9,7 +9,6 @@
 #include "llvm/Support/DynamicLibrary.h"
 #include "llvm/Config/config.h"
 #include "llvm/Support/FileSystem.h"
-#include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
 #include "gtest/gtest.h"
 
@@ -59,7 +58,6 @@
 TEST(DynamicLibrary, Overload) {
   {
 std::string Err;
-llvm_shutdown_obj Shutdown;
 DynamicLibrary DL =
 DynamicLibrary::getPermanentLibrary(LibPath().c_str(), );
 EXPECT_TRUE(DL.isValid());
@@ -109,9 +107,6 @@
   }
   EXPECT_TRUE(FuncPtr(DynamicLibrary::SearchForAddressOfSymbol(
   "TestA")) == nullptr);
-
-  // Check serach ordering is reset to default after call to llvm_shutdown
-  EXPECT_EQ(DynamicLibrary::SearchOrder, DynamicLibrary::SO_Linker);
 }
 
 TEST(DynamicLibrary, Shutdown) {
@@ -119,7 +114,6 @@
   std::vector Order;
   {
 std::string Err;
-llvm_shutdown_obj Shutdown;
 DynamicLibrary DL =
 DynamicLibrary::getPermanentLibrary(LibPath(A).c_str(), );
 EXPECT_TRUE(DL.isValid());
Index: llvm/unittests/ExecutionEngine/ExecutionEngineTest.cpp
===
--- llvm/unittests/ExecutionEngine/ExecutionEngineTest.cpp
+++ llvm/unittests/ExecutionEngine/ExecutionEngineTest.cpp
@@ -14,7 +14,6 @@
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Support/DynamicLibrary.h"
-#include "llvm/Support/ManagedStatic.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
@@ -22,9 +21,6 @@
 namespace {
 
 class ExecutionEngineTest : public testing::Test {
-private:
-  llvm_shutdown_obj Y; // Call llvm_shutdown() on exit.
-
 protected:
   ExecutionEngineTest() {
 auto Owner = std::make_unique("", Context);
Index: llvm/tools/llvm-gsymutil/llvm-gsymutil.cpp

[PATCH] D129131: Remove uses of llvm_shutdown

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added inline comments.



Comment at: polly/lib/External/isl/interface/extract_interface.cc:590
delete Clang;
-   llvm::llvm_shutdown();
 

Meinersbur wrote:
> This file is imported from the upstream project 
> (https://repo.or.cz/isl.git/blob/295cf91923295ca694e9e4433a0b818150421bee:/interface/extract_interface.cc#l590)
>  and this change will be lost when synchronizing with it. However, this file 
> is also not used within LLVM. I recommend to just keep as-is.
Thanks, will do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129131

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

[PATCH] D129118: CommandLine: add and use cl::SubCommand::get{All,TopLevel}

[Nicolai Hähnle 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 rGf7872cdce110: CommandLine: add and use 
cl::SubCommand::get{All,TopLevel} (authored by nhaehnle).

Changed prior to commit:
  https://reviews.llvm.org/D129118?vs=447694=449451#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129118

Files:
  bolt/lib/Utils/CommandLineOpts.cpp
  bolt/tools/driver/llvm-bolt.cpp
  clang/lib/Tooling/CommonOptionsParser.cpp
  clang/tools/clang-refactor/ClangRefactor.cpp
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp
  llvm/tools/llvm-xray/llvm-xray.cpp
  llvm/unittests/Support/CommandLineInit/CommandLineInitTest.cpp
  llvm/unittests/Support/CommandLineTest.cpp

Index: llvm/unittests/Support/CommandLineTest.cpp
===
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -99,7 +99,7 @@
   static const char ValueString[] = "Integer";
 
   StringMap  =
-  cl::getRegisteredOptions(*cl::TopLevelSubCommand);
+  cl::getRegisteredOptions(cl::SubCommand::getTopLevel());
 
   ASSERT_EQ(Map.count("test-option"), 1u) << "Could not find option in map.";
 
@@ -420,7 +420,7 @@
   << "Hid extra option that should be visable.";
 
   StringMap  =
-  cl::getRegisteredOptions(*cl::TopLevelSubCommand);
+  cl::getRegisteredOptions(cl::SubCommand::getTopLevel());
   ASSERT_TRUE(Map.count("help") == (size_t)0 ||
   cl::NotHidden == Map["help"]->getOptionHiddenFlag())
   << "Hid default option that should be visable.";
@@ -446,7 +446,7 @@
   << "Hid extra option that should be visable.";
 
   StringMap  =
-  cl::getRegisteredOptions(*cl::TopLevelSubCommand);
+  cl::getRegisteredOptions(cl::SubCommand::getTopLevel());
   ASSERT_TRUE(Map.count("help") == (size_t)0 ||
   cl::NotHidden == Map["help"]->getOptionHiddenFlag())
   << "Hid default option that should be visable.";
@@ -529,7 +529,7 @@
   cl::ResetCommandLineParser();
 
   StackSubCommand SC1("sc1", "First subcommand");
-  StackOption AllOpt("everywhere", cl::sub(*cl::AllSubCommands),
+  StackOption AllOpt("everywhere", cl::sub(cl::SubCommand::getAll()),
cl::init(false));
   StackSubCommand SC2("sc2", "Second subcommand");
 
@@ -566,8 +566,8 @@
 TEST(CommandLineTest, ReparseCommandLineOptions) {
   cl::ResetCommandLineParser();
 
-  StackOption TopLevelOpt("top-level", cl::sub(*cl::TopLevelSubCommand),
-cl::init(false));
+  StackOption TopLevelOpt(
+  "top-level", cl::sub(cl::SubCommand::getTopLevel()), cl::init(false));
 
   const char *args[] = {"prog", "-top-level"};
 
@@ -614,10 +614,12 @@
 TEST(CommandLineTest, RemoveFromTopLevelSubCommand) {
   cl::ResetCommandLineParser();
 
-  StackOption TopLevelRemove(
-  "top-level-remove", cl::sub(*cl::TopLevelSubCommand), cl::init(false));
-  StackOption TopLevelKeep(
-  "top-level-keep", cl::sub(*cl::TopLevelSubCommand), cl::init(false));
+  StackOption TopLevelRemove("top-level-remove",
+   cl::sub(cl::SubCommand::getTopLevel()),
+   cl::init(false));
+  StackOption TopLevelKeep("top-level-keep",
+ cl::sub(cl::SubCommand::getTopLevel()),
+ cl::init(false));
 
   const char *args[] = {"prog", "-top-level-remove"};
 
@@ -638,9 +640,9 @@
 
   StackSubCommand SC1("sc1", "First Subcommand");
   StackSubCommand SC2("sc2", "Second Subcommand");
-  StackOption RemoveOption("remove-option", cl::sub(*cl::AllSubCommands),
- cl::init(false));
-  StackOption KeepOption("keep-option", cl::sub(*cl::AllSubCommands),
+  StackOption RemoveOption(
+  "remove-option", cl::sub(cl::SubCommand::getAll()), cl::init(false));
+  StackOption KeepOption("keep-option", cl::sub(cl::SubCommand::getAll()),
cl::init(false));
 
   const char *args0[] = {"prog", "-remove-option"};
@@ -719,13 +721,13 @@
 TEST(CommandLineTest, DefaultOptions) {
   cl::ResetCommandLineParser();
 
-  StackOption Bar("bar", cl::sub(*cl::AllSubCommands),
+  StackOption Bar("bar", cl::sub(cl::SubCommand::getAll()),
cl::DefaultOption);
   StackOption Bar_Alias(
   "b", cl::desc("Alias for -bar"), cl::aliasopt(Bar), cl::DefaultOption);
 
-  StackOption Foo("foo", cl::init(false), cl::sub(*cl::AllSubCommands),
-cl::DefaultOption);
+  StackOption Foo("foo", cl::init(false),
+cl::sub(cl::SubCommand::getAll()), cl::DefaultOption);
   StackOption Foo_Alias("f", cl::desc("Alias for -foo"),
  cl::aliasopt(Foo), cl::DefaultOption);
 
@@ -1062,7 +1064,7 @@

[PATCH] D130576: ManagedStatic: remove from ASTMatchersInternal.h

[Nicolai Hähnle 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 rG7132bcdc428d: ManagedStatic: remove from 
ASTMatchersInternal.h (authored by nhaehnle).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130576

Files:
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp


Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -28,7 +28,6 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ErrorHandling.h"
-#include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Regex.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
Index: clang/include/clang/ASTMatchers/ASTMatchersInternal.h
===
--- clang/include/clang/ASTMatchers/ASTMatchersInternal.h
+++ clang/include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -59,7 +59,6 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/iterator.h"
 #include "llvm/Support/Casting.h"
-#include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Regex.h"
 #include 
 #include 
@@ -1931,8 +1930,8 @@
 
 public:
   static const Matcher () {
-static llvm::ManagedStatic Instance;
-return Instance->M;
+static Wrapper Instance;
+return Instance.M;
   }
 };
 


Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -28,7 +28,6 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ErrorHandling.h"
-#include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Regex.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
Index: clang/include/clang/ASTMatchers/ASTMatchersInternal.h
===
--- clang/include/clang/ASTMatchers/ASTMatchersInternal.h
+++ clang/include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -59,7 +59,6 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/iterator.h"
 #include "llvm/Support/Casting.h"
-#include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Regex.h"
 #include 
 #include 
@@ -1931,8 +1930,8 @@
 
 public:
   static const Matcher () {
-static llvm::ManagedStatic Instance;
-return Instance->M;
+static Wrapper Instance;
+return Instance.M;
   }
 };
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130575: clang: include ManagedStatic.h for llvm_shutdown

[Nicolai Hähnle 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 rG5a78ac21569a: clang: include ManagedStatic.h for 
llvm_shutdown (authored by nhaehnle).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130575

Files:
  clang/unittests/Interpreter/InterpreterTest.cpp
  clang/utils/TableGen/TableGen.cpp


Index: clang/utils/TableGen/TableGen.cpp
===
--- clang/utils/TableGen/TableGen.cpp
+++ clang/utils/TableGen/TableGen.cpp
@@ -13,6 +13,7 @@
 #include "TableGenBackends.h" // Declares all backends.
 #include "ASTTableGen.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/TableGen/Error.h"
Index: clang/unittests/Interpreter/InterpreterTest.cpp
===
--- clang/unittests/Interpreter/InterpreterTest.cpp
+++ clang/unittests/Interpreter/InterpreterTest.cpp
@@ -20,6 +20,7 @@
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Sema.h"
 
+#include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/TargetSelect.h"
 
 #include "gmock/gmock.h"


Index: clang/utils/TableGen/TableGen.cpp
===
--- clang/utils/TableGen/TableGen.cpp
+++ clang/utils/TableGen/TableGen.cpp
@@ -13,6 +13,7 @@
 #include "TableGenBackends.h" // Declares all backends.
 #include "ASTTableGen.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/TableGen/Error.h"
Index: clang/unittests/Interpreter/InterpreterTest.cpp
===
--- clang/unittests/Interpreter/InterpreterTest.cpp
+++ clang/unittests/Interpreter/InterpreterTest.cpp
@@ -20,6 +20,7 @@
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Sema.h"
 
+#include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/TargetSelect.h"
 
 #include "gmock/gmock.h"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130574: ClangLinkerWrapper: explicitly #include

[Nicolai Hähnle 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 rG3e874bcf0642: ClangLinkerWrapper: explicitly #include 
atomic (authored by nhaehnle).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130574

Files:
  clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp


Index: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
===
--- clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -51,6 +51,7 @@
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetMachine.h"
+#include 
 
 using namespace llvm;
 using namespace llvm::opt;


Index: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
===
--- clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -51,6 +51,7 @@
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetMachine.h"
+#include 
 
 using namespace llvm;
 using namespace llvm::opt;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129131: Remove uses of llvm_shutdown

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle updated this revision to Diff 447708.
nhaehnle added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129131

Files:
  bolt/tools/driver/llvm-bolt.cpp
  bolt/tools/merge-fdata/merge-fdata.cpp
  clang/include/clang/Frontend/CompilerInstance.h
  clang/tools/clang-repl/ClangRepl.cpp
  clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
  clang/unittests/Interpreter/InterpreterTest.cpp
  clang/utils/TableGen/TableGen.cpp
  libclc/utils/prepare-builtins.cpp
  lldb/tools/driver/Driver.cpp
  lldb/tools/lldb-test/lldb-test.cpp
  lldb/unittests/Utility/LogTest.cpp
  lldb/utils/TableGen/LLDBTableGen.cpp
  llvm/examples/BrainF/BrainFDriver.cpp
  llvm/examples/HowToUseJIT/HowToUseJIT.cpp
  llvm/include/llvm/PassRegistry.h
  llvm/include/llvm/Support/DynamicLibrary.h
  llvm/include/llvm/Support/InitLLVM.h
  llvm/lib/IR/Pass.cpp
  llvm/lib/Support/InitLLVM.cpp
  llvm/lib/Support/Unix/DynamicLibrary.inc
  llvm/lib/Support/Unix/Signals.inc
  llvm/lib/Support/Windows/DynamicLibrary.inc
  llvm/tools/gold/gold-plugin.cpp
  llvm/tools/llvm-gsymutil/llvm-gsymutil.cpp
  llvm/unittests/ExecutionEngine/ExecutionEngineTest.cpp
  llvm/unittests/Support/DynamicLibrary/DynamicLibraryTest.cpp
  llvm/utils/KillTheDoctor/KillTheDoctor.cpp
  mlir/tools/mlir-vulkan-runner/mlir-vulkan-runner.cpp
  polly/lib/External/isl/interface/extract_interface.cc

Index: polly/lib/External/isl/interface/extract_interface.cc
===
--- polly/lib/External/isl/interface/extract_interface.cc
+++ polly/lib/External/isl/interface/extract_interface.cc
@@ -48,7 +48,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -587,7 +586,6 @@
 
 	delete sema;
 	delete Clang;
-	llvm::llvm_shutdown();
 
 	if (Diags.hasErrorOccurred())
 		return EXIT_FAILURE;
Index: mlir/tools/mlir-vulkan-runner/mlir-vulkan-runner.cpp
===
--- mlir/tools/mlir-vulkan-runner/mlir-vulkan-runner.cpp
+++ mlir/tools/mlir-vulkan-runner/mlir-vulkan-runner.cpp
@@ -62,7 +62,6 @@
 }
 
 int main(int argc, char **argv) {
-  llvm::llvm_shutdown_obj x;
   registerPassManagerCLOptions();
 
   llvm::InitLLVM y(argc, argv);
Index: llvm/utils/KillTheDoctor/KillTheDoctor.cpp
===
--- llvm/utils/KillTheDoctor/KillTheDoctor.cpp
+++ llvm/utils/KillTheDoctor/KillTheDoctor.cpp
@@ -36,7 +36,6 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/CommandLine.h"
-#include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Signals.h"
@@ -297,7 +296,6 @@
   // Print a stack trace if we signal out.
   sys::PrintStackTraceOnErrorSignal(argv[0]);
   PrettyStackTraceProgram X(argc, argv);
-  llvm_shutdown_obj Y;  // Call llvm_shutdown() on exit.
 
   ToolName = argv[0];
 
Index: llvm/unittests/Support/DynamicLibrary/DynamicLibraryTest.cpp
===
--- llvm/unittests/Support/DynamicLibrary/DynamicLibraryTest.cpp
+++ llvm/unittests/Support/DynamicLibrary/DynamicLibraryTest.cpp
@@ -9,7 +9,6 @@
 #include "llvm/Support/DynamicLibrary.h"
 #include "llvm/Config/config.h"
 #include "llvm/Support/FileSystem.h"
-#include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
 #include "gtest/gtest.h"
 
@@ -59,7 +58,6 @@
 TEST(DynamicLibrary, Overload) {
   {
 std::string Err;
-llvm_shutdown_obj Shutdown;
 DynamicLibrary DL =
 DynamicLibrary::getPermanentLibrary(LibPath().c_str(), );
 EXPECT_TRUE(DL.isValid());
@@ -109,9 +107,6 @@
   }
   EXPECT_TRUE(FuncPtr(DynamicLibrary::SearchForAddressOfSymbol(
   "TestA")) == nullptr);
-
-  // Check serach ordering is reset to default after call to llvm_shutdown
-  EXPECT_EQ(DynamicLibrary::SearchOrder, DynamicLibrary::SO_Linker);
 }
 
 TEST(DynamicLibrary, Shutdown) {
@@ -119,7 +114,6 @@
   std::vector Order;
   {
 std::string Err;
-llvm_shutdown_obj Shutdown;
 DynamicLibrary DL =
 DynamicLibrary::getPermanentLibrary(LibPath(A).c_str(), );
 EXPECT_TRUE(DL.isValid());
Index: llvm/unittests/ExecutionEngine/ExecutionEngineTest.cpp
===
--- llvm/unittests/ExecutionEngine/ExecutionEngineTest.cpp
+++ llvm/unittests/ExecutionEngine/ExecutionEngineTest.cpp
@@ -14,7 +14,6 @@
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Support/DynamicLibrary.h"
-#include "llvm/Support/ManagedStatic.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
@@ -22,9 +21,6 @@
 namespace {
 
 class ExecutionEngineTest : public testing::Test {
-private:
-  llvm_shutdown_obj Y; // Call llvm_shutdown() on exit.
-
 protected:
   

[PATCH] D130578: ManagedStatic: remove a use from Clang

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle created this revision.
nhaehnle added reviewers: efriedma, lattner.
Herald added a project: All.
nhaehnle requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130578

Files:
  clang/lib/Frontend/PrecompiledPreamble.cpp


Index: clang/lib/Frontend/PrecompiledPreamble.cpp
===
--- clang/lib/Frontend/PrecompiledPreamble.cpp
+++ clang/lib/Frontend/PrecompiledPreamble.cpp
@@ -817,10 +817,9 @@
 }
 CommentHandler *PreambleCallbacks::getCommentHandler() { return nullptr; }
 
-static llvm::ManagedStatic 
BuildPreambleErrCategory;
-
 std::error_code clang::make_error_code(BuildPreambleError Error) {
-  return std::error_code(static_cast(Error), *BuildPreambleErrCategory);
+  static BuildPreambleErrorCategory BuildPreambleErrCategory;
+  return std::error_code(static_cast(Error), BuildPreambleErrCategory);
 }
 
 const char *BuildPreambleErrorCategory::name() const noexcept {


Index: clang/lib/Frontend/PrecompiledPreamble.cpp
===
--- clang/lib/Frontend/PrecompiledPreamble.cpp
+++ clang/lib/Frontend/PrecompiledPreamble.cpp
@@ -817,10 +817,9 @@
 }
 CommentHandler *PreambleCallbacks::getCommentHandler() { return nullptr; }
 
-static llvm::ManagedStatic BuildPreambleErrCategory;
-
 std::error_code clang::make_error_code(BuildPreambleError Error) {
-  return std::error_code(static_cast(Error), *BuildPreambleErrCategory);
+  static BuildPreambleErrorCategory BuildPreambleErrCategory;
+  return std::error_code(static_cast(Error), BuildPreambleErrCategory);
 }
 
 const char *BuildPreambleErrorCategory::name() const noexcept {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130577: clang-driver: use llvm_fast_shutdown

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle created this revision.
nhaehnle added reviewers: efriedma, lattner, mehdi_amini.
Herald added a project: All.
nhaehnle requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Avoid running almost all global destructors in the common case.

Use C->isForDiagnostics() for the decision of whether to do this, which
is the same condition as used elsewhere for triggering "-disable-free".


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130577

Files:
  clang/tools/driver/driver.cpp


Index: clang/tools/driver/driver.cpp
===
--- clang/tools/driver/driver.cpp
+++ clang/tools/driver/driver.cpp
@@ -34,6 +34,7 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/CrashRecoveryContext.h"
 #include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/FastShutdown.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Host.h"
 #include "llvm/Support/InitLLVM.h"
@@ -327,16 +328,17 @@
   return 1;
 }
 
-int clang_main(int Argc, char **Argv) {
-  noteBottomOfStack();
-  llvm::InitLLVM X(Argc, Argv);
-  llvm::setBugReportMsg("PLEASE submit a bug report to " BUG_REPORT_URL
-" and include the crash backtrace, preprocessed "
-"source, and associated run script.\n");
+// Return (FastShutdown, RetCode).
+//
+// Fast shutdown is implemented in the caller. We want the locals of this
+// function to be cleaned up even when fast shutdown is used, as at least the
+// driver's destructor must run to properly flush and close output streams (at
+// least the compilation database stream).
+static std::pair ExecuteClang(int Argc, char **Argv) {
   SmallVector Args(Argv, Argv + Argc);
 
   if (llvm::sys::Process::FixupStandardFileDescriptors())
-return 1;
+return {false, 1};
 
   llvm::InitializeAllTargets();
 
@@ -385,7 +387,7 @@
   auto newEnd = std::remove(Args.begin(), Args.end(), nullptr);
   Args.resize(newEnd - Args.begin());
 }
-return ExecuteCC1Tool(Args);
+return {false, ExecuteCC1Tool(Args)};
   }
 
   // Handle options that need handling before the real command line parsing in
@@ -494,7 +496,7 @@
 if (!Level) {
   llvm::errs() << "Unknown value for " << A->getSpelling() << ": '"
<< A->getValue() << "'\n";
-  return 1;
+  return {false, 1};
 }
 ReproLevel = *Level;
   }
@@ -572,5 +574,25 @@
 
   // If we have multiple failing commands, we return the result of the first
   // failing command.
+  bool FastShutdown = !C->isForDiagnostics();
+  return {FastShutdown, Res};
+}
+
+int clang_main(int Argc, char **Argv) {
+  noteBottomOfStack();
+  llvm::InitLLVM X(Argc, Argv);
+  llvm::setBugReportMsg("PLEASE submit a bug report to " BUG_REPORT_URL
+" and include the crash backtrace, preprocessed "
+"source, and associated run script.\n");
+
+  bool FastShutdown;
+  int Res;
+  std::tie(FastShutdown, Res) = ExecuteClang(Argc, Argv);
+
+  if (FastShutdown) {
+llvm::llvm_fast_shutdown();
+llvm::sys::Process::Exit(Res, /*NoCleanup=*/true);
+  }
+
   return Res;
 }


Index: clang/tools/driver/driver.cpp
===
--- clang/tools/driver/driver.cpp
+++ clang/tools/driver/driver.cpp
@@ -34,6 +34,7 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/CrashRecoveryContext.h"
 #include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/FastShutdown.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Host.h"
 #include "llvm/Support/InitLLVM.h"
@@ -327,16 +328,17 @@
   return 1;
 }
 
-int clang_main(int Argc, char **Argv) {
-  noteBottomOfStack();
-  llvm::InitLLVM X(Argc, Argv);
-  llvm::setBugReportMsg("PLEASE submit a bug report to " BUG_REPORT_URL
-" and include the crash backtrace, preprocessed "
-"source, and associated run script.\n");
+// Return (FastShutdown, RetCode).
+//
+// Fast shutdown is implemented in the caller. We want the locals of this
+// function to be cleaned up even when fast shutdown is used, as at least the
+// driver's destructor must run to properly flush and close output streams (at
+// least the compilation database stream).
+static std::pair ExecuteClang(int Argc, char **Argv) {
   SmallVector Args(Argv, Argv + Argc);
 
   if (llvm::sys::Process::FixupStandardFileDescriptors())
-return 1;
+return {false, 1};
 
   llvm::InitializeAllTargets();
 
@@ -385,7 +387,7 @@
   auto newEnd = std::remove(Args.begin(), Args.end(), nullptr);
   Args.resize(newEnd - Args.begin());
 }
-return ExecuteCC1Tool(Args);
+return {false, ExecuteCC1Tool(Args)};
   }
 
   // Handle options that need handling before the real command line parsing in
@@ -494,7 +496,7 @@
 if (!Level) {
   llvm::errs() << "Unknown value for " << 

[PATCH] D129118: CommandLine: add and use cl::SubCommand::get{All,TopLevel}

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle updated this revision to Diff 447694.
nhaehnle added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129118

Files:
  bolt/lib/Utils/CommandLineOpts.cpp
  bolt/tools/driver/llvm-bolt.cpp
  clang/lib/Tooling/CommonOptionsParser.cpp
  clang/tools/clang-refactor/ClangRefactor.cpp
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp
  llvm/tools/llvm-xray/llvm-xray.cpp
  llvm/unittests/Support/CommandLineInit/CommandLineInitTest.cpp
  llvm/unittests/Support/CommandLineTest.cpp

Index: llvm/unittests/Support/CommandLineTest.cpp
===
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -99,7 +99,7 @@
   static const char ValueString[] = "Integer";
 
   StringMap  =
-  cl::getRegisteredOptions(*cl::TopLevelSubCommand);
+  cl::getRegisteredOptions(cl::SubCommand::getTopLevel());
 
   ASSERT_EQ(Map.count("test-option"), 1u) << "Could not find option in map.";
 
@@ -420,7 +420,7 @@
   << "Hid extra option that should be visable.";
 
   StringMap  =
-  cl::getRegisteredOptions(*cl::TopLevelSubCommand);
+  cl::getRegisteredOptions(cl::SubCommand::getTopLevel());
   ASSERT_TRUE(Map.count("help") == (size_t)0 ||
   cl::NotHidden == Map["help"]->getOptionHiddenFlag())
   << "Hid default option that should be visable.";
@@ -446,7 +446,7 @@
   << "Hid extra option that should be visable.";
 
   StringMap  =
-  cl::getRegisteredOptions(*cl::TopLevelSubCommand);
+  cl::getRegisteredOptions(cl::SubCommand::getTopLevel());
   ASSERT_TRUE(Map.count("help") == (size_t)0 ||
   cl::NotHidden == Map["help"]->getOptionHiddenFlag())
   << "Hid default option that should be visable.";
@@ -529,7 +529,7 @@
   cl::ResetCommandLineParser();
 
   StackSubCommand SC1("sc1", "First subcommand");
-  StackOption AllOpt("everywhere", cl::sub(*cl::AllSubCommands),
+  StackOption AllOpt("everywhere", cl::sub(cl::SubCommand::getAll()),
cl::init(false));
   StackSubCommand SC2("sc2", "Second subcommand");
 
@@ -566,8 +566,8 @@
 TEST(CommandLineTest, ReparseCommandLineOptions) {
   cl::ResetCommandLineParser();
 
-  StackOption TopLevelOpt("top-level", cl::sub(*cl::TopLevelSubCommand),
-cl::init(false));
+  StackOption TopLevelOpt(
+  "top-level", cl::sub(cl::SubCommand::getTopLevel()), cl::init(false));
 
   const char *args[] = {"prog", "-top-level"};
 
@@ -614,10 +614,12 @@
 TEST(CommandLineTest, RemoveFromTopLevelSubCommand) {
   cl::ResetCommandLineParser();
 
-  StackOption TopLevelRemove(
-  "top-level-remove", cl::sub(*cl::TopLevelSubCommand), cl::init(false));
-  StackOption TopLevelKeep(
-  "top-level-keep", cl::sub(*cl::TopLevelSubCommand), cl::init(false));
+  StackOption TopLevelRemove("top-level-remove",
+   cl::sub(cl::SubCommand::getTopLevel()),
+   cl::init(false));
+  StackOption TopLevelKeep("top-level-keep",
+ cl::sub(cl::SubCommand::getTopLevel()),
+ cl::init(false));
 
   const char *args[] = {"prog", "-top-level-remove"};
 
@@ -638,9 +640,9 @@
 
   StackSubCommand SC1("sc1", "First Subcommand");
   StackSubCommand SC2("sc2", "Second Subcommand");
-  StackOption RemoveOption("remove-option", cl::sub(*cl::AllSubCommands),
- cl::init(false));
-  StackOption KeepOption("keep-option", cl::sub(*cl::AllSubCommands),
+  StackOption RemoveOption(
+  "remove-option", cl::sub(cl::SubCommand::getAll()), cl::init(false));
+  StackOption KeepOption("keep-option", cl::sub(cl::SubCommand::getAll()),
cl::init(false));
 
   const char *args0[] = {"prog", "-remove-option"};
@@ -719,13 +721,13 @@
 TEST(CommandLineTest, DefaultOptions) {
   cl::ResetCommandLineParser();
 
-  StackOption Bar("bar", cl::sub(*cl::AllSubCommands),
+  StackOption Bar("bar", cl::sub(cl::SubCommand::getAll()),
cl::DefaultOption);
   StackOption Bar_Alias(
   "b", cl::desc("Alias for -bar"), cl::aliasopt(Bar), cl::DefaultOption);
 
-  StackOption Foo("foo", cl::init(false), cl::sub(*cl::AllSubCommands),
-cl::DefaultOption);
+  StackOption Foo("foo", cl::init(false),
+cl::sub(cl::SubCommand::getAll()), cl::DefaultOption);
   StackOption Foo_Alias("f", cl::desc("Alias for -foo"),
  cl::aliasopt(Foo), cl::DefaultOption);
 
@@ -1062,7 +1064,7 @@
 
   cl::ResetAllOptionOccurrences();
 
-  for (auto  : cl::getRegisteredOptions(*cl::TopLevelSubCommand)) {
+  for (auto  : cl::getRegisteredOptions(cl::SubCommand::getTopLevel())) {
 cl::Option *O = OM.second;
 if (O->ArgStr == "opt2") 

[PATCH] D130576: ManagedStatic: remove from ASTMatchersInternal.h

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle created this revision.
nhaehnle added reviewers: efriedma, lattner.
Herald added a project: All.
nhaehnle requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130576

Files:
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp


Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -28,7 +28,6 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ErrorHandling.h"
-#include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Regex.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
Index: clang/include/clang/ASTMatchers/ASTMatchersInternal.h
===
--- clang/include/clang/ASTMatchers/ASTMatchersInternal.h
+++ clang/include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -59,7 +59,6 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/iterator.h"
 #include "llvm/Support/Casting.h"
-#include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Regex.h"
 #include 
 #include 
@@ -1931,8 +1930,8 @@
 
 public:
   static const Matcher () {
-static llvm::ManagedStatic Instance;
-return Instance->M;
+static Wrapper Instance;
+return Instance.M;
   }
 };
 


Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -28,7 +28,6 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ErrorHandling.h"
-#include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Regex.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
Index: clang/include/clang/ASTMatchers/ASTMatchersInternal.h
===
--- clang/include/clang/ASTMatchers/ASTMatchersInternal.h
+++ clang/include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -59,7 +59,6 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/iterator.h"
 #include "llvm/Support/Casting.h"
-#include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Regex.h"
 #include 
 #include 
@@ -1931,8 +1930,8 @@
 
 public:
   static const Matcher () {
-static llvm::ManagedStatic Instance;
-return Instance->M;
+static Wrapper Instance;
+return Instance.M;
   }
 };
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130575: clang: include ManagedStatic.h for llvm_shutdown

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle created this revision.
nhaehnle added reviewers: efriedma, lattner.
Herald added a project: All.
nhaehnle requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The code relied on ManagedStatic.h being included indirectly. This is
about to change as uses of ManagedStatic are removed throughout the
codebase.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130575

Files:
  clang/unittests/Interpreter/InterpreterTest.cpp
  clang/utils/TableGen/TableGen.cpp


Index: clang/utils/TableGen/TableGen.cpp
===
--- clang/utils/TableGen/TableGen.cpp
+++ clang/utils/TableGen/TableGen.cpp
@@ -13,6 +13,7 @@
 #include "TableGenBackends.h" // Declares all backends.
 #include "ASTTableGen.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/TableGen/Error.h"
Index: clang/unittests/Interpreter/InterpreterTest.cpp
===
--- clang/unittests/Interpreter/InterpreterTest.cpp
+++ clang/unittests/Interpreter/InterpreterTest.cpp
@@ -20,6 +20,7 @@
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Sema.h"
 
+#include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/TargetSelect.h"
 
 #include "gmock/gmock.h"


Index: clang/utils/TableGen/TableGen.cpp
===
--- clang/utils/TableGen/TableGen.cpp
+++ clang/utils/TableGen/TableGen.cpp
@@ -13,6 +13,7 @@
 #include "TableGenBackends.h" // Declares all backends.
 #include "ASTTableGen.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/TableGen/Error.h"
Index: clang/unittests/Interpreter/InterpreterTest.cpp
===
--- clang/unittests/Interpreter/InterpreterTest.cpp
+++ clang/unittests/Interpreter/InterpreterTest.cpp
@@ -20,6 +20,7 @@
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Sema.h"
 
+#include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/TargetSelect.h"
 
 #include "gmock/gmock.h"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130574: ClangLinkerWrapper: explicitly #include

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle created this revision.
nhaehnle added reviewers: efriedma, lattner.
Herald added a project: All.
nhaehnle requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This code relied on implicitly having std::atomic available via the
ManagedStatic.h header.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130574

Files:
  clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp


Index: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
===
--- clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -51,6 +51,7 @@
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetMachine.h"
+#include 
 
 using namespace llvm;
 using namespace llvm::opt;


Index: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
===
--- clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -51,6 +51,7 @@
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetMachine.h"
+#include 
 
 using namespace llvm;
 using namespace llvm::opt;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130406: Use llvm::sort instead of std::sort where possible

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle accepted this revision.
nhaehnle added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130406

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

[PATCH] D130224: [Clang][Attribute] Introduce maybe_undef attribute for function arguments which accepts undef values

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

In D130224#3668225 , @aaron.ballman 
wrote:

> I'm in C standards meetings this week and don't have a lot of ability to 
> thoroughly review this yet, but the most immediate concern which springs to 
> mind for me is that this is exposing LLVM implementation details to users. 
> Users should not have to think about things in terms of LLVM IR markings like 
> poison and undef, and I worry that this is an expert-only feature that will 
> be easy to misuse and introduce security issues.

Here's how I would tentatively describe the attribute in terms that mesh better 
with how I understand C and C++:

> As an exception to the rule that loading from an unitialized variable is 
> undefined behavior, if the loaded value is used immediately as an 
> `__attribute__((maybe_undef))` argument in a function call, the loaded value 
> is implementation-defined. It may vary between multiple runs of the program, 
> and it may vary between multiple uses of the uninitialized variable.

This requires no thinking about LLVM IR and undef/poison.

There may have to be some caveats about bools and enums (generally, types where 
not all possible values in memory are actually legal values of the type), but I 
don't know enough about those language standards to judge that.




Comment at: clang/include/clang/Basic/AttrDocs.td:276
+
+  void maybeundeffunc(void __attribute__((maybe_undef))param);
+  }];

`param` shouldn't be of void type, right?


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

https://reviews.llvm.org/D130224

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

[PATCH] D130089: update-test-checks: safely handle tests with #if's

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

My version of `diff` has a `--strip-trailing-cr` flag. But I don't really see 
that being used in many places at all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130089

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

[PATCH] D130089: update-test-checks: safely handle tests with #if's

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

I've been staring at ifdef.test vs. basic-cplusplus.test for a while now and 
don't see any relevant difference, including when I look at the line endings in 
a hex editor (on Linux). So I guess there is a difference that appears only on 
Windows somehow? Unfortunately, I simply have no idea at the moment where it 
could come from.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130089

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

[PATCH] D130089: update-test-checks: safely handle tests with #if's

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

In D130089#3666076 , @mizvekov wrote:

> This seems to have introduced a test which always fails on windows:
>
>   $ ":" "RUN: at line 4"
>   $ "cp" "clang\test\utils\update_cc_test_checks/Inputs/ifdef.c" 
> "build\llvm\tools\clang\test\utils\update_cc_test_checks\Output\ifdef.test.tmp.c"
>   $ "C:/Program Files (x86)/Microsoft Visual 
> Studio/Shared/Python39_64/python.exe" "llvm\utils\update_cc_test_checks.py" 
> "--clang" "build\llvm\RelWithDebInfo\bin\clang" "--opt" 
> "build\llvm\RelWithDebInfo\bin\opt" 
> "build\llvm\tools\clang\test\utils\update_cc_test_checks\Output\ifdef.test.tmp.c"
>   $ ":" "RUN: at line 5"
>   $ "diff" "-u" 
> "clang\test\utils\update_cc_test_checks/Inputs/ifdef.c.expected" 
> "build\llvm\tools\clang\test\utils\update_cc_test_checks\Output\ifdef.test.tmp.c"
>   # command output:
>   --- clang\test\utils\update_cc_test_checks/Inputs/ifdef.c.expected
>   +++ 
> build\llvm\tools\clang\test\utils\update_cc_test_checks\Output\ifdef.test.tmp.c
>   @@ -1,21 +1,21 @@
>   -// NOTE: Assertions have been autogenerated by 
> utils/update_cc_test_checks.py
>   -// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s | 
> FileCheck -check-prefixes=CHECK %s
>   -// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s 
> -DFOO | FileCheck -check-prefixes=CHECK,FOO %s
>   -
>   -#ifdef FOO
>   -// FOO-LABEL: @foo(
>   -// FOO-NEXT:  entry:
>   -// FOO-NEXT:ret i32 1
>   -//
>   -int foo() {
>   -  return 1;
>   -}
>   -#endif
>   -
>   -// CHECK-LABEL: @bar(
>   -// CHECK-NEXT:  entry:
>   -// CHECK-NEXT:ret i32 2
>   -//
>   -int bar() {
>   -  return 2;
>   -}
>   +// NOTE: Assertions have been autogenerated by 
> utils/update_cc_test_checks.py
>   +// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s | 
> FileCheck -check-prefixes=CHECK %s
>   +// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s 
> -DFOO | FileCheck -check-prefixes=CHECK,FOO %s
>   +
>   +#ifdef FOO
>   +// FOO-LABEL: @foo(
>   +// FOO-NEXT:  entry:
>   +// FOO-NEXT:ret i32 1
>   +//
>   +int foo() {
>   +  return 1;
>   +}
>   +#endif
>   +
>   +// CHECK-LABEL: @bar(
>   +// CHECK-NEXT:  entry:
>   +// CHECK-NEXT:ret i32 2
>   +//
>   +int bar() {
>   +  return 2;
>   +}
>   
>   error: command failed with exit status: 1
>   
>   --
>   
>   
>   Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..
>   
>   Failed Tests (1):
> Clang :: utils/update_cc_test_checks/ifdef.test
>   
>   
>   Testing Time: 0.28s
> Failed: 1

Aren't the before and after lines of the diff identical? Is this a Windows 
new-lines issue?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130089

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

[PATCH] D130089: update-test-checks: safely handle tests with #if's

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added inline comments.



Comment at: 
llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/arm_function_name.ll:2
 ; Check that we accept functions with '$' in the name.
 ; TODO: This is not handled correcly on 32bit ARM and needs to be fixed.
 

arichardson wrote:
> Remove this TODO?
Ok, will do before I commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130089

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

[PATCH] D130089: update-test-checks: safely handle tests with #if's

[Nicolai Hähnle via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
nhaehnle marked an inline comment as done.
Closed by commit rG5a4033c36716: update-test-checks: safely handle tests with 
#ifs (authored by nhaehnle).

Changed prior to commit:
  https://reviews.llvm.org/D130089?vs=445817=446085#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130089

Files:
  clang/test/utils/update_cc_test_checks/Inputs/ifdef.c
  clang/test/utils/update_cc_test_checks/Inputs/ifdef.c.expected
  clang/test/utils/update_cc_test_checks/ifdef.test
  
llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/arm_function_name.ll
  
llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/arm_function_name.ll.expected
  llvm/utils/UpdateTestChecks/asm.py
  llvm/utils/UpdateTestChecks/common.py
  llvm/utils/update_analyze_test_checks.py
  llvm/utils/update_cc_test_checks.py
  llvm/utils/update_llc_test_checks.py
  llvm/utils/update_test_checks.py

Index: llvm/utils/update_test_checks.py
===
--- llvm/utils/update_test_checks.py
+++ llvm/utils/update_test_checks.py
@@ -120,6 +120,7 @@
verbose=ti.args.verbose)
   builder.process_run_line(common.OPT_FUNCTION_RE, common.scrub_body,
   raw_tool_output, prefixes, False)
+  builder.processed_prefixes(prefixes)
 
 func_dict = builder.finish_and_get_func_dict()
 is_in_function = False
Index: llvm/utils/update_llc_test_checks.py
===
--- llvm/utils/update_llc_test_checks.py
+++ llvm/utils/update_llc_test_checks.py
@@ -137,6 +137,7 @@
 
   scrubber, function_re = output_type.get_run_handler(triple)
   builder.process_run_line(function_re, scrubber, raw_tool_output, prefixes, True)
+  builder.processed_prefixes(prefixes)
 
 func_dict = builder.finish_and_get_func_dict()
 global_vars_seen_dict = {}
Index: llvm/utils/update_cc_test_checks.py
===
--- llvm/utils/update_cc_test_checks.py
+++ llvm/utils/update_cc_test_checks.py
@@ -214,6 +214,7 @@
 builder.process_run_line(
 common.OPT_FUNCTION_RE, common.scrub_body, raw_tool_output,
 prefixes, False)
+builder.processed_prefixes(prefixes)
   else:
 print('The clang command line should include -emit-llvm as asm tests '
   'are discouraged in Clang testsuite.', file=sys.stderr)
Index: llvm/utils/update_analyze_test_checks.py
===
--- llvm/utils/update_analyze_test_checks.py
+++ llvm/utils/update_analyze_test_checks.py
@@ -124,6 +124,8 @@
 common.warn('Don\'t know how to deal with this output')
 continue
 
+  builder.processed_prefixes(prefixes)
+
 func_dict = builder.finish_and_get_func_dict()
 is_in_function = False
 is_in_function_start = False
Index: llvm/utils/UpdateTestChecks/common.py
===
--- llvm/utils/UpdateTestChecks/common.py
+++ llvm/utils/UpdateTestChecks/common.py
@@ -496,6 +496,7 @@
 self._func_dict = {}
 self._func_order = {}
 self._global_var_dict = {}
+self._processed_prefixes = set()
 for tuple in run_list:
   for prefix in tuple[0]:
 self._func_dict.update({prefix:dict()})
@@ -584,30 +585,43 @@
scrubbed_body)
 
 if func in self._func_dict[prefix]:
-  if (self._func_dict[prefix][func] is None or
-  str(self._func_dict[prefix][func]) != scrubbed_body or
-  self._func_dict[prefix][func].args_and_sig != args_and_sig or
-  self._func_dict[prefix][func].attrs != attrs):
-if (self._func_dict[prefix][func] is not None and
-self._func_dict[prefix][func].is_same_except_arg_names(
+  if (self._func_dict[prefix][func] is not None and
+  (str(self._func_dict[prefix][func]) != scrubbed_body or
+   self._func_dict[prefix][func].args_and_sig != args_and_sig or
+   self._func_dict[prefix][func].attrs != attrs)):
+if self._func_dict[prefix][func].is_same_except_arg_names(
 scrubbed_extra,
 args_and_sig,
 attrs,
-is_backend)):
+is_backend):
   self._func_dict[prefix][func].scrub = scrubbed_extra
   self._func_dict[prefix][func].args_and_sig = args_and_sig
-  continue
 else:
   # This means a previous RUN line produced a body for this function
   # that is different from the one produced by this current RUN line,
   # so the body can't be common 

[PATCH] D130089: update-test-checks: safely handle tests with #if's

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle created this revision.
nhaehnle added reviewers: MaskRay, arsenm, aemerson, arichardson.
Herald added subscribers: StephenFan, kristof.beyls.
Herald added a project: All.
nhaehnle requested review of this revision.
Herald added subscribers: cfe-commits, wdng.
Herald added projects: clang, LLVM.

There is at least one Clang test (clang/test/CodeGen/arm_acle.c) which
has functions guarded by #if's that cause those functions to be compiled
only for a subset of RUN lines.

This results in a case where one RUN line has a body for the function
and another doesn't. Treat this case as a conflict for any prefixes that
the two RUN lines have in common.

This change exposed a bug where functions with '$' in the name weren't
properly recognized in ARM assembly (despite there being a test case
that was supposed to catch the problem!). This bug is fixed as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130089

Files:
  clang/test/utils/update_cc_test_checks/Inputs/ifdef.c
  clang/test/utils/update_cc_test_checks/Inputs/ifdef.c.expected
  clang/test/utils/update_cc_test_checks/ifdef.test
  
llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/arm_function_name.ll
  
llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/arm_function_name.ll.expected
  llvm/utils/UpdateTestChecks/asm.py
  llvm/utils/UpdateTestChecks/common.py
  llvm/utils/update_analyze_test_checks.py
  llvm/utils/update_cc_test_checks.py
  llvm/utils/update_llc_test_checks.py
  llvm/utils/update_test_checks.py

Index: llvm/utils/update_test_checks.py
===
--- llvm/utils/update_test_checks.py
+++ llvm/utils/update_test_checks.py
@@ -120,6 +120,7 @@
verbose=ti.args.verbose)
   builder.process_run_line(common.OPT_FUNCTION_RE, common.scrub_body,
   raw_tool_output, prefixes, False)
+  builder.processed_prefixes(prefixes)
 
 func_dict = builder.finish_and_get_func_dict()
 is_in_function = False
Index: llvm/utils/update_llc_test_checks.py
===
--- llvm/utils/update_llc_test_checks.py
+++ llvm/utils/update_llc_test_checks.py
@@ -137,6 +137,7 @@
 
   scrubber, function_re = output_type.get_run_handler(triple)
   builder.process_run_line(function_re, scrubber, raw_tool_output, prefixes, True)
+  builder.processed_prefixes(prefixes)
 
 func_dict = builder.finish_and_get_func_dict()
 global_vars_seen_dict = {}
Index: llvm/utils/update_cc_test_checks.py
===
--- llvm/utils/update_cc_test_checks.py
+++ llvm/utils/update_cc_test_checks.py
@@ -214,6 +214,7 @@
 builder.process_run_line(
 common.OPT_FUNCTION_RE, common.scrub_body, raw_tool_output,
 prefixes, False)
+builder.processed_prefixes(prefixes)
   else:
 print('The clang command line should include -emit-llvm as asm tests '
   'are discouraged in Clang testsuite.', file=sys.stderr)
Index: llvm/utils/update_analyze_test_checks.py
===
--- llvm/utils/update_analyze_test_checks.py
+++ llvm/utils/update_analyze_test_checks.py
@@ -124,6 +124,8 @@
 common.warn('Don\'t know how to deal with this output')
 continue
 
+  builder.processed_prefixes(prefixes)
+
 func_dict = builder.finish_and_get_func_dict()
 is_in_function = False
 is_in_function_start = False
Index: llvm/utils/UpdateTestChecks/common.py
===
--- llvm/utils/UpdateTestChecks/common.py
+++ llvm/utils/UpdateTestChecks/common.py
@@ -496,6 +496,7 @@
 self._func_dict = {}
 self._func_order = {}
 self._global_var_dict = {}
+self._processed_prefixes = set()
 for tuple in run_list:
   for prefix in tuple[0]:
 self._func_dict.update({prefix:dict()})
@@ -584,30 +585,43 @@
scrubbed_body)
 
 if func in self._func_dict[prefix]:
-  if (self._func_dict[prefix][func] is None or
-  str(self._func_dict[prefix][func]) != scrubbed_body or
-  self._func_dict[prefix][func].args_and_sig != args_and_sig or
-  self._func_dict[prefix][func].attrs != attrs):
-if (self._func_dict[prefix][func] is not None and
-self._func_dict[prefix][func].is_same_except_arg_names(
+  if (self._func_dict[prefix][func] is not None and
+  (str(self._func_dict[prefix][func]) != scrubbed_body or
+   self._func_dict[prefix][func].args_and_sig != args_and_sig or
+   self._func_dict[prefix][func].attrs != attrs)):
+if self._func_dict[prefix][func].is_same_except_arg_names(
 scrubbed_extra,
 args_and_sig,
 attrs,
-

[PATCH] D125291: Introduce @llvm.threadlocal.address intrinsic to access TLS variable

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

I can't judge the Clang changes.

On the LLVM side, can you also add the implementation and a test for the 
GlobalISel path? Plus, I have some minor inline comments.




Comment at: llvm/docs/LangRef.rst:24546
+
+The first argument is pointer, which refers to a thread local variable.
+

The first argument is **a** pointer



Comment at: llvm/docs/LangRef.rst:24551-24553
+The LLVM treated the address of thread local variable as a constant expression.
+But it is not true. The ``llvm.threadlocal.address`` intrinsic would represent
+the address of the thread local variable.

LangRef should focus on how IR **is** defined, not the historical evolution. I 
think everything that needs to be said here is actually already said in the 
Semantics section.


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

https://reviews.llvm.org/D125291

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

[PATCH] D128166: ManagedStatic: Destroy from destructor

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.
Herald added a subscriber: anlunx.

Abandoning this in favor of removing ManagedStatic entirely via the stack of 
changes that ends in D129134 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128166

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

[PATCH] D129131: Remove uses of llvm_shutdown

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle created this revision.
nhaehnle added reviewers: efriedma, lattner.
Herald added a reviewer: bollu.
Herald added subscribers: anlunx, bzcheeseman, ayermolo, sdasgup3, wenzhicui, 
wrengr, dcaballe, cota, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, 
Kayjukh, grosul1, jvesely, Joonsoo, liufengdb, aartbik, mgester, arpith-jacob, 
antiagainst, shauheen, rriddle, mehdi_amini, hiraditya.
Herald added a reviewer: rafauler.
Herald added a reviewer: Amir.
Herald added a reviewer: maksfb.
Herald added a project: All.
nhaehnle requested review of this revision.
Herald added subscribers: lldb-commits, cfe-commits, yota9, 
stephenneuendorffer, nicolasvasilache.
Herald added projects: clang, LLDB, MLIR, LLVM.

With the removal of ManagedStatic, llvm_shutdown will be removed as
well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129131

Files:
  bolt/tools/driver/llvm-bolt.cpp
  bolt/tools/merge-fdata/merge-fdata.cpp
  clang/include/clang/Frontend/CompilerInstance.h
  clang/tools/clang-repl/ClangRepl.cpp
  clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
  clang/unittests/Interpreter/InterpreterTest.cpp
  clang/utils/TableGen/TableGen.cpp
  libclc/utils/prepare-builtins.cpp
  lldb/tools/driver/Driver.cpp
  lldb/tools/lldb-test/lldb-test.cpp
  lldb/unittests/Utility/LogTest.cpp
  lldb/utils/TableGen/LLDBTableGen.cpp
  llvm/examples/BrainF/BrainFDriver.cpp
  llvm/examples/HowToUseJIT/HowToUseJIT.cpp
  llvm/include/llvm/PassRegistry.h
  llvm/include/llvm/Support/DynamicLibrary.h
  llvm/include/llvm/Support/InitLLVM.h
  llvm/lib/IR/Pass.cpp
  llvm/lib/Support/InitLLVM.cpp
  llvm/lib/Support/Unix/DynamicLibrary.inc
  llvm/lib/Support/Unix/Signals.inc
  llvm/lib/Support/Windows/DynamicLibrary.inc
  llvm/tools/gold/gold-plugin.cpp
  llvm/tools/llvm-gsymutil/llvm-gsymutil.cpp
  llvm/unittests/ExecutionEngine/ExecutionEngineTest.cpp
  llvm/unittests/Support/DynamicLibrary/DynamicLibraryTest.cpp
  llvm/utils/KillTheDoctor/KillTheDoctor.cpp
  mlir/tools/mlir-vulkan-runner/mlir-vulkan-runner.cpp
  polly/lib/External/isl/interface/extract_interface.cc

Index: polly/lib/External/isl/interface/extract_interface.cc
===
--- polly/lib/External/isl/interface/extract_interface.cc
+++ polly/lib/External/isl/interface/extract_interface.cc
@@ -48,7 +48,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -587,7 +586,6 @@
 
 	delete sema;
 	delete Clang;
-	llvm::llvm_shutdown();
 
 	if (Diags.hasErrorOccurred())
 		return EXIT_FAILURE;
Index: mlir/tools/mlir-vulkan-runner/mlir-vulkan-runner.cpp
===
--- mlir/tools/mlir-vulkan-runner/mlir-vulkan-runner.cpp
+++ mlir/tools/mlir-vulkan-runner/mlir-vulkan-runner.cpp
@@ -62,7 +62,6 @@
 }
 
 int main(int argc, char **argv) {
-  llvm::llvm_shutdown_obj x;
   registerPassManagerCLOptions();
 
   llvm::InitLLVM y(argc, argv);
Index: llvm/utils/KillTheDoctor/KillTheDoctor.cpp
===
--- llvm/utils/KillTheDoctor/KillTheDoctor.cpp
+++ llvm/utils/KillTheDoctor/KillTheDoctor.cpp
@@ -36,7 +36,6 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/CommandLine.h"
-#include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Signals.h"
@@ -297,7 +296,6 @@
   // Print a stack trace if we signal out.
   sys::PrintStackTraceOnErrorSignal(argv[0]);
   PrettyStackTraceProgram X(argc, argv);
-  llvm_shutdown_obj Y;  // Call llvm_shutdown() on exit.
 
   ToolName = argv[0];
 
Index: llvm/unittests/Support/DynamicLibrary/DynamicLibraryTest.cpp
===
--- llvm/unittests/Support/DynamicLibrary/DynamicLibraryTest.cpp
+++ llvm/unittests/Support/DynamicLibrary/DynamicLibraryTest.cpp
@@ -9,7 +9,6 @@
 #include "llvm/Support/DynamicLibrary.h"
 #include "llvm/Config/config.h"
 #include "llvm/Support/FileSystem.h"
-#include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
 #include "gtest/gtest.h"
 
@@ -59,7 +58,6 @@
 TEST(DynamicLibrary, Overload) {
   {
 std::string Err;
-llvm_shutdown_obj Shutdown;
 DynamicLibrary DL =
 DynamicLibrary::getPermanentLibrary(LibPath().c_str(), );
 EXPECT_TRUE(DL.isValid());
@@ -109,9 +107,6 @@
   }
   EXPECT_TRUE(FuncPtr(DynamicLibrary::SearchForAddressOfSymbol(
   "TestA")) == nullptr);
-
-  // Check serach ordering is reset to default after call to llvm_shutdown
-  EXPECT_EQ(DynamicLibrary::SearchOrder, DynamicLibrary::SO_Linker);
 }
 
 TEST(DynamicLibrary, Shutdown) {
@@ -119,7 +114,6 @@
   std::vector Order;
   {
 std::string Err;
-llvm_shutdown_obj Shutdown;
 DynamicLibrary DL =
 DynamicLibrary::getPermanentLibrary(LibPath(A).c_str(), );
 EXPECT_TRUE(DL.isValid());

[PATCH] D129118: CommandLine: add and use cl::SubCommand::get{All,TopLevel}

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle created this revision.
nhaehnle added reviewers: efriedma, lattner.
Herald added subscribers: ayermolo, hiraditya.
Herald added a reviewer: rafauler.
Herald added a reviewer: Amir.
Herald added a reviewer: maksfb.
Herald added a project: All.
nhaehnle requested review of this revision.
Herald added subscribers: cfe-commits, yota9.
Herald added projects: clang, LLVM.

Prefer using these accessors to access the special sub-commands
corresponding to the top-level (no subcommand) and all sub-commands.

This is a preparatory step towards removing the use of ManagedStatic:
with a subsequent change, these global instances will be moved to
be regular function-scope statics.

It is split up to give downstream projects a (albeit short) window in
which they can switch to using the accessors in a forward-compatible
way.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129118

Files:
  bolt/lib/Utils/CommandLineOpts.cpp
  bolt/tools/driver/llvm-bolt.cpp
  clang/lib/Tooling/CommonOptionsParser.cpp
  clang/tools/clang-refactor/ClangRefactor.cpp
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp
  llvm/tools/llvm-xray/llvm-xray.cpp
  llvm/unittests/Support/CommandLineInit/CommandLineInitTest.cpp
  llvm/unittests/Support/CommandLineTest.cpp

Index: llvm/unittests/Support/CommandLineTest.cpp
===
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -99,7 +99,7 @@
   static const char ValueString[] = "Integer";
 
   StringMap  =
-  cl::getRegisteredOptions(*cl::TopLevelSubCommand);
+  cl::getRegisteredOptions(cl::SubCommand::getTopLevel());
 
   ASSERT_EQ(Map.count("test-option"), 1u) << "Could not find option in map.";
 
@@ -420,7 +420,7 @@
   << "Hid extra option that should be visable.";
 
   StringMap  =
-  cl::getRegisteredOptions(*cl::TopLevelSubCommand);
+  cl::getRegisteredOptions(cl::SubCommand::getTopLevel());
   ASSERT_TRUE(Map.count("help") == (size_t)0 ||
   cl::NotHidden == Map["help"]->getOptionHiddenFlag())
   << "Hid default option that should be visable.";
@@ -446,7 +446,7 @@
   << "Hid extra option that should be visable.";
 
   StringMap  =
-  cl::getRegisteredOptions(*cl::TopLevelSubCommand);
+  cl::getRegisteredOptions(cl::SubCommand::getTopLevel());
   ASSERT_TRUE(Map.count("help") == (size_t)0 ||
   cl::NotHidden == Map["help"]->getOptionHiddenFlag())
   << "Hid default option that should be visable.";
@@ -529,7 +529,7 @@
   cl::ResetCommandLineParser();
 
   StackSubCommand SC1("sc1", "First subcommand");
-  StackOption AllOpt("everywhere", cl::sub(*cl::AllSubCommands),
+  StackOption AllOpt("everywhere", cl::sub(cl::SubCommand::getAll()),
cl::init(false));
   StackSubCommand SC2("sc2", "Second subcommand");
 
@@ -566,8 +566,8 @@
 TEST(CommandLineTest, ReparseCommandLineOptions) {
   cl::ResetCommandLineParser();
 
-  StackOption TopLevelOpt("top-level", cl::sub(*cl::TopLevelSubCommand),
-cl::init(false));
+  StackOption TopLevelOpt(
+  "top-level", cl::sub(cl::SubCommand::getTopLevel()), cl::init(false));
 
   const char *args[] = {"prog", "-top-level"};
 
@@ -614,10 +614,12 @@
 TEST(CommandLineTest, RemoveFromTopLevelSubCommand) {
   cl::ResetCommandLineParser();
 
-  StackOption TopLevelRemove(
-  "top-level-remove", cl::sub(*cl::TopLevelSubCommand), cl::init(false));
-  StackOption TopLevelKeep(
-  "top-level-keep", cl::sub(*cl::TopLevelSubCommand), cl::init(false));
+  StackOption TopLevelRemove("top-level-remove",
+   cl::sub(cl::SubCommand::getTopLevel()),
+   cl::init(false));
+  StackOption TopLevelKeep("top-level-keep",
+ cl::sub(cl::SubCommand::getTopLevel()),
+ cl::init(false));
 
   const char *args[] = {"prog", "-top-level-remove"};
 
@@ -638,9 +640,9 @@
 
   StackSubCommand SC1("sc1", "First Subcommand");
   StackSubCommand SC2("sc2", "Second Subcommand");
-  StackOption RemoveOption("remove-option", cl::sub(*cl::AllSubCommands),
- cl::init(false));
-  StackOption KeepOption("keep-option", cl::sub(*cl::AllSubCommands),
+  StackOption RemoveOption(
+  "remove-option", cl::sub(cl::SubCommand::getAll()), cl::init(false));
+  StackOption KeepOption("keep-option", cl::sub(cl::SubCommand::getAll()),
cl::init(false));
 
   const char *args0[] = {"prog", "-remove-option"};
@@ -719,13 +721,13 @@
 TEST(CommandLineTest, DefaultOptions) {
   cl::ResetCommandLineParser();
 
-  StackOption Bar("bar", cl::sub(*cl::AllSubCommands),
+  StackOption Bar("bar", cl::sub(cl::SubCommand::getAll()),
cl::DefaultOption);
   StackOption Bar_Alias(
   "b", cl::desc("Alias for -bar"), 

[PATCH] D128166: ManagedStatic: Destroy from destructor

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle created this revision.
Herald added a reviewer: deadalnix.
Herald added subscribers: bzcheeseman, ayermolo, sdasgup3, wenzhicui, wrengr, 
dcaballe, cota, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh, 
grosul1, jvesely, Joonsoo, liufengdb, aartbik, mgester, arpith-jacob, 
antiagainst, shauheen, rriddle, mehdi_amini, mstorsjo, hiraditya, mgorny.
Herald added a reviewer: bollu.
Herald added a reviewer: MaskRay.
Herald added a reviewer: rafauler.
Herald added a reviewer: Amir.
Herald added a reviewer: maksfb.
Herald added a project: All.
nhaehnle requested review of this revision.
Herald added subscribers: lldb-commits, cfe-commits, yota9, StephenFan, 
stephenneuendorffer, nicolasvasilache.
Herald added projects: clang, LLDB, MLIR, LLVM.

This allows LLVM to be safely loaded and unloaded as a shared library
without leaking memory:

Consider the scenario where shared library Foo links against LLVM as
a shared library, and Foo may be loaded and unloaded multiple times
in the same process. Should Foo call llvm_shutdown or not? If it does
not, and LLVM is also loaded and unloaded multiple times, then
memory is leaked. If it does call llvm_shutdown and LLVM *isn't*
also unloaded, the state of LLVM's global statics is corrupted because
while the ManagedStatics may be re-initialized, *other* global
constructors aren't re-run.

Keep in mind that Foo itself may use ManagedStatic. If Foo is unloaded
but LLVM isn't, those statics should be destroyed.

The safe solution is to skip llvm_shutdown() altogether and destroy
managed statics from their destructor.

LLD relies on being able to call _exit() and still get some side-effects
of managed static destructors. We make this expectation much more
explicit.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128166

Files:
  bolt/tools/driver/llvm-bolt.cpp
  bolt/tools/merge-fdata/merge-fdata.cpp
  clang/include/clang/Frontend/CompilerInstance.h
  clang/tools/clang-repl/ClangRepl.cpp
  clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
  clang/unittests/Interpreter/InterpreterTest.cpp
  clang/utils/TableGen/TableGen.cpp
  libclc/utils/prepare-builtins.cpp
  lld/Common/ErrorHandler.cpp
  lldb/tools/driver/Driver.cpp
  lldb/tools/lldb-test/lldb-test.cpp
  lldb/unittests/Utility/LogTest.cpp
  lldb/utils/TableGen/LLDBTableGen.cpp
  llvm/docs/ProgrammersManual.rst
  llvm/examples/BrainF/BrainFDriver.cpp
  llvm/examples/HowToUseJIT/HowToUseJIT.cpp
  llvm/include/llvm-c/Core.h
  llvm/include/llvm/ADT/Statistic.h
  llvm/include/llvm/PassRegistry.h
  llvm/include/llvm/Support/DynamicLibrary.h
  llvm/include/llvm/Support/FastShutdown.h
  llvm/include/llvm/Support/InitLLVM.h
  llvm/include/llvm/Support/ManagedStatic.h
  llvm/lib/IR/Core.cpp
  llvm/lib/IR/Pass.cpp
  llvm/lib/IR/PassRegistry.cpp
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/FastShutdown.cpp
  llvm/lib/Support/InitLLVM.cpp
  llvm/lib/Support/ManagedStatic.cpp
  llvm/lib/Support/Parallel.cpp
  llvm/lib/Support/Statistic.cpp
  llvm/lib/Support/Unix/DynamicLibrary.inc
  llvm/lib/Support/Unix/Signals.inc
  llvm/lib/Support/Windows/DynamicLibrary.inc
  llvm/tools/gold/gold-plugin.cpp
  llvm/tools/llvm-gsymutil/llvm-gsymutil.cpp
  llvm/unittests/ExecutionEngine/ExecutionEngineTest.cpp
  llvm/unittests/Support/DynamicLibrary/DynamicLibraryTest.cpp
  llvm/utils/KillTheDoctor/KillTheDoctor.cpp
  mlir/tools/mlir-vulkan-runner/mlir-vulkan-runner.cpp
  polly/lib/External/isl/interface/extract_interface.cc

Index: polly/lib/External/isl/interface/extract_interface.cc
===
--- polly/lib/External/isl/interface/extract_interface.cc
+++ polly/lib/External/isl/interface/extract_interface.cc
@@ -587,7 +587,6 @@
 
 	delete sema;
 	delete Clang;
-	llvm::llvm_shutdown();
 
 	if (Diags.hasErrorOccurred())
 		return EXIT_FAILURE;
Index: mlir/tools/mlir-vulkan-runner/mlir-vulkan-runner.cpp
===
--- mlir/tools/mlir-vulkan-runner/mlir-vulkan-runner.cpp
+++ mlir/tools/mlir-vulkan-runner/mlir-vulkan-runner.cpp
@@ -61,7 +61,6 @@
 }
 
 int main(int argc, char **argv) {
-  llvm::llvm_shutdown_obj x;
   registerPassManagerCLOptions();
 
   llvm::InitLLVM y(argc, argv);
Index: llvm/utils/KillTheDoctor/KillTheDoctor.cpp
===
--- llvm/utils/KillTheDoctor/KillTheDoctor.cpp
+++ llvm/utils/KillTheDoctor/KillTheDoctor.cpp
@@ -297,7 +297,6 @@
   // Print a stack trace if we signal out.
   sys::PrintStackTraceOnErrorSignal(argv[0]);
   PrettyStackTraceProgram X(argc, argv);
-  llvm_shutdown_obj Y;  // Call llvm_shutdown() on exit.
 
   ToolName = argv[0];
 
Index: llvm/unittests/Support/DynamicLibrary/DynamicLibraryTest.cpp
===
--- llvm/unittests/Support/DynamicLibrary/DynamicLibraryTest.cpp
+++ 

[PATCH] D125291: Introduce @llvm.threadlocal.address intrinsic to access TLS variable (1/3)

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added inline comments.



Comment at: llvm/include/llvm/IR/IRBuilder.h:746-747
 
+  /// Create a call to llvm.threadlocal.address intrinsic.
+  CallInst *CreateThreadLocalAddress(Value *Ptr);
+

ChuanqiXu wrote:
> nhaehnle wrote:
> > This could be a `GlobalValue*` operand to reduce the risk of misuse, right?
> We could use `Value*` here to keep consistency with other functions and the 
> uses. We added assertion in implementations to avoid misuses.
Thanks!



Comment at: llvm/include/llvm/IR/Intrinsics.td:1393-1394
 
+def int_threadlocal_address : Intrinsic<[llvm_ptr_ty], [llvm_ptr_ty],
+[IntrNoMem, IntrWillReturn]>;
+

ChuanqiXu wrote:
> nhaehnle wrote:
> > Whether IntrNoMem is truly correct here depends on the overall solution of 
> > the thread identification issue, i.e. it depends on whether readnone 
> > implies "doesn't read thread ID". We'd best discuss that separately.
> Yeah, let's discuss this in discourse thread.
Just to follow up, based on the latest comments including that of @jyknight, 
IntroNoMem is correct.


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

https://reviews.llvm.org/D125291

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

[PATCH] D125291: Introduce @llvm.threadlocal.address intrinsic to access TLS variable (1/3)

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added inline comments.



Comment at: llvm/include/llvm/IR/Intrinsics.td:1393-1394
 
+def int_threadlocal_address : Intrinsic<[llvm_ptr_ty], [llvm_ptr_ty],
+[IntrNoMem, IntrWillReturn]>;
+

Whether IntrNoMem is truly correct here depends on the overall solution of the 
thread identification issue, i.e. it depends on whether readnone implies 
"doesn't read thread ID". We'd best discuss that separately.


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

https://reviews.llvm.org/D125291

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

[PATCH] D125291: Introduce @llvm.threadlocal.address intrinsic to access TLS variable (1/3)

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

You can use the "Edit Related Revisions" option in the right-hand side menu of 
Phabricator to link this revision with the others of the series. I can't really 
speak for the Clang parts, but the LLVM parts looks reasonable to me modulo 
some detail comments.




Comment at: llvm/docs/LangRef.rst:24392-24403
+Overview:
+""
+
+The LLVM treated the address of thread local variable as a constant expression.
+But it is not true. The ``llvm.threadlocal.address`` intrinsic would represent
+the address of the thread local variable.
+

LangRef should be written in present tense. Something like:

> The address of a thread local variable is not a constant, since it depends on 
> the calling thread. The ``llvm.threadlocal.address`` intrinsic returns the 
> address of the given thread local variable in the calling thread.



Comment at: llvm/include/llvm/IR/IRBuilder.h:746-747
 
+  /// Create a call to llvm.threadlocal.address intrinsic.
+  CallInst *CreateThreadLocalAddress(Value *Ptr);
+

This could be a `GlobalValue*` operand to reduce the risk of misuse, right?


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

https://reviews.llvm.org/D125291

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

[PATCH] D125378: [Attribute] Introduce shuffle attribute to be used for __shfl_sync like cross-lane APIs

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle requested changes to this revision.
nhaehnle added a comment.

I'm sorry to say that this patch seems confused about semantics. It lacks clear 
definitions, and in particular for the `shuffle` attribute in LLVM IR, you 
almost certainly just want the already existing `convergent` instead.

If, separately from that, you still find a need to prevent Clang from labeling 
certain function arguments or return values as `noundef`, then presumably that 
should be done with an explicit "maybe undef" attribute instead, as @jdoerfert 
suggests.


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

https://reviews.llvm.org/D125378

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

[PATCH] D124158: [Clang][Attr] Skip adding noundef attribute to arguments when function has convergent attribute

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

In D124158#3486110 , @jdoerfert wrote:

> I agree. As far as I can tell you have two options, both are specific to the 
> shuffle functions:
>
> 1. Do not set noundef for calls to them as they allow undef values for all 
> lanes we don't read the value.
> 2. Freeze the inputs unconditionally.

Right, with some nitpicks. Option #1 is semantically more accurate: 
__shfl_sync, subgroupShuffe, and all similar instructions across GPU 
programming languages are meant to be conceptually similar to `select` in that 
they select a value from a lane. The "data" argument and return value should 
allow undef and poison. The incoming value is simply returned as-is, and so 
poison is propagated instead of causing immediate UB, just as it is for a 
`select` instruction. However, the lane argument can (and arguably should) 
still be `noundef`.

There's a separate curious issue in that apparently, reading from an 
uninitialized variable is **not** UB in CUDA/HIP/GLSL/HLSL/etc. If it was, a 
lot of code existing out there in the wild would be broken. But that's a matter 
for the relevant language standards to decide (for the subset of languages that 
//have// proper standards to begin with).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124158

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

[PATCH] D69498: IR: Invert convergent attribute handling

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

In D69498#2711396 , @foad wrote:

> I don't have much to add to the conversation except to point out that this 
> definition defines `noconvergent` in terms of divergent control flow, but the 
> langref itself doesn't define what divergent control flow //is//, which makes 
> it an incomplete spec. (Perhaps I'm just restating @arsenm's objections.) 
> This seems unsatisfactory to me but I have no idea what to do about it. I 
> agree with @sameerds that the current definition of `convergent` is too 
> restrictive because in practice we really do want to be able to move 
> convergent calls past uniform control flow.

That is one of the things that D85603  
addresses. I suspect Sameer was assuming the language from there in the 
background.

I agree with Matt that it would be best to avoid too much TTI dependence. The 
existing situation with the intrinsics is already a bit of a strange one. I 
wonder if it is possible to move to a world where isSourceOfDivergence need to 
be target-dependent. (This requires e.g. new attributes on function arguments 
as well as new information about address spaces in the data layout, plus some 
new attributes to define intrinsic data flow. Likely beyond the scope of this 
patch.)


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

https://reviews.llvm.org/D69498

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

[PATCH] D92661: [RFC] Fix TLS and Coroutine

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added inline comments.



Comment at: llvm/include/llvm/IR/Intrinsics.td:1281-1282
 // The pseudoprobe intrinsic works as a place holder to the block it probes.
-// Like the sideeffect intrinsic defined above, this intrinsic is treated by 
the 
-// optimizer as having opaque side effects so that it won't be get rid of or 
moved 
+// Like the sideeffect intrinsic defined above, this intrinsic is treated by 
the
+// optimizer as having opaque side effects so that it won't be get rid of or 
moved
 // out of the block it probes.

Unrelated change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92661

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

[PATCH] D83088: Introduce CfgTraits abstraction

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle abandoned this revision.
nhaehnle added a comment.
Herald added a subscriber: teijeong.

Superseded by D92924 , D92925 
, D92926 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088

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

[PATCH] D83088: Introduce CfgTraits abstraction

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.
Herald added a subscriber: dexonsmith.

I'm going to follow up with another RFC about this on llvm-dev.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088

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

[PATCH] D83088: Introduce CfgTraits abstraction

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

I replied on llvm-dev.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088

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

[PATCH] D83088: Introduce CfgTraits abstraction

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

Hi Mehdi, this is not an appropriate place for this discussion. Yes, we have a 
general rule that patches can be reverted if they're obviously broken (e.g. 
build bot problems) or clearly violate some other standard. This is a good 
rule, but it doesn't apply here. If you think it does, please state your case 
in the email thread that I've started on llvm-dev for this very purpose. Just 
one thing:

> - the burden of convincing of the approach is on the patch author, reverting 
> is forcing the discussion here.

I was trying to have this conversation. I am more than happy to have it, and I 
would be happy for me people to participate! But what can I do if the only(!) 
person who voices concerns just goes into radio silence, and the total number 
of people who participate is small in any case, despite raising it on llvm-dev 
as well?

It is in fact the decision to **not** revert the change which is apparently 
required to force the discussion!

P.S.: It's easy to miss on Phabricator, but there is already a long stack of 
patches which build on this. In a way this is a good thing because it can 
inform the discussion, but I will hold off from pushing more for now even 
though many of them have already been accepted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088

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

[PATCH] D89814: [TableGen] Change !getop and !setop to !getdagop and !setdagop

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle accepted this revision.
nhaehnle added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89814

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

[PATCH] D83088: Introduce CfgTraits abstraction

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

David, I don't think this is appropriate here. Let's take the discussion to 
llvm-dev.




Comment at: mlir/include/mlir/IR/Dominance.h:49
+template <>
+struct llvm::CfgTraitsFor {
+  using CfgTraits = mlir::CfgTraits;

antiagainst wrote:
> rriddle wrote:
> > This seems to have broken the GCC5 build:
> > https://buildkite.com/mlir/mlir-core/builds/8739#7a957564-9850-487c-a814-c6818890bd14
> > 
> > ```
> > /mlir/include/mlir/IR/Dominance.h:49:14: error: specialization of 
> > 'template struct llvm::CfgTraitsFor' in different 
> > namespace [-fpermissive]
> >  struct llvm::CfgTraitsFor {
> >   ^
> > In file included from mlir/include/mlir/IR/Dominance.h:13:0,
> >  from mlir/lib/IR/Verifier.cpp:30:
> > llvm/include/llvm/Support/CfgTraits.h:294:44: error:   from definition of 
> > 'template struct llvm::CfgTraitsFor' [-fpermissive]
> >  template  struct CfgTraitsFor;
> > ```
> Pushed 
> https://github.com/llvm/llvm-project/commit/f2a06875b604c00cbe96e54363f4f5d28935d610
Apologies for the inconvenience, and thank you for taking care of it!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088

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

[PATCH] D83088: Introduce CfgTraits abstraction

[Nicolai Hähnle 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 rGc0cdd22c72fa: Introduce CfgTraits abstraction (authored by 
nhaehnle).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088

Files:
  clang/include/clang/Analysis/Analyses/Dominators.h
  llvm/include/llvm/CodeGen/MachineCfgTraits.h
  llvm/include/llvm/IR/CFG.h
  llvm/include/llvm/Support/CfgTraits.h
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/MachineCfgTraits.cpp
  llvm/lib/IR/CFG.cpp
  llvm/lib/IR/CMakeLists.txt
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/CfgTraits.cpp
  llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
  mlir/include/mlir/IR/Dominance.h

Index: mlir/include/mlir/IR/Dominance.h
===
--- mlir/include/mlir/IR/Dominance.h
+++ mlir/include/mlir/IR/Dominance.h
@@ -10,8 +10,46 @@
 #define MLIR_IR_DOMINANCE_H
 
 #include "mlir/IR/RegionGraphTraits.h"
+#include "llvm/Support/CfgTraits.h"
 #include "llvm/Support/GenericDomTree.h"
 
+namespace mlir {
+
+/// Partial CFG traits for MLIR's CFG, without a value type.
+class CfgTraitsBase : public llvm::CfgTraitsBase {
+public:
+  using ParentType = Region;
+  using BlockRef = Block *;
+  using ValueRef = void;
+
+  static llvm::CfgBlockRef wrapRef(BlockRef block) {
+return makeOpaque(block);
+  }
+  static BlockRef unwrapRef(llvm::CfgBlockRef block) {
+return static_cast(getOpaque(block));
+  }
+};
+
+class CfgTraits : public llvm::CfgTraits {
+public:
+  static Region *getBlockParent(Block *block) { return block->getParent(); }
+
+  static auto predecessors(Block *block) {
+return llvm::inverse_children(block);
+  }
+
+  static auto successors(Block *block) {
+return llvm::children(block);
+  }
+};
+
+} // namespace mlir
+
+template <>
+struct llvm::CfgTraitsFor {
+  using CfgTraits = mlir::CfgTraits;
+};
+
 extern template class llvm::DominatorTreeBase;
 extern template class llvm::DominatorTreeBase;
 
Index: llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
===
--- llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
+++ llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
@@ -18,9 +18,42 @@
 #include "VPlan.h"
 #include "llvm/ADT/GraphTraits.h"
 #include "llvm/IR/Dominators.h"
+#include "llvm/Support/CfgTraits.h"
 
 namespace llvm {
 
+/// Partial CFG traits for VPlan's CFG, without a value type.
+class VPCfgTraitsBase : public CfgTraitsBase {
+public:
+  using ParentType = VPRegionBlock;
+  using BlockRef = VPBlockBase *;
+  using ValueRef = void;
+
+  static CfgBlockRef wrapRef(BlockRef block) {
+return makeOpaque(block);
+  }
+  static BlockRef unwrapRef(CfgBlockRef block) {
+return static_cast(getOpaque(block));
+  }
+};
+
+class VPCfgTraits : public CfgTraits {
+public:
+  static VPRegionBlock *getBlockParent(VPBlockBase *block) {
+return block->getParent();
+  }
+
+  static auto predecessors(VPBlockBase *block) {
+return llvm::inverse_children(block);
+  }
+
+  static auto successors(VPBlockBase *block) {
+return llvm::children(block);
+  }
+};
+
+template <> struct CfgTraitsFor { using CfgTraits = VPCfgTraits; };
+
 /// Template specialization of the standard LLVM dominator tree utility for
 /// VPBlockBases.
 using VPDominatorTree = DomTreeBase;
Index: llvm/lib/Support/CfgTraits.cpp
===
--- /dev/null
+++ llvm/lib/Support/CfgTraits.cpp
@@ -0,0 +1,14 @@
+//===- CfgTraits.cpp - Traits for generically working on CFGs ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "llvm/Support/CfgTraits.h"
+
+using namespace llvm;
+
+void CfgInterface::anchor() {}
+void CfgPrinter::anchor() {}
Index: llvm/lib/Support/CMakeLists.txt
===
--- llvm/lib/Support/CMakeLists.txt
+++ llvm/lib/Support/CMakeLists.txt
@@ -97,6 +97,7 @@
   BranchProbability.cpp
   BuryPointer.cpp
   CachePruning.cpp
+  CfgTraits.cpp
   circular_raw_ostream.cpp
   Chrono.cpp
   COM.cpp
Index: llvm/lib/IR/CMakeLists.txt
===
--- llvm/lib/IR/CMakeLists.txt
+++ llvm/lib/IR/CMakeLists.txt
@@ -4,6 +4,7 @@
   Attributes.cpp
   AutoUpgrade.cpp
   BasicBlock.cpp
+  CFG.cpp
   Comdat.cpp
   ConstantFold.cpp
   ConstantRange.cpp
Index: llvm/lib/IR/CFG.cpp
===
--- /dev/null
+++ llvm/lib/IR/CFG.cpp
@@ -0,0 +1,56 @@
+//===- CFG.cpp 

[PATCH] D83088: Introduce CfgTraits abstraction

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.
Herald added a subscriber: rdzhabarov.

ping^2


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088

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

[PATCH] D83088: Introduce CfgTraits abstraction

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.
Herald added a subscriber: tatianashp.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088

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

[PATCH] D83088: Introduce CfgTraits abstraction

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added inline comments.



Comment at: llvm/include/llvm/Support/CfgTraits.h:51
+
+  operator bool() const { return ptr != nullptr; }
+

dblaikie wrote:
> `operator bool` should be `explicit`
Done.



Comment at: llvm/include/llvm/Support/CfgTraits.h:53-54
+
+  bool operator==(CfgOpaqueType other) const { return ptr == other.ptr; }
+  bool operator!=(CfgOpaqueType other) const { return ptr != other.ptr; }
+};

dblaikie wrote:
> Preferably make any operator overload that can be a non-member, a non-member 
> - this ensures equal conversion handling on both the left and right hand side 
> of symmetric operators like these. (they can be friends if needed, but 
> doesn't look like it in this case - non-friend, non-members that call get() 
> should be fine here)
Done.



Comment at: llvm/include/llvm/Support/CfgTraits.h:90
+/// operations such as traversal of the CFG.
+class CfgTraitsBase {
+protected:

dblaikie wrote:
> Not sure if this benefits from being inherited from, versus being freely 
> accessible?
The idea here is to enforce via the type system that people use 
CfgTraits::{wrap,unwrap}Ref instead of having makeOpaque calls freely in random 
code.



Comment at: llvm/include/llvm/Support/CfgTraits.h:271-273
+template  struct CfgTraitsFor {
+  // using CfgTraits = ...;
+};

dblaikie wrote:
> This probably shouldn't be defined if it's only needed for specialization,  
> instead it can be declared:
> ```
> template struct CfgTraitsFor;
> ```
Interesting. So GraphTraits should arguably be changed similarly.



Comment at: llvm/include/llvm/Support/CfgTraits.h:287
+public:
+  virtual ~CfgInterface() {}
+

dblaikie wrote:
> prefer `= default` where possible
Done.



Comment at: llvm/include/llvm/Support/CfgTraits.h:337
+return Printable(
+[this, block](raw_ostream ) { printBlockName(out, block); });
+  }

dblaikie wrote:
> generally capture everything by ref `[&]` if the lambda is only used 
> locally/within the same expression or block
The lambda is returned via `Printable`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088

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

[PATCH] D86154: AMDGPU: Add llvm.amdgcn.{read,readfirst,write}lane2 intrinsics with type overloads

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

>> ReplaceNodeResults expects the result type to be changed in semi-magical 
>> ways during vector type legalization, which is non-obvious since the method 
>> can be called from different places. I think it *could* be made to work with 
>> a lot of patience, but it's really a bad interface -- and besides, by doing 
>> it in IR we reduce code duplication between SelectionDAG and GlobalISel, 
>> which is an added benefit IMO.
>
> Well the globalisel handling should be much simpler. We have a lot of stuff 
> that's randomly handled in the IR to work around the DAG which long term 
> should be moved where it belongs in codegen

Is the GlobalISel handling simpler than the handling in IR? What would happen 
if we wanted to extend the handling to struct types?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86154

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

[PATCH] D83088: Introduce CfgTraits abstraction

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle updated this revision to Diff 287441.
nhaehnle added a comment.

- cleanup operators on CfgOpaqueType
- address other review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088

Files:
  clang/include/clang/Analysis/Analyses/Dominators.h
  llvm/include/llvm/CodeGen/MachineCfgTraits.h
  llvm/include/llvm/IR/CFG.h
  llvm/include/llvm/Support/CfgTraits.h
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/MachineCfgTraits.cpp
  llvm/lib/IR/CFG.cpp
  llvm/lib/IR/CMakeLists.txt
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/CfgTraits.cpp
  llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
  mlir/include/mlir/IR/Dominance.h

Index: mlir/include/mlir/IR/Dominance.h
===
--- mlir/include/mlir/IR/Dominance.h
+++ mlir/include/mlir/IR/Dominance.h
@@ -10,8 +10,46 @@
 #define MLIR_IR_DOMINANCE_H
 
 #include "mlir/IR/RegionGraphTraits.h"
+#include "llvm/Support/CfgTraits.h"
 #include "llvm/Support/GenericDomTree.h"
 
+namespace mlir {
+
+/// Partial CFG traits for MLIR's CFG, without a value type.
+class CfgTraitsBase : public llvm::CfgTraitsBase {
+public:
+  using ParentType = Region;
+  using BlockRef = Block *;
+  using ValueRef = void;
+
+  static llvm::CfgBlockRef wrapRef(BlockRef block) {
+return makeOpaque(block);
+  }
+  static BlockRef unwrapRef(llvm::CfgBlockRef block) {
+return static_cast(getOpaque(block));
+  }
+};
+
+class CfgTraits : public llvm::CfgTraits {
+public:
+  static Region *getBlockParent(Block *block) { return block->getParent(); }
+
+  static auto predecessors(Block *block) {
+return llvm::inverse_children(block);
+  }
+
+  static auto successors(Block *block) {
+return llvm::children(block);
+  }
+};
+
+} // namespace mlir
+
+template <>
+struct llvm::CfgTraitsFor {
+  using CfgTraits = mlir::CfgTraits;
+};
+
 extern template class llvm::DominatorTreeBase;
 extern template class llvm::DominatorTreeBase;
 
Index: llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
===
--- llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
+++ llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
@@ -18,9 +18,42 @@
 #include "VPlan.h"
 #include "llvm/ADT/GraphTraits.h"
 #include "llvm/IR/Dominators.h"
+#include "llvm/Support/CfgTraits.h"
 
 namespace llvm {
 
+/// Partial CFG traits for VPlan's CFG, without a value type.
+class VPCfgTraitsBase : public CfgTraitsBase {
+public:
+  using ParentType = VPRegionBlock;
+  using BlockRef = VPBlockBase *;
+  using ValueRef = void;
+
+  static CfgBlockRef wrapRef(BlockRef block) {
+return makeOpaque(block);
+  }
+  static BlockRef unwrapRef(CfgBlockRef block) {
+return static_cast(getOpaque(block));
+  }
+};
+
+class VPCfgTraits : public CfgTraits {
+public:
+  static VPRegionBlock *getBlockParent(VPBlockBase *block) {
+return block->getParent();
+  }
+
+  static auto predecessors(VPBlockBase *block) {
+return llvm::inverse_children(block);
+  }
+
+  static auto successors(VPBlockBase *block) {
+return llvm::children(block);
+  }
+};
+
+template <> struct CfgTraitsFor { using CfgTraits = VPCfgTraits; };
+
 /// Template specialization of the standard LLVM dominator tree utility for
 /// VPBlockBases.
 using VPDominatorTree = DomTreeBase;
Index: llvm/lib/Support/CfgTraits.cpp
===
--- /dev/null
+++ llvm/lib/Support/CfgTraits.cpp
@@ -0,0 +1,14 @@
+//===- CfgTraits.cpp - Traits for generically working on CFGs ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "llvm/Support/CfgTraits.h"
+
+using namespace llvm;
+
+void CfgInterface::anchor() {}
+void CfgPrinter::anchor() {}
Index: llvm/lib/Support/CMakeLists.txt
===
--- llvm/lib/Support/CMakeLists.txt
+++ llvm/lib/Support/CMakeLists.txt
@@ -83,6 +83,7 @@
   BranchProbability.cpp
   BuryPointer.cpp
   CachePruning.cpp
+  CfgTraits.cpp
   circular_raw_ostream.cpp
   Chrono.cpp
   COM.cpp
Index: llvm/lib/IR/CMakeLists.txt
===
--- llvm/lib/IR/CMakeLists.txt
+++ llvm/lib/IR/CMakeLists.txt
@@ -4,6 +4,7 @@
   Attributes.cpp
   AutoUpgrade.cpp
   BasicBlock.cpp
+  CFG.cpp
   Comdat.cpp
   ConstantFold.cpp
   ConstantRange.cpp
Index: llvm/lib/IR/CFG.cpp
===
--- /dev/null
+++ llvm/lib/IR/CFG.cpp
@@ -0,0 +1,56 @@
+//===- CFG.cpp --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache 

[PATCH] D83088: Introduce CfgTraits abstraction

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

 The most immediate problem is divergence analysis, which is extremely 
 complex and difficult to get right. If I had tried to fight the accidental 
 complexity that comes with attempting to write such an algorithm as C++ 
 templates in addition to the inherent complexity of the algorithm at the 
 same time, I'm not sure I would have been able to produce anything 
 workable at all.

 Frankly, I suspect that our dominator tree implementation also suffer 
 because of this, though at least dominator trees are much more well 
 studied in the academic literature, so that helps keep the inherent 
 complexity under control.
>>>
>>> I'm totally open to discussing making APIs more usable, for sure - though 
>>> I'm thinking it's likely a concept (like containers in the C++ standard 
>>> library) might be the better direction.
>>>
>>> Perhaps some code samples showing how one would interact (probably not 
>>> whole algorithms - maybe something simple like generating a dot diagram for 
>>> a graph) with these things given different APIs (traits, concepts, and 
>>> runtime polymorphism) - and implementations of each kind too.
>>
>> Take a look here for example: 
>> https://github.com/nhaehnle/llvm-project/blob/715450fa7f968ceefaf9c3b04b47066866c97206/llvm/lib/Analysis/GenericConvergenceUtils.cpp#L499
>>  -- this is obviously still fairly simple, but it's an example of printing 
>> out the results of an analysis in a way that's generic over the underlying 
>> CFG and SSA form.
>
> I'm having trouble following this example - I'm not sure what the CfgPrinter 
> abstraction is/why it's first-class, and why this "print" function is calling 
> what look like mutation operations like "appendBlocks". I guess perhaps the 
> question is - what's it printing from and what's it printing to?
>
> Ah, I see, the "append" functions are accessors, of a sort. Returning a 
> container might be more clear than using an out parameter - alternatively, a 
> functor parameter (ala std::for_each) that is called for each element, that 
> can then be used to populate an existing container if desired, or to do 
> immediate processing without the need for an intermediate container.

The code is trying to strike a balance here in terms of performance. Since 
dynamic polymorphism is used, a functor-based traversal can't be inlined and so 
the number of indirect function calls increases quite a bit. There are a number 
of use cases where you really do want to just append successors or predecessors 
to a vectors, like during a graph traversal. An example graph traversal is 
here: 
https://github.com/nhaehnle/llvm-project/blob/controlflow-wip-v7/llvm/lib/Analysis/GenericConvergenceUtils.cpp#L329

> Though the printer abstraction still strikes me as a bit strange - especially 
> since it doesn't seem to be printing itself. This function was passed a 
> printer and a stream - the printer prints to the stream (perhaps it'd make 
> more sense for the printer to take the stream on construction) and the 
> function isn't passed the thing to print at all - that thing is accessed from 
> the printer. That seems fairly awkward to me - I'd expect a printing 
> operation to take a thing to be printed and a thing to print to.

Keep in mind that where those `printBlockName` / `printValue` methods are used, 
they will always be mixed in with printing of other things. So the alternative 
you're describing would result in code like:

  dbgs() << " currently at: ";  // explicit stream
  printer.printBlockName(block); // implicit stream
  dbgs() << '\n'; // explicit stream

I would argue that that ends up being more awkward because it mixes implicit 
streams with explicitly given streams.

We could perhaps add versions of the methods that return a `Printable` object, 
so that we can write:

  dbgs() << "currently at: " << printer.printableBlockName(block) << '\n';

By the way, the main motivation for making the CfgPrinter a separate object is 
that printing LLVM IR efficiently requires keeping a ModuleSlotTracker around. 
Splitting the CfgPrinter off from the CfgInterface allows the fast-path of code 
that doesn't want debug prints to not have to carry a reference to a 
ModuleSlotTracker around, even if it's just an empty std::unique_ptr. That's a 
comparatively small burden though, so I'd be fine with merging the CfgPrinter 
back into the CfgInterface.

> Perhaps setting aside the complexities of printing things - could you provide 
> an example of code, given a CfgGraph, that walks the graph - perhaps just 
> numbering the nodes/edges/etc to produce a dot graph? Showing what the code 
> would look like if it were passed a GraphTraits-implementing graph, a static 
> polymorphic CfgGraph, and a dynamically polymorphic GfgGraph - and also 
> showing what would be fandemantally possible with the CfgGraph that wouldn't 
> be possible with GraphTraits, if any such things exist (it's still unclear to 
> 

[PATCH] D86154: AMDGPU: Add llvm.amdgcn.{read,readfirst,write}lane2 intrinsics with type overloads

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

In D86154#2224272 , @arsenm wrote:

> In D86154#2224270 , @nhaehnle wrote:
>
>> Note that part of my motivation here over D84639 
>>  is to support more general types on the 
>> lane intrinsics, since they also express some semantic content which would 
>> be interesting to be able to express e.g. on descriptors. I wasn't able to 
>> bend the SelectionDAG type legalization to my will, so that's why I instead 
>> "legalize" the intrinsics in the AMDGPUCodeGenPrepare pass.
>
> Don't you just need to handle this in ReplaceNodeResults the same way?

ReplaceNodeResults expects the result type to be changed in semi-magical ways 
during vector type legalization, which is non-obvious since the method can be 
called from different places. I think it *could* be made to work with a lot of 
patience, but it's really a bad interface -- and besides, by doing it in IR we 
reduce code duplication between SelectionDAG and GlobalISel, which is an added 
benefit IMO.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86154

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

[PATCH] D86154: AMDGPU: Add llvm.amdgcn.{read,readfirst,write}lane2 intrinsics with type overloads

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle updated this revision to Diff 286898.
nhaehnle added a comment.

Don't duplicate the intrinsics. Rely on D86317 
 to reduce the pain of this
change caused to downstream users.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86154

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGenOpenCL/builtins-amdgcn.cl
  llvm/include/llvm/IR/IntrinsicsAMDGPU.td
  llvm/lib/Target/AMDGPU/AMDGPUAtomicOptimizer.cpp
  llvm/lib/Target/AMDGPU/AMDGPUCodeGenPrepare.cpp
  llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp
  llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
  llvm/lib/Target/AMDGPU/SIInstructions.td
  llvm/test/CodeGen/AMDGPU/GlobalISel/inst-select-amdgcn.readfirstlane.mir
  llvm/test/CodeGen/AMDGPU/llvm.amdgcn.readfirstlane.ll
  llvm/test/CodeGen/AMDGPU/llvm.amdgcn.readlane.ll
  llvm/test/CodeGen/AMDGPU/llvm.amdgcn.writelane.ll
  llvm/test/Transforms/InstCombine/AMDGPU/amdgcn-intrinsics.ll

Index: llvm/test/Transforms/InstCombine/AMDGPU/amdgcn-intrinsics.ll
===
--- llvm/test/Transforms/InstCombine/AMDGPU/amdgcn-intrinsics.ll
+++ llvm/test/Transforms/InstCombine/AMDGPU/amdgcn-intrinsics.ll
@@ -2507,8 +2507,8 @@
 
 define amdgpu_kernel void @readfirstlane_constant(i32 %arg) {
 ; CHECK-LABEL: @readfirstlane_constant(
-; CHECK-NEXT:[[VAR:%.*]] = call i32 @llvm.amdgcn.readfirstlane(i32 [[ARG:%.*]])
-; CHECK-NEXT:store volatile i32 [[VAR]], i32* undef, align 4
+; CHECK-NEXT:[[TMP1:%.*]] = call i32 @llvm.amdgcn.readfirstlane.i32(i32 [[ARG:%.*]])
+; CHECK-NEXT:store volatile i32 [[TMP1]], i32* undef, align 4
 ; CHECK-NEXT:store volatile i32 0, i32* undef, align 4
 ; CHECK-NEXT:store volatile i32 123, i32* undef, align 4
 ; CHECK-NEXT:store volatile i32 ptrtoint (i32* @gv to i32), i32* undef, align 4
@@ -2530,8 +2530,8 @@
 
 define i32 @readfirstlane_idempotent(i32 %arg) {
 ; CHECK-LABEL: @readfirstlane_idempotent(
-; CHECK-NEXT:[[READ0:%.*]] = call i32 @llvm.amdgcn.readfirstlane(i32 [[ARG:%.*]])
-; CHECK-NEXT:ret i32 [[READ0]]
+; CHECK-NEXT:[[TMP1:%.*]] = call i32 @llvm.amdgcn.readfirstlane.i32(i32 [[ARG:%.*]])
+; CHECK-NEXT:ret i32 [[TMP1]]
 ;
   %read0 = call i32 @llvm.amdgcn.readfirstlane(i32 %arg)
   %read1 = call i32 @llvm.amdgcn.readfirstlane(i32 %read0)
@@ -2541,8 +2541,8 @@
 
 define i32 @readfirstlane_readlane(i32 %arg) {
 ; CHECK-LABEL: @readfirstlane_readlane(
-; CHECK-NEXT:[[READ0:%.*]] = call i32 @llvm.amdgcn.readfirstlane(i32 [[ARG:%.*]])
-; CHECK-NEXT:ret i32 [[READ0]]
+; CHECK-NEXT:[[TMP1:%.*]] = call i32 @llvm.amdgcn.readfirstlane.i32(i32 [[ARG:%.*]])
+; CHECK-NEXT:ret i32 [[TMP1]]
 ;
   %read0 = call i32 @llvm.amdgcn.readfirstlane(i32 %arg)
   %read1 = call i32 @llvm.amdgcn.readlane(i32 %read0, i32 0)
@@ -2552,11 +2552,11 @@
 define i32 @readfirstlane_readfirstlane_different_block(i32 %arg) {
 ; CHECK-LABEL: @readfirstlane_readfirstlane_different_block(
 ; CHECK-NEXT:  bb0:
-; CHECK-NEXT:[[READ0:%.*]] = call i32 @llvm.amdgcn.readfirstlane(i32 [[ARG:%.*]])
+; CHECK-NEXT:[[TMP0:%.*]] = call i32 @llvm.amdgcn.readfirstlane.i32(i32 [[ARG:%.*]])
 ; CHECK-NEXT:br label [[BB1:%.*]]
 ; CHECK:   bb1:
-; CHECK-NEXT:[[READ1:%.*]] = call i32 @llvm.amdgcn.readfirstlane(i32 [[READ0]])
-; CHECK-NEXT:ret i32 [[READ1]]
+; CHECK-NEXT:[[TMP1:%.*]] = call i32 @llvm.amdgcn.readfirstlane.i32(i32 [[TMP0]])
+; CHECK-NEXT:ret i32 [[TMP1]]
 ;
 bb0:
   %read0 = call i32 @llvm.amdgcn.readfirstlane(i32 %arg)
@@ -2570,11 +2570,11 @@
 define i32 @readfirstlane_readlane_different_block(i32 %arg) {
 ; CHECK-LABEL: @readfirstlane_readlane_different_block(
 ; CHECK-NEXT:  bb0:
-; CHECK-NEXT:[[READ0:%.*]] = call i32 @llvm.amdgcn.readlane(i32 [[ARG:%.*]], i32 0)
+; CHECK-NEXT:[[TMP0:%.*]] = call i32 @llvm.amdgcn.readlane.i32(i32 [[ARG:%.*]], i32 0)
 ; CHECK-NEXT:br label [[BB1:%.*]]
 ; CHECK:   bb1:
-; CHECK-NEXT:[[READ1:%.*]] = call i32 @llvm.amdgcn.readfirstlane(i32 [[READ0]])
-; CHECK-NEXT:ret i32 [[READ1]]
+; CHECK-NEXT:[[TMP1:%.*]] = call i32 @llvm.amdgcn.readfirstlane.i32(i32 [[TMP0]])
+; CHECK-NEXT:ret i32 [[TMP1]]
 ;
 bb0:
   %read0 = call i32 @llvm.amdgcn.readlane(i32 %arg, i32 0)
@@ -2585,6 +2585,41 @@
   ret i32 %read1
 }
 
+define i32 @readfirstlane_bitcast(float %arg) {
+; CHECK-LABEL: @readfirstlane_bitcast(
+; CHECK-NEXT:[[TMP1:%.*]] = call float @llvm.amdgcn.readfirstlane.f32(float [[ARG:%.*]])
+; CHECK-NEXT:[[TMP2:%.*]] = bitcast float [[TMP1]] to i32
+; CHECK-NEXT:ret i32 [[TMP2]]
+;
+  %bitcast.arg = bitcast float %arg to i32
+  %read = call i32 @llvm.amdgcn.readfirstlane(i32 %bitcast.arg)
+  ret i32 %read
+}
+
+define float @bitcast_readfirstlane_bitcast(float %arg) {
+; CHECK-LABEL: @bitcast_readfirstlane_bitcast(
+; CHECK-NEXT:[[TMP1:%.*]] = call float 

[PATCH] D83088: Introduce CfgTraits abstraction

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

> Based on your description and the DomTree patches, if I understand correctly, 
> the primary motivation is to facilitate writing CFG-representation-agnostic 
> algorithms/analyses (e.g., dominators, divergence, convergence analyses), 
> such that you can later lift the results back to the representation-aware 
> types? If that's correct, I support the overall goal. Having spent probably 
> ~weeks wrangling with domtree templates, this sounds like something that 
> could simplify life a lot and potentially cut down on compilation times & 
> sizes of llvm binaries.

Yes, that is the motivation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088

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

[PATCH] D83088: Introduce CfgTraits abstraction

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

In D83088#2225415 , @dblaikie wrote:

>>> But I guess coming back to the original/broader design: What problems is 
>>> this intended to solve? The inability to write non-template algorithms over 
>>> graphs? What cost does that come with? Are there algorithms that are a bit 
>>> too complicated/unwieldy when done as templates? 
>>> If it's specifically the static/dynamic dispatch issue - I'm not sure the 
>>> type erasure and runtime overhead may be worth the tradeoff here, though if 
>>> it is - it'd be good to keep the non-dynamic version common, rather than 
>>> now having GraphTraits and CfgTraits done a bit differently, etc.
>>
>> It's not just over graphs, but taking SSA values into account as well -- 
>> that is the key distinction between GraphTraits and CfgTraits.
>
> Not sure I follow - could you give an example of a graph where the 
> GraphTraits concept of the Graph and the CfgTraits concept of the graph (or, 
> perhaps more importantly - features of the graph/API surface area/properties 
> you can expose through the CFG API/concept/thing but not through GraphTraits?

See below.

>> The most immediate problem is divergence analysis, which is extremely 
>> complex and difficult to get right. If I had tried to fight the accidental 
>> complexity that comes with attempting to write such an algorithm as C++ 
>> templates in addition to the inherent complexity of the algorithm at the 
>> same time, I'm not sure I would have been able to produce anything workable 
>> at all.
>>
>> Frankly, I suspect that our dominator tree implementation also suffer 
>> because of this, though at least dominator trees are much more well studied 
>> in the academic literature, so that helps keep the inherent complexity under 
>> control.
>
> I'm totally open to discussing making APIs more usable, for sure - though I'm 
> thinking it's likely a concept (like containers in the C++ standard library) 
> might be the better direction.
>
> Perhaps some code samples showing how one would interact (probably not whole 
> algorithms - maybe something simple like generating a dot diagram for a 
> graph) with these things given different APIs (traits, concepts, and runtime 
> polymorphism) - and implementations of each kind too.

Take a look here for example: 
https://github.com/nhaehnle/llvm-project/blob/715450fa7f968ceefaf9c3b04b47066866c97206/llvm/lib/Analysis/GenericConvergenceUtils.cpp#L499
 -- this is obviously still fairly simple, but it's an example of printing out 
the results of an analysis in a way that's generic over the underlying CFG and 
SSA form. A statically polymorphic wrapper is here: 
https://github.com/nhaehnle/llvm-project/blob/715450fa7f968ceefaf9c3b04b47066866c97206/llvm/include/llvm/Analysis/GenericConvergenceUtils.h#L569

The simple example might be bearable writing as a template, precisely because 
it's simple -- so only looking at simple examples is unlikely to really capture 
the motivation. Really what the motivation boils down to is stuff like this: 
https://github.com/nhaehnle/llvm-project/blob/controlflow-wip-v7/llvm/lib/Analysis/GenericUniformAnalysis.cpp
 -- I don't fancy writing all this as a template.

Thid motivation would essentially go away if C++ could type-check against 
traits in the way that Rust and other languages like it can -- but it can't, so 
here we are.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088

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

[PATCH] D83088: Introduce CfgTraits abstraction

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

> Not sure that's the best place to be designing this fairly integral and 
> complicated piece of infrastructure from, but hoping we can find some good 
> places/solutions/etc.

I sent an email to llvm-dev several weeks ago, but things seem to have moved 
here. Either way is fine with me.

> But I guess coming back to the original/broader design: What problems is this 
> intended to solve? The inability to write non-template algorithms over 
> graphs? What cost does that come with? Are there algorithms that are a bit 
> too complicated/unwieldy when done as templates? 
> If it's specifically the static/dynamic dispatch issue - I'm not sure the 
> type erasure and runtime overhead may be worth the tradeoff here, though if 
> it is - it'd be good to keep the non-dynamic version common, rather than now 
> having GraphTraits and CfgTraits done a bit differently, etc.

It's not just over graphs, but taking SSA values into account as well -- that 
is the key distinction between GraphTraits and CfgTraits. The most immediate 
problem is divergence analysis, which is extremely complex and difficult to get 
right. If I had tried to fight the accidental complexity that comes with 
attempting to write such an algorithm as C++ templates in addition to the 
inherent complexity of the algorithm at the same time, I'm not sure I would 
have been able to produce anything workable at all.

Frankly, I suspect that our dominator tree implementation also suffer because 
of this, though at least dominator trees are much more well studied in the 
academic literature, so that helps keep the inherent complexity under control.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088

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

[PATCH] D86154: AMDGPU: Add llvm.amdgcn.{read,readfirst,write}lane2 intrinsics with type overloads

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

Note that part of my motivation here over D84639 
 is to support more general types on the lane 
intrinsics, since they also express some semantic content which would be 
interesting to be able to express e.g. on descriptors. I wasn't able to bend 
the SelectionDAG type legalization to my will, so that's why I instead 
"legalize" the intrinsics in the AMDGPUCodeGenPrepare pass.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86154

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

[PATCH] D86154: AMDGPU: Add llvm.amdgcn.{read,readfirst,write}lane2 intrinsics with type overloads

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle created this revision.
nhaehnle added a reviewer: arsenm.
Herald added subscribers: cfe-commits, kerbowa, jfb, hiraditya, t-tye, tpr, 
dstuttard, yaxunl, jvesely, kzhuravl.
Herald added projects: clang, LLVM.
nhaehnle requested review of this revision.
Herald added a subscriber: wdng.

These intrinsics should work at least with standard integer and floating
point sizes, pointers, and vectors of those.

This fixes selection for non-s32 types when readfirstlane is inserted
for SGPR return values.

Moving the atomic optimizer pass in the pass pipeline so that it can be
simplified and rely on the more general support of lane intrinsics.

API users should move to these new intrinsics so that we can remove the
old versions.

Change-Id: I1c5e7e7858890e1c30d3b46c8551e74ab7027552
Based-on: https://reviews.llvm.org/D84639


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86154

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGenOpenCL/builtins-amdgcn.cl
  llvm/include/llvm/IR/IntrinsicsAMDGPU.td
  llvm/lib/Target/AMDGPU/AMDGPUAtomicOptimizer.cpp
  llvm/lib/Target/AMDGPU/AMDGPUCallLowering.cpp
  llvm/lib/Target/AMDGPU/AMDGPUCodeGenPrepare.cpp
  llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp
  llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp
  llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp
  llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp
  llvm/lib/Target/AMDGPU/AMDGPUSearchableTables.td
  llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
  llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.cpp
  llvm/lib/Target/AMDGPU/SIInstructions.td
  llvm/lib/Target/AMDGPU/VOP1Instructions.td
  llvm/lib/Target/AMDGPU/VOP2Instructions.td
  llvm/test/CodeGen/AMDGPU/GlobalISel/inst-select-amdgcn.readfirstlane.mir
  llvm/test/CodeGen/AMDGPU/GlobalISel/irtranslator-amdgpu_ps.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/irtranslator-amdgpu_vs.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-amdgcn.readfirstlane.mir
  llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-amdgcn.readlane.mir
  llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-amdgcn.s.buffer.load.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-amdgcn.writelane.mir
  llvm/test/CodeGen/AMDGPU/llvm.amdgcn.readfirstlane.ll
  llvm/test/CodeGen/AMDGPU/llvm.amdgcn.readlane.ll
  llvm/test/CodeGen/AMDGPU/llvm.amdgcn.writelane.ll
  llvm/test/Transforms/InstCombine/AMDGPU/amdgcn-intrinsics.ll

Index: llvm/test/Transforms/InstCombine/AMDGPU/amdgcn-intrinsics.ll
===
--- llvm/test/Transforms/InstCombine/AMDGPU/amdgcn-intrinsics.ll
+++ llvm/test/Transforms/InstCombine/AMDGPU/amdgcn-intrinsics.ll
@@ -2507,8 +2507,8 @@
 
 define amdgpu_kernel void @readfirstlane_constant(i32 %arg) {
 ; CHECK-LABEL: @readfirstlane_constant(
-; CHECK-NEXT:[[VAR:%.*]] = call i32 @llvm.amdgcn.readfirstlane(i32 [[ARG:%.*]])
-; CHECK-NEXT:store volatile i32 [[VAR]], i32* undef, align 4
+; CHECK-NEXT:[[TMP1:%.*]] = call i32 @llvm.amdgcn.readfirstlane2.i32(i32 [[ARG:%.*]])
+; CHECK-NEXT:store volatile i32 [[TMP1]], i32* undef, align 4
 ; CHECK-NEXT:store volatile i32 0, i32* undef, align 4
 ; CHECK-NEXT:store volatile i32 123, i32* undef, align 4
 ; CHECK-NEXT:store volatile i32 ptrtoint (i32* @gv to i32), i32* undef, align 4
@@ -2530,8 +2530,8 @@
 
 define i32 @readfirstlane_idempotent(i32 %arg) {
 ; CHECK-LABEL: @readfirstlane_idempotent(
-; CHECK-NEXT:[[READ0:%.*]] = call i32 @llvm.amdgcn.readfirstlane(i32 [[ARG:%.*]])
-; CHECK-NEXT:ret i32 [[READ0]]
+; CHECK-NEXT:[[TMP1:%.*]] = call i32 @llvm.amdgcn.readfirstlane2.i32(i32 [[ARG:%.*]])
+; CHECK-NEXT:ret i32 [[TMP1]]
 ;
   %read0 = call i32 @llvm.amdgcn.readfirstlane(i32 %arg)
   %read1 = call i32 @llvm.amdgcn.readfirstlane(i32 %read0)
@@ -2541,8 +2541,8 @@
 
 define i32 @readfirstlane_readlane(i32 %arg) {
 ; CHECK-LABEL: @readfirstlane_readlane(
-; CHECK-NEXT:[[READ0:%.*]] = call i32 @llvm.amdgcn.readfirstlane(i32 [[ARG:%.*]])
-; CHECK-NEXT:ret i32 [[READ0]]
+; CHECK-NEXT:[[TMP1:%.*]] = call i32 @llvm.amdgcn.readfirstlane2.i32(i32 [[ARG:%.*]])
+; CHECK-NEXT:ret i32 [[TMP1]]
 ;
   %read0 = call i32 @llvm.amdgcn.readfirstlane(i32 %arg)
   %read1 = call i32 @llvm.amdgcn.readlane(i32 %read0, i32 0)
@@ -2552,11 +2552,11 @@
 define i32 @readfirstlane_readfirstlane_different_block(i32 %arg) {
 ; CHECK-LABEL: @readfirstlane_readfirstlane_different_block(
 ; CHECK-NEXT:  bb0:
-; CHECK-NEXT:[[READ0:%.*]] = call i32 @llvm.amdgcn.readfirstlane(i32 [[ARG:%.*]])
+; CHECK-NEXT:[[TMP0:%.*]] = call i32 @llvm.amdgcn.readfirstlane2.i32(i32 [[ARG:%.*]])
 ; CHECK-NEXT:br label [[BB1:%.*]]
 ; CHECK:   bb1:
-; CHECK-NEXT:[[READ1:%.*]] = call i32 @llvm.amdgcn.readfirstlane(i32 [[READ0]])
-; CHECK-NEXT:ret i32 [[READ1]]
+; CHECK-NEXT:[[TMP1:%.*]] = call i32 @llvm.amdgcn.readfirstlane2.i32(i32 [[TMP0]])
+; CHECK-NEXT:ret i32 [[TMP1]]
 ;
 

[PATCH] D85607: CfgTraits: add CfgInstructionRef

[Nicolai Hähnle via Phabricator via cfe-commits]

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



Comment at: llvm/include/llvm/CodeGen/MachineCfgTraits.h:100
+// provide one at all. We don't want to lay a subtle performance trap here.
+abort();
+  }

arsenm wrote:
> llvm_unreachable?
Makes sense, done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85607

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

[PATCH] D85607: CfgTraits: add CfgInstructionRef

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle updated this revision to Diff 285711.
nhaehnle added a comment.

- use llvm_unreachable


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85607

Files:
  clang/include/clang/Analysis/Analyses/Dominators.h
  llvm/include/llvm/CodeGen/MachineCfgTraits.h
  llvm/include/llvm/IR/CFG.h
  llvm/include/llvm/Support/CfgTraits.h
  llvm/lib/CodeGen/MachineCfgTraits.cpp
  llvm/lib/IR/CFG.cpp
  llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
  mlir/include/mlir/IR/Dominance.h

Index: mlir/include/mlir/IR/Dominance.h
===
--- mlir/include/mlir/IR/Dominance.h
+++ mlir/include/mlir/IR/Dominance.h
@@ -20,6 +20,7 @@
 public:
   using ParentType = Region;
   using BlockRef = Block *;
+  using InstructionRef = void;
   using ValueRef = void;
 
   static llvm::CfgBlockRef wrapRef(BlockRef block) {
Index: llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
===
--- llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
+++ llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
@@ -27,6 +27,7 @@
 public:
   using ParentType = VPRegionBlock;
   using BlockRef = VPBlockBase *;
+  using InstructionRef = void;
   using ValueRef = void;
 
   static CfgBlockRef wrapRef(BlockRef block) {
Index: llvm/lib/IR/CFG.cpp
===
--- llvm/lib/IR/CFG.cpp
+++ llvm/lib/IR/CFG.cpp
@@ -36,6 +36,11 @@
   }
 }
 
+void IrCfgTraits::Printer::printInstruction(raw_ostream ,
+InstructionRef instruction) const {
+  printValue(out, instruction);
+}
+
 void IrCfgTraits::Printer::printBlockName(raw_ostream ,
   BlockRef block) const {
   if (block->hasName()) {
Index: llvm/lib/CodeGen/MachineCfgTraits.cpp
===
--- llvm/lib/CodeGen/MachineCfgTraits.cpp
+++ llvm/lib/CodeGen/MachineCfgTraits.cpp
@@ -24,6 +24,11 @@
   }
 }
 
+void MachineCfgTraits::Printer::printInstruction(
+raw_ostream , MachineInstr *instruction) const {
+  instruction->print(out);
+}
+
 void MachineCfgTraits::Printer::printBlockName(raw_ostream ,
MachineBasicBlock *block) const {
   block->printName(out);
Index: llvm/include/llvm/Support/CfgTraits.h
===
--- llvm/include/llvm/Support/CfgTraits.h
+++ llvm/include/llvm/Support/CfgTraits.h
@@ -79,6 +79,9 @@
 class CfgBlockRefTag;
 using CfgBlockRef = CfgOpaqueType;
 
+class CfgInstructionRefTag;
+using CfgInstructionRef = CfgOpaqueType;
+
 class CfgValueRefTag;
 using CfgValueRef = CfgOpaqueType;
 
@@ -113,8 +116,10 @@
   //
   // - Static methods for converting BlockRef and ValueRef to and from
   //   static CfgBlockRef wrapRef(BlockRef);
+  //   static CfgInstructionRef wrapRef(InstructionRef);
   //   static CfgValueRef wrapRef(ValueRef);
   //   static BlockRef unwrapRef(CfgBlockRef);
+  //   static InstructionRef unwrapRef(CfgInstructionRef);
   //   static ValueRef unwrapRef(CfgValueRef);
 };
 
@@ -133,6 +138,7 @@
 class CfgTraits : public BaseTraits {
 public:
   using typename BaseTraits::BlockRef;
+  using typename BaseTraits::InstructionRef;
   using typename BaseTraits::ParentType;
   using typename BaseTraits::ValueRef;
 
@@ -159,6 +165,10 @@
   // a range of iterators dereferencing to ValueRef):
   //   static auto blockdefs(BlockRef block);
 
+  // Given two instructions in the same block, this returns true if lhs is
+  // strictly before rhs.
+  //   static bool comesBefore(InstructionRef lhs, InstructionRef rhs);
+
   // Get the block in which a given value is defined. Returns a null-like
   // BlockRef if the value is not defined in a block (e.g. it is a constant or
   // function argument).
@@ -168,6 +178,8 @@
   //   explicit Printer(const CfgTraits );
   //   void printBlockName(raw_ostream , BlockRef block) const;
   //   void printValue(raw_ostream , ValueRef value) const;
+  //   void printInstruction(raw_ostream ,
+  // InstructionRef instruction) const;
   // };
 
   ///@}
@@ -295,6 +307,9 @@
   virtual void appendBlocks(CfgParentRef parent,
 SmallVectorImpl ) const = 0;
 
+  virtual bool comesBefore(CfgInstructionRef lhs,
+   CfgInstructionRef rhs) const = 0;
+
   virtual void appendPredecessors(CfgBlockRef block,
   SmallVectorImpl ) const = 0;
   virtual void appendSuccessors(CfgBlockRef block,
@@ -331,6 +346,8 @@
 
   virtual void printBlockName(raw_ostream , CfgBlockRef block) const = 0;
   virtual void printValue(raw_ostream , CfgValueRef value) const = 0;
+  virtual void printInstruction(raw_ostream ,
+CfgInstructionRef instruction) const = 0;
 
   

[PATCH] D83088: Introduce CfgTraits abstraction

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

In D83088#2213886 , @dblaikie wrote:

> In D83088#2213864 , @nhaehnle wrote:
>
>> In D83088#2213802 , @dblaikie wrote:
>>
>>> In D83088#2213797 , @nhaehnle 
>>> wrote:
>>>
 In D83088#2208611 , @dblaikie 
 wrote:

> This seems like a strange hybrid between a static-polymorphism (with 
> traits) and dynamic polymorphism (with base classes/virtual functions). 
> Could this more readily be just one or the other? (sounds like you're 
> leaning towards dynamic polymorphism)

 No, it's very much this way on purpose. The idea is to support the same 
 set of functionality as much as possible in both static **and** dynamic 
 polymorphism.
>>>
>>> Could it be implemented statically as a primary interface, with a dynamic 
>>> wrapper? (eg: a base class, then a derived class template that takes the 
>>> static CFG type to wrap into the dynamic type) keeping the two concepts 
>>> more clearly separated?
>>
>> That is how it is implemented. CfgTraits is the primary static interface, 
>> and then CfgInterface / CfgInterfaceImpl is the dynamic wrapper.
>
> Ah, fair enough. The inheritance details in the traits class confused me a 
> bit/I had a hard time following, with all the features being in the one 
> patch. Might be easier separately, but not sure.
>
> Would it be possible for this not to use traits - I know @asbirlea and I had 
> trouble with some things using GraphTraits owing to the traits API. An 
> alternative would be to describe a CFGGraph concept (same as a standard 
> container concept, for instance) - where there is a concrete graph object and 
> that object is queried for things like nodes, edges, etc. (actually one of 
> the significant things we tripped over was the API choice to navigate edges 
> from a node itself without any extra state - which meant nodes/edge iteration 
> had to carry state (potentially pointers back to the graph, etc) to be able 
> to manifest their edges - trait or concept could both address this by, for 
> traits, passing the graph as well as the node when querying the trait for 
> edges, or for a concept passing the node back to the graph to query for 
> edges).

So there is a bit of a part here where I may admittedly be a bit confused with 
the C++ lingo, since I don't actually like template programming that much :)  
(Which is part of the motivation for this to begin with... so that I can do the 
later changes in the stack here without *everything* being in templates.)

The way the `CfgTraits` is used is that you never use the `CfgTraits` class 
directly except to inherit from it using CRTP (curiously recurring template 
pattern). When writing algorithms that want to be generic over the type of CFG, 
those algorithms then have a derived class of CfgTraits as a template 
parameter. For example, D83094  adds a 
`GenericCycleInfo` template class, where the template parameter 
should be set to e.g. `IrCfgTraits`, if you want cycle info on LLVM IR, or to 
`MachineCfgTraits`, if you want cycle info on MachineIR. Both of these classes 
are derived from `CfgTraits`.

It is definitely different from how `GraphTraits` works, which you use it as 
`GraphTraits`, and then `GraphTraits` etc. are 
specialized implementations. If `GraphTraits` worked the way that `CfgTraits` 
works, then we'd instead have classes like `BasicBlockGraphTraits`.

So to sum it up, all this sounds a bit to me like maybe calling `CfgTraits` 
"traits" is wrong? Is that what you're saying here? You can't just call it 
`Cfg` though, because it's *not* a CFG -- it's a kind of interface to a CFG 
which is designed for static polymorphism, unlike `CfgInterface` which is 
designed for dynamic polymorphism. Getting the names right is important, 
unfortunately I admit that I'm a bit lost there. "Traits" seemed like the 
closest thing to what I want, but I'm definitely open to suggestions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088

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

[PATCH] D83088: Introduce CfgTraits abstraction

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

In D83088#2213802 , @dblaikie wrote:

> In D83088#2213797 , @nhaehnle wrote:
>
>> In D83088#2208611 , @dblaikie wrote:
>>
>>> This seems like a strange hybrid between a static-polymorphism (with 
>>> traits) and dynamic polymorphism (with base classes/virtual functions). 
>>> Could this more readily be just one or the other? (sounds like you're 
>>> leaning towards dynamic polymorphism)
>>
>> No, it's very much this way on purpose. The idea is to support the same set 
>> of functionality as much as possible in both static **and** dynamic 
>> polymorphism.
>
> Could it be implemented statically as a primary interface, with a dynamic 
> wrapper? (eg: a base class, then a derived class template that takes the 
> static CFG type to wrap into the dynamic type) keeping the two concepts more 
> clearly separated?

That is how it is implemented. CfgTraits is the primary static interface, and 
then CfgInterface / CfgInterfaceImpl is the dynamic wrapper.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088

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

[PATCH] D83088: Introduce CfgTraits abstraction

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

In D83088#2208611 , @dblaikie wrote:

> This seems like a strange hybrid between a static-polymorphism (with traits) 
> and dynamic polymorphism (with base classes/virtual functions). Could this 
> more readily be just one or the other? (sounds like you're leaning towards 
> dynamic polymorphism)

No, it's very much this way on purpose. The idea is to support the same set of 
functionality as much as possible in both static **and** dynamic polymorphism.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088

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

[PATCH] D85607: CfgTraits: add CfgInstructionRef

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle created this revision.
nhaehnle added reviewers: arsenm, foad, sameerds.
Herald added subscribers: cfe-commits, msifontes, jurahul, Kayjukh, grosul1, 
Joonsoo, liufengdb, aartbik, lucyrfox, mgester, arpith-jacob, antiagainst, 
shauheen, jpienaar, rriddle, mehdi_amini, rogfer01, kuhar, hiraditya.
Herald added a reviewer: rriddle.
Herald added a reviewer: aartbik.
Herald added projects: clang, MLIR, LLVM.
nhaehnle requested review of this revision.
Herald added subscribers: vkmr, stephenneuendorffer, nicolasvasilache, wdng.

In CFGs where a single instruction can define multiple values, the
distinction is significant.

Change-Id: Ic15e8e34428ea71023cd644739b1114cebab9594


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85607

Files:
  clang/include/clang/Analysis/Analyses/Dominators.h
  llvm/include/llvm/CodeGen/MachineCfgTraits.h
  llvm/include/llvm/IR/CFG.h
  llvm/include/llvm/Support/CfgTraits.h
  llvm/lib/CodeGen/MachineCfgTraits.cpp
  llvm/lib/IR/CFG.cpp
  llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
  mlir/include/mlir/IR/Dominance.h

Index: mlir/include/mlir/IR/Dominance.h
===
--- mlir/include/mlir/IR/Dominance.h
+++ mlir/include/mlir/IR/Dominance.h
@@ -20,6 +20,7 @@
 public:
   using ParentType = Region;
   using BlockRef = Block *;
+  using InstructionRef = void;
   using ValueRef = void;
 
   static llvm::CfgBlockRef wrapRef(BlockRef block) {
Index: llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
===
--- llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
+++ llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
@@ -27,6 +27,7 @@
 public:
   using ParentType = VPRegionBlock;
   using BlockRef = VPBlockBase *;
+  using InstructionRef = void;
   using ValueRef = void;
 
   static CfgBlockRef wrapRef(BlockRef block) {
Index: llvm/lib/IR/CFG.cpp
===
--- llvm/lib/IR/CFG.cpp
+++ llvm/lib/IR/CFG.cpp
@@ -36,6 +36,11 @@
   }
 }
 
+void IrCfgTraits::Printer::printInstruction(raw_ostream ,
+InstructionRef instruction) const {
+  printValue(out, instruction);
+}
+
 void IrCfgTraits::Printer::printBlockName(raw_ostream ,
   BlockRef block) const {
   if (block->hasName()) {
Index: llvm/lib/CodeGen/MachineCfgTraits.cpp
===
--- llvm/lib/CodeGen/MachineCfgTraits.cpp
+++ llvm/lib/CodeGen/MachineCfgTraits.cpp
@@ -24,6 +24,11 @@
   }
 }
 
+void MachineCfgTraits::Printer::printInstruction(
+raw_ostream , MachineInstr *instruction) const {
+  instruction->print(out);
+}
+
 void MachineCfgTraits::Printer::printBlockName(raw_ostream ,
MachineBasicBlock *block) const {
   block->printName(out);
Index: llvm/include/llvm/Support/CfgTraits.h
===
--- llvm/include/llvm/Support/CfgTraits.h
+++ llvm/include/llvm/Support/CfgTraits.h
@@ -79,6 +79,9 @@
 class CfgBlockRefTag;
 using CfgBlockRef = CfgOpaqueType;
 
+class CfgInstructionRefTag;
+using CfgInstructionRef = CfgOpaqueType;
+
 class CfgValueRefTag;
 using CfgValueRef = CfgOpaqueType;
 
@@ -113,8 +116,10 @@
   //
   // - Static methods for converting BlockRef and ValueRef to and from
   //   static CfgBlockRef wrapRef(BlockRef);
+  //   static CfgInstructionRef wrapRef(InstructionRef);
   //   static CfgValueRef wrapRef(ValueRef);
   //   static BlockRef unwrapRef(CfgBlockRef);
+  //   static InstructionRef unwrapRef(CfgInstructionRef);
   //   static ValueRef unwrapRef(CfgValueRef);
 };
 
@@ -133,6 +138,7 @@
 class CfgTraits : public BaseTraits {
 public:
   using typename BaseTraits::BlockRef;
+  using typename BaseTraits::InstructionRef;
   using typename BaseTraits::ParentType;
   using typename BaseTraits::ValueRef;
 
@@ -159,6 +165,10 @@
   // a range of iterators dereferencing to ValueRef):
   //   static auto blockdefs(BlockRef block);
 
+  // Given two instructions in the same block, this returns true if lhs is
+  // strictly before rhs.
+  //   static bool comesBefore(InstructionRef lhs, InstructionRef rhs);
+
   // Get the block in which a given value is defined. Returns a null-like
   // BlockRef if the value is not defined in a block (e.g. it is a constant or
   // function argument).
@@ -168,6 +178,8 @@
   //   explicit Printer(const CfgTraits );
   //   void printBlockName(raw_ostream , BlockRef block) const;
   //   void printValue(raw_ostream , ValueRef value) const;
+  //   void printInstruction(raw_ostream ,
+  // InstructionRef instruction) const;
   // };
 
   ///@}
@@ -295,6 +307,9 @@
   virtual void appendBlocks(CfgParentRef parent,
 SmallVectorImpl ) const = 0;
 
+  virtual bool comesBefore(CfgInstructionRef lhs,
+   

[PATCH] D83088: Introduce CfgTraits abstraction

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle updated this revision to Diff 284195.
nhaehnle marked 2 inline comments as done.
nhaehnle added a comment.

  v7:
  - std::forward fix in wrapping_iterator
  - fix typos


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088

Files:
  clang/include/clang/Analysis/Analyses/Dominators.h
  llvm/include/llvm/CodeGen/MachineCfgTraits.h
  llvm/include/llvm/IR/CFG.h
  llvm/include/llvm/Support/CfgTraits.h
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/MachineCfgTraits.cpp
  llvm/lib/IR/CFG.cpp
  llvm/lib/IR/CMakeLists.txt
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/CfgTraits.cpp
  llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
  mlir/include/mlir/IR/Dominance.h

Index: mlir/include/mlir/IR/Dominance.h
===
--- mlir/include/mlir/IR/Dominance.h
+++ mlir/include/mlir/IR/Dominance.h
@@ -10,8 +10,46 @@
 #define MLIR_IR_DOMINANCE_H
 
 #include "mlir/IR/RegionGraphTraits.h"
+#include "llvm/Support/CfgTraits.h"
 #include "llvm/Support/GenericDomTree.h"
 
+namespace mlir {
+
+/// Partial CFG traits for MLIR's CFG, without a value type.
+class CfgTraitsBase : public llvm::CfgTraitsBase {
+public:
+  using ParentType = Region;
+  using BlockRef = Block *;
+  using ValueRef = void;
+
+  static llvm::CfgBlockRef wrapRef(BlockRef block) {
+return makeOpaque(block);
+  }
+  static BlockRef unwrapRef(llvm::CfgBlockRef block) {
+return static_cast(getOpaque(block));
+  }
+};
+
+class CfgTraits : public llvm::CfgTraits {
+public:
+  static Region *getBlockParent(Block *block) { return block->getParent(); }
+
+  static auto predecessors(Block *block) {
+return llvm::inverse_children(block);
+  }
+
+  static auto successors(Block *block) {
+return llvm::children(block);
+  }
+};
+
+} // namespace mlir
+
+template <>
+struct llvm::CfgTraitsFor {
+  using CfgTraits = mlir::CfgTraits;
+};
+
 extern template class llvm::DominatorTreeBase;
 extern template class llvm::DominatorTreeBase;
 
Index: llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
===
--- llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
+++ llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
@@ -18,9 +18,42 @@
 #include "VPlan.h"
 #include "llvm/ADT/GraphTraits.h"
 #include "llvm/IR/Dominators.h"
+#include "llvm/Support/CfgTraits.h"
 
 namespace llvm {
 
+/// Partial CFG traits for VPlan's CFG, without a value type.
+class VPCfgTraitsBase : public CfgTraitsBase {
+public:
+  using ParentType = VPRegionBlock;
+  using BlockRef = VPBlockBase *;
+  using ValueRef = void;
+
+  static CfgBlockRef wrapRef(BlockRef block) {
+return makeOpaque(block);
+  }
+  static BlockRef unwrapRef(CfgBlockRef block) {
+return static_cast(getOpaque(block));
+  }
+};
+
+class VPCfgTraits : public CfgTraits {
+public:
+  static VPRegionBlock *getBlockParent(VPBlockBase *block) {
+return block->getParent();
+  }
+
+  static auto predecessors(VPBlockBase *block) {
+return llvm::inverse_children(block);
+  }
+
+  static auto successors(VPBlockBase *block) {
+return llvm::children(block);
+  }
+};
+
+template <> struct CfgTraitsFor { using CfgTraits = VPCfgTraits; };
+
 /// Template specialization of the standard LLVM dominator tree utility for
 /// VPBlockBases.
 using VPDominatorTree = DomTreeBase;
Index: llvm/lib/Support/CfgTraits.cpp
===
--- /dev/null
+++ llvm/lib/Support/CfgTraits.cpp
@@ -0,0 +1,14 @@
+//===- CfgTraits.cpp - Traits for generically working on CFGs ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "llvm/Support/CfgTraits.h"
+
+using namespace llvm;
+
+void CfgInterface::anchor() {}
+void CfgPrinter::anchor() {}
Index: llvm/lib/Support/CMakeLists.txt
===
--- llvm/lib/Support/CMakeLists.txt
+++ llvm/lib/Support/CMakeLists.txt
@@ -71,6 +71,7 @@
   BranchProbability.cpp
   BuryPointer.cpp
   CachePruning.cpp
+  CfgTraits.cpp
   circular_raw_ostream.cpp
   Chrono.cpp
   COM.cpp
Index: llvm/lib/IR/CMakeLists.txt
===
--- llvm/lib/IR/CMakeLists.txt
+++ llvm/lib/IR/CMakeLists.txt
@@ -4,6 +4,7 @@
   Attributes.cpp
   AutoUpgrade.cpp
   BasicBlock.cpp
+  CFG.cpp
   Comdat.cpp
   ConstantFold.cpp
   ConstantRange.cpp
Index: llvm/lib/IR/CFG.cpp
===
--- /dev/null
+++ llvm/lib/IR/CFG.cpp
@@ -0,0 +1,56 @@
+//===- CFG.cpp --*- C++ -*-===//
+//
+// Part of 

[PATCH] D83088: Introduce CfgTraits abstraction

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle updated this revision to Diff 280481.
nhaehnle marked an inline comment as done.
nhaehnle added a comment.

  v6:
  - implement predecessors/successors for all CfgTraits implementations
  - fix error in unwrapRange
  - rename toGeneric/fromGeneric into wrapRef/unwrapRef to have naming
that is consistent with {wrap,unwrap}{Iterator,Range}
  - use getVRegDef instead of getUniqueVRegDef


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088

Files:
  clang/include/clang/Analysis/Analyses/Dominators.h
  llvm/include/llvm/CodeGen/MachineCfgTraits.h
  llvm/include/llvm/IR/CFG.h
  llvm/include/llvm/Support/CfgTraits.h
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/MachineCfgTraits.cpp
  llvm/lib/IR/CFG.cpp
  llvm/lib/IR/CMakeLists.txt
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/CfgTraits.cpp
  llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
  mlir/include/mlir/IR/Dominance.h

Index: mlir/include/mlir/IR/Dominance.h
===
--- mlir/include/mlir/IR/Dominance.h
+++ mlir/include/mlir/IR/Dominance.h
@@ -10,8 +10,46 @@
 #define MLIR_IR_DOMINANCE_H
 
 #include "mlir/IR/RegionGraphTraits.h"
+#include "llvm/Support/CfgTraits.h"
 #include "llvm/Support/GenericDomTree.h"
 
+namespace mlir {
+
+/// Partial CFG traits for MLIR's CFG, without a value type.
+class CfgTraitsBase : public llvm::CfgTraitsBase {
+public:
+  using ParentType = Region;
+  using BlockRef = Block *;
+  using ValueRef = void;
+
+  static llvm::CfgBlockRef wrapRef(BlockRef block) {
+return makeOpaque(block);
+  }
+  static BlockRef unwrapRef(llvm::CfgBlockRef block) {
+return static_cast(getOpaque(block));
+  }
+};
+
+class CfgTraits : public llvm::CfgTraits {
+public:
+  static Region *getBlockParent(Block *block) { return block->getParent(); }
+
+  static auto predecessors(Block *block) {
+return llvm::inverse_children(block);
+  }
+
+  static auto successors(Block *block) {
+return llvm::children(block);
+  }
+};
+
+} // namespace mlir
+
+template <>
+struct llvm::CfgTraitsFor {
+  using CfgTraits = mlir::CfgTraits;
+};
+
 extern template class llvm::DominatorTreeBase;
 extern template class llvm::DominatorTreeBase;
 
Index: llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
===
--- llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
+++ llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
@@ -18,9 +18,42 @@
 #include "VPlan.h"
 #include "llvm/ADT/GraphTraits.h"
 #include "llvm/IR/Dominators.h"
+#include "llvm/Support/CfgTraits.h"
 
 namespace llvm {
 
+/// Partial CFG traits for VPlan's CFG, without a value type.
+class VPCfgTraitsBase : public CfgTraitsBase {
+public:
+  using ParentType = VPRegionBlock;
+  using BlockRef = VPBlockBase *;
+  using ValueRef = void;
+
+  static CfgBlockRef wrapRef(BlockRef block) {
+return makeOpaque(block);
+  }
+  static BlockRef unwrapRef(CfgBlockRef block) {
+return static_cast(getOpaque(block));
+  }
+};
+
+class VPCfgTraits : public CfgTraits {
+public:
+  static VPRegionBlock *getBlockParent(VPBlockBase *block) {
+return block->getParent();
+  }
+
+  static auto predecessors(VPBlockBase *block) {
+return llvm::inverse_children(block);
+  }
+
+  static auto successors(VPBlockBase *block) {
+return llvm::children(block);
+  }
+};
+
+template <> struct CfgTraitsFor { using CfgTraits = VPCfgTraits; };
+
 /// Template specialization of the standard LLVM dominator tree utility for
 /// VPBlockBases.
 using VPDominatorTree = DomTreeBase;
Index: llvm/lib/Support/CfgTraits.cpp
===
--- /dev/null
+++ llvm/lib/Support/CfgTraits.cpp
@@ -0,0 +1,14 @@
+//===- CfgTraits.cpp - Traits for generically working on CFGs ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "llvm/Support/CfgTraits.h"
+
+using namespace llvm;
+
+void CfgInterface::anchor() {}
+void CfgPrinter::anchor() {}
Index: llvm/lib/Support/CMakeLists.txt
===
--- llvm/lib/Support/CMakeLists.txt
+++ llvm/lib/Support/CMakeLists.txt
@@ -71,6 +71,7 @@
   BranchProbability.cpp
   BuryPointer.cpp
   CachePruning.cpp
+  CfgTraits.cpp
   circular_raw_ostream.cpp
   Chrono.cpp
   COM.cpp
Index: llvm/lib/IR/CMakeLists.txt
===
--- llvm/lib/IR/CMakeLists.txt
+++ llvm/lib/IR/CMakeLists.txt
@@ -4,6 +4,7 @@
   Attributes.cpp
   AutoUpgrade.cpp
   BasicBlock.cpp
+  CFG.cpp
   Comdat.cpp
   ConstantFold.cpp
   ConstantRange.cpp
Index: llvm/lib/IR/CFG.cpp

[PATCH] D83088: Introduce CfgTraits abstraction

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle marked 3 inline comments as done.
nhaehnle added inline comments.



Comment at: llvm/include/llvm/CodeGen/MachineCfgTraits.h:44
+// use on a 32-bit architecture.
+assert(wrapped != (uintptr_t)-1 && wrapped != (uintptr_t)-2);
+

arsenm wrote:
> I feel like there should be a better way to do this; we should probably have 
> an assert where virtual registers are created
The reason for doing it here is that this is the place where the reinterpret 
happens. If the check is elsewhere, it's easy to miss by a user of this.



Comment at: llvm/include/llvm/CodeGen/MachineCfgTraits.h:101
+  return nullptr;
+return m_regInfo->getUniqueVRegDef(value)->getParent();
+  }

arsenm wrote:
> I think regular getVRegDef is preferable for SSA MIR
Fixed locally.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088



___
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

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle accepted this revision.
nhaehnle added a comment.
This revision is now accepted and ready to land.

This has had a month of good review that has been addressed, I'd say it's good 
to go.


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] D83088: Introduce CfgTraits abstraction

[Nicolai Hähnle via Phabricator via cfe-commits]

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



Comment at: llvm/include/llvm/CodeGen/MachineCfgTraits.h:136-138
+  // Prefer to avoid support for bundled instructions as long as we
+  // don't really need it.
+  assert(!m_instr->isBundle());

arsenm wrote:
> nhaehnle wrote:
> > arsenm wrote:
> > > I've been thinking about more aggressively using bundles around call 
> > > sites to handle waterfall looping around divergent calls with SGPR 
> > > arguments
> > Hmm, so what's the correct iteration behavior in the presence of bundles? 
> > Iterate over all instructions in the bundle (which is that 
> > MachineBasicBlock::instr_iterator does) and only iterate over explicit 
> > defs? I think that's what makes the most sense, and what I'm going with for 
> > now...
> I don't think this actually needs to specially consider bundles. The BUNDLE 
> itself is supposed to have the uses/defs that cover all the uses/defs inside 
> the bundle. You shouldn't need to worry about the individual instructions
This is what should be there with the last change :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088



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

[PATCH] D83087: DomTree: remove explicit use of DomTreeNodeBase::iterator

[Nicolai Hähnle via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3fa989d4fd6b: DomTree: remove explicit use of 
DomTreeNodeBase::iterator (authored by nhaehnle).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83087

Files:
  clang/include/clang/Analysis/Analyses/Dominators.h
  llvm/include/llvm/CodeGen/MachineDominators.h
  llvm/include/llvm/IR/Dominators.h
  llvm/lib/Target/X86/X86InstrInfo.cpp
  llvm/lib/Transforms/Scalar/EarlyCSE.cpp
  llvm/lib/Transforms/Scalar/Sink.cpp
  llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
  mlir/include/mlir/IR/Dominance.h
  mlir/lib/Transforms/CSE.cpp

Index: mlir/lib/Transforms/CSE.cpp
===
--- mlir/lib/Transforms/CSE.cpp
+++ mlir/lib/Transforms/CSE.cpp
@@ -64,7 +64,7 @@
 ScopedMapTy::ScopeTy scope;
 
 DominanceInfoNode *node;
-DominanceInfoNode::iterator childIterator;
+DominanceInfoNode::const_iterator childIterator;
 
 /// If this node has been fully processed yet or not.
 bool processed;
Index: mlir/include/mlir/IR/Dominance.h
===
--- mlir/include/mlir/IR/Dominance.h
+++ mlir/include/mlir/IR/Dominance.h
@@ -141,7 +141,7 @@
 /// DominatorTree GraphTraits specialization so the DominatorTree can be
 /// iterated by generic graph iterators.
 template <> struct GraphTraits {
-  using ChildIteratorType = mlir::DominanceInfoNode::iterator;
+  using ChildIteratorType = mlir::DominanceInfoNode::const_iterator;
   using NodeRef = mlir::DominanceInfoNode *;
 
   static NodeRef getEntryNode(NodeRef N) { return N; }
Index: llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
===
--- llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
+++ llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
@@ -30,7 +30,8 @@
 /// Template specializations of GraphTraits for VPDomTreeNode.
 template <>
 struct GraphTraits
-: public DomTreeGraphTraitsBase {};
+: public DomTreeGraphTraitsBase {};
 
 template <>
 struct GraphTraits
Index: llvm/lib/Transforms/Scalar/Sink.cpp
===
--- llvm/lib/Transforms/Scalar/Sink.cpp
+++ llvm/lib/Transforms/Scalar/Sink.cpp
@@ -166,8 +166,8 @@
   // dominated by one of the successors.
   // Look at all the dominated blocks and see if we can sink it in one.
   DomTreeNode *DTN = DT.getNode(Inst->getParent());
-  for (DomTreeNode::iterator I = DTN->begin(), E = DTN->end();
-  I != E && SuccToSinkTo == nullptr; ++I) {
+  for (auto I = DTN->begin(), E = DTN->end(); I != E && SuccToSinkTo == nullptr;
+   ++I) {
 BasicBlock *Candidate = (*I)->getBlock();
 // A node always immediate-dominates its children on the dominator
 // tree.
Index: llvm/lib/Transforms/Scalar/EarlyCSE.cpp
===
--- llvm/lib/Transforms/Scalar/EarlyCSE.cpp
+++ llvm/lib/Transforms/Scalar/EarlyCSE.cpp
@@ -620,8 +620,8 @@
   public:
 StackNode(ScopedHTType , LoadHTType ,
   InvariantHTType , CallHTType ,
-  unsigned cg, DomTreeNode *n, DomTreeNode::iterator child,
-  DomTreeNode::iterator end)
+  unsigned cg, DomTreeNode *n, DomTreeNode::const_iterator child,
+  DomTreeNode::const_iterator end)
 : CurrentGeneration(cg), ChildGeneration(cg), Node(n), ChildIter(child),
   EndIter(end),
   Scopes(AvailableValues, AvailableLoads, AvailableInvariants,
@@ -635,7 +635,7 @@
 unsigned childGeneration() { return ChildGeneration; }
 void childGeneration(unsigned generation) { ChildGeneration = generation; }
 DomTreeNode *node() { return Node; }
-DomTreeNode::iterator childIter() { return ChildIter; }
+DomTreeNode::const_iterator childIter() { return ChildIter; }
 
 DomTreeNode *nextChild() {
   DomTreeNode *child = *ChildIter;
@@ -643,7 +643,7 @@
   return child;
 }
 
-DomTreeNode::iterator end() { return EndIter; }
+DomTreeNode::const_iterator end() { return EndIter; }
 bool isProcessed() { return Processed; }
 void process() { Processed = true; }
 
@@ -651,8 +651,8 @@
 unsigned CurrentGeneration;
 unsigned ChildGeneration;
 DomTreeNode *Node;
-DomTreeNode::iterator ChildIter;
-DomTreeNode::iterator EndIter;
+DomTreeNode::const_iterator ChildIter;
+DomTreeNode::const_iterator EndIter;
 NodeScope Scopes;
 bool Processed = false;
   };
Index: llvm/lib/Target/X86/X86InstrInfo.cpp
===
--- llvm/lib/Target/X86/X86InstrInfo.cpp
+++ llvm/lib/Target/X86/X86InstrInfo.cpp
@@ -8660,8 +8660,7 @@
   }
 
   // Visit the children of this block in the dominator tree.
-  for (MachineDomTreeNode::iterator I = 

[PATCH] D83087: DomTree: remove explicit use of DomTreeNodeBase::iterator

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

In D83087#2134881 , @kuhar wrote:

> modulo accidental formatting changes.


I'm not aware of any. Some line breaks changed because "const_iterator" is 
longer than "iterator".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83087



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

[PATCH] D83084: DomTree: Remove the releaseMemory() method

[Nicolai Hähnle via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG723a44c9b5d6: DomTree: Remove the releaseMemory() method 
(authored by nhaehnle).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83084

Files:
  clang/include/clang/Analysis/Analyses/Dominators.h
  llvm/include/llvm/Analysis/PostDominators.h
  llvm/include/llvm/IR/Dominators.h
  llvm/include/llvm/Support/GenericDomTree.h


Index: llvm/include/llvm/Support/GenericDomTree.h
===
--- llvm/include/llvm/Support/GenericDomTree.h
+++ llvm/include/llvm/Support/GenericDomTree.h
@@ -325,8 +325,6 @@
 return false;
   }
 
-  void releaseMemory() { reset(); }
-
   /// getNode - return the (Post)DominatorTree node for the specified basic
   /// block.  This is the same as using operator[] on this class.  The result
   /// may (but is not required to) be null for a forward (backwards)
@@ -760,9 +758,6 @@
 return DomTreeBuilder::Verify(*this, VL);
   }
 
-protected:
-  void addRoot(NodeT *BB) { this->Roots.push_back(BB); }
-
   void reset() {
 DomTreeNodes.clear();
 Roots.clear();
@@ -772,6 +767,9 @@
 SlowQueries = 0;
   }
 
+protected:
+  void addRoot(NodeT *BB) { this->Roots.push_back(BB); }
+
   // NewBB is split and now it has one successor. Update dominator tree to
   // reflect this change.
   template 
Index: llvm/include/llvm/IR/Dominators.h
===
--- llvm/include/llvm/IR/Dominators.h
+++ llvm/include/llvm/IR/Dominators.h
@@ -277,7 +277,7 @@
 AU.setPreservesAll();
   }
 
-  void releaseMemory() override { DT.releaseMemory(); }
+  void releaseMemory() override { DT.reset(); }
 
   void print(raw_ostream , const Module *M = nullptr) const override;
 };
Index: llvm/include/llvm/Analysis/PostDominators.h
===
--- llvm/include/llvm/Analysis/PostDominators.h
+++ llvm/include/llvm/Analysis/PostDominators.h
@@ -88,9 +88,7 @@
 AU.setPreservesAll();
   }
 
-  void releaseMemory() override {
-DT.releaseMemory();
-  }
+  void releaseMemory() override { DT.reset(); }
 
   void print(raw_ostream , const Module*) const override;
 };
Index: clang/include/clang/Analysis/Analyses/Dominators.h
===
--- clang/include/clang/Analysis/Analyses/Dominators.h
+++ clang/include/clang/Analysis/Analyses/Dominators.h
@@ -167,9 +167,7 @@
   }
 
   /// Releases the memory held by the dominator tree.
-  virtual void releaseMemory() {
-DT.releaseMemory();
-  }
+  virtual void releaseMemory() { DT.reset(); }
 
   /// Converts the dominator tree to human readable form.
   virtual void print(raw_ostream , const llvm::Module* M= nullptr) const {


Index: llvm/include/llvm/Support/GenericDomTree.h
===
--- llvm/include/llvm/Support/GenericDomTree.h
+++ llvm/include/llvm/Support/GenericDomTree.h
@@ -325,8 +325,6 @@
 return false;
   }
 
-  void releaseMemory() { reset(); }
-
   /// getNode - return the (Post)DominatorTree node for the specified basic
   /// block.  This is the same as using operator[] on this class.  The result
   /// may (but is not required to) be null for a forward (backwards)
@@ -760,9 +758,6 @@
 return DomTreeBuilder::Verify(*this, VL);
   }
 
-protected:
-  void addRoot(NodeT *BB) { this->Roots.push_back(BB); }
-
   void reset() {
 DomTreeNodes.clear();
 Roots.clear();
@@ -772,6 +767,9 @@
 SlowQueries = 0;
   }
 
+protected:
+  void addRoot(NodeT *BB) { this->Roots.push_back(BB); }
+
   // NewBB is split and now it has one successor. Update dominator tree to
   // reflect this change.
   template 
Index: llvm/include/llvm/IR/Dominators.h
===
--- llvm/include/llvm/IR/Dominators.h
+++ llvm/include/llvm/IR/Dominators.h
@@ -277,7 +277,7 @@
 AU.setPreservesAll();
   }
 
-  void releaseMemory() override { DT.releaseMemory(); }
+  void releaseMemory() override { DT.reset(); }
 
   void print(raw_ostream , const Module *M = nullptr) const override;
 };
Index: llvm/include/llvm/Analysis/PostDominators.h
===
--- llvm/include/llvm/Analysis/PostDominators.h
+++ llvm/include/llvm/Analysis/PostDominators.h
@@ -88,9 +88,7 @@
 AU.setPreservesAll();
   }
 
-  void releaseMemory() override {
-DT.releaseMemory();
-  }
+  void releaseMemory() override { DT.reset(); }
 
   void print(raw_ostream , const Module*) const override;
 };
Index: clang/include/clang/Analysis/Analyses/Dominators.h
===
--- clang/include/clang/Analysis/Analyses/Dominators.h
+++ clang/include/clang/Analysis/Analyses/Dominators.h
@@ -167,9 

[PATCH] D83088: Introduce CfgTraits abstraction

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle updated this revision to Diff 275811.
nhaehnle marked 4 inline comments as done.
nhaehnle added a comment.

- fix MachineCfgTraits::blockdef_iterator and allow it to iterate over the 
instructions in a bundle
- use MachineBasicBlock::printName


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088

Files:
  clang/include/clang/Analysis/Analyses/Dominators.h
  llvm/include/llvm/CodeGen/MachineCfgTraits.h
  llvm/include/llvm/IR/CFG.h
  llvm/include/llvm/Support/CfgTraits.h
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/MachineCfgTraits.cpp
  llvm/lib/IR/CFG.cpp
  llvm/lib/IR/CMakeLists.txt
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/CfgTraits.cpp
  llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
  mlir/include/mlir/IR/Dominance.h

Index: mlir/include/mlir/IR/Dominance.h
===
--- mlir/include/mlir/IR/Dominance.h
+++ mlir/include/mlir/IR/Dominance.h
@@ -10,8 +10,37 @@
 #define MLIR_IR_DOMINANCE_H
 
 #include "mlir/IR/RegionGraphTraits.h"
+#include "llvm/Support/CfgTraits.h"
 #include "llvm/Support/GenericDomTree.h"
 
+namespace mlir {
+
+/// Partial CFG traits for MLIR's CFG, without a value type.
+class CfgTraitsBase : public llvm::CfgTraitsBase {
+public:
+  using ParentType = Region;
+  using BlockRef = Block *;
+  using ValueRef = void;
+
+  static llvm::CfgBlockRef toGeneric(BlockRef block) {
+return makeOpaque(block);
+  }
+  static BlockRef fromGeneric(llvm::CfgBlockRef block) {
+return static_cast(getOpaque(block));
+  }
+};
+
+class CfgTraits : public llvm::CfgTraits {
+  static Region *getBlockParent(Block *block) { return block->getParent(); }
+};
+
+} // namespace mlir
+
+template <>
+struct llvm::CfgTraitsFor {
+  using CfgTraits = mlir::CfgTraits;
+};
+
 extern template class llvm::DominatorTreeBase;
 extern template class llvm::DominatorTreeBase;
 
Index: llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
===
--- llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
+++ llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
@@ -18,9 +18,33 @@
 #include "VPlan.h"
 #include "llvm/ADT/GraphTraits.h"
 #include "llvm/IR/Dominators.h"
+#include "llvm/Support/CfgTraits.h"
 
 namespace llvm {
 
+/// Partial CFG traits for VPlan's CFG, without a value type.
+class VPCfgTraitsBase : public CfgTraitsBase {
+public:
+  using ParentType = VPRegionBlock;
+  using BlockRef = VPBlockBase *;
+  using ValueRef = void;
+
+  static CfgBlockRef toGeneric(BlockRef block) {
+return makeOpaque(block);
+  }
+  static BlockRef fromGeneric(CfgBlockRef block) {
+return static_cast(getOpaque(block));
+  }
+};
+
+class VPCfgTraits : public CfgTraits {
+  static VPRegionBlock *getBlockParent(VPBlockBase *block) {
+return block->getParent();
+  }
+};
+
+template <> struct CfgTraitsFor { using CfgTraits = VPCfgTraits; };
+
 /// Template specialization of the standard LLVM dominator tree utility for
 /// VPBlockBases.
 using VPDominatorTree = DomTreeBase;
Index: llvm/lib/Support/CfgTraits.cpp
===
--- /dev/null
+++ llvm/lib/Support/CfgTraits.cpp
@@ -0,0 +1,14 @@
+//===- CfgTraits.cpp - Traits for generically working on CFGs ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "llvm/Support/CfgTraits.h"
+
+using namespace llvm;
+
+void CfgInterface::anchor() {}
+void CfgPrinter::anchor() {}
Index: llvm/lib/Support/CMakeLists.txt
===
--- llvm/lib/Support/CMakeLists.txt
+++ llvm/lib/Support/CMakeLists.txt
@@ -71,6 +71,7 @@
   BranchProbability.cpp
   BuryPointer.cpp
   CachePruning.cpp
+  CfgTraits.cpp
   circular_raw_ostream.cpp
   Chrono.cpp
   COM.cpp
Index: llvm/lib/IR/CMakeLists.txt
===
--- llvm/lib/IR/CMakeLists.txt
+++ llvm/lib/IR/CMakeLists.txt
@@ -4,6 +4,7 @@
   Attributes.cpp
   AutoUpgrade.cpp
   BasicBlock.cpp
+  CFG.cpp
   Comdat.cpp
   ConstantFold.cpp
   ConstantRange.cpp
Index: llvm/lib/IR/CFG.cpp
===
--- /dev/null
+++ llvm/lib/IR/CFG.cpp
@@ -0,0 +1,56 @@
+//===- CFG.cpp --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "llvm/IR/CFG.h"

[PATCH] D83088: Introduce CfgTraits abstraction

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added inline comments.



Comment at: llvm/include/llvm/CodeGen/MachineCfgTraits.h:133
+  ++m_def;
+  if (m_def == m_instr->defs().end()) {
+++m_instr;

arsenm wrote:
> != return early?
The logic is actually subtly broken in the presence of instructions without 
defs, I just didn't notice it because it currently affects only debug printing 
logic. Going to fix it.



Comment at: llvm/include/llvm/CodeGen/MachineCfgTraits.h:136-138
+  // Prefer to avoid support for bundled instructions as long as we
+  // don't really need it.
+  assert(!m_instr->isBundle());

arsenm wrote:
> I've been thinking about more aggressively using bundles around call sites to 
> handle waterfall looping around divergent calls with SGPR arguments
Hmm, so what's the correct iteration behavior in the presence of bundles? 
Iterate over all instructions in the bundle (which is that 
MachineBasicBlock::instr_iterator does) and only iterate over explicit defs? I 
think that's what makes the most sense, and what I'm going with for now...



Comment at: llvm/lib/CodeGen/MachineCfgTraits.cpp:27-29
+void MachineCfgTraits::Printer::printBlockName(raw_ostream ,
+   MachineBasicBlock *block) const 
{
+  out << "bb." << block->getNumber();

arsenm wrote:
> I think this should be added to MachineBasicBlock. The same logic is already 
> repeated in MIRPrinter (and the MBB dump function uses a different prefix)
D83253


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088



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

[PATCH] D83084: DomTree: Remove the releaseMemory() method

[Nicolai Hähnle via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG723a44c9b5d6: DomTree: Remove the releaseMemory() method 
(authored by nhaehnle).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83084

Files:
  clang/include/clang/Analysis/Analyses/Dominators.h
  llvm/include/llvm/Analysis/PostDominators.h
  llvm/include/llvm/IR/Dominators.h
  llvm/include/llvm/Support/GenericDomTree.h


Index: llvm/include/llvm/Support/GenericDomTree.h
===
--- llvm/include/llvm/Support/GenericDomTree.h
+++ llvm/include/llvm/Support/GenericDomTree.h
@@ -325,8 +325,6 @@
 return false;
   }
 
-  void releaseMemory() { reset(); }
-
   /// getNode - return the (Post)DominatorTree node for the specified basic
   /// block.  This is the same as using operator[] on this class.  The result
   /// may (but is not required to) be null for a forward (backwards)
@@ -760,9 +758,6 @@
 return DomTreeBuilder::Verify(*this, VL);
   }
 
-protected:
-  void addRoot(NodeT *BB) { this->Roots.push_back(BB); }
-
   void reset() {
 DomTreeNodes.clear();
 Roots.clear();
@@ -772,6 +767,9 @@
 SlowQueries = 0;
   }
 
+protected:
+  void addRoot(NodeT *BB) { this->Roots.push_back(BB); }
+
   // NewBB is split and now it has one successor. Update dominator tree to
   // reflect this change.
   template 
Index: llvm/include/llvm/IR/Dominators.h
===
--- llvm/include/llvm/IR/Dominators.h
+++ llvm/include/llvm/IR/Dominators.h
@@ -277,7 +277,7 @@
 AU.setPreservesAll();
   }
 
-  void releaseMemory() override { DT.releaseMemory(); }
+  void releaseMemory() override { DT.reset(); }
 
   void print(raw_ostream , const Module *M = nullptr) const override;
 };
Index: llvm/include/llvm/Analysis/PostDominators.h
===
--- llvm/include/llvm/Analysis/PostDominators.h
+++ llvm/include/llvm/Analysis/PostDominators.h
@@ -88,9 +88,7 @@
 AU.setPreservesAll();
   }
 
-  void releaseMemory() override {
-DT.releaseMemory();
-  }
+  void releaseMemory() override { DT.reset(); }
 
   void print(raw_ostream , const Module*) const override;
 };
Index: clang/include/clang/Analysis/Analyses/Dominators.h
===
--- clang/include/clang/Analysis/Analyses/Dominators.h
+++ clang/include/clang/Analysis/Analyses/Dominators.h
@@ -167,9 +167,7 @@
   }
 
   /// Releases the memory held by the dominator tree.
-  virtual void releaseMemory() {
-DT.releaseMemory();
-  }
+  virtual void releaseMemory() { DT.reset(); }
 
   /// Converts the dominator tree to human readable form.
   virtual void print(raw_ostream , const llvm::Module* M= nullptr) const {


Index: llvm/include/llvm/Support/GenericDomTree.h
===
--- llvm/include/llvm/Support/GenericDomTree.h
+++ llvm/include/llvm/Support/GenericDomTree.h
@@ -325,8 +325,6 @@
 return false;
   }
 
-  void releaseMemory() { reset(); }
-
   /// getNode - return the (Post)DominatorTree node for the specified basic
   /// block.  This is the same as using operator[] on this class.  The result
   /// may (but is not required to) be null for a forward (backwards)
@@ -760,9 +758,6 @@
 return DomTreeBuilder::Verify(*this, VL);
   }
 
-protected:
-  void addRoot(NodeT *BB) { this->Roots.push_back(BB); }
-
   void reset() {
 DomTreeNodes.clear();
 Roots.clear();
@@ -772,6 +767,9 @@
 SlowQueries = 0;
   }
 
+protected:
+  void addRoot(NodeT *BB) { this->Roots.push_back(BB); }
+
   // NewBB is split and now it has one successor. Update dominator tree to
   // reflect this change.
   template 
Index: llvm/include/llvm/IR/Dominators.h
===
--- llvm/include/llvm/IR/Dominators.h
+++ llvm/include/llvm/IR/Dominators.h
@@ -277,7 +277,7 @@
 AU.setPreservesAll();
   }
 
-  void releaseMemory() override { DT.releaseMemory(); }
+  void releaseMemory() override { DT.reset(); }
 
   void print(raw_ostream , const Module *M = nullptr) const override;
 };
Index: llvm/include/llvm/Analysis/PostDominators.h
===
--- llvm/include/llvm/Analysis/PostDominators.h
+++ llvm/include/llvm/Analysis/PostDominators.h
@@ -88,9 +88,7 @@
 AU.setPreservesAll();
   }
 
-  void releaseMemory() override {
-DT.releaseMemory();
-  }
+  void releaseMemory() override { DT.reset(); }
 
   void print(raw_ostream , const Module*) const override;
 };
Index: clang/include/clang/Analysis/Analyses/Dominators.h
===
--- clang/include/clang/Analysis/Analyses/Dominators.h
+++ clang/include/clang/Analysis/Analyses/Dominators.h
@@ -167,9 

[PATCH] D83088: Introduce CfgTraits abstraction

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle created this revision.
nhaehnle added reviewers: arsenm, RKSimon, mehdi_amini, courbet.
Herald added subscribers: cfe-commits, msifontes, jurahul, Kayjukh, vkmr, 
grosul1, Joonsoo, stephenneuendorffer, liufengdb, aartbik, lucyrfox, mgester, 
arpith-jacob, nicolasvasilache, antiagainst, shauheen, jpienaar, rriddle, 
rogfer01, kuhar, hiraditya, mgorny, wdng.
Herald added a reviewer: rriddle.
Herald added a reviewer: aartbik.
Herald added projects: clang, MLIR, LLVM.
nhaehnle added a parent revision: D83087: DomTree: remove explicit use of 
DomTreeNodeBase::iterator.
nhaehnle added a child revision: D83089: DomTree: Extract (mostly) read-only 
logic into type-erased base classes.

The CfgTraits abstraction simplfies writing algorithms that are
generic over the type of CFG, and enables writing such algorithms
as regular non-template code that operates on opaque references
to CFG blocks and values.

Implementations of CfgTraits provide operations on the concrete
CFG types, e.g. `IrCfgTraits::BlockRef` is `BasicBlock *`.

CfgInterface is an abstract base class which provides operations
on opaque types CfgBlockRef and CfgValueRef. Those opaque types
encapsulate a `void *`, but the meaning depends on the concrete
CFG type. For example, MachineCfgTraits -- for use with MachineIR
in SSA form -- encodes a Register inside CfgValueRef. Converting
between concrete references and opaque/generic ones is done by
CfgTraits::{fromGeneric,toGeneric}. Convenience methods
CfgTraits::{un}wrap{Iterator,Range} are available as well.

Writing algorithms in terms of CfgInterface adds some overhead
(virtual method calls, plus in same cases it removes the
opportunity to inline iterators), but can be much more convenient
since generic algorithms can be written as non-templates.

This patch adds implementations of CfgTraits for all CFGs on
which dominator trees are calculated, so that the dominator
tree can be ported to this machinery. Only IrCfgTraits (LLVM IR)
and MachineCfgTraits (Machine IR in SSA form) are complete, the
other implementations are limited to the absolute minimum
required to make the upcoming dominator tree changes work.

Change-Id: Ia75f4f268fded33fca11218a7d578c9aec1f3f4d


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83088

Files:
  clang/include/clang/Analysis/Analyses/Dominators.h
  llvm/include/llvm/CodeGen/MachineCfgTraits.h
  llvm/include/llvm/IR/CFG.h
  llvm/include/llvm/Support/CfgTraits.h
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/MachineCfgTraits.cpp
  llvm/lib/IR/CFG.cpp
  llvm/lib/IR/CMakeLists.txt
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/CfgTraits.cpp
  llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
  mlir/include/mlir/IR/Dominance.h

Index: mlir/include/mlir/IR/Dominance.h
===
--- mlir/include/mlir/IR/Dominance.h
+++ mlir/include/mlir/IR/Dominance.h
@@ -10,8 +10,37 @@
 #define MLIR_IR_DOMINANCE_H
 
 #include "mlir/IR/RegionGraphTraits.h"
+#include "llvm/Support/CfgTraits.h"
 #include "llvm/Support/GenericDomTree.h"
 
+namespace mlir {
+
+/// Partial CFG traits for MLIR's CFG, without a value type.
+class CfgTraitsBase : public llvm::CfgTraitsBase {
+public:
+  using ParentType = Region;
+  using BlockRef = Block *;
+  using ValueRef = void;
+
+  static llvm::CfgBlockRef toGeneric(BlockRef block) {
+return makeOpaque(block);
+  }
+  static BlockRef fromGeneric(llvm::CfgBlockRef block) {
+return static_cast(getOpaque(block));
+  }
+};
+
+class CfgTraits : public llvm::CfgTraits {
+  static Region *getBlockParent(Block *block) { return block->getParent(); }
+};
+
+} // namespace mlir
+
+template <>
+struct llvm::CfgTraitsFor {
+  using CfgTraits = mlir::CfgTraits;
+};
+
 extern template class llvm::DominatorTreeBase;
 extern template class llvm::DominatorTreeBase;
 
Index: llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
===
--- llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
+++ llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
@@ -18,9 +18,33 @@
 #include "VPlan.h"
 #include "llvm/ADT/GraphTraits.h"
 #include "llvm/IR/Dominators.h"
+#include "llvm/Support/CfgTraits.h"
 
 namespace llvm {
 
+/// Partial CFG traits for VPlan's CFG, without a value type.
+class VPCfgTraitsBase : public CfgTraitsBase {
+public:
+  using ParentType = VPRegionBlock;
+  using BlockRef = VPBlockBase *;
+  using ValueRef = void;
+
+  static CfgBlockRef toGeneric(BlockRef block) {
+return makeOpaque(block);
+  }
+  static BlockRef fromGeneric(CfgBlockRef block) {
+return static_cast(getOpaque(block));
+  }
+};
+
+class VPCfgTraits : public CfgTraits {
+  static VPRegionBlock *getBlockParent(VPBlockBase *block) {
+return block->getParent();
+  }
+};
+
+template <> struct CfgTraitsFor { using CfgTraits = VPCfgTraits; };
+
 /// Template specialization of the standard LLVM dominator tree utility for
 /// VPBlockBases.
 using 

[PATCH] D83087: DomTree: remove explicit use of DomTreeNodeBase::iterator

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle created this revision.
nhaehnle added reviewers: arsenm, RKSimon, mehdi_amini, courbet.
Herald added subscribers: cfe-commits, msifontes, jurahul, Kayjukh, vkmr, 
grosul1, Joonsoo, stephenneuendorffer, liufengdb, aartbik, lucyrfox, mgester, 
arpith-jacob, nicolasvasilache, antiagainst, shauheen, jpienaar, rriddle, 
rogfer01, kuhar, hiraditya, Prazek, wdng.
Herald added a reviewer: rriddle.
Herald added a reviewer: aartbik.
Herald added projects: clang, MLIR, LLVM.
nhaehnle added a parent revision: D83086: DomTree: add private 
create{Child,Node} helpers.
nhaehnle added a child revision: D83088: Introduce CfgTraits abstraction.

Almost all uses of these iterators, including implicit ones, really
only need the const variant (as it should be). The only exception is
in NewGVN, which changes the order of dominator tree child nodes.

Change-Id: I4b5bd71e32d71b0c67b03d4927d93fe9413726d4


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83087

Files:
  clang/include/clang/Analysis/Analyses/Dominators.h
  llvm/include/llvm/CodeGen/MachineDominators.h
  llvm/include/llvm/IR/Dominators.h
  llvm/lib/Target/X86/X86InstrInfo.cpp
  llvm/lib/Transforms/Scalar/EarlyCSE.cpp
  llvm/lib/Transforms/Scalar/Sink.cpp
  llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
  mlir/include/mlir/IR/Dominance.h
  mlir/lib/Transforms/CSE.cpp

Index: mlir/lib/Transforms/CSE.cpp
===
--- mlir/lib/Transforms/CSE.cpp
+++ mlir/lib/Transforms/CSE.cpp
@@ -64,7 +64,7 @@
 ScopedMapTy::ScopeTy scope;
 
 DominanceInfoNode *node;
-DominanceInfoNode::iterator childIterator;
+DominanceInfoNode::const_iterator childIterator;
 
 /// If this node has been fully processed yet or not.
 bool processed;
Index: mlir/include/mlir/IR/Dominance.h
===
--- mlir/include/mlir/IR/Dominance.h
+++ mlir/include/mlir/IR/Dominance.h
@@ -141,7 +141,7 @@
 /// DominatorTree GraphTraits specialization so the DominatorTree can be
 /// iterated by generic graph iterators.
 template <> struct GraphTraits {
-  using ChildIteratorType = mlir::DominanceInfoNode::iterator;
+  using ChildIteratorType = mlir::DominanceInfoNode::const_iterator;
   using NodeRef = mlir::DominanceInfoNode *;
 
   static NodeRef getEntryNode(NodeRef N) { return N; }
Index: llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
===
--- llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
+++ llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
@@ -30,7 +30,8 @@
 /// Template specializations of GraphTraits for VPDomTreeNode.
 template <>
 struct GraphTraits
-: public DomTreeGraphTraitsBase {};
+: public DomTreeGraphTraitsBase {};
 
 template <>
 struct GraphTraits
Index: llvm/lib/Transforms/Scalar/Sink.cpp
===
--- llvm/lib/Transforms/Scalar/Sink.cpp
+++ llvm/lib/Transforms/Scalar/Sink.cpp
@@ -166,8 +166,8 @@
   // dominated by one of the successors.
   // Look at all the dominated blocks and see if we can sink it in one.
   DomTreeNode *DTN = DT.getNode(Inst->getParent());
-  for (DomTreeNode::iterator I = DTN->begin(), E = DTN->end();
-  I != E && SuccToSinkTo == nullptr; ++I) {
+  for (auto I = DTN->begin(), E = DTN->end(); I != E && SuccToSinkTo == nullptr;
+   ++I) {
 BasicBlock *Candidate = (*I)->getBlock();
 // A node always immediate-dominates its children on the dominator
 // tree.
Index: llvm/lib/Transforms/Scalar/EarlyCSE.cpp
===
--- llvm/lib/Transforms/Scalar/EarlyCSE.cpp
+++ llvm/lib/Transforms/Scalar/EarlyCSE.cpp
@@ -620,8 +620,8 @@
   public:
 StackNode(ScopedHTType , LoadHTType ,
   InvariantHTType , CallHTType ,
-  unsigned cg, DomTreeNode *n, DomTreeNode::iterator child,
-  DomTreeNode::iterator end)
+  unsigned cg, DomTreeNode *n, DomTreeNode::const_iterator child,
+  DomTreeNode::const_iterator end)
 : CurrentGeneration(cg), ChildGeneration(cg), Node(n), ChildIter(child),
   EndIter(end),
   Scopes(AvailableValues, AvailableLoads, AvailableInvariants,
@@ -635,7 +635,7 @@
 unsigned childGeneration() { return ChildGeneration; }
 void childGeneration(unsigned generation) { ChildGeneration = generation; }
 DomTreeNode *node() { return Node; }
-DomTreeNode::iterator childIter() { return ChildIter; }
+DomTreeNode::const_iterator childIter() { return ChildIter; }
 
 DomTreeNode *nextChild() {
   DomTreeNode *child = *ChildIter;
@@ -643,7 +643,7 @@
   return child;
 }
 
-DomTreeNode::iterator end() { return EndIter; }
+DomTreeNode::const_iterator end() { return EndIter; }
 bool isProcessed() { return Processed; }
 void process() { Processed = true; }
 
@@ -651,8 +651,8 @@
 

[PATCH] D83084: DomTree: Remove the releaseMemory() method

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle created this revision.
nhaehnle added reviewers: arsenm, RKSimon, mehdi_amini, courbet.
Herald added subscribers: cfe-commits, wdng.
Herald added projects: clang, LLVM.
nhaehnle added a parent revision: D83083: DomTree: Remove getChildren() 
accessor.
nhaehnle added a child revision: D83085: DomTree: Remove getRoots() accessor.

It is fully redundant with reset().

Change-Id: I25850b9f08eace757cf03cbb8780e970aca7f51a


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83084

Files:
  clang/include/clang/Analysis/Analyses/Dominators.h
  llvm/include/llvm/Analysis/PostDominators.h
  llvm/include/llvm/IR/Dominators.h
  llvm/include/llvm/Support/GenericDomTree.h


Index: llvm/include/llvm/Support/GenericDomTree.h
===
--- llvm/include/llvm/Support/GenericDomTree.h
+++ llvm/include/llvm/Support/GenericDomTree.h
@@ -325,8 +325,6 @@
 return false;
   }
 
-  void releaseMemory() { reset(); }
-
   /// getNode - return the (Post)DominatorTree node for the specified basic
   /// block.  This is the same as using operator[] on this class.  The result
   /// may (but is not required to) be null for a forward (backwards)
@@ -760,9 +758,6 @@
 return DomTreeBuilder::Verify(*this, VL);
   }
 
-protected:
-  void addRoot(NodeT *BB) { this->Roots.push_back(BB); }
-
   void reset() {
 DomTreeNodes.clear();
 Roots.clear();
@@ -772,6 +767,9 @@
 SlowQueries = 0;
   }
 
+protected:
+  void addRoot(NodeT *BB) { this->Roots.push_back(BB); }
+
   // NewBB is split and now it has one successor. Update dominator tree to
   // reflect this change.
   template 
Index: llvm/include/llvm/IR/Dominators.h
===
--- llvm/include/llvm/IR/Dominators.h
+++ llvm/include/llvm/IR/Dominators.h
@@ -277,7 +277,7 @@
 AU.setPreservesAll();
   }
 
-  void releaseMemory() override { DT.releaseMemory(); }
+  void releaseMemory() override { DT.reset(); }
 
   void print(raw_ostream , const Module *M = nullptr) const override;
 };
Index: llvm/include/llvm/Analysis/PostDominators.h
===
--- llvm/include/llvm/Analysis/PostDominators.h
+++ llvm/include/llvm/Analysis/PostDominators.h
@@ -88,9 +88,7 @@
 AU.setPreservesAll();
   }
 
-  void releaseMemory() override {
-DT.releaseMemory();
-  }
+  void releaseMemory() override { DT.reset(); }
 
   void print(raw_ostream , const Module*) const override;
 };
Index: clang/include/clang/Analysis/Analyses/Dominators.h
===
--- clang/include/clang/Analysis/Analyses/Dominators.h
+++ clang/include/clang/Analysis/Analyses/Dominators.h
@@ -167,9 +167,7 @@
   }
 
   /// Releases the memory held by the dominator tree.
-  virtual void releaseMemory() {
-DT.releaseMemory();
-  }
+  virtual void releaseMemory() { DT.reset(); }
 
   /// Converts the dominator tree to human readable form.
   virtual void print(raw_ostream , const llvm::Module* M= nullptr) const {


Index: llvm/include/llvm/Support/GenericDomTree.h
===
--- llvm/include/llvm/Support/GenericDomTree.h
+++ llvm/include/llvm/Support/GenericDomTree.h
@@ -325,8 +325,6 @@
 return false;
   }
 
-  void releaseMemory() { reset(); }
-
   /// getNode - return the (Post)DominatorTree node for the specified basic
   /// block.  This is the same as using operator[] on this class.  The result
   /// may (but is not required to) be null for a forward (backwards)
@@ -760,9 +758,6 @@
 return DomTreeBuilder::Verify(*this, VL);
   }
 
-protected:
-  void addRoot(NodeT *BB) { this->Roots.push_back(BB); }
-
   void reset() {
 DomTreeNodes.clear();
 Roots.clear();
@@ -772,6 +767,9 @@
 SlowQueries = 0;
   }
 
+protected:
+  void addRoot(NodeT *BB) { this->Roots.push_back(BB); }
+
   // NewBB is split and now it has one successor. Update dominator tree to
   // reflect this change.
   template 
Index: llvm/include/llvm/IR/Dominators.h
===
--- llvm/include/llvm/IR/Dominators.h
+++ llvm/include/llvm/IR/Dominators.h
@@ -277,7 +277,7 @@
 AU.setPreservesAll();
   }
 
-  void releaseMemory() override { DT.releaseMemory(); }
+  void releaseMemory() override { DT.reset(); }
 
   void print(raw_ostream , const Module *M = nullptr) const override;
 };
Index: llvm/include/llvm/Analysis/PostDominators.h
===
--- llvm/include/llvm/Analysis/PostDominators.h
+++ llvm/include/llvm/Analysis/PostDominators.h
@@ -88,9 +88,7 @@
 AU.setPreservesAll();
   }
 
-  void releaseMemory() override {
-DT.releaseMemory();
-  }
+  void releaseMemory() override { DT.reset(); }
 
   void print(raw_ostream , const Module*) const override;
 };
Index: clang/include/clang/Analysis/Analyses/Dominators.h

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

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added inline comments.



Comment at: llvm/include/llvm/Transforms/InstCombine/InstCombiner.h:46
+/// combine them.
+class LLVM_LIBRARY_VISIBILITY InstCombiner {
+public:

lattner wrote:
> I would really rather not make this be a public class - this is a very thick 
> interface.  Can this be cut down to something much smaller than the 
> implementation details of InstCombine?
> 
> If you're curious for a pattern that could be followed, the MLIR AsmParser is 
> a reasonable example.  The parser is spread across a bunch of classes in the 
> lib/ directory:
> https://github.com/llvm/llvm-project/blob/master/mlir/lib/Parser/Parser.cpp
> 
> But then there is a much smaller public API exposed through a header:
> https://github.com/llvm/llvm-project/blob/master/mlir/include/mlir/IR/OpImplementation.h#L229
> 
> 
I agree with the sentiment, but note @Flakebi has split up the `InstCombiner` 
class into `InstCombiner` and `InstCombinerImpl` classes, which addresses those 
concerns already as far as I'm concerned.

Looking through the new `InstCombiner`, aside from methods that are core to the 
workings of InstCombine (modifying instructions while keeping track of the 
Worklist) and methods for accessing the analyses, what's left is:

* A bunch of static methods that should arguably just be global functions in a 
utils header somewhere.
* CreateOverflowTuple and CreateNonTerminatorUnreachable

Moving those methods feels sensible, but is likely to touch a lot of code, so I 
think it would be better to do it in a separate commit.


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

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

We've been handling target-specific intrinsics in InstCombine for a long time, 
and that's the place where they should naturally sit. This is a pretty clean 
refactoring in my opinion, I'm in favor. It's substantial enough as a change 
that it should probably receive a heads-up on llvm-dev, though.


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] D75661: Remove SequentialType from the type heirarchy.

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

The AMDGPU changes LGTM, modulo a stylistic nitpick.

As for the overall change, I'm perfectly fine with the general direction of 
SequentialType, though I don't know where we stand in terms of enough people 
having had a chance to look at it.

I'm concerned though about having getSequentialElementType and 
getSequentialNumElements apply to different sets of types. That's bound to lead 
to confusion which causes bugs. Can we have one name for all types that are 
homogenous sequences of the same element type, and a different name for all 
types which are *fixed-length* homogenous sequences?




Comment at: llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp:370-372
+if (VectorType::isValidElementType(ArrayTy->getElementType()) &&
+ArrayTy->getNumElements() > 0)
+  VectorTy = arrayTypeToVecType(ArrayTy);

Please put braces for the outer if (multi-line body).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75661



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

[PATCH] D75660: Remove CompositeType class

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle accepted this revision.
nhaehnle added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75660



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

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

In D74935#1887245 , @efriedma wrote:

> In D74935#1886020 , @nhaehnle wrote:
>
> > I find this phrasing pretty confusing. How about the following:
> >
> > > This indicates that objects accessed via pointer values based on the 
> > > argument or return value are not **modified**, during the execution of 
> > > the function, via pointer values not based on the argument or return 
> > > value. [...]
>
>
> This isn't equivalent.  If the memory location is modified, we want to forbid 
> both reads and writes that aren't based on the noalias pointer.


Oh I see. So another way to put this is:

- If an object is accessed via a pointer based on the noalias argument, then it 
cannot be modified via other pointer values.
- If an object is modified via a pointer based on the noalias argument, then it 
cannot be accessed via other pointer values in any way.

Correct? I think it's useful to spell it out like this, because it makes the 
alias implications clearer (at least to me).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74935



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

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

I find this phrasing pretty confusing. How about the following:

> This indicates that objects accessed via pointer values based on the argument 
> or return value are not **modified**, during the execution of the function, 
> via pointer values not based on the argument or return value. The attribute 
> on a return value also has additional semantics [...]

Does this exactly capture your intended meaning? If yes, can we please use that 
instead because it seems much clearer at least to me. If not, can you give an 
example for why it is different?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74935



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

[PATCH] D74787: [IRBuilder] Always respect inserter/folder

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle accepted this revision.
nhaehnle added a comment.
This revision is now accepted and ready to land.

LGTM. I think it's a pretty reasonable idea to add this threshold check to 
track down things that should be minor, but can add up in terms of compile-time 
performance.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74787



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

[PATCH] D74673: CGBuiltin: Remove uses of deprecated CreateCall overloads

[Nicolai Hähnle via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbf197304a66a: CGBuiltin: Remove uses of deprecated 
CreateCall overloads (authored by nhaehnle).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74673

Files:
  clang/lib/CodeGen/CGBuiltin.cpp


Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -13303,7 +13303,7 @@
 llvm::Value *Src2 = EmitScalarExpr(E->getArg(2));
 
 // FIXME-GFX10: How should 32 bit mask be handled?
-Value *F = CGM.getIntrinsic(Intrinsic::amdgcn_icmp,
+Function *F = CGM.getIntrinsic(Intrinsic::amdgcn_icmp,
   { Builder.getInt64Ty(), Src0->getType() });
 return Builder.CreateCall(F, { Src0, Src1, Src2 });
   }
@@ -13314,7 +13314,7 @@
 llvm::Value *Src2 = EmitScalarExpr(E->getArg(2));
 
 // FIXME-GFX10: How should 32 bit mask be handled?
-Value *F = CGM.getIntrinsic(Intrinsic::amdgcn_fcmp,
+Function *F = CGM.getIntrinsic(Intrinsic::amdgcn_fcmp,
   { Builder.getInt64Ty(), Src0->getType() });
 return Builder.CreateCall(F, { Src0, Src1, Src2 });
   }


Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -13303,7 +13303,7 @@
 llvm::Value *Src2 = EmitScalarExpr(E->getArg(2));
 
 // FIXME-GFX10: How should 32 bit mask be handled?
-Value *F = CGM.getIntrinsic(Intrinsic::amdgcn_icmp,
+Function *F = CGM.getIntrinsic(Intrinsic::amdgcn_icmp,
   { Builder.getInt64Ty(), Src0->getType() });
 return Builder.CreateCall(F, { Src0, Src1, Src2 });
   }
@@ -13314,7 +13314,7 @@
 llvm::Value *Src2 = EmitScalarExpr(E->getArg(2));
 
 // FIXME-GFX10: How should 32 bit mask be handled?
-Value *F = CGM.getIntrinsic(Intrinsic::amdgcn_fcmp,
+Function *F = CGM.getIntrinsic(Intrinsic::amdgcn_fcmp,
   { Builder.getInt64Ty(), Src0->getType() });
 return Builder.CreateCall(F, { Src0, Src1, Src2 });
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74673: CGBuiltin: Remove uses of deprecated CreateCall overloads

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle created this revision.
nhaehnle added a reviewer: t.p.northover.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74673

Files:
  clang/lib/CodeGen/CGBuiltin.cpp


Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -13305,7 +13305,7 @@
 llvm::Value *Src2 = EmitScalarExpr(E->getArg(2));
 
 // FIXME-GFX10: How should 32 bit mask be handled?
-Value *F = CGM.getIntrinsic(Intrinsic::amdgcn_icmp,
+Function *F = CGM.getIntrinsic(Intrinsic::amdgcn_icmp,
   { Builder.getInt64Ty(), Src0->getType() });
 return Builder.CreateCall(F, { Src0, Src1, Src2 });
   }
@@ -13316,7 +13316,7 @@
 llvm::Value *Src2 = EmitScalarExpr(E->getArg(2));
 
 // FIXME-GFX10: How should 32 bit mask be handled?
-Value *F = CGM.getIntrinsic(Intrinsic::amdgcn_fcmp,
+Function *F = CGM.getIntrinsic(Intrinsic::amdgcn_fcmp,
   { Builder.getInt64Ty(), Src0->getType() });
 return Builder.CreateCall(F, { Src0, Src1, Src2 });
   }


Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -13305,7 +13305,7 @@
 llvm::Value *Src2 = EmitScalarExpr(E->getArg(2));
 
 // FIXME-GFX10: How should 32 bit mask be handled?
-Value *F = CGM.getIntrinsic(Intrinsic::amdgcn_icmp,
+Function *F = CGM.getIntrinsic(Intrinsic::amdgcn_icmp,
   { Builder.getInt64Ty(), Src0->getType() });
 return Builder.CreateCall(F, { Src0, Src1, Src2 });
   }
@@ -13316,7 +13316,7 @@
 llvm::Value *Src2 = EmitScalarExpr(E->getArg(2));
 
 // FIXME-GFX10: How should 32 bit mask be handled?
-Value *F = CGM.getIntrinsic(Intrinsic::amdgcn_fcmp,
+Function *F = CGM.getIntrinsic(Intrinsic::amdgcn_fcmp,
   { Builder.getInt64Ty(), Src0->getType() });
 return Builder.CreateCall(F, { Src0, Src1, Src2 });
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74672: Fix various occurences of redundant std::move [gcc 9 -Wredundant-move]

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle created this revision.
Herald added subscribers: cfe-commits, hiraditya.
Herald added projects: clang, LLVM.

There are hundreds (thousands?) of instances of this warning left.

A blog post on the new warning in gcc 9 is in
https://developers.redhat.com/blog/2019/04/12/understanding-when-not-to-stdmove-in-c/


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74672

Files:
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
  llvm/include/llvm/Bitstream/BitstreamReader.h
  llvm/include/llvm/DebugInfo/CodeView/CVRecord.h
  llvm/include/llvm/DebugInfo/CodeView/SymbolDeserializer.h
  llvm/include/llvm/Object/ELF.h
  llvm/include/llvm/Object/ELFObjectFile.h
  llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
  llvm/lib/ObjectYAML/CodeViewYAMLSymbols.cpp
  llvm/lib/ObjectYAML/CodeViewYAMLTypes.cpp
  llvm/lib/Support/APFloat.cpp
  llvm/lib/Support/JSON.cpp
  llvm/lib/Support/MemoryBuffer.cpp
  llvm/lib/Support/Path.cpp
  llvm/lib/Support/YAMLTraits.cpp
  llvm/lib/TableGen/JSONBackend.cpp
  llvm/lib/XRay/Profile.cpp
  llvm/tools/obj2yaml/elf2yaml.cpp
  llvm/utils/TableGen/GlobalISelEmitter.cpp

Index: llvm/utils/TableGen/GlobalISelEmitter.cpp
===
--- llvm/utils/TableGen/GlobalISelEmitter.cpp
+++ llvm/utils/TableGen/GlobalISelEmitter.cpp
@@ -3791,7 +3791,7 @@
   if (auto Error =
   importChildMatcher(Rule, InsnMatcher, SrcChild, OperandIsAPointer,
  OperandIsImmArg, OpIdx++, TempOpIdx))
-return std::move(Error);
+return Error;
 }
   }
 
@@ -4063,7 +4063,7 @@
   auto InsertPtOrError = createAndImportSubInstructionRenderer(
   ++InsertPt, Rule, DstChild, TempRegID);
   if (auto Error = InsertPtOrError.takeError())
-return std::move(Error);
+return Error;
   return InsertPtOrError.get();
 }
 
@@ -4142,7 +4142,7 @@
 const TreePatternNode *Dst) {
   auto InsertPtOrError = createInstructionRenderer(M.actions_end(), M, Dst);
   if (auto Error = InsertPtOrError.takeError())
-return std::move(Error);
+return Error;
 
   action_iterator InsertPt = InsertPtOrError.get();
   BuildMIAction  = *static_cast(InsertPt->get());
@@ -4162,7 +4162,7 @@
 
   if (auto Error = importExplicitUseRenderers(InsertPt, M, DstMIBuilder, Dst)
.takeError())
-return std::move(Error);
+return Error;
 
   return DstMIBuilder;
 }
@@ -4176,7 +4176,7 @@
   // TODO: Assert there's exactly one result.
 
   if (auto Error = InsertPtOrError.takeError())
-return std::move(Error);
+return Error;
 
   BuildMIAction  =
   *static_cast(InsertPtOrError.get()->get());
@@ -4187,7 +4187,7 @@
   InsertPtOrError =
   importExplicitUseRenderers(InsertPtOrError.get(), M, DstMIBuilder, Dst);
   if (auto Error = InsertPtOrError.takeError())
-return std::move(Error);
+return Error;
 
   // We need to make sure that when we import an INSERT_SUBREG as a
   // subinstruction that it ends up being constrained to the correct super
@@ -4359,7 +4359,7 @@
 auto InsertPtOrError =
 importExplicitUseRenderer(InsertPt, M, DstMIBuilder, ValChild);
 if (auto Error = InsertPtOrError.takeError())
-  return std::move(Error);
+  return Error;
 InsertPt = InsertPtOrError.get();
 DstMIBuilder.addRenderer(SubIdx);
   }
@@ -4420,7 +4420,7 @@
   DagInit *DefaultOps = DstIOperand.Rec->getValueAsDag("DefaultOps");
   if (auto Error = importDefaultOperandRenderers(
 InsertPt, M, DstMIBuilder, DefaultOps))
-return std::move(Error);
+return Error;
   ++NumDefaultOps;
   continue;
 }
@@ -4428,7 +4428,7 @@
 auto InsertPtOrError = importExplicitUseRenderer(InsertPt, M, DstMIBuilder,
  Dst->getChild(Child));
 if (auto Error = InsertPtOrError.takeError())
-  return std::move(Error);
+  return Error;
 InsertPt = InsertPtOrError.get();
 ++Child;
   }
@@ -4629,7 +4629,7 @@
   llvm::to_string(*P.getDstPattern()));
 
   if (auto Error = importRulePredicates(M, P.getPredicates()))
-return std::move(Error);
+return Error;
 
   // Next, analyze the pattern operators.
   TreePatternNode *Src = P.getSrcPattern();
@@ -4669,7 +4669,7 @@
   auto InsnMatcherOrError =
   createAndImportSelDAGMatcher(M, InsnMatcherTemp, Src, TempOpIdx);
   if (auto Error = InsnMatcherOrError.takeError())
-return std::move(Error);
+return Error;
   InstructionMatcher  = InsnMatcherOrError.get();
 
   if (Dst->isLeaf()) {
@@ -4697,7 +4697,7 @@
   // We're done with this pattern!  It's eligible for GISel emission; return
   // it.
   ++NumPatternImported;
-  return std::move(M);
+  return M;
 }
 
 return failedImport("Dst pattern root isn't a known leaf");
@@ -4787,13 +4787,13 @@
   auto DstMIBuilderOrError 

[PATCH] D71213: [Alignment][NFC] CreateMemSet use MaybeAlign

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

In D71213#1781322 , @gchatelet wrote:

> LLVM has this `LLVM_ATTRIBUTE_DEPRECATED` macro, it's convenient to get a 
> warning but it only works when building without `-Wall`.


Did you mean to write _with_ -Wall? I fail to see anything that stops the macro 
from working with -Wall?

Generally, I'd say using that macro a bit more for this kind of thing would be 
a good idea.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71213



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