[clang] ddb0101 - Revert "[analyzer] RetainCountChecker: Add a suppression for OSSymbols."

[Artem Dergachev via cfe-commits]

Author: Artem Dergachev
Date: 2021-02-09T23:44:33-08:00
New Revision: ddb01010b275c68deb7340ec32e0d1beaa9bbf13

URL: 
https://github.com/llvm/llvm-project/commit/ddb01010b275c68deb7340ec32e0d1beaa9bbf13
DIFF: 
https://github.com/llvm/llvm-project/commit/ddb01010b275c68deb7340ec32e0d1beaa9bbf13.diff

LOG: Revert "[analyzer] RetainCountChecker: Add a suppression for OSSymbols."

This reverts commit 3500cc8d891bb3825bb3275affe6db8b12f2f695.

This old commit was made over a completely false premise. OSSymbols
aren't different from other OSObjects and we shouldn't treat them
differently for the purposes of static analysis.

Added: 


Modified: 
clang/lib/Analysis/RetainSummaryManager.cpp
clang/test/Analysis/osobject-retain-release.cpp

Removed: 




diff  --git a/clang/lib/Analysis/RetainSummaryManager.cpp 
b/clang/lib/Analysis/RetainSummaryManager.cpp
index 9f45a8efe546..00bc854a8804 100644
--- a/clang/lib/Analysis/RetainSummaryManager.cpp
+++ b/clang/lib/Analysis/RetainSummaryManager.cpp
@@ -146,9 +146,7 @@ static bool isSubclass(const Decl *D,
 }
 
 static bool isOSObjectSubclass(const Decl *D) {
-  // OSSymbols are particular OSObjects that are allocated globally
-  // and therefore aren't really refcounted, so we ignore them.
-  return D && isSubclass(D, "OSMetaClassBase") && !isSubclass(D, "OSSymbol");
+  return D && isSubclass(D, "OSMetaClassBase");
 }
 
 static bool isOSObjectDynamicCast(StringRef S) {

diff  --git a/clang/test/Analysis/osobject-retain-release.cpp 
b/clang/test/Analysis/osobject-retain-release.cpp
index d88349dcd807..41606a30c39f 100644
--- a/clang/test/Analysis/osobject-retain-release.cpp
+++ b/clang/test/Analysis/osobject-retain-release.cpp
@@ -53,9 +53,6 @@ struct MyArray : public OSArray {
   OSObject *generateObject(OSObject *input) override;
 };
 
-// These are never refcounted.
-struct OSSymbol : OSObject {};
-
 struct OtherStruct {
   static void doNothingToArray(OSArray *array);
   OtherStruct(OSArray *arr);
@@ -757,10 +754,3 @@ void test() {
   b(0);
 }
 } // namespace inherited_constructor_crash
-
-namespace ossymbol_suppression {
-OSSymbol *createSymbol();
-void test() {
-  OSSymbol *sym = createSymbol(); // no-warning
-}
-} // namespace ossymbol_suppression



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

[PATCH] D94973: [clang][OpenMP] Use OpenMPIRBuilder for workshare loops.

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added inline comments.



Comment at: clang/include/clang/AST/StmtOpenMP.h:97
+/// `std::vector::iterator::difference_type` aka `ptrdiff_t`).
+/// Therefore, the distance function will be 
+///   [&](size_t ) { Result = __end - __begin; }

jdenny wrote:
> 
Unfortunately, clang-format does this when it wants to word-wrap a paragraph.



Comment at: clang/include/clang/AST/StmtOpenMP.h:166
+assert((isa(S) || isa(S)) &&
+   "Canonical loop must be a for loop (range-based or otherwise)");
+SubStmts[LOOPY_STMT] = S;

jdenny wrote:
> To convert run-time errors into compile-time errors, what if `setLoopStmt` is 
> overloaded to take either a `ForStmt` or `CXXForRangeStmt` instead of any 
> `Stmt`?
This would also require callers to call two different versions and just removes 
the problem up the call stack. For instance, StmtReader just receives a Stmt 
from ReadStmt. If we had setLoopStmt, we'd need a switch to call one of 
overloads.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94973

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

[PATCH] D95536: [clang][sema] Note decl location on missing member

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a comment.

In D95536#2552258 , @aaron.ballman 
wrote:

> In D95536#2551332 , @tbaeder wrote:
>
>> Any update on this?
>
> Thank you for the patch! Do you have some motivating examples of when this 
> would really add clarity to the diagnostic? I'm not opposed to the patch per 
> se, but I'm a bit wary about adding an additional note because that makes the 
> diagnostic output seem that much longer, which makes the salient diagnostics 
> a bit harder to find (colorization in the terminal helps with this, though).

I run into this a lot when looking into a larger, new code base. When adding 
new code, I often look at surrounding code and infer how e.g. an enum member is 
called. Take https://godbolt.org/z/Tcoxf5 for example, I could've seen code 
using `OsType::Unix` and inferred that the OsType for Windows will be called 
`OsType::Windows`, but it's `Win32`. The next step for me as a human is of 
course to grep the source code for the declaration of `OsType` and check the 
members. (On the other hand, a "Foo is not a member of enum FooEnum, existing 
members are: Bar1, Bar2, Bar3, ..." diagnostic would probably be more useful. 
But that has its own drawbacks).


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

https://reviews.llvm.org/D95536

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

[PATCH] D95915: [clang][driver] Only warn once about invalid -stdlib value

[Douglas Yung via Phabricator via cfe-commits]

dyung added a comment.

It also failed on the PS4 linux bot: 
http://lab.llvm.org:8014/#/builders/125/builds/125


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95915

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

[PATCH] D95915: [clang][driver] Only warn once about invalid -stdlib value

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a comment.

Looks like this broke the tests because `-stdlib` is not a thing on Windows? 
Would passing a linux `-target x86_64-linux-gnu` be an appropriate fix for the 
tests?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95915

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

[PATCH] D90457: [clang][driver] Set LTO mode based on input files

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a comment.

I don't think `.bc` files are affected by this patch since they aren't 
`types::TY_Object` files, but I might misremember.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90457

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

[PATCH] D95915: [clang][driver] Only warn once about invalid -stdlib value

[Timm Bäder via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa6439b52088b: [clang][driver] Only warn once about invalid 
library values (authored by tbaeder).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95915

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/ToolChain.cpp
  clang/test/Driver/undefined-libs.cpp

Index: clang/test/Driver/undefined-libs.cpp
===
--- /dev/null
+++ clang/test/Driver/undefined-libs.cpp
@@ -0,0 +1,15 @@
+
+// Check that all the following options print a warning when given a non-existent
+// value. But only one warning.
+
+// RUN: not %clangxx -stdlib=nostdlib %s 2>&1 | FileCheck --check-prefix=STDLIB %s
+// STDLIB: error: invalid library name in argument '-stdlib=nostdlib'
+// STDLIB-EMPTY:
+//
+// RUN: not %clangxx -rtlib=nortlib %s 2>&1 | FileCheck --check-prefix=RTLIB %s
+// RTLIB: error: invalid runtime library name in argument '-rtlib=nortlib'
+// RTLIB-EMPTY:
+//
+// RUN: not %clangxx -unwindlib=nounwindlib %s 2>&1 | FileCheck --check-prefix=UNWINDLIB %s
+// UNWINDLIB: error: invalid unwind library name in argument '-unwindlib=nounwindlib'
+// UNWINDLIB-EMPTY:
Index: clang/lib/Driver/ToolChain.cpp
===
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -884,66 +884,86 @@
 
 ToolChain::RuntimeLibType ToolChain::GetRuntimeLibType(
 const ArgList ) const {
+  if (runtimeLibType)
+return *runtimeLibType;
+
   const Arg* A = Args.getLastArg(options::OPT_rtlib_EQ);
   StringRef LibName = A ? A->getValue() : CLANG_DEFAULT_RTLIB;
 
   // Only use "platform" in tests to override CLANG_DEFAULT_RTLIB!
   if (LibName == "compiler-rt")
-return ToolChain::RLT_CompilerRT;
+runtimeLibType = ToolChain::RLT_CompilerRT;
   else if (LibName == "libgcc")
-return ToolChain::RLT_Libgcc;
+runtimeLibType = ToolChain::RLT_Libgcc;
   else if (LibName == "platform")
-return GetDefaultRuntimeLibType();
+runtimeLibType = GetDefaultRuntimeLibType();
+  else {
+if (A)
+  getDriver().Diag(diag::err_drv_invalid_rtlib_name)
+  << A->getAsString(Args);
 
-  if (A)
-getDriver().Diag(diag::err_drv_invalid_rtlib_name) << A->getAsString(Args);
+runtimeLibType = GetDefaultRuntimeLibType();
+  }
 
-  return GetDefaultRuntimeLibType();
+  return *runtimeLibType;
 }
 
 ToolChain::UnwindLibType ToolChain::GetUnwindLibType(
 const ArgList ) const {
+  if (unwindLibType)
+return *unwindLibType;
+
   const Arg *A = Args.getLastArg(options::OPT_unwindlib_EQ);
   StringRef LibName = A ? A->getValue() : CLANG_DEFAULT_UNWINDLIB;
 
   if (LibName == "none")
-return ToolChain::UNW_None;
+unwindLibType = ToolChain::UNW_None;
   else if (LibName == "platform" || LibName == "") {
 ToolChain::RuntimeLibType RtLibType = GetRuntimeLibType(Args);
 if (RtLibType == ToolChain::RLT_CompilerRT)
-  return ToolChain::UNW_None;
+  unwindLibType = ToolChain::UNW_None;
 else if (RtLibType == ToolChain::RLT_Libgcc)
-  return ToolChain::UNW_Libgcc;
+  unwindLibType = ToolChain::UNW_Libgcc;
   } else if (LibName == "libunwind") {
 if (GetRuntimeLibType(Args) == RLT_Libgcc)
   getDriver().Diag(diag::err_drv_incompatible_unwindlib);
-return ToolChain::UNW_CompilerRT;
+unwindLibType = ToolChain::UNW_CompilerRT;
   } else if (LibName == "libgcc")
-return ToolChain::UNW_Libgcc;
+unwindLibType = ToolChain::UNW_Libgcc;
+  else {
+if (A)
+  getDriver().Diag(diag::err_drv_invalid_unwindlib_name)
+  << A->getAsString(Args);
 
-  if (A)
-getDriver().Diag(diag::err_drv_invalid_unwindlib_name)
-<< A->getAsString(Args);
+unwindLibType = GetDefaultUnwindLibType();
+  }
 
-  return GetDefaultUnwindLibType();
+  return *unwindLibType;
 }
 
 ToolChain::CXXStdlibType ToolChain::GetCXXStdlibType(const ArgList ) const{
+  if (cxxStdlibType)
+return *cxxStdlibType;
+
   const Arg *A = Args.getLastArg(options::OPT_stdlib_EQ);
   StringRef LibName = A ? A->getValue() : CLANG_DEFAULT_CXX_STDLIB;
 
   // Only use "platform" in tests to override CLANG_DEFAULT_CXX_STDLIB!
   if (LibName == "libc++")
-return ToolChain::CST_Libcxx;
+cxxStdlibType = ToolChain::CST_Libcxx;
   else if (LibName == "libstdc++")
-return ToolChain::CST_Libstdcxx;
+cxxStdlibType = ToolChain::CST_Libstdcxx;
   else if (LibName == "platform")
-return GetDefaultCXXStdlibType();
+cxxStdlibType = GetDefaultCXXStdlibType();
+  else {
+if (A)
+  getDriver().Diag(diag::err_drv_invalid_stdlib_name)
+  << A->getAsString(Args);
 
-  if (A)
-getDriver().Diag(diag::err_drv_invalid_stdlib_name) << A->getAsString(Args);
+cxxStdlibType = GetDefaultCXXStdlibType();
+  }
 
-  return GetDefaultCXXStdlibType();
+  return 

[clang] a6439b5 - [clang][driver] Only warn once about invalid library values

[Timm Bäder via cfe-commits]

Author: Timm Bäder
Date: 2021-02-10T06:19:52+01:00
New Revision: a6439b52088b1d58d8e7aa9891c9011648710593

URL: 
https://github.com/llvm/llvm-project/commit/a6439b52088b1d58d8e7aa9891c9011648710593
DIFF: 
https://github.com/llvm/llvm-project/commit/a6439b52088b1d58d8e7aa9891c9011648710593.diff

LOG: [clang][driver] Only warn once about invalid library values

Since ToolChain::GetCXXStdlibType() is a simple getter that might emit
the "invalid library name in argument" warning, it can conceivably be
called several times while initializing the build pipeline.

Before this patch, a simple 'clang++ -stdlib=foo ./test.cpp' would print
the warning twice, -rt=lib=foo would print 6 times.

Change this and always only print the warning once. Keep the rest of the
semantics of the functions.

Differential Revision: https://reviews.llvm.org/D95915

Added: 
clang/test/Driver/undefined-libs.cpp

Modified: 
clang/include/clang/Driver/ToolChain.h
clang/lib/Driver/ToolChain.cpp

Removed: 




diff  --git a/clang/include/clang/Driver/ToolChain.h 
b/clang/include/clang/Driver/ToolChain.h
index 59fdd2997fec..fed688c0f1ce 100644
--- a/clang/include/clang/Driver/ToolChain.h
+++ b/clang/include/clang/Driver/ToolChain.h
@@ -166,6 +166,10 @@ class ToolChain {
 EffectiveTriple = std::move(ET);
   }
 
+  mutable llvm::Optional cxxStdlibType;
+  mutable llvm::Optional runtimeLibType;
+  mutable llvm::Optional unwindLibType;
+
 protected:
   MultilibSet Multilibs;
   Multilib SelectedMultilib;

diff  --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index c83638086048..d0f404d8cbaa 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -884,66 +884,86 @@ void ToolChain::addProfileRTLibs(const llvm::opt::ArgList 
,
 
 ToolChain::RuntimeLibType ToolChain::GetRuntimeLibType(
 const ArgList ) const {
+  if (runtimeLibType)
+return *runtimeLibType;
+
   const Arg* A = Args.getLastArg(options::OPT_rtlib_EQ);
   StringRef LibName = A ? A->getValue() : CLANG_DEFAULT_RTLIB;
 
   // Only use "platform" in tests to override CLANG_DEFAULT_RTLIB!
   if (LibName == "compiler-rt")
-return ToolChain::RLT_CompilerRT;
+runtimeLibType = ToolChain::RLT_CompilerRT;
   else if (LibName == "libgcc")
-return ToolChain::RLT_Libgcc;
+runtimeLibType = ToolChain::RLT_Libgcc;
   else if (LibName == "platform")
-return GetDefaultRuntimeLibType();
+runtimeLibType = GetDefaultRuntimeLibType();
+  else {
+if (A)
+  getDriver().Diag(diag::err_drv_invalid_rtlib_name)
+  << A->getAsString(Args);
 
-  if (A)
-getDriver().Diag(diag::err_drv_invalid_rtlib_name) << A->getAsString(Args);
+runtimeLibType = GetDefaultRuntimeLibType();
+  }
 
-  return GetDefaultRuntimeLibType();
+  return *runtimeLibType;
 }
 
 ToolChain::UnwindLibType ToolChain::GetUnwindLibType(
 const ArgList ) const {
+  if (unwindLibType)
+return *unwindLibType;
+
   const Arg *A = Args.getLastArg(options::OPT_unwindlib_EQ);
   StringRef LibName = A ? A->getValue() : CLANG_DEFAULT_UNWINDLIB;
 
   if (LibName == "none")
-return ToolChain::UNW_None;
+unwindLibType = ToolChain::UNW_None;
   else if (LibName == "platform" || LibName == "") {
 ToolChain::RuntimeLibType RtLibType = GetRuntimeLibType(Args);
 if (RtLibType == ToolChain::RLT_CompilerRT)
-  return ToolChain::UNW_None;
+  unwindLibType = ToolChain::UNW_None;
 else if (RtLibType == ToolChain::RLT_Libgcc)
-  return ToolChain::UNW_Libgcc;
+  unwindLibType = ToolChain::UNW_Libgcc;
   } else if (LibName == "libunwind") {
 if (GetRuntimeLibType(Args) == RLT_Libgcc)
   getDriver().Diag(diag::err_drv_incompatible_unwindlib);
-return ToolChain::UNW_CompilerRT;
+unwindLibType = ToolChain::UNW_CompilerRT;
   } else if (LibName == "libgcc")
-return ToolChain::UNW_Libgcc;
+unwindLibType = ToolChain::UNW_Libgcc;
+  else {
+if (A)
+  getDriver().Diag(diag::err_drv_invalid_unwindlib_name)
+  << A->getAsString(Args);
 
-  if (A)
-getDriver().Diag(diag::err_drv_invalid_unwindlib_name)
-<< A->getAsString(Args);
+unwindLibType = GetDefaultUnwindLibType();
+  }
 
-  return GetDefaultUnwindLibType();
+  return *unwindLibType;
 }
 
 ToolChain::CXXStdlibType ToolChain::GetCXXStdlibType(const ArgList ) 
const{
+  if (cxxStdlibType)
+return *cxxStdlibType;
+
   const Arg *A = Args.getLastArg(options::OPT_stdlib_EQ);
   StringRef LibName = A ? A->getValue() : CLANG_DEFAULT_CXX_STDLIB;
 
   // Only use "platform" in tests to override CLANG_DEFAULT_CXX_STDLIB!
   if (LibName == "libc++")
-return ToolChain::CST_Libcxx;
+cxxStdlibType = ToolChain::CST_Libcxx;
   else if (LibName == "libstdc++")
-return ToolChain::CST_Libstdcxx;
+cxxStdlibType = ToolChain::CST_Libstdcxx;
   else if (LibName == "platform")
-return GetDefaultCXXStdlibType();
+

[PATCH] D96360: [CMake] Delete LLVM_RUNTIME_BUILD_ID_LINK_TARGETS

[Petr Hosek via Phabricator via cfe-commits]

phosek accepted this revision.
phosek 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/D96360/new/

https://reviews.llvm.org/D96360

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

[PATCH] D93003: [libunwind][ELF] Hide unw_getcontext with LIBUNWIND_HIDE_SYMBOLS

[Ryan Prichard via Phabricator via cfe-commits]

rprichard added a subscriber: emaste.
rprichard added a comment.

emaste: Just a heads up in case FreeBSD is affected. FWIW, I noticed that 
libgcc_eh.a on FreeBSD 12.1 doesn't hide the `_Unwind_*` or `unw_*` symbols. 
The build system 

 is defining `VISIBILITY_HIDDEN` but libunwind doesn't respond to that name. 
FreeBSD libgcc_eh.a also defines hidden symbols named `logAPIs`, 
`logUnwinding`, and `logDWARF`. libunwind defines these internal names if 
`NDEBUG` isn't defined. They can break statically-linked programs on FreeBSD 
(e.g. duplicate symbol linker errors).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93003

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

[PATCH] D94476: [analyzer] Implement conversion from Clang diagnostics to PathDiagnostics.

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus accepted this revision.
Szelethus added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: nullptr.cpp.

LGTM! As a side note, are we aware of anyone that uses short messages instead 
of the full length one?




Comment at: 
clang/lib/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp:69-70
+
+  // For now we assume that every diagnostic is either a warning or a note
+  // logically attached to a previous warning.
+  // Notes do not come in actually attached to their respective warnings

For later, consider testing and handling `-werror` and `-analyzer-werror`.



Comment at: 
clang/unittests/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp:139
+
+class PathDiagnosticConverterDiagnosticConsumerTestAction
+: public ASTFrontendAction {

NoQ wrote:
> steakhal wrote:
> > vsavchenko wrote:
> > > WE NEED MORE NOUNS!
> > What about calling this `DiagnosticMock`?
> We still need to discriminate between `FrontendAction` mock, `ASTConsumer` 
> mock, and `PathDiagnosticConsumer` mock. Dropped some of the nouns though.
Do we need really need the postfix `DiagnosticConsumer` everywhere? Can't we 
convert it into a namespace? Maybe at least for `PathDiagnosticConsumer`s? Its 
not a big deal, and it is certainly descriptive, but looks a bit funny.

Also, RIP `ClangDiagPathDiagConsumer`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94476

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

[PATCH] D93003: [libunwind][ELF] Hide unw_getcontext with LIBUNWIND_HIDE_SYMBOLS

[Ryan Prichard via Phabricator via cfe-commits]

rprichard added inline comments.



Comment at: libunwind/src/assembly.h:82
   .globl SYMBOL_NAME(aliasname) SEPARATOR  
\
-  WEAK_SYMBOL(aliasname) SEPARATOR 
\
+  EXPORT_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR  
\
   SYMBOL_NAME(aliasname) = SYMBOL_NAME(name)

rprichard wrote:
> steven_wu wrote:
> > rprichard wrote:
> > > compnerd wrote:
> > > > Does this not change the behaviour on MachO?  This symbol is now 
> > > > `private_extern` rather than a `weak_reference`.  A weak reference will 
> > > > be set to 0 by the loader if it is not found, and a `private_extern` is 
> > > > a strong internal reference.
> > > Is `.weak_reference` the right directive to use here, instead of 
> > > `.weak_definition`? We're defining a symbol (`aliasname`) and setting its 
> > > value to that of another symbol (`name`).
> > > 
> > > I think marking `unw_*` weak is intended to let some other strong 
> > > definition override it. Its value won't ever be set to 0.
> > > 
> > > Currently on Mach-O, the hide-symbols flag hides almost everything 
> > > (including `_Unwind_*`) but leaves all of the `unw_*` alias symbols as 
> > > extern (and not private-extern) and not weak. With my change, they're 
> > > still not weak, but they're private-extern.
> > > 
> > > libunwind's current assembly.h behavior for a weak alias:
> > > 
> > > .globl aliasname
> > > .weak_reference aliasname
> > > aliasname = name
> > > 
> > > The LLVM Mach-O assembler ignores the `.weak_reference` directive. If I 
> > > change it to `.weak_definition`, it is still ignored. AFAICT, the LLVM 
> > > assembler uses the WeakDef/WeakRef attributes from the `name` symbol.
> > > 
> > > e.g.
> > > 
> > > ```
> > > $ cat test.S
> > > .text
> > > .space 0x42
> > > 
> > > // Define foo.
> > > .globl foo
> > > foo:
> > > ret
> > > 
> > > // Define a weak alias, bar.
> > > .globl bar
> > > .weak_reference bar
> > > bar = foo
> > > 
> > > $ ~/clang11/bin/clang test.S -c && ~/clang11/bin/llvm-readobj --syms 
> > > test.o
> > > 
> > > File: test.o
> > > Format: Mach-O 64-bit x86-64
> > > Arch: x86_64
> > > AddressSize: 64bit
> > > Symbols [
> > >   Symbol {
> > > Name: bar (1)
> > > Extern
> > > Type: Section (0xE)
> > > Section: __text (0x1)
> > > RefType: UndefinedNonLazy (0x0)
> > > Flags [ (0x0)
> > > ]
> > > Value: 0x42
> > >   }
> > >   Symbol {
> > > Name: foo (5)
> > > Extern
> > > Type: Section (0xE)
> > > Section: __text (0x1)
> > > RefType: UndefinedNonLazy (0x0)
> > > Flags [ (0x0)
> > > ]
> > > Value: 0x42
> > >   }
> > > ]
> > > ```
> > > 
> > > The Flags are empty.
> > > 
> > > OTOH, if I flip things around:
> > > 
> > > ```
> > > .text
> > > .space 0x42
> > > 
> > > // Define a weak function, foo.
> > > .globl foo
> > > .weak_reference foo
> > > foo:
> > > ret
> > > 
> > > // Define an alias, bar.
> > > .globl bar
> > > bar = foo
> > > ```
> > > 
> > > Now both symbols are WeakRef:
> > > 
> > > ```
> > > File: test.o
> > > Format: Mach-O 64-bit x86-64
> > > Arch: x86_64
> > > AddressSize: 64bit
> > > Symbols [
> > >   Symbol {
> > > Name: bar (1)
> > > Extern
> > > Type: Section (0xE)
> > > Section: __text (0x1)
> > > RefType: UndefinedNonLazy (0x0)
> > > Flags [ (0x40)
> > >   WeakRef (0x40)
> > > ]
> > > Value: 0x42
> > >   }
> > >   Symbol {
> > > Name: foo (5)
> > > Extern
> > > Type: Section (0xE)
> > > Section: __text (0x1)
> > > RefType: UndefinedNonLazy (0x0)
> > > Flags [ (0x40)
> > >   WeakRef (0x40)
> > > ]
> > > Value: 0x42
> > >   }
> > > ]
> > > ```
> > > 
> > > I'm wondering if this LLVM behavior is actually correct, but I'm also 
> > > unfamiliar with Mach-O. (i.e. We want to copy the symbol's address, but 
> > > should we be copying the WeakRef/WeakDef flags?) It looks like the 
> > > behavior is controlled in `MachObjectWriter::writeNlist`. `writeNlist` 
> > > finds the aliased symbol and uses its flags:
> > > ```
> > >   // The Mach-O streamer uses the lowest 16-bits of the flags for the 
> > > 'desc'
> > >   // value.
> > >   bool EncodeAsAltEntry =
> > > IsAlias && cast(OrigSymbol).isAltEntry();
> > >   
> > > W.write(cast(Symbol)->getEncodedFlags(EncodeAsAltEntry));
> > > ```
> > > 
> > > The PrivateExtern attribute, on the other hand, isn't part of these 
> > > encoded flags:
> > > ```
> > >   if (Data.isPrivateExtern())
> > > Type |= MachO::N_PEXT;
> > > ```
> > > `Data` continues to refer to the alias symbol rather than the aliased 
> > > symbol. However, apparently the author isn't completely sure about things?
> > > ```
> > > // FIXME: Should this update Data as well?
> > > ```
> > > 
> > > In libunwind, there is one usage of assembly.h's WEAK_ALIAS. 
> > > 

[PATCH] D93003: [libunwind][ELF] Hide unw_getcontext with LIBUNWIND_HIDE_SYMBOLS

[Ryan Prichard via Phabricator via cfe-commits]

rprichard updated this revision to Diff 322574.
rprichard edited the summary of this revision.
rprichard added a comment.

Restore Mach-O alias fixes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93003

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake
  libunwind/CMakeLists.txt
  libunwind/src/CMakeLists.txt
  libunwind/src/assembly.h
  libunwind/src/config.h

Index: libunwind/src/config.h
===
--- libunwind/src/config.h
+++ libunwind/src/config.h
@@ -52,7 +52,8 @@
   #endif
 #endif
 
-#if defined(_LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS)
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
+  // The CMake file passes -fvisibility=hidden to control ELF/Mach-O visibility.
   #define _LIBUNWIND_EXPORT
   #define _LIBUNWIND_HIDDEN
 #else
@@ -70,11 +71,15 @@
 #define SYMBOL_NAME(name) XSTR(__USER_LABEL_PREFIX__) #name
 
 #if defined(__APPLE__)
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
+#define _LIBUNWIND_ALIAS_VISIBILITY(name) __asm__(".private_extern " name)
+#else
+#define _LIBUNWIND_ALIAS_VISIBILITY(name)
+#endif
 #define _LIBUNWIND_WEAK_ALIAS(name, aliasname) \
   __asm__(".globl " SYMBOL_NAME(aliasname));   \
   __asm__(SYMBOL_NAME(aliasname) " = " SYMBOL_NAME(name)); \
-  extern "C" _LIBUNWIND_EXPORT __typeof(name) aliasname\
-  __attribute__((weak_import));
+  _LIBUNWIND_ALIAS_VISIBILITY(SYMBOL_NAME(aliasname));
 #elif defined(__ELF__)
 #define _LIBUNWIND_WEAK_ALIAS(name, aliasname) \
   extern "C" _LIBUNWIND_EXPORT __typeof(name) aliasname\
Index: libunwind/src/assembly.h
===
--- libunwind/src/assembly.h
+++ libunwind/src/assembly.h
@@ -70,12 +70,15 @@
 #if defined(__APPLE__)
 
 #define SYMBOL_IS_FUNC(name)
-#define EXPORT_SYMBOL(name)
 #define HIDDEN_SYMBOL(name) .private_extern name
-#define WEAK_SYMBOL(name) .weak_reference name
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
+#define EXPORT_SYMBOL(name) HIDDEN_SYMBOL(name)
+#else
+#define EXPORT_SYMBOL(name)
+#endif
 #define WEAK_ALIAS(name, aliasname)\
   .globl SYMBOL_NAME(aliasname) SEPARATOR  \
-  WEAK_SYMBOL(aliasname) SEPARATOR \
+  EXPORT_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR  \
   SYMBOL_NAME(aliasname) = SYMBOL_NAME(name)
 
 #define NO_EXEC_STACK_DIRECTIVE
@@ -87,17 +90,23 @@
 #else
 #define SYMBOL_IS_FUNC(name) .type name,@function
 #endif
-#define EXPORT_SYMBOL(name)
 #define HIDDEN_SYMBOL(name) .hidden name
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
+#define EXPORT_SYMBOL(name) HIDDEN_SYMBOL(name)
+#else
+#define EXPORT_SYMBOL(name)
+#endif
 #define WEAK_SYMBOL(name) .weak name
 
 #if defined(__hexagon__)
-#define WEAK_ALIAS(name, aliasname) \
-  WEAK_SYMBOL(aliasname) SEPARATOR \
+#define WEAK_ALIAS(name, aliasname)\
+  EXPORT_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR  \
+  WEAK_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR\
   .equiv SYMBOL_NAME(aliasname), SYMBOL_NAME(name)
 #else
 #define WEAK_ALIAS(name, aliasname)\
-  WEAK_SYMBOL(aliasname) SEPARATOR \
+  EXPORT_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR  \
+  WEAK_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR\
   SYMBOL_NAME(aliasname) = SYMBOL_NAME(name)
 #endif
 
@@ -119,7 +128,7 @@
   .section .drectve,"yn" SEPARATOR \
   .ascii "-export:", #name, "\0" SEPARATOR \
   .text
-#if defined(_LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS)
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
 #define EXPORT_SYMBOL(name)
 #else
 #define EXPORT_SYMBOL(name) EXPORT_SYMBOL2(name)
Index: libunwind/src/CMakeLists.txt
===
--- libunwind/src/CMakeLists.txt
+++ libunwind/src/CMakeLists.txt
@@ -160,11 +160,11 @@
 LINKER_LANGUAGE C
 OUTPUT_NAME "unwind")
 
-  if(LIBUNWIND_HERMETIC_STATIC_LIBRARY)
+  if(LIBUNWIND_HIDE_SYMBOLS)
 append_flags_if_supported(UNWIND_STATIC_LIBRARY_FLAGS -fvisibility=hidden)
 append_flags_if_supported(UNWIND_STATIC_LIBRARY_FLAGS -fvisibility-global-new-delete-hidden)
 target_compile_options(unwind_static PRIVATE ${UNWIND_STATIC_LIBRARY_FLAGS})
-target_compile_definitions(unwind_static PRIVATE _LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS)
+target_compile_definitions(unwind_static PRIVATE _LIBUNWIND_HIDE_SYMBOLS)
   endif()
 
   list(APPEND 

[PATCH] D93222: [analyzer] Introduce MacroExpansionContext to libAnalysis

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added inline comments.



Comment at: clang/include/clang/Analysis/MacroExpansionContext.h:105
+  using MacroExpansionText = SmallString<40>;
+  using ExpansionMap = llvm::DenseMap;
+  using ExpansionRangeMap = llvm::DenseMap;

Hmm, I'm by no means an expert, but isn't a string-like structure a bit big for 
a `DenseMap`?


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

https://reviews.llvm.org/D93222

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

[PATCH] D93224: [analyzer] Use the MacroExpansionContext for macro expansions in plists

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus accepted this revision.
Szelethus added a comment.
Herald added a subscriber: nullptr.cpp.

Happy to see this mess go. It was obvious even after the first few hurdles that 
it wouldn't work out in the long term.




Comment at: clang/test/Analysis/plist-macros-with-expansion.cpp:137
 
-// TODO: Expand arguments.
-// CHECK: namePASS_PTR_TO_MACRO_THAT_WILL_BE_UNDEFD

Nice.


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

https://reviews.llvm.org/D93224

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

[clang] 66ac53f - [clang][cli] Fix gcc "enumeral and non-enumeral type in conditional expression" warning (NFC)

[Yang Fan via cfe-commits]

Author: Yang Fan
Date: 2021-02-10T11:15:39+08:00
New Revision: 66ac53fe319be932d31ef96bdca34ad2b897cf04

URL: 
https://github.com/llvm/llvm-project/commit/66ac53fe319be932d31ef96bdca34ad2b897cf04
DIFF: 
https://github.com/llvm/llvm-project/commit/66ac53fe319be932d31ef96bdca34ad2b897cf04.diff

LOG: [clang][cli] Fix gcc "enumeral and non-enumeral type in conditional 
expression" warning (NFC)

Added: 


Modified: 
clang/lib/Frontend/CompilerInvocation.cpp

Removed: 




diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index 928009ebf2f4..d06b2c9e677f 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -386,7 +386,7 @@ template  static T extractForwardValue(T 
KeyPath) {
 
 template 
 static T extractMaskValue(T KeyPath) {
-  return ((KeyPath & Value) == Value) ? Value : T();
+  return ((KeyPath & Value) == Value) ? static_cast(Value) : T();
 }
 
 #define PARSE_OPTION_WITH_MARSHALLING(ARGS, DIAGS, SUCCESS, ID, FLAGS, PARAM,  
\



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

[PATCH] D93223: [analyzer] Create MacroExpansionContext member in AnalysisConsumer

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus accepted this revision.
Szelethus added a comment.
Herald added a subscriber: nullptr.cpp.

My no1. thought is that I wish the new functionality you're implementing was in 
the interface of the `Preprocessor`. I found, and still find it so hard to 
believe that you couldn't just retrieve this information -- your projects seems 
to plug this gaping hole in the design. I respect that this is an opt-in 
functionality for now though, and I guess we could make this a default feature 
with relative ease.

The actual patch is straightforward, LGTM!


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

https://reviews.llvm.org/D93223

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

[PATCH] D93222: [analyzer] Introduce MacroExpansionContext to libAnalysis

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus accepted this revision.
Szelethus added a comment.
Herald added a subscriber: nullptr.cpp.

This is amazing. We longed for a sensible implementation for this for a long 
time. Really liking the unit tests as well!

There are a number of inlines you didn't mark as done but seem to have 
addressed.

> The main goal is to substitute the current macro expansion generator in the 
> PlistsDiagnostics, but all the other DiagnosticsConsumer could benefit from 
> this.

Good! Burn it, bury it, lets forget that it ever existed. It was a stupid idea 
from the get-go and got so much worse over time.




Comment at: clang/include/clang/Analysis/MacroExpansionContext.h:64-66
+/// \remark Currently we don't respect the whitespaces between expanded tokens,
+/// so the output for this example might differ from the -E compiler
+/// invocation.

That might be very tricky. I recall stumbling across the same issue when using 
the `Preprocessor`. I think I used this or something similar:

https://clang.llvm.org/doxygen/classclang_1_1Preprocessor.html#adb53a8b33c6ea7a5e0953126da5fb121



Comment at: clang/include/clang/Analysis/MacroExpansionContext.h:91
+
+  /// \param MacroExpansionLoc Must be the expansion location of a macro.
+  /// \return The text from the original source code which were substituted by

Consider mentioning `getExpansionLoc`, since that is almost always the source 
of the expansion loc (no other comes to my mind).



Comment at: clang/lib/Analysis/MacroExpansionContext.cpp:41-43
+  if (Range.getBegin() == Range.getEnd())
+return SM.getExpansionLoc(
+MacroName.getLocation().getLocWithOffset(MacroName.getLength()));

Well that is interesting, so the `Range` parameter is **not** (always?) the 
range of the expansion. [[ 
https://clang.llvm.org/doxygen/classclang_1_1PPCallbacks.html#a1f99f55fc3c5df84b152f9bb2057633f
 | Doxygen]] says that 

> Called by Preprocessor::HandleMacroExpandedIdentifier when a macro invocation 
> is found.

in which there is a TODO:

```
  // FIXME: We lose macro args info with delayed callback.
  Callbacks->MacroExpands(Info.Tok, Info.MD, Info.Range,
  /*Args=*/nullptr);
```

I suppose you're handling this corner case?



Comment at: clang/lib/Analysis/MacroExpansionContext.cpp:183-186
+// FIXME: For now, we don't respect whitespaces between macro expanded
+// tokens. We just emit a space after every identifier to produce a valid
+// code for `int a ;` like expansions.
+//  ^-^-- Space after the 'int' and 'a' identifiers.

In the  `TokenPrinter` in the plist implementation, I used 
`Preprocessor::getSpelling()`.


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

https://reviews.llvm.org/D93222

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

[PATCH] D96163: [analyzer] Add 12.0.0. release notes

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus updated this revision to Diff 322550.
Szelethus added a comment.

Fix'd!


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

https://reviews.llvm.org/D96163

Files:
  clang/docs/ReleaseNotes.rst
  clang/docs/analyzer/checkers.rst


Index: clang/docs/analyzer/checkers.rst
===
--- clang/docs/analyzer/checkers.rst
+++ clang/docs/analyzer/checkers.rst
@@ -2538,6 +2538,8 @@
 - casts
 - unary operators like ``&`` or ``*``
 
+.. _alpha-webkit-UncountedLocalVarsChecker:
+
 alpha.webkit.UncountedLocalVarsChecker
 ""
 The goal of this rule is to make sure that any uncounted local variable is 
backed by a ref-counted object with lifetime that is strictly larger than the 
scope of the uncounted local variable. To be on the safe side we require the 
scope of an uncounted variable to be embedded in the scope of ref-counted 
object that backs it.
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -318,7 +318,38 @@
 Static Analyzer
 ---
 
-- ...
+.. 3ff220de9009 [analyzer][StdLibraryFunctionsChecker] Add POSIX networking 
functions
+.. ...And a million other patches.
+- Improve the analyzer's understanding of several POSIX functions.
+
+.. https://reviews.llvm.org/D86533#2238207
+- Greatly improved the analyzer’s constraint solver by better understanding
+  when constraints are imposed on multiple symbolic values that are known to be
+  equal or known to be non-equal. It will now also efficiently reject 
impossible
+  if-branches between known comparison expressions. (Incorrectly stated as a
+  11.0.0 feature in the previous release notes)
+
+.. 820e8d8656ec [Analyzer][WebKit] UncountedLambdaCaptureChecker
+- New checker: 
:ref:`webkit.UncountedLambdaCapturesChecker`
+  is a WebKit coding convention checker that flags raw pointers to
+  reference-counted objects captured by lambdas and suggests using intrusive
+  reference-counting smart pointers instead.
+
+.. 8a64689e264c [Analyzer][WebKit] UncountedLocalVarsChecker
+- New checker: 
:ref:`alpha.webkit.UncountedLocalVarsChecker`
+  is a WebKit coding convention checker that intends to make sure that any
+  uncounted local variable is backed by a ref-counted object with lifetime that
+  is strictly larger than the scope of the uncounted local variable.
+
+.. i914f6c4ff8a4 [StaticAnalyzer] Support struct annotations in 
FuchsiaHandleChecker
+- ``fuchia.HandleChecker`` now recognizes handles in structs; All the handles
+  referenced by the structure (direct value or ptr) would be treated as
+  containing the release/use/acquire annotations directly.
+
+.. 8deaec122ec6 [analyzer] Update Fuchsia checker to catch releasing unowned 
handles.
+- Fuchsia checkers can detect the release of an unowned handle.
+
+- Numerous fixes and improvements to bug report generation.
 
 .. _release-notes-ubsan:
 


Index: clang/docs/analyzer/checkers.rst
===
--- clang/docs/analyzer/checkers.rst
+++ clang/docs/analyzer/checkers.rst
@@ -2538,6 +2538,8 @@
 - casts
 - unary operators like ``&`` or ``*``
 
+.. _alpha-webkit-UncountedLocalVarsChecker:
+
 alpha.webkit.UncountedLocalVarsChecker
 ""
 The goal of this rule is to make sure that any uncounted local variable is backed by a ref-counted object with lifetime that is strictly larger than the scope of the uncounted local variable. To be on the safe side we require the scope of an uncounted variable to be embedded in the scope of ref-counted object that backs it.
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -318,7 +318,38 @@
 Static Analyzer
 ---
 
-- ...
+.. 3ff220de9009 [analyzer][StdLibraryFunctionsChecker] Add POSIX networking functions
+.. ...And a million other patches.
+- Improve the analyzer's understanding of several POSIX functions.
+
+.. https://reviews.llvm.org/D86533#2238207
+- Greatly improved the analyzer’s constraint solver by better understanding
+  when constraints are imposed on multiple symbolic values that are known to be
+  equal or known to be non-equal. It will now also efficiently reject impossible
+  if-branches between known comparison expressions. (Incorrectly stated as a
+  11.0.0 feature in the previous release notes)
+
+.. 820e8d8656ec [Analyzer][WebKit] UncountedLambdaCaptureChecker
+- New checker: :ref:`webkit.UncountedLambdaCapturesChecker`
+  is a WebKit coding convention checker that flags raw pointers to
+  reference-counted objects captured by lambdas and suggests using intrusive
+  reference-counting smart pointers instead.
+
+.. 8a64689e264c [Analyzer][WebKit] UncountedLocalVarsChecker
+- New checker: 

[PATCH] D94973: [clang][OpenMP] Use OpenMPIRBuilder for workshare loops.

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

In D94973#2547383 , @jdoerfert wrote:

> I have a single last comment/request. @jdenny I'll take it you finish the 
> review and accept as you see fit.

@jdoerfert, if you've already reviewed everything and want to accept so this 
can move forward, I'm fine with that.  Otherwise, I'll review more as I find 
time.  There's a lot of code.




Comment at: clang/include/clang/AST/StmtOpenMP.h:45
+/// known before entering the loop and allow skipping to an arbitrary 
iteration.
+/// The OMPCanonicalLoop AST node wraps a ForStmt or CXXRangeForStmt that is
+/// known to fulfill OpenMP's canonical loop requirements.





Comment at: clang/include/clang/AST/StmtOpenMP.h:54
+///   for range-based for-statement, this is the hidden iterator '__begin'. For
+///   other loops, it is identical to the loop variable. Must be a 
random-access
+///   iterator, pointer or integer type.





Comment at: clang/include/clang/AST/StmtOpenMP.h:58
+///   incrementing by one at each iteration. Allows abstracting over the type
+///   of the loop counter and is always an unsigned integer type appropriate to
+///   represent the range of the loop counter variable. Its value corresponds 
to





Comment at: clang/include/clang/AST/StmtOpenMP.h:59
+///   of the loop counter and is always an unsigned integer type appropriate to
+///   represent the range of the loop counter variable. Its value corresponds 
to
+///   the logical iteration number in the OpenMP specification.





Comment at: clang/include/clang/AST/StmtOpenMP.h:97
+/// `std::vector::iterator::difference_type` aka `ptrdiff_t`).
+/// Therefore, the distance function will be 
+///   [&](size_t ) { Result = __end - __begin; }





Comment at: clang/include/clang/AST/StmtOpenMP.h:166
+assert((isa(S) || isa(S)) &&
+   "Canonical loop must be a for loop (range-based or otherwise)");
+SubStmts[LOOPY_STMT] = S;

To convert run-time errors into compile-time errors, what if `setLoopStmt` is 
overloaded to take either a `ForStmt` or `CXXForRangeStmt` instead of any 
`Stmt`?



Comment at: clang/include/clang/AST/StmtOpenMP.h:190
+
+  /// The function that compute the loop  user variable from a logical 
iteration
+  /// counter. Can be evaluated as first statement in the loop.





Comment at: clang/include/clang/AST/StmtOpenMP.h:196
+  /// value, step size) are captured by the closure. In particular, the initial
+  /// value of loop counter is captured by value to be unaffected by previous
+  /// iterations.





Comment at: clang/include/clang/AST/StmtOpenMP.h:92
+/// 
+///   [&,__begin](std::vector::iterator , size_t Logical) {
+///   Result = __begin + Logical; }

Meinersbur wrote:
> jdenny wrote:
> > Why is `__begin` an explicit capture here but not for the distance function?
> Because `__begin` is captured by-value, everything lese uses the `&`default 
> capture. This is the loop counter variable is modified inside the loop body.
> Because `__begin` is captured by-value, everything lese uses the `&`default 
> capture. This is the loop counter variable is modified inside the loop body.





Comment at: clang/include/clang/AST/StmtOpenMP.h:92
+/// 
+///   [&,__begin](std::vector::iterator , size_t Logical) {
+///   Result = __begin + Logical; }

jdenny wrote:
> Meinersbur wrote:
> > jdenny wrote:
> > > Why is `__begin` an explicit capture here but not for the distance 
> > > function?
> > Because `__begin` is captured by-value, everything lese uses the `&`default 
> > capture. This is the loop counter variable is modified inside the loop body.
> > Because `__begin` is captured by-value, everything lese uses the `&`default 
> > capture. This is the loop counter variable is modified inside the loop body.
> 
> 
Ah, I now see there are comments about this in the code.  I think a brief note 
here too would help because it stands out in the example, but your call.



Comment at: clang/include/clang/AST/StmtOpenMP.h:101
+  enum {
+LOOPY_STMT,
+DISTANCE_FUNC,

Meinersbur wrote:
> jdenny wrote:
> > Why "loopy" with a "y"?
> `loopy` was meant to refer to the property of the statement (ForStmt, 
> CXXForRangeStmt, potentially others such as WhileStmt, DoStmt, functions from 
> `#include `, etc) instead of a specific loop node (such as 
> OMPLoopDirective or OMPCanonicalLoop itself), although I did not apply this 
> idea consistently. Do you prefer a plain 'LOOP_STMT'?
> 
Yes.  In my mind, "loopy" doesn't help with that distinction.  But your call.



Comment at: clang/include/clang/Sema/Sema.h:10486
+
+  /// Called for syntactical loops 

[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

[Pengfei Wang via Phabricator via cfe-commits]

pengfei commandeered this revision.
pengfei edited reviewers, added: RKSimon; removed: pengfei.
pengfei added a comment.

@RKSimon Sure, with pleasure


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93179

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

[PATCH] D96381: [AArch64] Adding SHA3 Intrinsics support

[Ryan Santhirarajan via Phabricator via cfe-commits]

rsanthir.quic added a comment.

This is the second of three patches to address the following:
https://bugs.llvm.org/show_bug.cgi?id=47828


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96381

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

[PATCH] D93594: [X86] Pass to transform amx intrinsics to scalar operation.

[Xiang Zhang via Phabricator via cfe-commits]

xiangzhangllvm added inline comments.



Comment at: llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp:211-212
+IRBuilderBase , DomTreeUpdater ,
+LoopInfo , Value *Row, Value *Col,
+Value *K, Value *Acc, Value *LHS,
+Value *RHS) {

In fact, no need handle Row, Col, K here, just use fix size 16x16, the result 
of calculation is some in effective area. (just need tileload "keep" the 
"unused" area is 0). 
Then can use vector to handle all of the them, let type legalization to split 
the type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93594

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

[PATCH] D37538: [libc++] Remove problematic ADL in container implementations.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

Everything in this PR is obsolete and/or fixed at this point, //except// for 
the change to , which as Eric said, fixes the lower-hanging fruit but 
sadly can't "ADL-proof" it completely.
(Grepping for "ADL" in the git history of `libcxx/include/` will turn up a lot 
of patches from me and from Logan Smith, on the subject of ADL-proofing.)
I suggest that this PR could be closed at this point.


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

https://reviews.llvm.org/D37538

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

[PATCH] D96381: [AArch64] Adding SHA3 Intrinsics support

[Ryan Santhirarajan via Phabricator via cfe-commits]

rsanthir.quic created this revision.
rsanthir.quic added reviewers: apazos, t.p.northover, labrinea, pbarrio.
Herald added subscribers: danielkiss, hiraditya, kristof.beyls.
rsanthir.quic requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

  This patch adds the following SHA3 Intrinsics:
  vsha512hq_u64,
  vsha512h2q_u64,
  vsha512su0q_u64,
  vsha512su1q_u64
  veor3q_u8
  veor3q_u16
  veor3q_u32
  veor3q_u64
  veor3q_s8
  veor3q_s16
  veor3q_s32
  veor3q_s64
  vrax1q_u64
  vxarq_u64
  vbcaxq_u8
  vbcaxq_u16
  vbcaxq_u32
  vbcaxq_u64
  vbcaxq_s8
  vbcaxq_s16
  vbcaxq_s32
  vbcaxq_s64
  
  Note need to include +sha3 and +crypto when building from the front-end


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96381

Files:
  clang/include/clang/Basic/arm_neon.td
  clang/include/clang/Basic/arm_neon_incl.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/aarch64-neon-range-checks.c
  clang/test/CodeGen/aarch64-neon-sha3.c
  clang/utils/TableGen/NeonEmitter.cpp
  llvm/include/llvm/IR/IntrinsicsAArch64.td
  llvm/lib/Target/AArch64/AArch64InstrFormats.td
  llvm/lib/Target/AArch64/AArch64InstrInfo.td
  llvm/test/CodeGen/AArch64/neon-sha3.ll

Index: llvm/test/CodeGen/AArch64/neon-sha3.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/neon-sha3.ll
@@ -0,0 +1,105 @@
+; RUN: llc %s -mtriple=aarch64 -mattr=+v8.3a,+sha3 -o - | FileCheck %s
+
+define <2 x i64> @test_vsha512h(<2 x i64> %a, <2 x i64> %b, <2 x i64> %c) {
+; CHECK-LABEL: test_vsha512h:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:sha512h q0, q1, v2.2d
+; CHECK-NEXT:ret
+entry:
+  %vsha512h.i = tail call <2 x i64> @llvm.aarch64.crypto.sha512h(<2 x i64> %a, <2 x i64> %b, <2 x i64> %c)
+  ret <2 x i64> %vsha512h.i
+}
+
+define <2 x i64> @test_vsha512h2(<2 x i64> %a, <2 x i64> %b, <2 x i64> %c) {
+; CHECK-LABEL: test_vsha512h2:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:sha512h2 q0, q1, v2.2d
+; CHECK-NEXT:ret
+entry:
+  %vsha512h2.i = tail call <2 x i64> @llvm.aarch64.crypto.sha512h2(<2 x i64> %a, <2 x i64> %b, <2 x i64> %c)
+  ret <2 x i64> %vsha512h2.i
+}
+
+define <2 x i64> @test_vsha512su0(<2 x i64> %a, <2 x i64> %b) {
+; CHECK-LABEL: test_vsha512su0:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:sha512su0 v0.2d, v1.2d
+; CHECK-NEXT:ret
+entry:
+  %vsha512su0.i = tail call <2 x i64> @llvm.aarch64.crypto.sha512su0(<2 x i64> %a, <2 x i64> %b)
+  ret <2 x i64> %vsha512su0.i
+}
+
+define <2 x i64> @test_vsha512su1(<2 x i64> %a, <2 x i64> %b, <2 x i64> %c) {
+; CHECK-LABEL: test_vsha512su1:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:sha512su1 v0.2d, v1.2d, v2.2d
+; CHECK-NEXT:ret
+entry:
+  %vsha512su1.i = tail call <2 x i64> @llvm.aarch64.crypto.sha512su1(<2 x i64> %a, <2 x i64> %b, <2 x i64> %c)
+  ret <2 x i64> %vsha512su1.i
+}
+
+define <2 x i64> @test_vrax1(<2 x i64> %a, <2 x i64> %b) {
+; CHECK-LABEL: test_vrax1:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:rax1 v0.2d, v0.2d, v1.2d
+; CHECK-NEXT:ret
+entry:
+  %vrax1.i = tail call <2 x i64> @llvm.aarch64.crypto.rax1(<2 x i64> %a, <2 x i64> %b)
+  ret <2 x i64> %vrax1.i
+}
+
+define <2 x i64> @test_vxar(<2 x i64> %a,  <2 x i64> %b) {
+; CHECK-LABEL: test_vxar:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:xar v0.2d, v0.2d, v1.2d, #1
+; CHECK-NEXT:ret
+entry:
+  %vxar.i = tail call  <2 x i64> @llvm.aarch64.crypto.xar(<2 x i64> %a, <2 x i64> %b, i64 1)
+  ret <2 x i64> %vxar.i
+}
+
+define <16 x i8> @test_bcax_8(<16 x i8> %a, <16 x i8> %b, <16 x i8> %c) {
+; CHECK-LABEL: test_bcax_8:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:bcax v0.16b, v0.16b, v1.16b, v2.16b
+; CHECK-NEXT:ret
+entry:
+  %vbcax_8.i = tail call <16 x i8> @llvm.aarch64.crypto.bcaxu.v16i8(<16 x i8> %a, <16 x i8> %b, <16 x i8> %c)
+  ret <16 x i8> %vbcax_8.i
+}
+
+define <16 x i8> @test_eor3_8(<16 x i8> %a, <16 x i8> %b, <16 x i8> %c) {
+; CHECK-LABEL: test_eor3_8:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:eor3 v0.16b, v0.16b, v1.16b, v2.16b
+; CHECK-NEXT:ret
+entry:
+  %veor3_8.i = tail call <16 x i8> @llvm.aarch64.crypto.eor3u.v16i8(<16 x i8> %a, <16 x i8> %b, <16 x i8> %c)
+  ret <16 x i8> %veor3_8.i
+}
+
+declare <2 x i64> @llvm.aarch64.crypto.sha512h(<2 x i64>, <2 x i64>, <2 x i64>)
+declare <2 x i64> @llvm.aarch64.crypto.sha512h2(<2 x i64>, <2 x i64>, <2 x i64>)
+declare <2 x i64> @llvm.aarch64.crypto.sha512su0(<2 x i64>, <2 x i64>)
+declare <2 x i64> @llvm.aarch64.crypto.sha512su1(<2 x i64>, <2 x i64>, <2 x i64>)
+declare <2 x i64> @llvm.aarch64.crypto.rax1(<2 x i64>, <2 x i64>)
+declare <2 x i64> @llvm.aarch64.crypto.xar(<2 x i64>, <2 x i64>, i64 immarg)
+declare <16 x i8> @llvm.aarch64.crypto.bcaxu.v16i8(<16 x i8>, <16 x i8>, <16 x i8>)
+declare <8 x i16> 

[PATCH] D96367: Partially Revert "scan-view: Remove Reporter.py and associated AppleScript files"

[Artem Dergachev via Phabricator via cfe-commits]

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

Ok no problem!

I'll try to find time to clean this up properly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96367

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

[PATCH] D96070: [clang] [driver] Enable static linking to libc++

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Making -stdlib=libc++ not affect linker option for `-static` and `-static-pie` 
looks good to me, for the GNU toolchain. They are expected to pass `-lc++ 
-lc++abi` if they use separate `libc++abi.a`.
Just specifying `-lc++abi` does not work with GNU ld and gold anyway.
It may make some users unhappy if we do that for dynamic linking.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96070

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

[PATCH] D86376: [HIP] Emit kernel symbol

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D86376#2552524 , @tra wrote:

> In D86376#2552419 , @yaxunl wrote:
>
>> For triple chevron with kernel name, it is not needed. We only need 
>> indirection for a triple chevron with a function pointer, in which case we 
>> do not know its stub function at compile time. This is allowed by CUDA/HIP.
>
> Got it. We'll need to map the address of the symbol into the address of the 
> stub.
>
> Adding an indirection brings another question -- what's supposed to happen if 
> we're passed a pointer that's *not* a pointer to the symbol. I.e. it does not 
> point to the pointer to the stub.

The same thing could happen before this change, i.e., a function pointer does 
not contain the address of a stub function. In either case it will be UB. This 
change does not make the situation worse.

> Can we backtrack a bit and review our constraints/assumptions. I vaguely 
> recall AMD inproduced `__device_stub` because debugger needed to distinguish 
> host-side stub from the device-side kernel.
> If we add the data with the same name, would not it cause the same confusion 
> about what `kernel` is? If we are allowed to use 'kernel' on the host, is 
> there a reason not to rename `__device_stubkernel` back to `kernel` and just 
> use the stub address everywhere?

We have confirmed with our debugger team that emitting this symbol is OK for 
rocgdb since it is a variable symbol, not a function symbol.

> Another question -- assuming that the stub can't be renamed, can we give the 
> stub an alias with the name `kernel`? This way no matter how we take the 
> address, it will always point to the stub.

We have tried this and it did not work. The alias will ends up as a symbol to a 
function which is not allowed by rocgdb.


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

https://reviews.llvm.org/D86376

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

[PATCH] D96376: [CodeComplete] Member completion: heuristically resolve some dependent base exprs

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: hokein.
Herald added subscribers: usaxena95, kadircet, arphaman.
sammccall requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Today, inside a template, you can get completion for:

Foo t;
t.^

t has dependent type Foo, and we use the primary template to find its 
members.
However we also want this to work:

t.foo.bar().^

The type of t.foo.bar() is DependentTy, so we attempt to resolve using similar
heuristics (e.g. primary template).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96376

Files:
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/test/CodeCompletion/member-access.cpp

Index: clang/test/CodeCompletion/member-access.cpp
===
--- clang/test/CodeCompletion/member-access.cpp
+++ clang/test/CodeCompletion/member-access.cpp
@@ -84,6 +84,9 @@
   T function() { }
   T field;
 
+  TemplateClass 
+  BaseTemplate ();
+
   void overload1(const T &);
   void overload1(const S &);
 };
@@ -102,8 +105,12 @@
 // CHECK-CC2: overload1 : [#void#]overload1(<#const T 

[PATCH] D86376: [HIP] Emit kernel symbol

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D86376#2552419 , @yaxunl wrote:

> For triple chevron with kernel name, it is not needed. We only need 
> indirection for a triple chevron with a function pointer, in which case we do 
> not know its stub function at compile time. This is allowed by CUDA/HIP.

Got it. We'll need to map the address of the symbol into the address of the 
stub.

Adding an indirection brings another question -- what's supposed to happen if 
we're passed a pointer that's *not* a pointer to the symbol. I.e. it does not 
point to the pointer to the stub.

Can we backtrack a bit and review our constraints/assumptions. I vaguely recall 
AMD inproduced `__device_stub` because debugger needed to distinguish host-side 
stub from the device-side kernel.
If we add the data with the same name, would not it cause the same confusion 
about what `kernel` is? If we are allowed to use 'kernel' on the host, is there 
a reason not to rename `__device_stubkernel` back to `kernel` and just use the 
stub address everywhere?

Another question -- assuming that the stub can't be renamed, can we give the 
stub an alias with the name `kernel`? This way no matter how we take the 
address, it will always point to the stub.


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

https://reviews.llvm.org/D86376

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

[PATCH] D96280: [clang][cli] Round-trip the whole CompilerInvocation

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

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

LGTM! One more more comment inline.




Comment at: clang/lib/Frontend/CompilerInvocation.cpp:4534-4547
+  InputKind DashX = FrontendOpts.DashX;
+  if (DashX.getFormat() == InputKind::Precompiled ||
+  DashX.getLanguage() == Language::LLVM_IR) {
+if (LangOpts->ObjCAutoRefCount)
+  GenerateArg(Args, OPT_fobjc_arc, SA);
+if (LangOpts->PICLevel != 0)
+  GenerateArg(Args, OPT_pic_level, Twine(LangOpts->PICLevel), SA);

jansvoboda11 wrote:
> dexonsmith wrote:
> > This change, like the commented out code I already pointed at, seems not 
> > strictly-related to changing round-tripping to happen at a higher level. Is 
> > there some reason it's tied to landing at the same time? (If that somehow 
> > avoids a bunch of refactoring that'll have to be undone after, fine by me, 
> > but otherwise it'd be nice to make this particular patch as clean as 
> > possible I think and land the more functional changes ahead of time.)
> > 
> > I also wonder if this special-case logic could/should be buried inside 
> > `GenerateLangArgs`, possibly by passing in `DashX` as an argument. (Maybe 
> > with a FIXME to handle via ShouldParseIf? E.g., maybe almost all lang 
> > options could be moved to a `let` scope that turns on ShouldParseIf with 
> > this logic, and these four are left outside of it. Up to you, whether you 
> > think that's a good idea.)
> This change is necessary for the patch.
> 
> Until now, `GenerateLangArgs` was called (during round-trip) from 
> `ParseLangArgs`, which is behind the same condition in `CreateFromArgs`: `if 
> (!(DashX.getFormat() == InputKind::Precompiled || DashX.getLanguage() == 
> Language::LLVM_IR))`. This change attempts to preserve the previous behavior 
> and also to avoid calling `GenerateLangArgs` with unexpected/invalid `DashX`.
> 
> I agree eventually moving the logic into `{Parse,Generate}LangArgs` would be 
> nice, but I'd like to do that in a follow up patch.
That makes sense; please add a FIXME to that effect.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96280

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

[PATCH] D96323: [clang][cli] NFC: Remove intermediate command line parsing functions

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith accepted this revision.
dexonsmith 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/D96323/new/

https://reviews.llvm.org/D96323

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

[PATCH] D96070: [clang] [driver] Enable static linking to libc++

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

I think the root cause of the push back you're getting is that this is normally 
handled by vendors when they decide how they're going to ship libc++ on their 
platform or in their toolchain.

For example, on Apple platforms, we ship `libc++.dylib`, and we make sure to 
re-export `libc++abi.dylib`'s symbols from `libc++.dylib`. The result is that 
you only need to specify `-lc++` (which the compiler does by default), and 
everything "works". Since we don't ship `libc++.a`, there's no out-of-the-box 
solution to make it work - it's not officially supported. Some folks do diverge 
from that, however we assume they are willing to get their hands dirty and use 
the flags required to make it work (i.e. `-lc++ -lc++abi`). My understanding is 
that the same goes for other platforms, where each has their preferred way of 
shipping libc++. If we were to officially ship `libc++.a` in the SDKs for Apple 
platforms, I could imagine that we'd want to merge `libc++abi.a` into 
`libc++.a`, and there again you'd only need to specify `-static -lc++` and 
everything would work (unless I'm missing something).

Making that work out-of-the-box would be something we'd want to achieve in how 
we're building and shipping libc++, not by modifying the behaviour of the Clang 
driver on multiple platforms.

In D96070#2544297 , @tbaeder wrote:

> [...]
>
> Or should clang just not do anything here and people should be aware they 
> need to pass additional flags when using libc++ (on Linux)?

The TL;DR of what I wrote above is that I believe this is the correct answer. 
My main area is Apple platforms and not Linux, though, so this is just my .02 
as someone who ships libc++.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96070

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

[PATCH] D86376: [HIP] Emit kernel symbol

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D86376#2552066 , @tra wrote:

> In D86376#2551298 , @yaxunl wrote:
>
>> Actually there is one issue with this approach.
>>
>> HIP have API's to launch kernels, which accept kernel as function pointer 
>> argument. Currently when taking address of kernel, we get the stub function. 
>> These kernel launching API's will not work if we use kernel symbol to 
>> register the kernel. A solution is to return the kernel symbol instead of 
>> stub function when taking address of the kernel in host compilation, i.e. if 
>> a function pointer is assigned to a kernel in host code, it gets the kernel 
>> symbol instead of the stub function. This will make the kernel launching API 
>> work.
>>
>> To keep the triple chevron working, the kernel symbol will be initialized 
>> with the address of the stub function. For triple chevron call, the address 
>> of the stub function is loaded from the kernel symbol and invoked.
>
> This could work.
> Do we really need an indirection? If we know the stub address when we 
> initialize the symbol with it, we should be able to use that address for 
> `<<<>>>`.

For triple chevron with kernel name, it is not needed. We only need indirection 
for a triple chevron with a function pointer, in which case we do not know its 
stub function at compile time. This is allowed by CUDA/HIP.


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

https://reviews.llvm.org/D86376

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

[PATCH] D96274: [clang][cli] Generate and round-trip Diagnostic options

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

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

LGTM!

In D96274#2551206 , @probinson wrote:

> I don't claim to be very familiar with this area but "round-trip" and "test" 
> would make me think those bits should be in a lit test or unittest. As it is, 
> it's not obvious what is functional and what is testing here.
> Possibly I am misunderstanding something fundamental about how these options 
> work?

The idea is to eventually / soon turn `-round-trip-args` on by default in 
asserts builds. The option changes "parse `-cc1`" to "parse1 => generate1 => 
parse2 => generate2", returning the result of "parse2" (instead of "parse1") so 
that new options cannot be added without adding matching generate code, and 
comparing "generate1" against "generate2" to ensure generation is consistent / 
deterministic. Having it on by default in asserts builds will add coverage for 
all `-cc1` options used in any test.

Note that the per-option-group round-tripping is temporary, to enable some of 
this verification to land incrementally. The following patch combines it into a 
single high-level operation:
https://reviews.llvm.org/D96280




Comment at: clang/lib/Frontend/CompilerInvocation.cpp:745
+
+A->render(Args, Rendered);
   }

jansvoboda11 wrote:
> dexonsmith wrote:
> > It's not obvious why this renders the args instead of calling 
> > `A->getAsString()` to get the literal command-line argument and putting it 
> > directly in DiagnosticsAsWritten. If that'll work, I suggest switching to 
> > that, since it's a bit more straightforward; if there's some reason you 
> > can't do that, please leave a comment explaining why.
> I removed `DiagnosticsAsWritten` completely.
> 
> Previously, the code looped over `A->getValues()`, which gave me the 
> impression it handles a flag that might indeed have multiple values (e.g. 
> `-Wsomething=a,b,c`). If the code only stored `{a,b,c}` into `Diagnostics`, 
> `GenerateDiagnosticArgs` could never reconstruct the original argument.
> 
> However, after looking closely at all `-W` and `-R` options, this never 
> happens and `A->getValues()` always has exactly one element. I've refactored 
> the code and clarified the comment for the branch above.
Great!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96274

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

[PATCH] D92808: [ObjC][ARC] Use operand bundle 'clang.arc.rv' instead of explicitly emitting retainRV or claimRV calls in the IR

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

The ultimate code-generation consequences of this feature aren't very generic, 
so we shouldn't let ourselves get caught up designing an extremely general 
feature that ultimately either doesn't do what we need it to do or doesn't 
actually work for any of the generalizations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92808

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

[PATCH] D96328: [Clang, NewPM] Add KMSan support

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis accepted this revision.
eugenis 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/D96328/new/

https://reviews.llvm.org/D96328

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

[PATCH] D96367: Partially Revert "scan-view: Remove Reporter.py and associated AppleScript files"

[Tom Stellard via Phabricator via cfe-commits]

tstellar created this revision.
Herald added a subscriber: mgorny.
tstellar requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This reverts some of commit dbb01536f6f49fa428f170e34466072ef439b3e9 
.

The Reporter module was still being used by the ScanView.py module and deleting
it caused scan-view to fail.  This commit adds back Reporter.py but removes the
code the references the AppleScript files which were removed in
dbb01536f6f49fa428f170e34466072ef439b3e9 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96367

Files:
  clang/tools/scan-view/CMakeLists.txt
  clang/tools/scan-view/share/Reporter.py

Index: clang/tools/scan-view/share/Reporter.py
===
--- /dev/null
+++ clang/tools/scan-view/share/Reporter.py
@@ -0,0 +1,183 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+"""Methods for reporting bugs."""
+
+import subprocess, sys, os
+
+__all__ = ['ReportFailure', 'BugReport', 'getReporters']
+
+#
+
+class ReportFailure(Exception):
+"""Generic exception for failures in bug reporting."""
+def __init__(self, value):
+self.value = value
+
+# Collect information about a bug.
+
+class BugReport(object):
+def __init__(self, title, description, files):
+self.title = title
+self.description = description
+self.files = files
+
+# Reporter interfaces.
+
+import os
+
+import email, mimetypes, smtplib
+from email import encoders
+from email.message import Message
+from email.mime.base import MIMEBase
+from email.mime.multipart import MIMEMultipart
+from email.mime.text import MIMEText
+
+#======#
+# ReporterParameter
+#======#
+
+class ReporterParameter(object):
+  def __init__(self, n):
+self.name = n
+  def getName(self):
+return self.name
+  def getValue(self,r,bugtype,getConfigOption):
+ return getConfigOption(r.getName(),self.getName())
+  def saveConfigValue(self):
+return True
+
+class TextParameter (ReporterParameter):
+  def getHTML(self,r,bugtype,getConfigOption):
+return """\
+
+%s:
+
+"""%(self.getName(),r.getName(),self.getName(),self.getValue(r,bugtype,getConfigOption))
+
+class SelectionParameter (ReporterParameter):
+  def __init__(self, n, values):
+ReporterParameter.__init__(self,n)
+self.values = values
+
+  def getHTML(self,r,bugtype,getConfigOption):
+default = self.getValue(r,bugtype,getConfigOption)
+return """\
+
+%s:
+%s
+"""%(self.getName(),r.getName(),self.getName(),'\n'.join(["""\
+%s"""%(o[0],
+ o[0] == default and ' selected="selected"' or '',
+ o[1]) for o in self.values]))
+
+#======#
+# Reporters
+#======#
+
+class EmailReporter(object):
+def getName(self):
+return 'Email'
+
+def getParameters(self):
+return [TextParameter(x) for x in ['To', 'From', 'SMTP Server', 'SMTP Port']]
+
+# Lifted from python email module examples.
+def attachFile(self, outer, path):
+# Guess the content type based on the file's extension.  Encoding
+# will be ignored, although we should check for simple things like
+# gzip'd or compressed files.
+ctype, encoding = mimetypes.guess_type(path)
+if ctype is None or encoding is not None:
+# No guess could be made, or the file is encoded (compressed), so
+# use a generic bag-of-bits type.
+ctype = 'application/octet-stream'
+maintype, subtype = ctype.split('/', 1)
+if maintype == 'text':
+fp = open(path)
+# Note: we should handle calculating the charset
+msg = MIMEText(fp.read(), _subtype=subtype)
+fp.close()
+else:
+fp = open(path, 'rb')
+msg = MIMEBase(maintype, subtype)
+msg.set_payload(fp.read())
+fp.close()
+# Encode the payload using Base64
+encoders.encode_base64(msg)
+# Set the filename parameter
+msg.add_header('Content-Disposition', 'attachment', filename=os.path.basename(path))
+outer.attach(msg)
+
+def fileReport(self, report, parameters):
+mainMsg = """\
+BUG REPORT
+---
+Title: %s
+Description: %s
+"""%(report.title, report.description)
+
+if not parameters.get('To'):
+raise ReportFailure('No "To" address specified.')
+if not parameters.get('From'):
+raise ReportFailure('No "From" address specified.')
+
+  

[PATCH] D92808: [ObjC][ARC] Use operand bundle 'clang.arc.rv' instead of explicitly emitting retainRV or claimRV calls in the IR

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

I have a couple of suggestions below to add to the mix (but I don't feel 
strongly about either of them, just adding them for consideration).

In D92808#2552100 , @ahatanak wrote:

>   define void @test() {
> %r = call i8* @foo() [ "clang.arc.rv"(i64 1) ]
> call void (...) @llvm.objc.clang.arc.noop.use(i8* %r)
> ret void
>   }

Firstly, it feels to me like this IR would clarify that the bundle looks at the 
return value:

  define void @test() {
%r = call i8* @foo() [ "clang.arc.rv"(i64 1, i8* %r) ]
ret void
  }

Notice this passes `%r` to the operand bundle, which the verifier rejects. I 
wonder if the IR rule could safely be weakened to allow an operand bundle to 
reference the call/invoke it's attached to? (Not sure what all the implications 
would be, or if this would be sufficient...)

In D92808#2552100 , @ahatanak wrote:

> Or we could use a bundle that is more generic than `clang.arc.rv` (for 
> example, `"implicitly.used.by"(@llvm.objc.retainAutoreleasedReturnValue)`, 
> which indicates the result is used by an instruction that isn't explicitly 
> emitted in the IR).

Secondly, I agree it might be cleaner to formalize a generic operand bundle 
that has the semantics ARC needs. It might be nice to have this also model the 
inlining semantics, maybe something like "returnfilter":

  %x = call type @f1(...) [ "returnfilter"(type(type)* @f2) ]

The semantics could be that `%x` is implicitly passed through `@f2`. 
Concretely, maybe that's:

- `%x` has an implicit use.
- `@f1`'s return type cannot be changed.
- Inlining `@f1` generates explicit calls to `@f2`.
- The backend lowers the bundle as late as possible to calls to `@f2`.

Thirdly, you could combine the two previous ideas, generalizing the semantics 
to an "onreturn" bundle that is allowed to self-reference the callable, that 
the inliner knows about, etc., something like:

  %x = call type @f1(...) [ "onreturn"(type(type)* @f2, type %x) ]

This `@f2` is implicitly called and its return used in place of `%x`, but it 
explicitly lists its arguments. Compared to "returnfilter", it has the 
flexibility to specify argument order and might be more useful for other 
applications.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92808

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

[PATCH] D87587: [clang-format][PR47290] Add ShortNamespaceLines format option

[Marek Kurdej via Phabricator via cfe-commits]

curdeius requested changes to this revision.
curdeius added a comment.
This revision now requires changes to proceed.

Please regroup tests and add release notes.




Comment at: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp:1126
+
+  EXPECT_EQ(UnwrappedLines, Style.ShortNamespaceLines);
+}

This could be tested together with another test.



Comment at: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp:1129
+
+TEST_F(ShortNamespaceLinesTest,
+   ZeroUnwrappedLines_DoesNotAddEndCommentForOneLinerNamespace) {

In general, I think you can merge some test cases to match the current testing 
"style".



Comment at: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp:1141
+  auto Style = getLLVMStyle();
+  Style.ShortNamespaceLines = 0;
+

From the tests one could imply that 0 means "never add comments", or "add 
comments for non empty namespaces". Please add a test with at least one line in 
the namespace.



Comment at: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp:1161
+TEST_F(ShortNamespaceLinesTest,
+   OneUnwrappedLine_AddsEndCommentForLongNamespace) {
+  EXPECT_EQ("namespace LongNamespace {\n"

You should check that Style.ShortNamespaceLines is 1 here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87587

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

[clang] d5d8c52 - PR48545: Access check the inherited constructor, not the inheriting

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2021-02-09T13:27:55-08:00
New Revision: d5d8c529abe65ce55c24b0333842bb31746a8bf8

URL: 
https://github.com/llvm/llvm-project/commit/d5d8c529abe65ce55c24b0333842bb31746a8bf8
DIFF: 
https://github.com/llvm/llvm-project/commit/d5d8c529abe65ce55c24b0333842bb31746a8bf8.diff

LOG: PR48545: Access check the inherited constructor, not the inheriting
constructor.

We got this wrong only when forming a CXXTemporaryObjectExpr, which
caused the bug to only appear for certain syntactic forms.

Added: 


Modified: 
clang/lib/Sema/SemaInit.cpp
clang/test/SemaCXX/cxx11-inheriting-ctors.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 640755cec222..c1a2be744853 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -6536,22 +6536,23 @@ PerformConstructorInitialization(Sema ,
 ? SourceRange(LBraceLoc, RBraceLoc)
 : Kind.getParenOrBraceRange();
 
+CXXConstructorDecl *CalleeDecl = Constructor;
 if (auto *Shadow = dyn_cast(
 Step.Function.FoundDecl.getDecl())) {
-  Constructor = S.findInheritingConstructor(Loc, Constructor, Shadow);
-  if (S.DiagnoseUseOfDecl(Constructor, Loc))
+  CalleeDecl = S.findInheritingConstructor(Loc, Constructor, Shadow);
+  if (S.DiagnoseUseOfDecl(CalleeDecl, Loc))
 return ExprError();
 }
-S.MarkFunctionReferenced(Loc, Constructor);
+S.MarkFunctionReferenced(Loc, CalleeDecl);
 
 CurInit = S.CheckForImmediateInvocation(
 CXXTemporaryObjectExpr::Create(
-S.Context, Constructor,
+S.Context, CalleeDecl,
 Entity.getType().getNonLValueExprType(S.Context), TSInfo,
 ConstructorArgs, ParenOrBraceRange, HadMultipleCandidates,
 IsListInitialization, IsStdInitListInitialization,
 ConstructorInitRequiresZeroInit),
-Constructor);
+CalleeDecl);
   } else {
 CXXConstructExpr::ConstructionKind ConstructKind =
   CXXConstructExpr::CK_Complete;

diff  --git a/clang/test/SemaCXX/cxx11-inheriting-ctors.cpp 
b/clang/test/SemaCXX/cxx11-inheriting-ctors.cpp
index 5be428401fa0..39582660984b 100644
--- a/clang/test/SemaCXX/cxx11-inheriting-ctors.cpp
+++ b/clang/test/SemaCXX/cxx11-inheriting-ctors.cpp
@@ -142,3 +142,24 @@ namespace PR47555 {
   };
   template void f();
 }
+
+namespace PR48545 {
+  struct B {
+  void f();
+  private:
+  B(int, int = 0);
+  };
+  struct D : B { using B::B; };
+  void B::f() {
+  D{0};
+  D{0, 0};
+  D(0);
+  D(0, 0);
+  D u = {0};
+  D v = {0, 0};
+  D w{0};
+  D x{0, 0};
+  D y(0);
+  D z(0, 0);
+  }
+}



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

[PATCH] D93446: [RISCV] Add vadd with mask and without mask builtin.

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/lib/Basic/Targets/RISCV.cpp:89
+#define BUILTIN(ID, TYPE, ATTRS)   
\
+  {"__builtin_rvv_" #ID, TYPE, ATTRS, nullptr, ALL_LANGUAGES, nullptr},
+#include "clang/Basic/BuiltinsRISCV.def"

khchen wrote:
> Jim wrote:
> > Builtins for other extension don't have "__builtin_rvv_" prefix.
> maybe we could rename BuiltinsRISCV.def as BuiltinsRVV.def, and other 
> extension defines their own .def file?
> 
> @Jim do you have any suggestion?
Don't most targets pass the full name with the prefix to the BUILTIN macro?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93446

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

[PATCH] D95536: [clang][sema] Note decl location on missing member

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D95536#2551332 , @tbaeder wrote:

> Any update on this?

Thank you for the patch! Do you have some motivating examples of when this 
would really add clarity to the diagnostic? I'm not opposed to the patch per 
se, but I'm a bit wary about adding an additional note because that makes the 
diagnostic output seem that much longer, which makes the salient diagnostics a 
bit harder to find (colorization in the terminal helps with this, though).

> The patch only emits the note if clang does not output a typo correction. I 
> thought it might not be as useful in this case since it would clutter the 
> output and chances are that the typo fix _is_ the right thing to show. 
> Opinions on this?

I think that makes sense -- the typo correction already has a note that points 
to somewhere which is likely to get the user into the right area anyway: 
https://godbolt.org/z/1Yo3Pj




Comment at: clang/include/clang/Sema/Sema.h:9795
 
+  /// Report a Note for an unknown member of the given decl context
+  void reportDeclForUnknownMember(const DeclContext *Ctx);





Comment at: clang/lib/Sema/Sema.cpp:2549
+void Sema::reportDeclForUnknownMember(const DeclContext *Ctx) {
+  // Don't report the global scope decl
+  if (isa(Ctx))





Comment at: clang/lib/Sema/Sema.cpp:2553
+
+  // Lambdas already report their declaration location themselves
+  if (isa(Ctx) && cast(Ctx)->isLambda())





Comment at: clang/lib/Sema/Sema.cpp:2557-2560
+  if (const Decl *D = dyn_cast(Ctx)) {
+Diag(D->getLocation(), diag::note_entity_declared_at)
+<< Ctx << D->getLocation();
+  }




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

https://reviews.llvm.org/D95536

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

[PATCH] D96363: Mark output as text if it is really text

[Abhina Sree via Phabricator via cfe-commits]

abhina.sreeskantharajan updated this revision to Diff 322479.
abhina.sreeskantharajan added a comment.

Remove ARCMigrate change in this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96363

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Frontend/Rewrite/FrontendActions.cpp
  llvm/tools/dsymutil/DwarfLinkerForBinary.cpp


Index: llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
===
--- llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
+++ llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
@@ -252,7 +252,10 @@
   }
 
   std::error_code EC;
-  raw_fd_ostream OS(Options.NoOutput ? "-" : Path.str(), EC, sys::fs::OF_None);
+  raw_fd_ostream OS(Options.NoOutput ? "-" : Path.str(), EC,
+Options.RemarksFormat == remarks::Format::Bitstream
+? sys::fs::OF_None
+: sys::fs::OF_Text);
   if (EC)
 return errorCodeToError(EC);
 
Index: clang/lib/Frontend/Rewrite/FrontendActions.cpp
===
--- clang/lib/Frontend/Rewrite/FrontendActions.cpp
+++ clang/lib/Frontend/Rewrite/FrontendActions.cpp
@@ -185,7 +185,7 @@
 void RewriteMacrosAction::ExecuteAction() {
   CompilerInstance  = getCompilerInstance();
   std::unique_ptr OS =
-  CI.createDefaultOutputFile(true, getCurrentFileOrBufferName());
+  CI.createDefaultOutputFile(false, getCurrentFileOrBufferName());
   if (!OS) return;
 
   RewriteMacrosInInput(CI.getPreprocessor(), OS.get());
@@ -270,7 +270,7 @@
 bool RewriteIncludesAction::BeginSourceFileAction(CompilerInstance ) {
   if (!OutputStream) {
 OutputStream =
-CI.createDefaultOutputFile(true, getCurrentFileOrBufferName());
+CI.createDefaultOutputFile(false, getCurrentFileOrBufferName());
 if (!OutputStream)
   return false;
   }
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -1434,7 +1434,9 @@
   llvm::SmallString<128> Script(CrashInfo.Filename);
   llvm::sys::path::replace_extension(Script, "sh");
   std::error_code EC;
-  llvm::raw_fd_ostream ScriptOS(Script, EC, llvm::sys::fs::CD_CreateNew);
+  llvm::raw_fd_ostream ScriptOS(Script, EC, llvm::sys::fs::CD_CreateNew,
+llvm::sys::fs::FA_Write,
+llvm::sys::fs::OF_Text);
   if (EC) {
 Diag(clang::diag::note_drv_command_failed_diag_msg)
 << "Error generating run script: " << Script << " " << EC.message();


Index: llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
===
--- llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
+++ llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
@@ -252,7 +252,10 @@
   }
 
   std::error_code EC;
-  raw_fd_ostream OS(Options.NoOutput ? "-" : Path.str(), EC, sys::fs::OF_None);
+  raw_fd_ostream OS(Options.NoOutput ? "-" : Path.str(), EC,
+Options.RemarksFormat == remarks::Format::Bitstream
+? sys::fs::OF_None
+: sys::fs::OF_Text);
   if (EC)
 return errorCodeToError(EC);
 
Index: clang/lib/Frontend/Rewrite/FrontendActions.cpp
===
--- clang/lib/Frontend/Rewrite/FrontendActions.cpp
+++ clang/lib/Frontend/Rewrite/FrontendActions.cpp
@@ -185,7 +185,7 @@
 void RewriteMacrosAction::ExecuteAction() {
   CompilerInstance  = getCompilerInstance();
   std::unique_ptr OS =
-  CI.createDefaultOutputFile(true, getCurrentFileOrBufferName());
+  CI.createDefaultOutputFile(false, getCurrentFileOrBufferName());
   if (!OS) return;
 
   RewriteMacrosInInput(CI.getPreprocessor(), OS.get());
@@ -270,7 +270,7 @@
 bool RewriteIncludesAction::BeginSourceFileAction(CompilerInstance ) {
   if (!OutputStream) {
 OutputStream =
-CI.createDefaultOutputFile(true, getCurrentFileOrBufferName());
+CI.createDefaultOutputFile(false, getCurrentFileOrBufferName());
 if (!OutputStream)
   return false;
   }
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -1434,7 +1434,9 @@
   llvm::SmallString<128> Script(CrashInfo.Filename);
   llvm::sys::path::replace_extension(Script, "sh");
   std::error_code EC;
-  llvm::raw_fd_ostream ScriptOS(Script, EC, llvm::sys::fs::CD_CreateNew);
+  llvm::raw_fd_ostream ScriptOS(Script, EC, llvm::sys::fs::CD_CreateNew,
+llvm::sys::fs::FA_Write,
+llvm::sys::fs::OF_Text);
   if (EC) {
 Diag(clang::diag::note_drv_command_failed_diag_msg)
 << "Error generating run script: " << Script << " " << EC.message();

[PATCH] D94500: [clang-format] Rework Whitesmiths mode to use line-level values in UnwrappedLineParser

[Tim Wojtulewicz via Phabricator via cfe-commits]

timwoj updated this revision to Diff 322477.
timwoj added a comment.

Added release note entry


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94500

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13543,6 +13543,7 @@
WhitesmithsBraceStyle);
   */
 
+  WhitesmithsBraceStyle.NamespaceIndentation = FormatStyle::NI_None;
   verifyFormat("namespace a\n"
"  {\n"
"class A\n"
@@ -13567,6 +13568,89 @@
"  } // namespace a",
WhitesmithsBraceStyle);
 
+  verifyFormat("namespace a\n"
+   "  {\n"
+   "namespace b\n"
+   "  {\n"
+   "class A\n"
+   "  {\n"
+   "  void f()\n"
+   "{\n"
+   "if (true)\n"
+   "  {\n"
+   "  a();\n"
+   "  b();\n"
+   "  }\n"
+   "}\n"
+   "  void g()\n"
+   "{\n"
+   "return;\n"
+   "}\n"
+   "  };\n"
+   "struct B\n"
+   "  {\n"
+   "  int x;\n"
+   "  };\n"
+   "  } // namespace b\n"
+   "  } // namespace a",
+   WhitesmithsBraceStyle);
+
+  WhitesmithsBraceStyle.NamespaceIndentation = FormatStyle::NI_Inner;
+  verifyFormat("namespace a\n"
+   "  {\n"
+   "namespace b\n"
+   "  {\n"
+   "  class A\n"
+   "{\n"
+   "void f()\n"
+   "  {\n"
+   "  if (true)\n"
+   "{\n"
+   "a();\n"
+   "b();\n"
+   "}\n"
+   "  }\n"
+   "void g()\n"
+   "  {\n"
+   "  return;\n"
+   "  }\n"
+   "};\n"
+   "  struct B\n"
+   "{\n"
+   "int x;\n"
+   "};\n"
+   "  } // namespace b\n"
+   "  } // namespace a",
+   WhitesmithsBraceStyle);
+
+  WhitesmithsBraceStyle.NamespaceIndentation = FormatStyle::NI_All;
+  verifyFormat("namespace a\n"
+   "  {\n"
+   "  namespace b\n"
+   "{\n"
+   "class A\n"
+   "  {\n"
+   "  void f()\n"
+   "{\n"
+   "if (true)\n"
+   "  {\n"
+   "  a();\n"
+   "  b();\n"
+   "  }\n"
+   "}\n"
+   "  void g()\n"
+   "{\n"
+   "return;\n"
+   "}\n"
+   "  };\n"
+   "struct B\n"
+   "  {\n"
+   "  int x;\n"
+   "  };\n"
+   "} // namespace b\n"
+   "  }   // namespace a",
+   WhitesmithsBraceStyle);
+
   verifyFormat("void f()\n"
"  {\n"
"  if (true)\n"
@@ -13601,7 +13685,7 @@
"  }\n",
WhitesmithsBraceStyle);
 
-  WhitesmithsBraceStyle.IndentCaseBlocks = true;
+  WhitesmithsBraceStyle.IndentCaseLabels = true;
   verifyFormat("void switchTest1(int a)\n"
"  {\n"
"  switch (a)\n"
@@ -13609,7 +13693,7 @@
"case 2:\n"
"  {\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
"}\n"
"  }\n",
WhitesmithsBraceStyle);
@@ -13619,7 +13703,7 @@
"  switch (a)\n"
"{\n"
"case 0:\n"
-   "break;\n"
+   "  break;\n"
"case 1:\n"
"  {\n"
"  break;\n"
@@ -13627,9 +13711,9 @@
"case 2:\n"
"  {\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
"default:\n"
-   "break;\n"
+   "  break;\n"
"}\n"
"  }\n",
WhitesmithsBraceStyle);
@@ -13642,17 +13726,17 @@
"  {\n"
"  foo(x);\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
   

[PATCH] D87587: [clang-format][PR47290] Add ShortNamespaceLines format option

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

Apart from my comments, this looks good to me and I really wonder why it hasn't 
been pushed. (I would directly use it!)




Comment at: clang/include/clang/Format/Format.h:2821
MaxEmptyLinesToKeep == R.MaxEmptyLinesToKeep &&
+   ShortNamespaceLines == R.ShortNamespaceLines &&
NamespaceIndentation == R.NamespaceIndentation &&

Could you sort this one alphabetically?



Comment at: clang/lib/Format/Format.cpp:576
 IO.mapOptional("MaxEmptyLinesToKeep", Style.MaxEmptyLinesToKeep);
+IO.mapOptional("ShortNamespaceLines", Style.ShortNamespaceLines);
 IO.mapOptional("NamespaceIndentation", Style.NamespaceIndentation);

Dito



Comment at: clang/lib/Format/Format.cpp:939
   LLVMStyle.MaxEmptyLinesToKeep = 1;
+  LLVMStyle.ShortNamespaceLines = 1;
   LLVMStyle.KeepEmptyLinesAtTheStartOfBlocks = true;

Dito


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87587

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

[PATCH] D95655: [AArch64] Adding Neon Sm3 & Sm4 Intrinsics

[Ryan Santhirarajan via Phabricator via cfe-commits]

rsanthir.quic updated this revision to Diff 322470.
rsanthir.quic added a comment.

Updated test name and checks performed


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

https://reviews.llvm.org/D95655

Files:
  clang/include/clang/Basic/arm_neon.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/aarch64-neon-range-checks.c
  clang/test/CodeGen/aarch64-neon-sm4-sm3.c
  llvm/include/llvm/IR/IntrinsicsAArch64.td
  llvm/lib/Target/AArch64/AArch64InstrFormats.td
  llvm/lib/Target/AArch64/AArch64InstrInfo.td
  llvm/test/CodeGen/AArch64/neon-sm4-sm3.ll

Index: llvm/test/CodeGen/AArch64/neon-sm4-sm3.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/neon-sm4-sm3.ll
@@ -0,0 +1,102 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc %s -mtriple=aarch64 -mattr=+v8.3a,+sm4 -o - | FileCheck %s
+
+define <4 x i32> @test_vsm3partw1(<4 x i32> %a, <4 x i32> %b, <4 x i32> %c) {
+; CHECK-LABEL: test_vsm3partw1:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:sm3partw1 v0.4s, v1.4s, v2.4s
+; CHECK-NEXT:ret
+entry:
+  %vsm3partw1.i = tail call <4 x i32> @llvm.aarch64.crypto.sm3partw1(<4 x i32> %a, <4 x i32> %b, <4 x i32> %c)
+  ret <4 x i32> %vsm3partw1.i
+}
+
+define <4 x i32> @test_vsm3partw2(<4 x i32> %a, <4 x i32> %b, <4 x i32> %c) {
+; CHECK-LABEL: test_vsm3partw2:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:sm3partw2 v0.4s, v1.4s, v2.4s
+; CHECK-NEXT:ret
+entry:
+  %vsm3partw2.i = tail call <4 x i32> @llvm.aarch64.crypto.sm3partw2(<4 x i32> %a, <4 x i32> %b, <4 x i32> %c)
+  ret <4 x i32> %vsm3partw2.i
+}
+
+define <4 x i32> @test_vsm3ss1(<4 x i32> %a, <4 x i32> %b, <4 x i32> %c) {
+; CHECK-LABEL: test_vsm3ss1:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:sm3ss1 v0.4s, v0.4s, v1.4s, v2.4s
+; CHECK-NEXT:ret
+entry:
+  %vsm3ss1.i = tail call <4 x i32> @llvm.aarch64.crypto.sm3ss1(<4 x i32> %a, <4 x i32> %b, <4 x i32> %c)
+  ret <4 x i32> %vsm3ss1.i
+}
+
+define <4 x i32> @test_vsm3tt1a(<4 x i32> %a, <4 x i32> %b, <4 x i32> %c) {
+; CHECK-LABEL: test_vsm3tt1a:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:sm3tt1a v0.4s, v1.4s, v2.s[2]
+; CHECK-NEXT:ret
+entry:
+  %vsm3tt1a.i = tail call <4 x i32> @llvm.aarch64.crypto.sm3tt1a(<4 x i32> %a, <4 x i32> %b, <4 x i32> %c, i64 2)
+  ret <4 x i32> %vsm3tt1a.i
+}
+
+define <4 x i32> @test_vsm3tt1b(<4 x i32> %a, <4 x i32> %b, <4 x i32> %c) {
+; CHECK-LABEL: test_vsm3tt1b:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:sm3tt1b v0.4s, v1.4s, v2.s[2]
+; CHECK-NEXT:ret
+entry:
+  %vsm3tt1b.i = tail call <4 x i32> @llvm.aarch64.crypto.sm3tt1b(<4 x i32> %a, <4 x i32> %b, <4 x i32> %c, i64 2)
+  ret <4 x i32> %vsm3tt1b.i
+}
+
+define <4 x i32> @test_vsm3tt2a(<4 x i32> %a, <4 x i32> %b, <4 x i32> %c) {
+; CHECK-LABEL: test_vsm3tt2a:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:sm3tt2a v0.4s, v1.4s, v2.s[2]
+; CHECK-NEXT:ret
+entry:
+  %vsm3tt2a.i = tail call <4 x i32> @llvm.aarch64.crypto.sm3tt2a(<4 x i32> %a, <4 x i32> %b, <4 x i32> %c, i64 2)
+  ret <4 x i32> %vsm3tt2a.i
+}
+
+define <4 x i32> @test_vsm3tt2b(<4 x i32> %a, <4 x i32> %b, <4 x i32> %c) {
+; CHECK-LABEL: test_vsm3tt2b:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:sm3tt2b v0.4s, v1.4s, v2.s[2]
+; CHECK-NEXT:ret
+entry:
+  %vsm3tt2b.i = tail call <4 x i32> @llvm.aarch64.crypto.sm3tt2b(<4 x i32> %a, <4 x i32> %b, <4 x i32> %c, i64 2)
+  ret <4 x i32> %vsm3tt2b.i
+}
+
+define <4 x i32> @test_vsm4e(<4 x i32> %a, <4 x i32> %b) {
+; CHECK-LABEL: test_vsm4e:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:sm4e v0.4s, v1.4s
+; CHECK-NEXT:ret
+entry:
+  %vsm4e.i = tail call <4 x i32> @llvm.aarch64.crypto.sm4e(<4 x i32> %a, <4 x i32> %b)
+  ret <4 x i32> %vsm4e.i
+}
+
+define <4 x i32> @test_vsm4ekey(<4 x i32> %a, <4 x i32> %b) {
+; CHECK-LABEL: test_vsm4ekey:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:sm4ekey v0.4s, v0.4s, v1.4s
+; CHECK-NEXT:ret
+entry:
+  %vsm4ekey.i = tail call <4 x i32> @llvm.aarch64.crypto.sm4ekey(<4 x i32> %a, <4 x i32> %b)
+  ret <4 x i32> %vsm4ekey.i
+}
+
+declare <4 x i32> @llvm.aarch64.crypto.sm3partw1(<4 x i32>, <4 x i32>, <4 x i32>)
+declare <4 x i32> @llvm.aarch64.crypto.sm3partw2(<4 x i32>, <4 x i32>, <4 x i32>)
+declare <4 x i32> @llvm.aarch64.crypto.sm3ss1(<4 x i32>, <4 x i32>, <4 x i32>)
+declare <4 x i32> @llvm.aarch64.crypto.sm3tt1a(<4 x i32>, <4 x i32>, <4 x i32>, i64 immarg)
+declare <4 x i32> @llvm.aarch64.crypto.sm3tt2b(<4 x i32>, <4 x i32>, <4 x i32>, i64 immarg)
+declare <4 x i32> @llvm.aarch64.crypto.sm3tt2a(<4 x i32>, <4 x i32>, <4 x i32>, i64 immarg)
+declare <4 x i32> @llvm.aarch64.crypto.sm3tt1b(<4 x i32>, <4 x i32>, <4 x i32>, i64 immarg)
+declare <4 x i32> @llvm.aarch64.crypto.sm4e(<4 x i32>, <4 x i32>)
+declare <4 x i32> @llvm.aarch64.crypto.sm4ekey(<4 x i32>, <4 x i32>)
Index: llvm/lib/Target/AArch64/AArch64InstrInfo.td

[PATCH] D85223: [CUDA][HIP] Support accessing static device variable in host code for -fgpu-rdc

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 6 inline comments as done.
yaxunl added a comment.

In D85223#2551894 , @JonChesterfield 
wrote:

> This works around the limitations of the binary format nvptx and amdgpu are 
> using in the compiler. It's the wrong place in the stack to fix it - we could 
> introduce another symbol table in the binary to capture the 
> per-tu-between-arch scoping.
>
> However, if we later reach consensus on what to do in the elf instead, we can 
> still do that. In particular, embedding an elf for one arch in a named 
> section of an elf for a host arch is crude. This workaround seems acceptable 
> in the meantime.

Yes we should revisit this if there is a better solution.




Comment at: clang/test/CodeGenCUDA/device-var-linkage.cu:40
 // NORDC-DAG: @_ZL3sv1 = dso_local addrspace(1) externally_initialized global 
i32 0
-// RDC-DAG: @_ZL3sv1 = internal addrspace(1) global i32 0
+// RDC-DAG: @_ZL3sv1.static.[[HASH:b04fd23c98500190]] = dso_local addrspace(1) 
externally_initialized global i32 0
 // HOST-DAG: @_ZL3sv1 = internal global i32 undef

tra wrote:
> It should probably be a regex after `HASH:`, not the hash value itself.
will do



Comment at: clang/test/CodeGenCUDA/managed-var.cu:42
+// NORDC-D-DAG: @_ZL2sx = dso_local addrspace(1) externally_initialized global 
i32 addrspace(1)* null
+// RDC-D-DAG: @_ZL2sx.static.[[HASH:b04fd23c98500190]] = dso_local 
addrspace(1) externally_initialized global i32 addrspace(1)* null
 // HOST-DAG: @_ZL2sx.init = internal global i32 1

tra wrote:
> Same here.
will do



Comment at: clang/test/CodeGenCUDA/static-device-var-rdc.cu:34
+// Test externalized static device variables
+// EXT-DEV-DAG: @_ZL1x.static.[[HASH:b04fd23c98500190]] = dso_local 
addrspace(1) externally_initialized global i32 0
+// EXT-HOST-DAG: @[[DEVNAMEX:[0-9]+]] = 
{{.*}}c"_ZL1x.static.[[HASH:b04fd23c98500190]]\00"

tra wrote:
> ditto.
will do



Comment at: clang/test/SemaCUDA/static-device-var.cu:10
+
+// expected-no-diagnostics
+

tra wrote:
> A comment explaining what we're testing would be helpful. `no-diagnostics` 
> gives no clues about what is it we're looking for here.
> 
> 
will do



Comment at: clang/test/SemaCUDA/static-device-var.cu:14-22
+__device__ void f1() {
+  const static int b = 123;
+  static int a;
+}
+
+__global__ void k1() {
+  const static int b = 123;

tra wrote:
> So, this verifies that we're allowed to use static local vars in device code. 
> A comment would be useful.
will do



Comment at: clang/test/SemaCUDA/static-device-var.cu:23-37
+
+static __device__ int x;
+static __constant__ int y;
+
+__global__ void kernel(int *a) {
+  a[0] = x;
+  a[1] = y;

tra wrote:
> And this verifies that global static vars can be referenced from both host 
> and device. 
> I'd also add a negative test with `static int host_only;` and would verify 
> that we still don't allow accessing it from the device.
will do


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

https://reviews.llvm.org/D85223

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

[PATCH] D95655: [AArch64] Adding Neon Sm3 & Sm4 Intrinsics

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added inline comments.



Comment at: clang/test/CodeGen/aarch64-neon-sm4-sm3.c:5
+
+// RUN: not %clang_cc1 -triple aarch64-linux-gnu -target-feature +neon \
+// RUN: -S -emit-llvm -o - %s 2>&1 | FileCheck --check-prefix=CHECK-NO-CRYPTO 
%s

apazos wrote:
> The test is written for aarch64 triple, in addition @t.p.northover might want 
> to add arm64 triple checks. Tim what do you think?
Thanks for thinking of us, but generally I don't think it's worth duplicating 
tests along those lines, it's really difficult to write something that cares 
about the difference between `arm64` and `aarch64`. 

`linux-gnu` vs `apple-ios` (or others, these days) is more likely to be an 
issue, but even there I tend to think it's fine to just check one unless 
there's a specific reason to think it'll affect the targets differently.


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

https://reviews.llvm.org/D95655

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

[PATCH] D77811: [clangd] Implement semanticTokens modifiers

[Nathan Ridge via Phabricator via cfe-commits]

nridge added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:136
+// Whether D is const in a loose sense (should it be highlighted as such?)
+bool isConst(const Decl *D) {
+  if (llvm::isa(D) || llvm::isa(D))

nridge wrote:
> sammccall wrote:
> > nridge wrote:
> > > Do you think in the future it might make sense to have the `readonly` 
> > > modifier reflect, at least for variables, whether //the particular 
> > > reference// to the variable is a readonly reference (e.g. binding the 
> > > variable to a `const X&` vs. an `X&`)?
> > Do I understand correctly?
> > 
> > ```
> > std::vector X; // X is not readonly
> > X.push_back(42); // X is not readonly
> > X.size(); // X is readonly
> > ```
> > 
> > Distinguishing potentially-mutating from non-mutating uses seems really 
> > useful to me.
> > My only question is whether mapping this concept onto `readonly` is the 
> > right thing to do:
> >  - there are really three sets: const access, mutable access, and 
> > non-access (e.g. declaration, or on RHS of `using a=b`). And I think maybe 
> > the most useful thing to distinguish is mutable access vs everything else, 
> > which is awkward with an attribute called "readonly".
> >  - this also seems likely to diverge from how other languages use the 
> > attribute (most don't have this concept)
> >  - on the other hand, standard attribute names will be better supported by 
> > clients
> > 
> > This also might be somewhat complicated to implement :-)
> > I'd like to leave in this simple decl-based thing as a placeholder, and 
> > either we can replace it and add an additional "mutation" attribute later 
> > (once we work out how to implement it!) I've left a comment about this...
> > 
> > (Related: a while ago Google's style guide dropped its requirement to use 
> > pointers rather than references for mutable function params. Having `` at 
> > the callsite rather than just `x` is a useful hint, but obviously diverging 
> > from common practice has a cost. We discussed how we could use semantic 
> > highlighting to highlight where a param was being passed by mutable 
> > reference, though didn't have client-side support for it yet)
> > Do I understand correctly?
> > 
> > ```
> > std::vector X; // X is not readonly
> > X.push_back(42); // X is not readonly
> > X.size(); // X is readonly
> > ```
> 
> Yup, that's what I had in mind.
> 
> > Distinguishing potentially-mutating from non-mutating uses seems really 
> > useful to me.
> > My only question is whether mapping this concept onto `readonly` is the 
> > right thing to do:
> >  - there are really three sets: const access, mutable access, and 
> > non-access (e.g. declaration, or on RHS of `using a=b`). And I think maybe 
> > the most useful thing to distinguish is mutable access vs everything else, 
> > which is awkward with an attribute called "readonly".
> >  - this also seems likely to diverge from how other languages use the 
> > attribute (most don't have this concept)
> >  - on the other hand, standard attribute names will be better supported by 
> > clients
> > 
> > This also might be somewhat complicated to implement :-)
> > I'd like to leave in this simple decl-based thing as a placeholder, and 
> > either we can replace it and add an additional "mutation" attribute later 
> > (once we work out how to implement it!) I've left a comment about this...
> 
> Sounds good!
> 
> > (Related: a while ago Google's style guide dropped its requirement to use 
> > pointers rather than references for mutable function params. Having `` at 
> > the callsite rather than just `x` is a useful hint, but obviously diverging 
> > from common practice has a cost. We discussed how we could use semantic 
> > highlighting to highlight where a param was being passed by mutable 
> > reference, though didn't have client-side support for it yet)
> 
> Funny you mention this, because I implemented this exact highlighting in 
> Eclipse a few years ago:
> 
> https://wiki.eclipse.org/CDT/User/NewIn92#Syntax_coloring_for_variables_passed_by_non-const_reference
> 
> (and it's my main motivation for the suggestion I've made here).
> I implemented this exact highlighting in Eclipse a few years ago

I will readily admit that classifying references as mutable or not can get 
tricky. For example:

```
int* array = new int[10];
array[0] = 42;
```

Is the reference to `array` on the second line mutable? Does the answer change 
if the declaration is changed to `int array[10];`?

Fun questions to figure out later :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77811

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

[PATCH] D77811: [clangd] Implement semanticTokens modifiers

[Nathan Ridge via Phabricator via cfe-commits]

nridge added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:136
+// Whether D is const in a loose sense (should it be highlighted as such?)
+bool isConst(const Decl *D) {
+  if (llvm::isa(D) || llvm::isa(D))

sammccall wrote:
> nridge wrote:
> > Do you think in the future it might make sense to have the `readonly` 
> > modifier reflect, at least for variables, whether //the particular 
> > reference// to the variable is a readonly reference (e.g. binding the 
> > variable to a `const X&` vs. an `X&`)?
> Do I understand correctly?
> 
> ```
> std::vector X; // X is not readonly
> X.push_back(42); // X is not readonly
> X.size(); // X is readonly
> ```
> 
> Distinguishing potentially-mutating from non-mutating uses seems really 
> useful to me.
> My only question is whether mapping this concept onto `readonly` is the right 
> thing to do:
>  - there are really three sets: const access, mutable access, and non-access 
> (e.g. declaration, or on RHS of `using a=b`). And I think maybe the most 
> useful thing to distinguish is mutable access vs everything else, which is 
> awkward with an attribute called "readonly".
>  - this also seems likely to diverge from how other languages use the 
> attribute (most don't have this concept)
>  - on the other hand, standard attribute names will be better supported by 
> clients
> 
> This also might be somewhat complicated to implement :-)
> I'd like to leave in this simple decl-based thing as a placeholder, and 
> either we can replace it and add an additional "mutation" attribute later 
> (once we work out how to implement it!) I've left a comment about this...
> 
> (Related: a while ago Google's style guide dropped its requirement to use 
> pointers rather than references for mutable function params. Having `` at 
> the callsite rather than just `x` is a useful hint, but obviously diverging 
> from common practice has a cost. We discussed how we could use semantic 
> highlighting to highlight where a param was being passed by mutable 
> reference, though didn't have client-side support for it yet)
> Do I understand correctly?
> 
> ```
> std::vector X; // X is not readonly
> X.push_back(42); // X is not readonly
> X.size(); // X is readonly
> ```

Yup, that's what I had in mind.

> Distinguishing potentially-mutating from non-mutating uses seems really 
> useful to me.
> My only question is whether mapping this concept onto `readonly` is the right 
> thing to do:
>  - there are really three sets: const access, mutable access, and non-access 
> (e.g. declaration, or on RHS of `using a=b`). And I think maybe the most 
> useful thing to distinguish is mutable access vs everything else, which is 
> awkward with an attribute called "readonly".
>  - this also seems likely to diverge from how other languages use the 
> attribute (most don't have this concept)
>  - on the other hand, standard attribute names will be better supported by 
> clients
> 
> This also might be somewhat complicated to implement :-)
> I'd like to leave in this simple decl-based thing as a placeholder, and 
> either we can replace it and add an additional "mutation" attribute later 
> (once we work out how to implement it!) I've left a comment about this...

Sounds good!

> (Related: a while ago Google's style guide dropped its requirement to use 
> pointers rather than references for mutable function params. Having `` at 
> the callsite rather than just `x` is a useful hint, but obviously diverging 
> from common practice has a cost. We discussed how we could use semantic 
> highlighting to highlight where a param was being passed by mutable 
> reference, though didn't have client-side support for it yet)

Funny you mention this, because I implemented this exact highlighting in 
Eclipse a few years ago:

https://wiki.eclipse.org/CDT/User/NewIn92#Syntax_coloring_for_variables_passed_by_non-const_reference

(and it's my main motivation for the suggestion I've made here).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77811

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

[PATCH] D40259: [libcxx] LWG2993: reference_wrapper conversion from T&

[Louis Dionne via Phabricator via cfe-commits]

ldionne abandoned this revision.
ldionne added a comment.

Closing as this is superseded by D92725 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D40259

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

[PATCH] D40259: [libcxx] LWG2993: reference_wrapper conversion from T&

[Louis Dionne via Phabricator via cfe-commits]

ldionne commandeered this revision.
ldionne added a reviewer: K-ballo.
ldionne added a comment.
Herald added a subscriber: jkorous.

This is superseded by D92725 , so I'm going to 
commandeer and abandon this to clear up the review queue. Thanks for your 
contribution - it seems to me that part of D92725 
 is actually taken from this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D40259

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

[PATCH] D95706: [clangd] Expose more dependent-name detail via semanticTokens

[Sam McCall via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
sammccall marked an inline comment as done.
Closed by commit rG59c1139d3ee1: [clangd] Expose more dependent-name detail via 
semanticTokens (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D95706?vs=320234=322459#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95706

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/test/semantic-tokens.test
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -58,8 +58,8 @@
   {HighlightingKind::Method, "Method"},
   {HighlightingKind::StaticMethod, "StaticMethod"},
   {HighlightingKind::Typedef, "Typedef"},
-  {HighlightingKind::DependentType, "DependentType"},
-  {HighlightingKind::DependentName, "DependentName"},
+  {HighlightingKind::Type, "Type"},
+  {HighlightingKind::Unknown, "Unknown"},
   {HighlightingKind::TemplateParameter, "TemplateParameter"},
   {HighlightingKind::Concept, "Concept"},
   {HighlightingKind::Primitive, "Primitive"},
@@ -208,7 +208,7 @@
   }
   template
   struct $Class_decl[[C]] : $Namespace[[abc]]::$Class[[A]]<$TemplateParameter[[T]]> {
-typename $TemplateParameter[[T]]::$DependentType[[A]]* $Field_decl[[D]];
+typename $TemplateParameter[[T]]::$Type_dependentName[[A]]* $Field_decl[[D]];
   };
   $Namespace[[abc]]::$Class[[A]] $Variable_decl[[AA]];
   typedef $Namespace[[abc]]::$Class[[A]] $Class_decl[[AAA]];
@@ -342,11 +342,8 @@
   R"cpp(
   template 
   struct $Class_decl[[Tmpl]] {$TemplateParameter[[T]] $Field_decl[[x]] = 0;};
-  // FIXME: highlights dropped due to conflicts.
-  // explicitReferenceTargets reports ClassTemplateSpecializationDecl twice
-  // at its location, calling it a declaration/non-declaration once each.
-  extern template struct Tmpl;
-  template struct Tmpl;
+  extern template struct $Class_decl[[Tmpl]];
+  template struct $Class_decl[[Tmpl]];
 )cpp",
   // This test is to guard against highlightings disappearing when using
   // conversion operators as their behaviour in the clang AST differ from
@@ -569,7 +566,7 @@
   template 
   void $Function_decl[[foo]]($TemplateParameter[[T]] $Parameter_decl[[P]]) {
 $Function[[phase1]]($Parameter[[P]]);
-$DependentName[[phase2]]($Parameter[[P]]);
+$Unknown_dependentName[[phase2]]($Parameter[[P]]);
   }
 )cpp",
   R"cpp(
@@ -598,20 +595,20 @@
 )cpp",
   R"cpp(
   template 
-  void $Function_decl[[foo]](typename $TemplateParameter[[T]]::$DependentType[[Type]]
-= $TemplateParameter[[T]]::$DependentName[[val]]);
+  void $Function_decl[[foo]](typename $TemplateParameter[[T]]::$Type_dependentName[[Type]]
+= $TemplateParameter[[T]]::$Unknown_dependentName[[val]]);
 )cpp",
   R"cpp(
   template 
   void $Function_decl[[foo]]($TemplateParameter[[T]] $Parameter_decl[[P]]) {
-$Parameter[[P]].$DependentName[[Field]];
+$Parameter[[P]].$Unknown_dependentName[[Field]];
   }
 )cpp",
   R"cpp(
   template 
   class $Class_decl[[A]] {
 int $Method_decl[[foo]]() {
-  return $TemplateParameter[[T]]::$DependentName[[Field]];
+  return $TemplateParameter[[T]]::$Unknown_dependentName[[Field]];
 }
   };
 )cpp",
@@ -674,15 +671,15 @@
   template 
   struct $Class_decl[[Waldo]] {
 using $Typedef_decl[[Location1]] = typename $TemplateParameter[[T]]
-::$DependentType[[Resolver]]::$DependentType[[Location]];
+::$Type_dependentName[[Resolver]]::$Type_dependentName[[Location]];
 using $Typedef_decl[[Location2]] = typename $TemplateParameter[[T]]
-::template $DependentType[[Resolver]]<$TemplateParameter[[T]]>
-::$DependentType[[Location]];
+::template $Type_dependentName[[Resolver]]<$TemplateParameter[[T]]>
+::$Type_dependentName[[Location]];
 using $Typedef_decl[[Location3]] = typename $TemplateParameter[[T]]
-::$DependentType[[Resolver]]
-::template $DependentType[[Location]]<$TemplateParameter[[T]]>;
+::$Type_dependentName[[Resolver]]
+::template $Type_dependentName[[Location]]<$TemplateParameter[[T]]>;
 

[clang-tools-extra] 59c1139 - [clangd] Expose more dependent-name detail via semanticTokens

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2021-02-09T20:40:59+01:00
New Revision: 59c1139d3ee127a2049de5c711f81d46d8ec4e41

URL: 
https://github.com/llvm/llvm-project/commit/59c1139d3ee127a2049de5c711f81d46d8ec4e41
DIFF: 
https://github.com/llvm/llvm-project/commit/59c1139d3ee127a2049de5c711f81d46d8ec4e41.diff

LOG: [clangd] Expose more dependent-name detail via semanticTokens

This change makes dependentName a modifier, rather than a token type.
It can be combined with:
- type (new, standard) - this combination replaces dependentType like 
T::typename Foo
- unknown (new, nonstandard) - for general dependent names
- Field, etc - when the name is dependent but we heuristically resolve it

While here, fix cases where template-template-parameter cases were
incorrectly flagged as type-dependent.
And the merging of modifiers when resolving conflicts accidentally
happens to work around a bug that showed up in a test.

The behavior observed through the pre-standard protocol should be mostly
unchanged (it'll see the bugfixes only). This is done in a somehat
fragile way but it's not expected to live long.

Differential Revision: https://reviews.llvm.org/D95706

Added: 


Modified: 
clang-tools-extra/clangd/SemanticHighlighting.cpp
clang-tools-extra/clangd/SemanticHighlighting.h
clang-tools-extra/clangd/test/initialize-params.test
clang-tools-extra/clangd/test/semantic-tokens.test
clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp 
b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index 29afae3746f8..0e50fc133e4d 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -213,21 +213,31 @@ SourceLocation 
getHighlightableSpellingToken(SourceLocation L,
   return getHighlightableSpellingToken(SM.getImmediateSpellingLoc(L), SM);
 }
 
-unsigned evaluateHighlightPriority(HighlightingKind Kind) {
+unsigned evaluateHighlightPriority(const HighlightingToken ) {
   enum HighlightPriority { Dependent = 0, Resolved = 1 };
-  return Kind == HighlightingKind::DependentType ||
- Kind == HighlightingKind::DependentName
+  return (Tok.Modifiers & (1 << uint32_t(HighlightingModifier::DependentName)))
  ? Dependent
  : Resolved;
 }
 
-// Sometimes we get conflicts between findExplicitReferences() returning
-// a heuristic result for a dependent name (e.g. Method) and
-// CollectExtraHighlighting returning a fallback dependent highlighting (e.g.
-// DependentName). In such cases, resolve the conflict in favour of the
-// resolved (non-dependent) highlighting.
-// With macros we can get other conflicts (if a spelled token has multiple
-// expansions with 
diff erent token types) which we can't usefully resolve.
+// Sometimes we get multiple tokens at the same location:
+//
+// - findExplicitReferences() returns a heuristic result for a dependent name
+//   (e.g. Method) and CollectExtraHighlighting returning a fallback dependent
+//   highlighting (e.g. Unknown+Dependent).
+// - macro arguments are expanded multiple times and have 
diff erent roles
+// - broken code recovery produces several AST nodes at the same location
+//
+// We should either resolve these to a single token, or drop them all.
+// Our heuristics are:
+//
+// - token kinds that come with "dependent-name" modifiers are less reliable
+//   (these tend to be vague, like Type or Unknown)
+// - if we have multiple equally reliable kinds, drop token rather than guess
+// - take the union of modifiers from all tokens
+//
+// In particular, heuristically resolved dependent names get their heuristic
+// kind, plus the dependent modifier.
 llvm::Optional
 resolveConflict(ArrayRef Tokens) {
   if (Tokens.size() == 1)
@@ -236,11 +246,13 @@ resolveConflict(ArrayRef Tokens) {
   if (Tokens.size() != 2)
 return llvm::None;
 
-  unsigned Priority1 = evaluateHighlightPriority(Tokens[0].Kind);
-  unsigned Priority2 = evaluateHighlightPriority(Tokens[1].Kind);
-  if (Priority1 == Priority2)
+  unsigned Priority1 = evaluateHighlightPriority(Tokens[0]);
+  unsigned Priority2 = evaluateHighlightPriority(Tokens[1]);
+  if (Priority1 == Priority2 && Tokens[0].Kind != Tokens[1].Kind)
 return llvm::None;
-  return Priority1 > Priority2 ? Tokens[0] : Tokens[1];
+  auto Result = Priority1 > Priority2 ? Tokens[0] : Tokens[1];
+  Result.Modifiers = Tokens[0].Modifiers | Tokens[1].Modifiers;
+  return Result;
 }
 
 /// Consumes source locations and maps them to text ranges for highlightings.
@@ -430,7 +442,8 @@ class CollectExtraHighlightings
   bool VisitOverloadExpr(OverloadExpr *E) {
 if (!E->decls().empty())
   return true; // handled by findExplicitReferences.
-auto  = H.addToken(E->getNameLoc(), HighlightingKind::DependentName);
+auto  = H.addToken(E->getNameLoc(), 

[PATCH] D95701: [clangd] Add semanticTokens modifiers for function/class/file/global scope

[Nathan Ridge via Phabricator via cfe-commits]

nridge added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.h:82
+  FileScope,
+  GlobalScope,
+

sammccall wrote:
> nridge wrote:
> > Would it make sense to call this `NamespaceScope` instead?
> > 
> > I understand it's "global" in the sense that it's visible to other 
> > translation units, but it's not "global" in the sense that it's not 
> > necessarily in the global namespace.
> > 
> > (On the other hand, I can see how `FileScope` symbols are also "namespace 
> > scope", so... could go either way.)
> Yeah, the file-scope thing. (Maybe it's not an important distinction, but it 
> seems pretty interesting for functions at least)
> 
> The other consideration is that I harbor a little bit of hope we could get 
> some convergence across implementations, so avoiding lang-specific terms is 
> nice. In any case, client implementers are probably not C++ people!
> (of course we can use different names internally than we use in the protocol, 
> but it seems less confusing to align them)
+1 for a distinct `FileScope` being useful


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95701

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

[PATCH] D92808: [ObjC][ARC] Use operand bundle 'clang.arc.rv' instead of explicitly emitting retainRV or claimRV calls in the IR

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

This patch causes opt to crash when the following IR is compiled:

$ cat test.ll

  @g0 = global i8 zeroinitializer, align 1
  
  define internal i8* @foo() {
ret i8* @g0
  }
  
  define void @test() {
%r = call i8* @foo() [ "clang.arc.rv"(i64 1) ]
call void (...) @llvm.objc.clang.arc.noop.use(i8* %r)
ret void
  }
  
  declare void @llvm.objc.clang.arc.noop.use(...)
  
  !llvm.module.flags = !{!0}
  
  !0 = !{i32 1, !"clang.arc.retainAutoreleasedReturnValueMarker", !"mov\09fp, 
fp\09\09// marker for objc_retainAutoreleaseReturnValue"}

$ opt -ipsccp -deadargelim -objc-arc -o - -S test.ll

What's happening is that ipsccp replaces argument `%r` passed to call 
`@llvm.objc.clang.arc.noop.use` with `@g0` and then deadargelim changes the 
return type of `@foo` to `void` since there are no explicit users of its 
return. ARC optimizer crashes because it expects a call with the operand bundle 
to have a return.

To fix the crash, we can modify `tryToReplaceWithConstant` to prevent ipsccp 
from replacing `@llvm.objc.clang.arc.noop.use`'s argument. Alternatively, we 
can restore the check in `DeadArgumentEliminationPass::SurveyFunction`, which 
was in earlier versions of the patch (see 
https://reviews.llvm.org/D92808?id=319082#inline-892965). I feel like the 
latter option is better since we wouldn't' have to prevent ipsccp from 
optimizing the IR, which is perfectly legal. If it's not desirable to add a 
check that is too specific to ObjC, we could add a new generic function to the 
core library, which indicates the function's return type shouldn't be changed, 
and use it to tell deadargelim not to change the return type. Or we could use a 
bundle that is more generic than `clang.arc.rv` (for example, 
`"implicitly.used.by"(@llvm.objc.retainAutoreleasedReturnValue)`, which 
indicates the result is used by an instruction that isn't explicitly emitted in 
the IR).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92808

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

[PATCH] D96363: Mark output as text if it is really text

[Abhina Sree via Phabricator via cfe-commits]

abhina.sreeskantharajan created this revision.
Herald added a reviewer: JDevlieghere.
abhina.sreeskantharajan requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

This is a continuation of https://reviews.llvm.org/D67696. The following places 
need to set the OF_Text flag correctly.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96363

Files:
  clang/lib/ARCMigrate/FileRemapper.cpp
  clang/lib/Driver/Driver.cpp
  clang/lib/Frontend/Rewrite/FrontendActions.cpp
  llvm/tools/dsymutil/DwarfLinkerForBinary.cpp


Index: llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
===
--- llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
+++ llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
@@ -252,7 +252,10 @@
   }
 
   std::error_code EC;
-  raw_fd_ostream OS(Options.NoOutput ? "-" : Path.str(), EC, sys::fs::OF_None);
+  raw_fd_ostream OS(Options.NoOutput ? "-" : Path.str(), EC,
+Options.RemarksFormat == remarks::Format::Bitstream
+? sys::fs::OF_None
+: sys::fs::OF_Text);
   if (EC)
 return errorCodeToError(EC);
 
Index: clang/lib/Frontend/Rewrite/FrontendActions.cpp
===
--- clang/lib/Frontend/Rewrite/FrontendActions.cpp
+++ clang/lib/Frontend/Rewrite/FrontendActions.cpp
@@ -185,7 +185,7 @@
 void RewriteMacrosAction::ExecuteAction() {
   CompilerInstance  = getCompilerInstance();
   std::unique_ptr OS =
-  CI.createDefaultOutputFile(true, getCurrentFileOrBufferName());
+  CI.createDefaultOutputFile(false, getCurrentFileOrBufferName());
   if (!OS) return;
 
   RewriteMacrosInInput(CI.getPreprocessor(), OS.get());
@@ -270,7 +270,7 @@
 bool RewriteIncludesAction::BeginSourceFileAction(CompilerInstance ) {
   if (!OutputStream) {
 OutputStream =
-CI.createDefaultOutputFile(true, getCurrentFileOrBufferName());
+CI.createDefaultOutputFile(false, getCurrentFileOrBufferName());
 if (!OutputStream)
   return false;
   }
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -1434,7 +1434,9 @@
   llvm::SmallString<128> Script(CrashInfo.Filename);
   llvm::sys::path::replace_extension(Script, "sh");
   std::error_code EC;
-  llvm::raw_fd_ostream ScriptOS(Script, EC, llvm::sys::fs::CD_CreateNew);
+  llvm::raw_fd_ostream ScriptOS(Script, EC, llvm::sys::fs::CD_CreateNew,
+llvm::sys::fs::FA_Write,
+llvm::sys::fs::OF_Text);
   if (EC) {
 Diag(clang::diag::note_drv_command_failed_diag_msg)
 << "Error generating run script: " << Script << " " << EC.message();
Index: clang/lib/ARCMigrate/FileRemapper.cpp
===
--- clang/lib/ARCMigrate/FileRemapper.cpp
+++ clang/lib/ARCMigrate/FileRemapper.cpp
@@ -121,7 +121,7 @@
 
   std::error_code EC;
   std::string infoFile = std::string(outputPath);
-  llvm::raw_fd_ostream infoOut(infoFile, EC, llvm::sys::fs::OF_None);
+  llvm::raw_fd_ostream infoOut(infoFile, EC, llvm::sys::fs::OF_Text);
   if (EC)
 return report(EC.message(), Diag);
 


Index: llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
===
--- llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
+++ llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
@@ -252,7 +252,10 @@
   }
 
   std::error_code EC;
-  raw_fd_ostream OS(Options.NoOutput ? "-" : Path.str(), EC, sys::fs::OF_None);
+  raw_fd_ostream OS(Options.NoOutput ? "-" : Path.str(), EC,
+Options.RemarksFormat == remarks::Format::Bitstream
+? sys::fs::OF_None
+: sys::fs::OF_Text);
   if (EC)
 return errorCodeToError(EC);
 
Index: clang/lib/Frontend/Rewrite/FrontendActions.cpp
===
--- clang/lib/Frontend/Rewrite/FrontendActions.cpp
+++ clang/lib/Frontend/Rewrite/FrontendActions.cpp
@@ -185,7 +185,7 @@
 void RewriteMacrosAction::ExecuteAction() {
   CompilerInstance  = getCompilerInstance();
   std::unique_ptr OS =
-  CI.createDefaultOutputFile(true, getCurrentFileOrBufferName());
+  CI.createDefaultOutputFile(false, getCurrentFileOrBufferName());
   if (!OS) return;
 
   RewriteMacrosInInput(CI.getPreprocessor(), OS.get());
@@ -270,7 +270,7 @@
 bool RewriteIncludesAction::BeginSourceFileAction(CompilerInstance ) {
   if (!OutputStream) {
 OutputStream =
-CI.createDefaultOutputFile(true, getCurrentFileOrBufferName());
+CI.createDefaultOutputFile(false, getCurrentFileOrBufferName());
 if (!OutputStream)
   return false;
   }
Index: clang/lib/Driver/Driver.cpp

[PATCH] D95706: [clangd] Expose more dependent-name detail via semanticTokens

[Nathan Ridge via Phabricator via cfe-commits]

nridge added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.h:78
   Abstract,
+  DependentName,
 

sammccall wrote:
> nridge wrote:
> > Just `Dependent` might be enough
> AFAICS the current implementation is closer to "dependent name" than 
> "dependent", I think.
> 
> For example, in the RHS of `template  using Val = T::value_type`, 
> both "T" and "value_type" certainly refer to dependent types. But only 
> `value_type` is a dependent name, and only `value_type` gets the old 
> kinds/new modifier.
> 
> ---
> 
> Or are you proposing we change the implementation too?
> 
> It's not intuitively which version would be more useful. FWIW I have the 
> current DependentName kind highlighted in bold bright orange, and I find it 
> really helpful :-) When there are errors and RecoveryExpr kicks in, it colors 
> the resulting unresolved names, which I like surprisingly much.
I like the current impl as well, was not proposing a change to it. I was just 
quibbling about the name. Please feel free to ignore, it makes sense to be 
consistent with clang's terminology.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95706

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

[PATCH] D86376: [HIP] Emit kernel symbol

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D86376#2551298 , @yaxunl wrote:

> Actually there is one issue with this approach.
>
> HIP have API's to launch kernels, which accept kernel as function pointer 
> argument. Currently when taking address of kernel, we get the stub function. 
> These kernel launching API's will not work if we use kernel symbol to 
> register the kernel. A solution is to return the kernel symbol instead of 
> stub function when taking address of the kernel in host compilation, i.e. if 
> a function pointer is assigned to a kernel in host code, it gets the kernel 
> symbol instead of the stub function. This will make the kernel launching API 
> work.
>
> To keep the triple chevron working, the kernel symbol will be initialized 
> with the address of the stub function. For triple chevron call, the address 
> of the stub function is loaded from the kernel symbol and invoked.

This could work.
Do we really need an indirection? If we know the stub address when we 
initialize the symbol with it, we should be able to use that address for 
`<<<>>>`.


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

https://reviews.llvm.org/D86376

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

[PATCH] D96161: [OpenCL] Fix printing of types with signed prefix in arg info metadata

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

The clarification to the spec: 
https://github.com/KhronosGroup/OpenCL-Docs/pull/558


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96161

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

[PATCH] D96360: [CMake] Delete LLVM_RUNTIME_BUILD_ID_LINK_TARGETS

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: beanz, phosek, smeenai.
Herald added a subscriber: mgorny.
Herald added a reviewer: alexshap.
MaskRay requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96360

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake
  llvm/runtimes/CMakeLists.txt
  llvm/runtimes/llvm-strip-link.in


Index: llvm/runtimes/llvm-strip-link.in
===
--- llvm/runtimes/llvm-strip-link.in
+++ /dev/null
@@ -1,27 +0,0 @@
-#!/usr/bin/env python
-# -*- coding: utf-8 -*-
-
-import os
-import sys
-import subprocess
-
-
-ELF_MAGIC = '\x7fELF'
-
-with open(sys.argv[1], "rb") as f:
-buf = f.read(len(ELF_MAGIC))
-if buf != ELF_MAGIC:
-sys.exit(0)
-
-llvm_objcopy = os.path.join('@LLVM_RUNTIME_OUTPUT_INTDIR@', 'llvm-objcopy')
-install_dir = os.path.join(os.getenv('DESTDIR', ''), '@CMAKE_INSTALL_PREFIX@')
-link_dir = os.path.join(install_dir, 'lib', 'debug', '.build-id')
-
-sys.exit(subprocess.call([
-llvm_objcopy,
-'--strip-all',
-'--build-id-link-dir=' + link_dir,
-'--build-id-link-input=.debug',
-'--build-id-link-output=',
-sys.argv[1],
-]))
Index: llvm/runtimes/CMakeLists.txt
===
--- llvm/runtimes/CMakeLists.txt
+++ llvm/runtimes/CMakeLists.txt
@@ -187,14 +187,6 @@
   list(APPEND runtime_names ${projName})
 endforeach()
 
-if(LLVM_RUNTIME_BUILD_ID_LINK_TARGETS)
-  configure_file(
-${CMAKE_CURRENT_SOURCE_DIR}/llvm-strip-link.in
-${CMAKE_CURRENT_BINARY_DIR}/llvm-strip-link
-@ONLY
-  )
-endif()
-
 function(runtime_default_target)
   cmake_parse_arguments(ARG "" "" "DEPENDS;PREFIXES" ${ARGN})
 
@@ -329,10 +321,6 @@
 list(APPEND ${name}_extra_args 
-DLLVM_ENABLE_RUNTIMES=${LLVM_ENABLE_RUNTIMES_PASSTHROUGH})
   endif()
 
-  if(target IN_LIST LLVM_RUNTIME_BUILD_ID_LINK_TARGETS)
-list(APPEND EXTRA_ARGS STRIP_TOOL 
${CMAKE_CURRENT_BINARY_DIR}/llvm-strip-link)
-  endif()
-
   llvm_ExternalProject_Add(runtimes-${name}
${CMAKE_CURRENT_SOURCE_DIR}/../../runtimes
DEPENDS ${${name}_deps}
Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -249,7 +249,6 @@
 
 set(LLVM_BUILTIN_TARGETS "${BUILTIN_TARGETS}" CACHE STRING "")
 set(LLVM_RUNTIME_TARGETS "${RUNTIME_TARGETS}" CACHE STRING "")
-set(LLVM_RUNTIME_BUILD_ID_LINK_TARGETS "${RUNTIME_BUILD_ID_LINK}" CACHE STRING 
"")
 
 # Setup toolchain.
 set(LLVM_INSTALL_TOOLCHAIN_ONLY ON CACHE BOOL "")


Index: llvm/runtimes/llvm-strip-link.in
===
--- llvm/runtimes/llvm-strip-link.in
+++ /dev/null
@@ -1,27 +0,0 @@
-#!/usr/bin/env python
-# -*- coding: utf-8 -*-
-
-import os
-import sys
-import subprocess
-
-
-ELF_MAGIC = '\x7fELF'
-
-with open(sys.argv[1], "rb") as f:
-buf = f.read(len(ELF_MAGIC))
-if buf != ELF_MAGIC:
-sys.exit(0)
-
-llvm_objcopy = os.path.join('@LLVM_RUNTIME_OUTPUT_INTDIR@', 'llvm-objcopy')
-install_dir = os.path.join(os.getenv('DESTDIR', ''), '@CMAKE_INSTALL_PREFIX@')
-link_dir = os.path.join(install_dir, 'lib', 'debug', '.build-id')
-
-sys.exit(subprocess.call([
-llvm_objcopy,
-'--strip-all',
-'--build-id-link-dir=' + link_dir,
-'--build-id-link-input=.debug',
-'--build-id-link-output=',
-sys.argv[1],
-]))
Index: llvm/runtimes/CMakeLists.txt
===
--- llvm/runtimes/CMakeLists.txt
+++ llvm/runtimes/CMakeLists.txt
@@ -187,14 +187,6 @@
   list(APPEND runtime_names ${projName})
 endforeach()
 
-if(LLVM_RUNTIME_BUILD_ID_LINK_TARGETS)
-  configure_file(
-${CMAKE_CURRENT_SOURCE_DIR}/llvm-strip-link.in
-${CMAKE_CURRENT_BINARY_DIR}/llvm-strip-link
-@ONLY
-  )
-endif()
-
 function(runtime_default_target)
   cmake_parse_arguments(ARG "" "" "DEPENDS;PREFIXES" ${ARGN})
 
@@ -329,10 +321,6 @@
 list(APPEND ${name}_extra_args -DLLVM_ENABLE_RUNTIMES=${LLVM_ENABLE_RUNTIMES_PASSTHROUGH})
   endif()
 
-  if(target IN_LIST LLVM_RUNTIME_BUILD_ID_LINK_TARGETS)
-list(APPEND EXTRA_ARGS STRIP_TOOL ${CMAKE_CURRENT_BINARY_DIR}/llvm-strip-link)
-  endif()
-
   llvm_ExternalProject_Add(runtimes-${name}
${CMAKE_CURRENT_SOURCE_DIR}/../../runtimes
DEPENDS ${${name}_deps}
Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -249,7 +249,6 @@
 
 set(LLVM_BUILTIN_TARGETS "${BUILTIN_TARGETS}" CACHE STRING "")
 set(LLVM_RUNTIME_TARGETS "${RUNTIME_TARGETS}" CACHE STRING "")

[PATCH] D88645: [Annotation] Allows annotation to carry some additional constant arguments.

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert requested changes to this revision.
jdoerfert added a comment.
This revision now requires changes to proceed.

status change, see last message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88645

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

[PATCH] D88645: [Annotation] Allows annotation to carry some additional constant arguments.

[Johannes Doerfert via Phabricator via cfe-commits]

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

The lang ref needs to reflect the new type 
https://llvm.org/docs/LangRef.html#llvm-var-annotation-intrinsic
Please also review the auto-upgrade patch for this: 
https://reviews.llvm.org/D95993


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88645

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

[PATCH] D95655: [AArch64] Adding Neon Sm3 & Sm4 Intrinsics

[Ana Pazos via Phabricator via cfe-commits]

apazos added inline comments.



Comment at: clang/test/CodeGen/aarch64-neon-sm4-sm3-invalid.c:8
+
+void test_range_check_vsm3tt1a(uint32x4_t a, uint32x4_t b, uint32x4_t c) {
+  // CHECK-RANGE: error: argument value 5 is outside the valid range [0, 3]

Consider renaming the test with range-check suffix so it is clear it is 
checking for invalid  range for the immediate
In addition, consider rewriting it with //expected-error checks



Comment at: clang/test/CodeGen/aarch64-neon-sm4-sm3.c:5
+
+// RUN: not %clang_cc1 -triple aarch64-linux-gnu -target-feature +neon \
+// RUN: -S -emit-llvm -o - %s 2>&1 | FileCheck --check-prefix=CHECK-NO-CRYPTO 
%s

The test is written for aarch64 triple, in addition @t.p.northover might want 
to add arm64 triple checks. Tim what do you think?


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

https://reviews.llvm.org/D95655

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

[clang] 2aa01cc - [CUDA, NVPTX] Allow targeting sm_86 GPUs.

[Artem Belevich via cfe-commits]

Author: Artem Belevich
Date: 2021-02-09T11:01:10-08:00
New Revision: 2aa01ccec30109fbcc65934c5d7c8907793e0660

URL: 
https://github.com/llvm/llvm-project/commit/2aa01ccec30109fbcc65934c5d7c8907793e0660
DIFF: 
https://github.com/llvm/llvm-project/commit/2aa01ccec30109fbcc65934c5d7c8907793e0660.diff

LOG: [CUDA, NVPTX] Allow targeting sm_86 GPUs.

The patch only plumbs through the option necessary for targeting sm_86 GPUs w/o
adding any new functionality.

Differential Revision: https://reviews.llvm.org/D95974

Added: 


Modified: 
clang/include/clang/Basic/BuiltinsNVPTX.def
clang/include/clang/Basic/Cuda.h
clang/lib/Basic/Cuda.cpp
clang/lib/Basic/Targets/NVPTX.cpp
clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
clang/lib/Driver/ToolChains/Cuda.cpp
llvm/lib/Target/NVPTX/NVPTX.td

Removed: 




diff  --git a/clang/include/clang/Basic/BuiltinsNVPTX.def 
b/clang/include/clang/Basic/BuiltinsNVPTX.def
index d149fa0127b9..44a5e4ae01c1 100644
--- a/clang/include/clang/Basic/BuiltinsNVPTX.def
+++ b/clang/include/clang/Basic/BuiltinsNVPTX.def
@@ -21,7 +21,9 @@
 #pragma push_macro("SM_72")
 #pragma push_macro("SM_75")
 #pragma push_macro("SM_80")
-#define SM_80 "sm_80"
+#pragma push_macro("SM_86")
+#define SM_86 "sm_86"
+#define SM_80 "sm_80|" SM_86
 #define SM_75 "sm_75|" SM_80
 #define SM_72 "sm_72|" SM_75
 #define SM_70 "sm_70|" SM_72
@@ -35,7 +37,9 @@
 #pragma push_macro("PTX64")
 #pragma push_macro("PTX65")
 #pragma push_macro("PTX70")
-#define PTX70 "ptx70"
+#pragma push_macro("PTX71")
+#define PTX71 "ptx71"
+#define PTX70 "ptx70|" PTX71
 #define PTX65 "ptx65|" PTX70
 #define PTX64 "ptx64|" PTX65
 #define PTX63 "ptx63|" PTX64
@@ -728,9 +732,11 @@ TARGET_BUILTIN(__imma_m8n8k32_st_c_i32, "vi*iC*UiIi", "", 
AND(SM_75,PTX63))
 #pragma pop_macro("SM_72")
 #pragma pop_macro("SM_75")
 #pragma pop_macro("SM_80")
+#pragma pop_macro("SM_86")
 #pragma pop_macro("PTX60")
 #pragma pop_macro("PTX61")
 #pragma pop_macro("PTX63")
 #pragma pop_macro("PTX64")
 #pragma pop_macro("PTX65")
 #pragma pop_macro("PTX70")
+#pragma pop_macro("PTX71")

diff  --git a/clang/include/clang/Basic/Cuda.h 
b/clang/include/clang/Basic/Cuda.h
index b3a2e99fe931..12ffa3e04fb8 100644
--- a/clang/include/clang/Basic/Cuda.h
+++ b/clang/include/clang/Basic/Cuda.h
@@ -29,7 +29,9 @@ enum class CudaVersion {
   CUDA_101,
   CUDA_102,
   CUDA_110,
-  LATEST = CUDA_110,
+  CUDA_111,
+  CUDA_112,
+  LATEST = CUDA_112,
   LATEST_SUPPORTED = CUDA_101,
 };
 const char *CudaVersionToString(CudaVersion V);
@@ -55,6 +57,7 @@ enum class CudaArch {
   SM_72,
   SM_75,
   SM_80,
+  SM_86,
   GFX600,
   GFX601,
   GFX602,

diff  --git a/clang/lib/Basic/Cuda.cpp b/clang/lib/Basic/Cuda.cpp
index 144113f2d2e7..22eea1fb9cf6 100644
--- a/clang/lib/Basic/Cuda.cpp
+++ b/clang/lib/Basic/Cuda.cpp
@@ -32,6 +32,10 @@ const char *CudaVersionToString(CudaVersion V) {
 return "10.2";
   case CudaVersion::CUDA_110:
 return "11.0";
+  case CudaVersion::CUDA_111:
+return "11.1";
+  case CudaVersion::CUDA_112:
+return "11.2";
   }
   llvm_unreachable("invalid enum");
 }
@@ -48,6 +52,8 @@ CudaVersion CudaStringToVersion(const llvm::Twine ) {
   .Case("10.1", CudaVersion::CUDA_101)
   .Case("10.2", CudaVersion::CUDA_102)
   .Case("11.0", CudaVersion::CUDA_110)
+  .Case("11.1", CudaVersion::CUDA_111)
+  .Case("11.2", CudaVersion::CUDA_112)
   .Default(CudaVersion::UNKNOWN);
 }
 
@@ -71,7 +77,7 @@ CudaArchToStringMap arch_names[] = {
 SM(60), SM(61), SM(62),  // Pascal
 SM(70), SM(72),  // Volta
 SM(75),  // Turing
-SM(80),  // Ampere
+SM(80), SM(86),  // Ampere
 GFX(600),  // gfx600
 GFX(601),  // gfx601
 GFX(602),  // gfx602
@@ -164,6 +170,8 @@ CudaVersion MinVersionForCudaArch(CudaArch A) {
 return CudaVersion::CUDA_100;
   case CudaArch::SM_80:
 return CudaVersion::CUDA_110;
+  case CudaArch::SM_86:
+return CudaVersion::CUDA_111;
   default:
 llvm_unreachable("invalid enum");
   }
@@ -209,6 +217,10 @@ CudaVersion ToCudaVersion(llvm::VersionTuple Version) {
 return CudaVersion::CUDA_102;
   case 110:
 return CudaVersion::CUDA_110;
+  case 111:
+return CudaVersion::CUDA_111;
+  case 112:
+return CudaVersion::CUDA_112;
   default:
 return CudaVersion::UNKNOWN;
   }

diff  --git a/clang/lib/Basic/Targets/NVPTX.cpp 
b/clang/lib/Basic/Targets/NVPTX.cpp
index b7f0dce33d2b..da8a578fa557 100644
--- a/clang/lib/Basic/Targets/NVPTX.cpp
+++ b/clang/lib/Basic/Targets/NVPTX.cpp
@@ -45,6 +45,8 @@ NVPTXTargetInfo::NVPTXTargetInfo(const llvm::Triple ,
 if (!Feature.startswith("+ptx"))
   continue;
 PTXVersion = llvm::StringSwitch(Feature)
+ .Case("+ptx72", 72)
+ .Case("+ptx71", 71)
  .Case("+ptx70", 70)
  

[PATCH] D95974: [CUDA, NVPTX] Allow targeting sm_86 GPUs.

[Artem Belevich 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 rG2aa01ccec301: [CUDA, NVPTX] Allow targeting sm_86 GPUs. 
(authored by tra).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95974

Files:
  clang/include/clang/Basic/BuiltinsNVPTX.def
  clang/include/clang/Basic/Cuda.h
  clang/lib/Basic/Cuda.cpp
  clang/lib/Basic/Targets/NVPTX.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/Driver/ToolChains/Cuda.cpp
  llvm/lib/Target/NVPTX/NVPTX.td

Index: llvm/lib/Target/NVPTX/NVPTX.td
===
--- llvm/lib/Target/NVPTX/NVPTX.td
+++ llvm/lib/Target/NVPTX/NVPTX.td
@@ -57,6 +57,8 @@
  "Target SM 7.5">;
 def SM80 : SubtargetFeature<"sm_80", "SmVersion", "80",
  "Target SM 8.0">;
+def SM86 : SubtargetFeature<"sm_86", "SmVersion", "86",
+ "Target SM 8.6">;
 
 // PTX Versions
 def PTX32 : SubtargetFeature<"ptx32", "PTXVersion", "32",
@@ -83,6 +85,10 @@
  "Use PTX version 6.5">;
 def PTX70 : SubtargetFeature<"ptx70", "PTXVersion", "70",
  "Use PTX version 7.0">;
+def PTX71 : SubtargetFeature<"ptx71", "PTXVersion", "71",
+ "Use PTX version 7.1">;
+def PTX72 : SubtargetFeature<"ptx72", "PTXVersion", "72",
+ "Use PTX version 7.2">;
 
 //===--===//
 // NVPTX supported processors.
@@ -107,6 +113,7 @@
 def : Proc<"sm_72", [SM72, PTX61]>;
 def : Proc<"sm_75", [SM75, PTX63]>;
 def : Proc<"sm_80", [SM80, PTX70]>;
+def : Proc<"sm_86", [SM86, PTX71]>;
 
 def NVPTXInstrInfo : InstrInfo {
 }
Index: clang/lib/Driver/ToolChains/Cuda.cpp
===
--- clang/lib/Driver/ToolChains/Cuda.cpp
+++ clang/lib/Driver/ToolChains/Cuda.cpp
@@ -75,6 +75,8 @@
 return CudaVersion::CUDA_102;
   if (raw_version < 11010)
 return CudaVersion::CUDA_110;
+  if (raw_version < 11020)
+return CudaVersion::CUDA_111;
   return CudaVersion::LATEST;
 }
 
@@ -720,6 +722,8 @@
 CudaVersionStr = #CUDA_VER;\
 PtxFeature = "+ptx" #PTX_VER;  \
 break;
+CASE_CUDA_VERSION(112, 72);
+CASE_CUDA_VERSION(111, 71);
 CASE_CUDA_VERSION(110, 70);
 CASE_CUDA_VERSION(102, 65);
 CASE_CUDA_VERSION(101, 64);
Index: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
@@ -4613,6 +4613,7 @@
   case CudaArch::SM_72:
   case CudaArch::SM_75:
   case CudaArch::SM_80:
+  case CudaArch::SM_86:
   case CudaArch::GFX600:
   case CudaArch::GFX601:
   case CudaArch::GFX602:
@@ -4680,6 +4681,7 @@
   case CudaArch::SM_72:
   case CudaArch::SM_75:
   case CudaArch::SM_80:
+  case CudaArch::SM_86:
 return {84, 32};
   case CudaArch::GFX600:
   case CudaArch::GFX601:
Index: clang/lib/Basic/Targets/NVPTX.cpp
===
--- clang/lib/Basic/Targets/NVPTX.cpp
+++ clang/lib/Basic/Targets/NVPTX.cpp
@@ -45,6 +45,8 @@
 if (!Feature.startswith("+ptx"))
   continue;
 PTXVersion = llvm::StringSwitch(Feature)
+ .Case("+ptx72", 72)
+ .Case("+ptx71", 71)
  .Case("+ptx70", 70)
  .Case("+ptx65", 65)
  .Case("+ptx64", 64)
@@ -246,6 +248,8 @@
 return "750";
   case CudaArch::SM_80:
 return "800";
+  case CudaArch::SM_86:
+return "860";
   }
   llvm_unreachable("unhandled CudaArch");
 }();
Index: clang/lib/Basic/Cuda.cpp
===
--- clang/lib/Basic/Cuda.cpp
+++ clang/lib/Basic/Cuda.cpp
@@ -32,6 +32,10 @@
 return "10.2";
   case CudaVersion::CUDA_110:
 return "11.0";
+  case CudaVersion::CUDA_111:
+return "11.1";
+  case CudaVersion::CUDA_112:
+return "11.2";
   }
   llvm_unreachable("invalid enum");
 }
@@ -48,6 +52,8 @@
   .Case("10.1", CudaVersion::CUDA_101)
   .Case("10.2", CudaVersion::CUDA_102)
   .Case("11.0", CudaVersion::CUDA_110)
+  .Case("11.1", CudaVersion::CUDA_111)
+  .Case("11.2", CudaVersion::CUDA_112)
   .Default(CudaVersion::UNKNOWN);
 }
 
@@ -71,7 +77,7 @@
 SM(60), SM(61), SM(62),  // Pascal
 SM(70), SM(72),  // Volta
 SM(75),  // Turing
-SM(80),  // Ampere
+SM(80), SM(86),  // Ampere
 GFX(600),  // gfx600
 GFX(601),  

[PATCH] D85223: [CUDA][HIP] Support accessing static device variable in host code for -fgpu-rdc

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

What breaks existing abstractions is that we produce N ELF objects from a 
single TU and the meaning of `static` becomes fuzzy. On one hand, we don't want 
that static symbol to be visible across objects on the same target, at the same 
time we do want it to be visible across host/device objects compiled from the 
same TU.  ELF does not have a way to express it. Making such symbols visible 
with an unique suffix seems to be a reasonable tradeoff. We probably have more 
options available for AMDGPU. E.g. as you've suggested, give runtime extra 
clues about referencing these variables across host/device boundary without 
resorting to making them externally visible. However, we don't have that 
flexibility for NVPTX.


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

https://reviews.llvm.org/D85223

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

[PATCH] D94779: [Clang] Ensure vector predication pragma is ignored only when vectorization width is 1.

[Malhar via Phabricator via cfe-commits]

malharJ updated this revision to Diff 322445.
malharJ added a comment.

This update removes the dependence of (emitting) vectorize_predicate metadata 
on whether vectorization
is disabled (via vectorize(disable) or vector_width(1)) or not.

This means that vectorize_predicate loop metadata is emitted whenever the 
pragma has been specified.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94779

Files:
  clang/lib/CodeGen/CGLoopInfo.cpp
  clang/test/CodeGenCXX/pragma-loop-predicate.cpp


Index: clang/test/CodeGenCXX/pragma-loop-predicate.cpp
===
--- clang/test/CodeGenCXX/pragma-loop-predicate.cpp
+++ clang/test/CodeGenCXX/pragma-loop-predicate.cpp
@@ -58,6 +58,49 @@
 List[i] = i * 2;
 }
 
+// Check that vectorize_predicate is ignored when vectorization width is 1
+void test6(int *List, int Length) {
+// CHECK-LABEL: @{{.*}}test6{{.*}}(
+// CHECK: br label {{.*}}, !llvm.loop ![[LOOP6:.*]]
+
+#pragma clang loop vectorize_predicate(disable) vectorize_width(1)
+  for (int i = 0; i < Length; i++)
+List[i] = i * 2;
+}
+
+
+// Check that vectorize_width(!=1) does not affect vectorize_predicate.
+void test7(int *List, int Length) {
+// CHECK-LABEL: @{{.*}}test7{{.*}}(
+// CHECK: br label {{.*}}, !llvm.loop ![[LOOP7:.*]]
+
+#pragma clang loop vectorize_predicate(disable) vectorize_width(4)
+  for (int i = 0; i < Length; i++)
+List[i] = i * 2;
+}
+
+
+// Check that vectorize_predicate is ignored when vectorization width is 1
+void test8(int *List, int Length) {
+// CHECK-LABEL: @{{.*}}test8{{.*}}(
+// CHECK: br label {{.*}}, !llvm.loop ![[LOOP8:.*]]
+
+#pragma clang loop vectorize_predicate(enable) vectorize_width(1)
+  for (int i = 0; i < Length; i++)
+List[i] = i * 2;
+}
+
+
+// Check that vectorize_width(!=1) does not affect vectorize_predicate.
+void test9(int *List, int Length) {
+// CHECK-LABEL: @{{.*}}test9{{.*}}(
+// CHECK: br label {{.*}}, !llvm.loop ![[LOOP9:.*]]
+
+#pragma clang loop vectorize_predicate(enable) vectorize_width(4)
+  for (int i = 0; i < Length; i++)
+List[i] = i * 2;
+}
+
 // CHECK:  ![[LOOP0]] = distinct !{![[LOOP0]], [[MP:![0-9]+]], 
[[GEN3:![0-9]+]]}
 // CHECK:  [[MP]] = !{!"llvm.loop.mustprogress"}
 // CHECK-NEXT: [[GEN3]] = !{!"llvm.loop.vectorize.enable", i1 true}
@@ -73,4 +116,14 @@
 // CHECK-NEXT: ![[LOOP4]] = distinct !{![[LOOP4]], [[MP]], [[GEN10:![0-9]+]]}
 // CHECK-NEXT: [[GEN10]] = !{!"llvm.loop.vectorize.width", i32 1}
 
-// CHECK-NEXT: ![[LOOP5]] = distinct !{![[LOOP5]], [[MP]], [[GEN10]]}
+// CHECK-NEXT: ![[LOOP5]] = distinct !{![[LOOP5]], [[MP]], [[GEN6]], [[GEN10]]}
+
+// CHECK-NEXT: ![[LOOP6]] = distinct !{![[LOOP6]], [[MP]], [[GEN8]], 
[[GEN10]], [[GEN11:![0-9]+]]}
+// CHECK-NEXT: [[GEN11]] = !{!"llvm.loop.vectorize.scalable.enable", i1 false}
+
+// CHECK-NEXT: ![[LOOP7]] = distinct !{![[LOOP7]], [[MP]], [[GEN8]], 
[[GEN12:![0-9]+]], [[GEN11]], [[GEN3]]}
+// CHECK-NEXT: [[GEN12]] = !{!"llvm.loop.vectorize.width", i32 4}
+
+// CHECK-NEXT: ![[LOOP8]] = distinct !{![[LOOP8]], [[MP]], [[GEN6]], 
[[GEN10]], [[GEN11]]}
+
+// CHECK-NEXT: ![[LOOP9]] = distinct !{![[LOOP9]], [[MP]], [[GEN6]], 
[[GEN12]], [[GEN11]], [[GEN3]]}
Index: clang/lib/CodeGen/CGLoopInfo.cpp
===
--- clang/lib/CodeGen/CGLoopInfo.cpp
+++ clang/lib/CodeGen/CGLoopInfo.cpp
@@ -250,12 +250,10 @@
   Args.push_back(nullptr);
   Args.append(LoopProperties.begin(), LoopProperties.end());
 
-  // Setting vectorize.predicate
+  // Setting vectorize.predicate when it has been specified and vectorization
+  // has not been disabled.
   bool IsVectorPredicateEnabled = false;
-  if (Attrs.VectorizePredicateEnable != LoopAttributes::Unspecified &&
-  Attrs.VectorizeEnable != LoopAttributes::Disable &&
-  Attrs.VectorizeWidth < 1) {
-
+  if (Attrs.VectorizePredicateEnable != LoopAttributes::Unspecified) {
 IsVectorPredicateEnabled =
 (Attrs.VectorizePredicateEnable == LoopAttributes::Enable);
 
@@ -303,7 +301,8 @@
   //explicitly requested fixed-width vectorization, i.e.
   //vectorize.scalable.enable is false.
   if (Attrs.VectorizeEnable != LoopAttributes::Unspecified ||
-  IsVectorPredicateEnabled || Attrs.VectorizeWidth > 1 ||
+  (IsVectorPredicateEnabled && Attrs.VectorizeWidth != 1) ||
+  Attrs.VectorizeWidth > 1 ||
   Attrs.VectorizeScalable == LoopAttributes::Enable ||
   (Attrs.VectorizeScalable == LoopAttributes::Disable &&
Attrs.VectorizeWidth != 1)) {


Index: clang/test/CodeGenCXX/pragma-loop-predicate.cpp
===
--- clang/test/CodeGenCXX/pragma-loop-predicate.cpp
+++ clang/test/CodeGenCXX/pragma-loop-predicate.cpp
@@ -58,6 +58,49 @@
 List[i] = i * 2;
 }
 
+// Check that vectorize_predicate is ignored when vectorization width is 1
+void test6(int *List, 

[PATCH] D95915: [clang][driver] Only warn once about invalid -stdlib value

[Yaxun Liu via Phabricator via cfe-commits]

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

LGTM. Thanks.


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

https://reviews.llvm.org/D95915

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

[PATCH] D85223: [CUDA][HIP] Support accessing static device variable in host code for -fgpu-rdc

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield accepted this revision.
JonChesterfield added a comment.

This works around the limitations of the binary format nvptx and amdgpu are 
using in the compiler. It's the wrong place in the stack to fix it - we could 
introduce another symbol table in the binary to capture the per-tu-between-arch 
scoping.

However, if we later reach consensus on what to do in the elf instead, we can 
still do that. In particular, embedding an elf for one arch in a named section 
of an elf for a host arch is crude. This workaround seems acceptable in the 
meantime.


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

https://reviews.llvm.org/D85223

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

[PATCH] D96285: [clang][Arm] Fix handling of -Wa,-implicit-it=

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 322439.
nickdesaulniers marked 2 inline comments as done.
nickdesaulniers added a comment.

- address code review comments, add moar tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96285

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/arm-target-as-mimplicit-it.s


Index: clang/test/Driver/arm-target-as-mimplicit-it.s
===
--- /dev/null
+++ clang/test/Driver/arm-target-as-mimplicit-it.s
@@ -0,0 +1,44 @@
+/// Simple tests for valid input.
+/// -Wa,-implicit-it=
+// RUN: %clang -target arm-linux-gnueabi -### -Wa,-mimplicit-it=always %s 2>&1 
| FileCheck %s --check-prefix=ALWAYS
+// RUN: %clang -target arm-linux-gnueabi -### -Wa,-mimplicit-it=never %s 2>&1 
| FileCheck %s --check-prefix=NEVER
+// RUN: %clang -target arm-linux-gnueabi -### -Wa,-mimplicit-it=arm %s 2>&1 | 
FileCheck %s --check-prefix=ARM
+// RUN: %clang -target arm-linux-gnueabi -### -Wa,-mimplicit-it=thumb %s 2>&1 
| FileCheck %s --check-prefix=THUMB
+/// -Xassembler -mimplicit-it=
+// RUN: %clang -target arm-linux-gnueabi -### -Xassembler -mimplicit-it=always 
%s 2>&1 | FileCheck %s --check-prefix=ALWAYS
+// RUN: %clang -target arm-linux-gnueabi -### -Xassembler -mimplicit-it=never 
%s 2>&1 | FileCheck %s --check-prefix=NEVER
+// RUN: %clang -target arm-linux-gnueabi -### -Xassembler -mimplicit-it=arm %s 
2>&1 | FileCheck %s --check-prefix=ARM
+// RUN: %clang -target arm-linux-gnueabi -### -Xassembler -mimplicit-it=thumb 
%s 2>&1 | FileCheck %s --check-prefix=THUMB
+/// Test space separated -Wa,- arguments (latter wins).
+// RUN: %clang -target arm-linux-gnueabi -### -Wa,-mimplicit-it=never 
-Wa,-mimplicit-it=always %s 2>&1 | FileCheck %s --check-prefix=ALWAYS
+// RUN: %clang -target arm-linux-gnueabi -### -Wa,-mimplicit-it=always 
-Wa,-mimplicit-it=never %s 2>&1 | FileCheck %s --check-prefix=NEVER
+// RUN: %clang -target arm-linux-gnueabi -### -Wa,-mimplicit-it=always 
-Wa,-mimplicit-it=arm %s 2>&1 | FileCheck %s --check-prefix=ARM
+// RUN: %clang -target arm-linux-gnueabi -### -Wa,-mimplicit-it=always 
-Wa,-mimplicit-it=thumb %s 2>&1 | FileCheck %s --check-prefix=THUMB
+/// Test comma separated -Wa,- arguments (latter wins).
+// RUN: %clang -target arm-linux-gnueabi -### 
-Wa,-mimplicit-it=never,-mimplicit-it=always %s 2>&1 | FileCheck %s 
--check-prefix=ALWAYS
+// RUN: %clang -target arm-linux-gnueabi -### 
-Wa,-mimplicit-it=always,-mimplicit-it=never %s 2>&1 | FileCheck %s 
--check-prefix=NEVER
+// RUN: %clang -target arm-linux-gnueabi -### 
-Wa,-mimplicit-it=always,-mimplicit-it=arm %s 2>&1 | FileCheck %s 
--check-prefix=ARM
+// RUN: %clang -target arm-linux-gnueabi -### 
-Wa,-mimplicit-it=always,-mimplicit-it=thumb %s 2>&1 | FileCheck %s 
--check-prefix=THUMB
+
+/// Mix -implicit-it= (compiler) with -Wa,-mimplicit-it= (assembler), assembler
+/// takes priority. -mllvm -arm-implicit-it= will be repeated, with the
+/// assembler flag appearing last (latter wins).
+// RUN: %clang -target arm-linux-gnueabi -### -mimplicit-it=never 
-Wa,-mimplicit-it=always %S/Inputs/wildcard1.c 2>&1 | FileCheck %s 
--check-prefix=NEVER_ALWAYS
+// RUN: %clang -target arm-linux-gnueabi -### -mimplicit-it=always 
-Wa,-mimplicit-it=never %S/Inputs/wildcard1.c 2>&1 | FileCheck %s 
--check-prefix=ALWAYS_NEVER
+// RUN: %clang -target arm-linux-gnueabi -### -Wa,-mimplicit-it=never 
-mimplicit-it=always %S/Inputs/wildcard1.c 2>&1 | FileCheck %s 
--check-prefix=ALWAYS_NEVER
+
+/// Test invalid input.
+// RUN: %clang -target arm-linux-gnueabi -### -Wa,-mimplicit-it=foo %s 2>&1 | 
FileCheck %s --check-prefix=INVALID
+// RUN: %clang -target arm-linux-gnueabi -### -Xassembler -mimplicit-it=foo %s 
2>&1 | FileCheck %s --check-prefix=XINVALID
+// RUN: %clang -target arm-linux-gnueabi -### -Wa,-mimplicit-it=always 
-Wa,-mimplicit-it=foo %s 2>&1 | FileCheck %s --check-prefix=INVALID
+// RUN: %clang -target arm-linux-gnueabi -### 
-Wa,-mimplicit-it=always,-mimplicit-it=foo %s 2>&1 | FileCheck %s 
--check-prefix=INVALID
+
+
+// ALWAYS: "-mllvm" "-arm-implicit-it=always"
+// NEVER: "-mllvm" "-arm-implicit-it=never"
+// ARM: "-mllvm" "-arm-implicit-it=arm"
+// THUMB: "-mllvm" "-arm-implicit-it=thumb"
+// INVALID: error: unsupported argument '-mimplicit-it=foo' to option 'Wa,'
+// XINVALID: error: unsupported argument '-mimplicit-it=foo' to option 
'Xassembler'
+// ALWAYS_NEVER: "-mllvm" "-arm-implicit-it=always" "-mllvm" 
"-arm-implicit-it=never"
+// NEVER_ALWAYS: "-mllvm" "-arm-implicit-it=never" "-mllvm" 
"-arm-implicit-it=always"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -2304,6 +2304,17 @@
   DumpCompilationDatabase(C, "", Target, Output, Input, Args);
 }
 
+static bool AddARMImplicitITArgs(const 

[PATCH] D85223: [CUDA][HIP] Support accessing static device variable in host code for -fgpu-rdc

[Artem Belevich via Phabricator via cfe-commits]

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

LGTM with new test nits.

@JonChesterfield -- are you OK with the patch?




Comment at: clang/test/CodeGenCUDA/device-var-linkage.cu:40
 // NORDC-DAG: @_ZL3sv1 = dso_local addrspace(1) externally_initialized global 
i32 0
-// RDC-DAG: @_ZL3sv1 = internal addrspace(1) global i32 0
+// RDC-DAG: @_ZL3sv1.static.[[HASH:b04fd23c98500190]] = dso_local addrspace(1) 
externally_initialized global i32 0
 // HOST-DAG: @_ZL3sv1 = internal global i32 undef

It should probably be a regex after `HASH:`, not the hash value itself.



Comment at: clang/test/CodeGenCUDA/managed-var.cu:42
+// NORDC-D-DAG: @_ZL2sx = dso_local addrspace(1) externally_initialized global 
i32 addrspace(1)* null
+// RDC-D-DAG: @_ZL2sx.static.[[HASH:b04fd23c98500190]] = dso_local 
addrspace(1) externally_initialized global i32 addrspace(1)* null
 // HOST-DAG: @_ZL2sx.init = internal global i32 1

Same here.



Comment at: clang/test/CodeGenCUDA/static-device-var-rdc.cu:34
+// Test externalized static device variables
+// EXT-DEV-DAG: @_ZL1x.static.[[HASH:b04fd23c98500190]] = dso_local 
addrspace(1) externally_initialized global i32 0
+// EXT-HOST-DAG: @[[DEVNAMEX:[0-9]+]] = 
{{.*}}c"_ZL1x.static.[[HASH:b04fd23c98500190]]\00"

ditto.



Comment at: clang/test/SemaCUDA/static-device-var.cu:10
+
+// expected-no-diagnostics
+

A comment explaining what we're testing would be helpful. `no-diagnostics` 
gives no clues about what is it we're looking for here.





Comment at: clang/test/SemaCUDA/static-device-var.cu:14-22
+__device__ void f1() {
+  const static int b = 123;
+  static int a;
+}
+
+__global__ void k1() {
+  const static int b = 123;

So, this verifies that we're allowed to use static local vars in device code. A 
comment would be useful.



Comment at: clang/test/SemaCUDA/static-device-var.cu:23-37
+
+static __device__ int x;
+static __constant__ int y;
+
+__global__ void kernel(int *a) {
+  a[0] = x;
+  a[1] = y;

And this verifies that global static vars can be referenced from both host and 
device. 
I'd also add a negative test with `static int host_only;` and would verify that 
we still don't allow accessing it from the device.


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

https://reviews.llvm.org/D85223

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

[PATCH] D96354: Avoid conflicts between debug-info and pseudo-probe profiling

[Hongtao Yu via Phabricator via cfe-commits]

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

Thanks for fixing these issues! LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96354

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

[PATCH] D96355: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with optionally considering differently qualified types mixable

[Whisperity via Phabricator via cfe-commits]

whisperity created this revision.
whisperity added reviewers: aaron.ballman, alexfh, hokein, njames93.
whisperity added projects: clang, clang-tools-extra.
Herald added subscribers: nullptr.cpp, rnkovacs, xazax.hun.
whisperity requested review of this revision.
Herald added a subscriber: cfe-commits.

Adds a relaxation option **`QualifiersMix`** which will make the check report 
for cases where parameters refer to the same type if they only differ in 
qualifiers.

This makes cases, such as the following, not warned about by default, produce a 
warning.

  void* memcpy(void* dst, const void* src, unsigned size) {}

The reasoning behind this is that several projects, code styles, and 
preferences might consider `T` and `const T` fundamentally different types. The 
related C++ Core Guidelines 

 rule itself shows an example where `void T*, const void T*` is considered a 
"good solution".

However, unless people meticulously `const` their local variables, 
unfortunately, even such a function carry a potential swap:

  T* obj = new T; // Not const!!!
  void* buf = malloc(sizeof(T));
  
  memcpy(obj, buf, sizeof(T));
  // ^~~  ^~~ accidental swap here, even though the interface "specified" a 
const.

Of course, keeping your things `const` where appropriate results in an error:

  const T* obj = new T;
  void* buf = malloc(sizeof(T));
  
  memcpy(obj, buf, sizeof(T));
  // error: 1st argument ('const T*') loses qualifiers

Due to the rationale behind this depending on project-specific guidelines and 
preferences, the modelling is introduced as a check option. The default value 
is **`false`**, i.e. `T*` and `const T*` are considered **unmixable, distinct 
types**.

> Originally, the implementation of this feature was part of the very first 
> patch related to this check, D69560  (diff 
> 259320 ). It has been factored out 
> for clarity and to allow more on-point discussion.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96355

Files:
  clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.h
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-ignore.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len3.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-qualifiermixing.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
@@ -2,7 +2,8 @@
 // RUN:   -config='{CheckOptions: [ \
 // RUN: {key: bugprone-easily-swappable-parameters.MinimumLength, value: 2}, \
 // RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: ""}, \
-// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: "bool;MyBool;struct U;MAKE_LOGICAL_TYPE(int)"} \
+// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: "bool;MyBool;struct U;MAKE_LOGICAL_TYPE(int)"}, \
+// RUN: {key: bugprone-easily-swappable-parameters.QualifiersMix, value: 0} \
 // RUN:  ]}' -- -x c
 
 #define bool _Bool
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-qualifiermixing.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-qualifiermixing.cpp
@@ -0,0 +1,112 @@
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: [ \
+// RUN: {key: bugprone-easily-swappable-parameters.MinimumLength, value: 2}, \
+// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: ""}, \
+// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: ""}, \
+// RUN: {key: bugprone-easily-swappable-parameters.QualifiersMix, value: 1} \
+// RUN:  ]}' --
+
+typedef int MyInt1;
+typedef int MyInt2;
+using CInt = const int;
+using CMyInt1 = const MyInt1;
+using CMyInt2 = const MyInt2;
+
+void qualified1(int I, const int CI) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: 2 adjacent parameters of 'qualified1' of similar type are easily swapped by mistake [bugprone-easily-swappable-parameters]
+// CHECK-MESSAGES: 

[PATCH] D95736: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with `typedef` and `const &` diagnostics

[Whisperity via Phabricator via cfe-commits]

whisperity updated this revision to Diff 322431.
whisperity marked 6 inline comments as done.
whisperity added a comment.
Herald added a subscriber: nullptr.cpp.

- **NFC** Code style, formatting, wording, etc. changes as per review comments.
- **NFC** Removed a stale FIXME


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95736

Files:
  clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
@@ -86,20 +86,38 @@
 // CHECK-MESSAGES: :[[@LINE-2]]:32: note: the first parameter in the range is 'I1'
 // CHECK-MESSAGES: :[[@LINE-3]]:43: note: the last parameter in the range is 'I2'
 
-void throughTypedef(int I, MyInt1 J) {}
-// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 2 adjacent parameters of 'throughTypedef' of similar type ('int')
-// CHECK-MESSAGES: :[[@LINE-2]]:25: note: the first parameter in the range is 'I'
-// CHECK-MESSAGES: :[[@LINE-3]]:35: note: the last parameter in the range is 'J'
+void typedefMultiple(MyInt1 I1, MyInt2 I2x, MyInt2 I2y) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 3 adjacent parameters of 'typedefMultiple' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:29: note: the first parameter in the range is 'I1'
+// CHECK-MESSAGES: :[[@LINE-3]]:52: note: the last parameter in the range is 'I2y'
+// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases, the common type of 'MyInt1' and 'MyInt2' is 'int'
+
+void throughTypedef1(int I, MyInt1 J) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 2 adjacent parameters of 'throughTypedef1' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:26: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:36: note: the last parameter in the range is 'J'
+// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases, 'int' and 'MyInt1' are the same
+
+void betweenTypedef2(MyInt1 I, MyInt2 J) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 2 adjacent parameters of 'betweenTypedef2' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:29: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:39: note: the last parameter in the range is 'J'
+// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases, the common type of 'MyInt1' and 'MyInt2' is 'int'
+
+typedef MyInt2 MyInt2b;
 
-void betweenTypedef(MyInt1 I, MyInt2 J) {}
-// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 2 adjacent parameters of 'betweenTypedef' of similar type ('MyInt1')
-// CHECK-MESSAGES: :[[@LINE-2]]:28: note: the first parameter in the range is 'I'
-// CHECK-MESSAGES: :[[@LINE-3]]:38: note: the last parameter in the range is 'J'
+void typedefChain(int I, MyInt1 MI1, MyInt2 MI2, MyInt2b MI2b) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: 4 adjacent parameters of 'typedefChain' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:23: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:58: note: the last parameter in the range is 'MI2b'
+// CHECK-MESSAGES: :[[@LINE-4]]:19: note: after resolving type aliases, 'int' and 'MyInt1' are the same
+// CHECK-MESSAGES: :[[@LINE-5]]:19: note: after resolving type aliases, 'int' and 'MyInt2' are the same
+// CHECK-MESSAGES: :[[@LINE-6]]:19: note: after resolving type aliases, 'int' and 'MyInt2b' are the same
 
 typedef long MyLong1;
 using MyLong2 = long;
 
-void throughTypedefToOtherType(MyInt1 I, MyLong1 J) {} // NO-WARN: Not the same type.
+void throughTypedefToOtherType(MyInt1 I, MyLong1 J) {} // NO-WARN: int and long.
 
 void qualified1(int I, const int CI) {} // NO-WARN: Not the same type.
 
@@ -113,18 +131,73 @@
 
 void qualifiedThroughTypedef1(int I, CInt CI) {} // NO-WARN: Not the same type.
 
-void qualifiedThroughTypedef2(CInt CI1, const int CI2) {} // NO-WARN: Not the same type.
-// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: 2 adjacent parameters of 'qualifiedThroughTypedef2' of similar type ('CInt')
+void qualifiedThroughTypedef2(CInt CI1, const int CI2) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: 2 adjacent parameters of 'qualifiedThroughTypedef2' of similar type are
 // CHECK-MESSAGES: :[[@LINE-2]]:36: note: the first parameter in the range is 'CI1'
 // CHECK-MESSAGES: :[[@LINE-3]]:51: note: the last parameter in the range is 'CI2'
-
-void reference1(int I, int ) {} // NO-WARN: Not the same type.
-
-void reference2(int I, const int ) {} // NO-WARN: Not the same type.
-
-void reference3(int I, int 

[PATCH] D96248: [OpenMP][AMDGPU] Add support for linking libomptarget bitcode

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

Using one option for both targets seems great - if both have put the devicertl 
in the same folder. Which I suppose they might not have.

Maybe keep it separate for one, one for nvptx and one for amdgcn, and hope for 
a common 'device' later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96248

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

[PATCH] D92808: [ObjC][ARC] Use operand bundle 'clang.arc.rv' instead of explicitly emitting retainRV or claimRV calls in the IR

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

It looks like there is a pre-existing bug in `replaceUsesOfNonProtoConstant` 
where the operand bundles of all call sites are accumulated into `newBundles`. 
This will be fixed if its declaration is moved into the loop body. Also, we 
found another case of deadargelim changing the return type of the called 
function to `void`. I'll post a new patch when I have the fixes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92808

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

[PATCH] D96354: Avoid conflicts between debug-info and pseudo-probe profiling

[Paul Robinson via Phabricator via cfe-commits]

probinson created this revision.
probinson added reviewers: hoy, wmi.
probinson requested review of this revision.
Herald added a project: clang.

After D93264 , using both 
-fdebug-info-for-profiling and -fpseudo-probe-for-profiling will cause the 
compiler to crash.  Diagnose these conflicting options in the driver.

Also, the driver had a redundant pass-through of the option, and the existing 
CodeGen test was using the driver when it should be running cc1.  Fixed up 
those as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96354

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/pseudo-probe-emit.c
  clang/test/Driver/pseudo-probe.c


Index: clang/test/Driver/pseudo-probe.c
===
--- /dev/null
+++ clang/test/Driver/pseudo-probe.c
@@ -0,0 +1,7 @@
+// RUN: %clang -### -fpseudo-probe-for-profiling 2>&1 | FileCheck %s 
--check-prefix=YESPROBE
+// RUN: %clang -### -fno-pseudo-probe-for-profiling 2>&1 | FileCheck %s 
--check-prefix=NOPROBE
+// RUN: %clang -### -fpseudo-probe-for-profiling -fdebug-info-for-profiling 
2>&1 | FileCheck %s --check-prefix=CONFLICT
+
+// YESPROBE: -fpseudo-probe-for-profiling
+// NOPROBE-NOT: -fpseudo-probe-for-profiling
+// CONFLICT: invalid argument
Index: clang/test/CodeGen/pseudo-probe-emit.c
===
--- clang/test/CodeGen/pseudo-probe-emit.c
+++ clang/test/CodeGen/pseudo-probe-emit.c
@@ -1,4 +1,4 @@
-// RUN: %clang -O2  -fexperimental-new-pass-manager 
-fpseudo-probe-for-profiling -g -emit-llvm -S -o - %s | FileCheck %s
+// RUN: %clang_cc1 -O2 -fno-legacy-pass-manager -fpseudo-probe-for-profiling 
-debug-info-kind=limited -emit-llvm -o - %s | FileCheck %s
 
 // Check the generation of pseudoprobe intrinsic call
 
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3746,6 +3746,12 @@
ArgStringList ,
codegenoptions::DebugInfoKind ,
DwarfFissionKind ) {
+  // These two forms of profiling info can't be used together.
+  if (const Arg *A1 = 
Args.getLastArg(options::OPT_fpseudo_probe_for_profiling))
+if (const Arg *A2 = 
Args.getLastArg(options::OPT_fdebug_info_for_profiling))
+  D.Diag(diag::err_drv_argument_not_allowed_with)
+  << A1->getAsString(Args) << A2->getAsString(Args);
+
   if (Args.hasFlag(options::OPT_fdebug_info_for_profiling,
options::OPT_fno_debug_info_for_profiling, false) &&
   checkDebugInfoOption(
@@ -5727,10 +5733,6 @@
   }
   Args.AddLastArg(CmdArgs, options::OPT_fprofile_remapping_file_EQ);
 
-  if (Args.hasFlag(options::OPT_fpseudo_probe_for_profiling,
-   options::OPT_fno_pseudo_probe_for_profiling, false))
-CmdArgs.push_back("-fpseudo-probe-for-profiling");
-
   RenderBuiltinOptions(TC, RawTriple, Args, CmdArgs);
 
   if (!Args.hasFlag(options::OPT_fassume_sane_operator_new,


Index: clang/test/Driver/pseudo-probe.c
===
--- /dev/null
+++ clang/test/Driver/pseudo-probe.c
@@ -0,0 +1,7 @@
+// RUN: %clang -### -fpseudo-probe-for-profiling 2>&1 | FileCheck %s --check-prefix=YESPROBE
+// RUN: %clang -### -fno-pseudo-probe-for-profiling 2>&1 | FileCheck %s --check-prefix=NOPROBE
+// RUN: %clang -### -fpseudo-probe-for-profiling -fdebug-info-for-profiling 2>&1 | FileCheck %s --check-prefix=CONFLICT
+
+// YESPROBE: -fpseudo-probe-for-profiling
+// NOPROBE-NOT: -fpseudo-probe-for-profiling
+// CONFLICT: invalid argument
Index: clang/test/CodeGen/pseudo-probe-emit.c
===
--- clang/test/CodeGen/pseudo-probe-emit.c
+++ clang/test/CodeGen/pseudo-probe-emit.c
@@ -1,4 +1,4 @@
-// RUN: %clang -O2  -fexperimental-new-pass-manager -fpseudo-probe-for-profiling -g -emit-llvm -S -o - %s | FileCheck %s
+// RUN: %clang_cc1 -O2 -fno-legacy-pass-manager -fpseudo-probe-for-profiling -debug-info-kind=limited -emit-llvm -o - %s | FileCheck %s
 
 // Check the generation of pseudoprobe intrinsic call
 
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3746,6 +3746,12 @@
ArgStringList ,
codegenoptions::DebugInfoKind ,
DwarfFissionKind ) {
+  // These two forms of profiling info can't be used together.
+  if (const Arg *A1 = Args.getLastArg(options::OPT_fpseudo_probe_for_profiling))
+if (const Arg *A2 = Args.getLastArg(options::OPT_fdebug_info_for_profiling))
+  D.Diag(diag::err_drv_argument_not_allowed_with)
+ 

[PATCH] D96353: [clangd] Use ML Code completion ranking as default.

[Utkarsh Saxena via Phabricator via cfe-commits]

usaxena95 created this revision.
usaxena95 added a reviewer: hokein.
Herald added subscribers: kadircet, arphaman.
usaxena95 requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang.

Also treat Keywords separately as they are not recorded by the training set 
generator.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96353

Files:
  clang-tools-extra/clangd/CodeComplete.h
  clang-tools-extra/clangd/Quality.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp


Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -647,13 +647,12 @@
 }
 
 TEST(CompletionTest, ReferencesAffectRanking) {
-  auto Results = completions("int main() { abs^ }", {ns("absl"), 
func("absb")});
+  auto Results =
+  completions("int main() { abs^ }", {func("absl"), func("absb")});
   EXPECT_THAT(Results.Completions,
   HasSubsequence(Named("absb"), Named("absl")));
   Results = completions("int main() { abs^ }",
-{withReferences(1, ns("absl")), func("absb")});
-  EXPECT_THAT(Results.Completions,
-  HasSubsequence(Named("absl"), Named("absb")));
+{withReferences(100, func("absl")), func("absb")});
 }
 
 TEST(CompletionTest, ContextWords) {
Index: clang-tools-extra/clangd/Quality.cpp
===
--- clang-tools-extra/clangd/Quality.cpp
+++ clang-tools-extra/clangd/Quality.cpp
@@ -580,12 +580,16 @@
   // multiplciative boost (like NameMatch). This allows us to weigh the
   // prediciton score and NameMatch appropriately.
   Scores.ExcludingName = pow(Base, Evaluate(E));
-  // NeedsFixIts is not part of the DecisionForest as generating training
-  // data that needs fixits is not-feasible.
+  // Following cases are not part of the generated training dataset:
+  //  - Symbols with `NeedsFixIts`.
+  //  - Forbidden symbols.
+  //  - Keywords: Dataset contains only macros and decls.
   if (Relevance.NeedsFixIts)
 Scores.ExcludingName *= 0.5;
   if (Relevance.Forbidden)
 Scores.ExcludingName *= 0;
+  if (Quality.Category == SymbolQualitySignals::Keyword)
+Scores.ExcludingName *= 4;
 
   // NameMatch should be a multiplier on total score to support rescoring.
   Scores.Total = Relevance.NameMatch * Scores.ExcludingName;
Index: clang-tools-extra/clangd/CodeComplete.h
===
--- clang-tools-extra/clangd/CodeComplete.h
+++ clang-tools-extra/clangd/CodeComplete.h
@@ -133,7 +133,7 @@
   enum CodeCompletionRankingModel {
 Heuristics,
 DecisionForest,
-  } RankingModel = Heuristics;
+  } RankingModel = DecisionForest;
 
   /// Callback used to score a CompletionCandidate if DecisionForest ranking
   /// model is enabled.


Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -647,13 +647,12 @@
 }
 
 TEST(CompletionTest, ReferencesAffectRanking) {
-  auto Results = completions("int main() { abs^ }", {ns("absl"), func("absb")});
+  auto Results =
+  completions("int main() { abs^ }", {func("absl"), func("absb")});
   EXPECT_THAT(Results.Completions,
   HasSubsequence(Named("absb"), Named("absl")));
   Results = completions("int main() { abs^ }",
-{withReferences(1, ns("absl")), func("absb")});
-  EXPECT_THAT(Results.Completions,
-  HasSubsequence(Named("absl"), Named("absb")));
+{withReferences(100, func("absl")), func("absb")});
 }
 
 TEST(CompletionTest, ContextWords) {
Index: clang-tools-extra/clangd/Quality.cpp
===
--- clang-tools-extra/clangd/Quality.cpp
+++ clang-tools-extra/clangd/Quality.cpp
@@ -580,12 +580,16 @@
   // multiplciative boost (like NameMatch). This allows us to weigh the
   // prediciton score and NameMatch appropriately.
   Scores.ExcludingName = pow(Base, Evaluate(E));
-  // NeedsFixIts is not part of the DecisionForest as generating training
-  // data that needs fixits is not-feasible.
+  // Following cases are not part of the generated training dataset:
+  //  - Symbols with `NeedsFixIts`.
+  //  - Forbidden symbols.
+  //  - Keywords: Dataset contains only macros and decls.
   if (Relevance.NeedsFixIts)
 Scores.ExcludingName *= 0.5;
   if (Relevance.Forbidden)
 Scores.ExcludingName *= 0;
+  if (Quality.Category == SymbolQualitySignals::Keyword)
+Scores.ExcludingName *= 4;
 
   // NameMatch should be a multiplier on total score to support rescoring.
   

[PATCH] D90457: [clang][driver] Set LTO mode based on input files

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D90457#2551698 , @tbaeder wrote:

> Hey @MaskRay, have you thought about this some more? Or is it too much magic 
> and you'd rather reject it altogether?

Yes, it should be rejected. I've seen cases where people want `clang -c a.bc` 
to produce an object file instead of enabling LTO.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90457

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

[PATCH] D90646: [clang] Add warning when `-include-pch` is passed multiple times

[Keith Smiley via Phabricator via cfe-commits]

keith added a comment.

Yea seems like a reasonable request, looking through the uses nothing 
immediately pops out as being not supporting multiple PCHs but I guess before I 
went down that path I'd want someone who knew the code better to agree on that


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90646

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

[PATCH] D95915: [clang][driver] Only warn once about invalid -stdlib value

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a comment.

Is this good to go now? Or still something left? @yaxunl?


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

https://reviews.llvm.org/D95915

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

[PATCH] D90457: [clang][driver] Set LTO mode based on input files

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a comment.

Hey @MaskRay, have you thought about this some more? Or is it too much magic 
and you'd rather reject it altogether?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90457

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

[PATCH] D96070: [clang] [driver] Enable static linking to libc++

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a comment.

In D96070#2546330 , @MaskRay wrote:

> we cannot control `(optional -lc++abi/-lcxxrt/others)`, which is depended by 
> the installation configuration (e.g. 
> LIBCXX_STATICALLY_LINK_ABI_IN_STATIC_LIBRARY, or libc++.a as a linker script 
> (https://bugs.llvm.org/show_bug.cgi?id=46321), or .deplibs) and whether you 
> are specifying --sysroot. Perhaps I had the opportunity to redesign the 
> matter, I would let `-stdlib=libc++` not affect linker options at all. Users 
> just should specify `-lc++` and related options by themselves.

Right, but at least some of these sound like the can or should be solved in 
libc++ then. Reading https://reviews.llvm.org/D60794, I thought this was not 
going to happen and the solution should instead be in clang.

What is the ideal solution here? If `-static -stdlib=libc++` is not meant to 
work, is the working solution documented anywhere that we could link people to? 
If your blog post isn't official of course :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96070

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

[PATCH] D95706: [clangd] Expose more dependent-name detail via semanticTokens

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:239
+  auto Result = Priority1 > Priority2 ? Tokens[0] : Tokens[1];
+  Result.Modifiers = Tokens[0].Modifiers | Tokens[1].Modifiers;
+  return Result;

nridge wrote:
> I like that this ends up with a concrete kind + dependent modifier, to give a 
> hint that the concrete kind was determined heuristically :)
> 
> Maybe we could adjust the comment at the top of the function to call this 
> behaviour out.
Right! I like this too but forgot do doc it. Done.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.h:78
   Abstract,
+  DependentName,
 

nridge wrote:
> Just `Dependent` might be enough
AFAICS the current implementation is closer to "dependent name" than 
"dependent", I think.

For example, in the RHS of `template  using Val = T::value_type`, 
both "T" and "value_type" certainly refer to dependent types. But only 
`value_type` is a dependent name, and only `value_type` gets the old kinds/new 
modifier.

---

Or are you proposing we change the implementation too?

It's not intuitively which version would be more useful. FWIW I have the 
current DependentName kind highlighted in bold bright orange, and I find it 
really helpful :-) When there are errors and RecoveryExpr kicks in, it colors 
the resulting unresolved names, which I like surprisingly much.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95706

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

[PATCH] D96248: [OpenMP][AMDGPU] Add support for linking libomptarget bitcode

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 requested changes to this revision.
tianshilei1992 added a comment.
This revision now requires changes to proceed.

In D96248#2551503 , @JonChesterfield 
wrote:

> @tianshilei1992 @jdoerfert can we agree on 'libomptarget-device-bc-path' 
> being the better one to recommend in the error message, despite that being a 
> minor change to the current behaviour? It'll slightly surprise people parsing 
> error messages, such as our test, but shouldn't cause any confusion.

After a second thought, I don't think it is feasible. Consider the following 
senecio:

  clang -fopenmp -fopenmp-targets=nvptx64,amdgcn source.cpp

We cannot use one option for the two different targets, and the alias might not 
work as well, especially in terms of the driver.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96248

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

[PATCH] D96090: [analyzer] Replace StoreManager::CastRetrievedVal with SValBuilder::evalCast

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 322422.
ASDenysPetrov added a comment.

Updated. Unlinked parent revision D95799  from 
the stack. Made corresponding changes in this patch.


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

https://reviews.llvm.org/D96090

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
  clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  clang/lib/StaticAnalyzer/Core/Store.cpp

Index: clang/lib/StaticAnalyzer/Core/Store.cpp
===
--- clang/lib/StaticAnalyzer/Core/Store.cpp
+++ clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -394,48 +394,6 @@
   return UnknownVal();
 }
 
-static bool hasSameUnqualifiedPointeeType(QualType ty1, QualType ty2) {
-  return ty1->getPointeeType().getCanonicalType().getTypePtr() ==
- ty2->getPointeeType().getCanonicalType().getTypePtr();
-}
-
-/// CastRetrievedVal - Used by subclasses of StoreManager to implement
-///  implicit casts that arise from loads from regions that are reinterpreted
-///  as another region.
-SVal StoreManager::CastRetrievedVal(SVal V, const TypedValueRegion *R,
-QualType castTy) {
-  if (castTy.isNull() || V.isUnknownOrUndef())
-return V;
-
-  // The dispatchCast() call below would convert the int into a float.
-  // What we want, however, is a bit-by-bit reinterpretation of the int
-  // as a float, which usually yields nothing garbage. For now skip casts
-  // from ints to floats.
-  // TODO: What other combinations of types are affected?
-  if (castTy->isFloatingType()) {
-SymbolRef Sym = V.getAsSymbol();
-if (Sym && !Sym->getType()->isFloatingType())
-  return UnknownVal();
-  }
-
-  // When retrieving symbolic pointer and expecting a non-void pointer,
-  // wrap them into element regions of the expected type if necessary.
-  // SValBuilder::dispatchCast() doesn't do that, but it is necessary to
-  // make sure that the retrieved value makes sense, because there's no other
-  // cast in the AST that would tell us to cast it to the correct pointer type.
-  // We might need to do that for non-void pointers as well.
-  // FIXME: We really need a single good function to perform casts for us
-  // correctly every time we need it.
-  if (castTy->isPointerType() && !castTy->isVoidPointerType())
-if (const auto *SR = dyn_cast_or_null(V.getAsRegion())) {
-  QualType sr = SR->getSymbol()->getType();
-  if (!hasSameUnqualifiedPointeeType(sr, castTy))
-  return loc::MemRegionVal(castRegion(SR, castTy));
-}
-
-  return svalBuilder.dispatchCast(V, castTy);
-}
-
 SVal StoreManager::getLValueFieldOrIvar(const Decl *D, SVal Base) {
   if (Base.isUnknownOrUndef())
 return Base;
Index: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -66,9 +66,7 @@
 //===--===//
 
 SVal SimpleSValBuilder::dispatchCast(SVal Val, QualType CastTy) {
-  assert(Val.getAs() || Val.getAs());
-  return Val.getAs() ? evalCastFromLoc(Val.castAs(), CastTy)
-   : evalCastFromNonLoc(Val.castAs(), CastTy);
+  return evalCast(Val, CastTy);
 }
 
 SVal SimpleSValBuilder::evalCastFromNonLoc(NonLoc val, QualType castTy) {
Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -536,20 +536,29 @@
 // `evalCastKind` and `evalCastSubKind` are helpers
 //===--===//
 
-SVal SValBuilder::evalCast(SVal V, QualType CastTy, QualType OriginalTy) {
-  CastTy = Context.getCanonicalType(CastTy);
-  OriginalTy = Context.getCanonicalType(OriginalTy);
-  if (CastTy == OriginalTy)
+// In case when `OriginalTy.isNull() == true` we cast `V` a bit differently.
+SVal SValBuilder::evalCast(SVal V, QualType CastTy,
+   QualType OriginalTy /*= QualType{}*/) {
+  if (CastTy.isNull())
 return V;
 
-  // FIXME: Move this check to the most appropriate evalCastKind/evalCastSubKind
-  // function.
-  // For const casts, casts to void, just propagate the value.
-  if (!CastTy->isVariableArrayType() && !OriginalTy->isVariableArrayType())
-if (shouldBeModeledWithNoOp(Context, Context.getPointerType(CastTy),
-Context.getPointerType(OriginalTy)))
+  CastTy = Context.getCanonicalType(CastTy);
+
+  if (!OriginalTy.isNull()) {
+OriginalTy = 

[PATCH] D90646: [clang] Add warning when `-include-pch` is passed multiple times

[Dave Lee via Phabricator via cfe-commits]

kastiglione added a comment.

Is there any reason that `-include-pch` shouldn't follow the precedent of 
`-include`, which can be used multiple times? If not, then the end goal should 
be to support multiple uses, but in the mean time a warning is helpful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90646

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

[PATCH] D95701: [clangd] Add semanticTokens modifiers for function/class/file/global scope

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:379
+  // ExternalLinkage threshold could be tweaked, e.g. module-visible as global.
+  // Avoid caching linkage if it may change after enclosing code completion.
+  if (hasUnstableLinkage(D) || D->getLinkageInternal() < ExternalLinkage)

nridge wrote:
> code completion? I'm a bit lost :)
Whoops, fixed.

(This was copied from a utility function that's used in code completion ranking 
- I planned to share it, but it turns out we want slightly different semantics. 
Code completion happens inside callbacks that happen mid-parse, and if you call 
getLinkageInternal() at just the wrong time, you end up caching an intermediate 
state and crash. But I removed the check as it's not relevant here)



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:466
 case TemplateArgument::TemplateExpansion:
+  // FIXME: I don't understand why this is DependentType.
   H.addToken(L.getTemplateNameLoc(), HighlightingKind::DependentType);

nridge wrote:
> nridge wrote:
> > The testcase which relies on this is this one:
> > 
> > ```
> >   // Dependent template name
> >   R"cpp(
> >   template  class> struct $Class[[A]] {};
> >   template 
> >   using $Typedef[[W]] = $Class[[A]]<
> > $TemplateParameter[[T]]::template $DependentType[[Waldo]]
> >   >;
> > ```
> > 
> > However, it does appear that we get into here even for non-dependent 
> > template template arguments (but then also get a non-dependent highlighting 
> > kind via `findExplicitReferences()`, and end up discarding the 
> > `DependentType` via `resolveConflict()`).
> I see you've already figured this out in D95706 :)
Oops, yes. I've adjusted the comment here so the intermediate state makes sense 
:-)
(I'd rather not emit dubious tokens and then rely on conflicts to discard them, 
it seems hard to maintain)



Comment at: clang-tools-extra/clangd/SemanticHighlighting.h:82
+  FileScope,
+  GlobalScope,
+

nridge wrote:
> Would it make sense to call this `NamespaceScope` instead?
> 
> I understand it's "global" in the sense that it's visible to other 
> translation units, but it's not "global" in the sense that it's not 
> necessarily in the global namespace.
> 
> (On the other hand, I can see how `FileScope` symbols are also "namespace 
> scope", so... could go either way.)
Yeah, the file-scope thing. (Maybe it's not an important distinction, but it 
seems pretty interesting for functions at least)

The other consideration is that I harbor a little bit of hope we could get some 
convergence across implementations, so avoiding lang-specific terms is nice. In 
any case, client implementers are probably not C++ people!
(of course we can use different names internally than we use in the protocol, 
but it seems less confusing to align them)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95701

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

[PATCH] D95701: [clangd] Add semanticTokens modifiers for function/class/file/global scope

[Sam McCall via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
sammccall marked 5 inline comments as done.
Closed by commit rG46cc7ce35ade: [clangd] Add semanticTokens modifiers for 
function/class/file/global scope (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D95701?vs=320221=322406#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95701

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/test/semantic-tokens.test
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
  clang-tools-extra/clangd/unittests/tweaks/AnnotateHighlightingsTests.cpp

Index: clang-tools-extra/clangd/unittests/tweaks/AnnotateHighlightingsTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/AnnotateHighlightingsTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/AnnotateHighlightingsTests.cpp
@@ -18,17 +18,18 @@
 TEST_F(AnnotateHighlightingsTest, Test) {
   EXPECT_AVAILABLE("^vo^id^ ^f(^) {^}^"); // available everywhere.
   EXPECT_AVAILABLE("[[int a; int b;]]");
-  EXPECT_EQ("void /* Function [decl] */f() {}", apply("void ^f() {}"));
+  EXPECT_EQ("void /* Function [decl] [globalScope] */f() {}",
+apply("void ^f() {}"));
 
-  EXPECT_EQ(
-  apply("[[int f1(); const int x = f1();]]"),
-  "int /* Function [decl] */f1(); "
-  "const int /* Variable [decl] [readonly] */x = /* Function */f1();");
+  EXPECT_EQ(apply("[[int f1(); const int x = f1();]]"),
+"int /* Function [decl] [globalScope] */f1(); "
+"const int /* Variable [decl] [readonly] [fileScope] */x = "
+"/* Function [globalScope] */f1();");
 
   // Only the targeted range is annotated.
   EXPECT_EQ(apply("void f1(); void f2() {^}"),
 "void f1(); "
-"void /* Function [decl] */f2() {}");
+"void /* Function [decl] [globalScope] */f2() {}");
 }
 
 } // namespace
Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -113,7 +113,8 @@
 void checkHighlightings(llvm::StringRef Code,
 std::vector>
-AdditionalFiles = {}) {
+AdditionalFiles = {},
+uint32_t ModifierMask = -1) {
   Annotations Test(Code);
   TestTU TU;
   TU.Code = std::string(Test.code());
@@ -126,8 +127,11 @@
   for (auto File : AdditionalFiles)
 TU.AdditionalFiles.insert({File.first, std::string(File.second)});
   auto AST = TU.build();
+  auto Actual = getSemanticHighlightings(AST);
+  for (auto  : Actual)
+Token.Modifiers &= ModifierMask;
 
-  EXPECT_EQ(Code, annotate(Test.code(), getSemanticHighlightings(AST)));
+  EXPECT_EQ(Code, annotate(Test.code(), Actual));
 }
 
 // Any annotations in OldCode and NewCode are converted into their corresponding
@@ -163,6 +167,12 @@
   << OldCode;
 }
 
+constexpr static uint32_t ScopeModifierMask =
+1 << unsigned(HighlightingModifier::FunctionScope) |
+1 << unsigned(HighlightingModifier::ClassScope) |
+1 << unsigned(HighlightingModifier::FileScope) |
+1 << unsigned(HighlightingModifier::GlobalScope);
+
 TEST(SemanticHighlighting, GetsCorrectTokens) {
   const char *TestCases[] = {
   R"cpp(
@@ -720,9 +730,10 @@
   <:[deprecated]:> int $Variable_decl_deprecated[[x]];
   )cpp",
   };
-  for (const auto  : TestCases) {
-checkHighlightings(TestCase);
-  }
+  for (const auto  : TestCases)
+// Mask off scope modifiers to keep the tests manageable.
+// They're tested separately.
+checkHighlightings(TestCase, {}, ~ScopeModifierMask);
 
   checkHighlightings(R"cpp(
 class $Class_decl[[A]] {
@@ -732,7 +743,8 @@
  {{"imp.h", R"cpp(
 int someMethod();
 void otherMethod();
-  )cpp"}});
+  )cpp"}},
+ ~ScopeModifierMask);
 
   // A separate test for macros in headers.
   checkHighlightings(R"cpp(
@@ -745,7 +757,59 @@
 #define DXYZ_Y(Y) DXYZ(x##Y)
 #define DEFINE(X) int X;
 #define DEFINE_Y DEFINE(Y)
-  )cpp"}});
+  )cpp"}},
+ ~ScopeModifierMask);
+}
+
+TEST(SemanticHighlighting, ScopeModifiers) {
+  const char *TestCases[] = {
+  R"cpp(
+static int $Variable_fileScope[[x]];
+namespace $Namespace_globalScope[[ns]] {
+  class $Class_globalScope[[x]];
+}
+namespace {
+  void $Function_fileScope[[foo]]();
+}
+  )cpp",
+  R"cpp(
+void $Function_globalScope[[foo]](int 

[clang-tools-extra] 46cc7ce - [clangd] Add semanticTokens modifiers for function/class/file/global scope

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2021-02-09T17:57:36+01:00
New Revision: 46cc7ce35ade2d0b3b49301fbf8135c1eca5b048

URL: 
https://github.com/llvm/llvm-project/commit/46cc7ce35ade2d0b3b49301fbf8135c1eca5b048
DIFF: 
https://github.com/llvm/llvm-project/commit/46cc7ce35ade2d0b3b49301fbf8135c1eca5b048.diff

LOG: [clangd] Add semanticTokens modifiers for function/class/file/global scope

These allow (function-) local variables to be distinguished, but also a
bunch more cases.
It's not quite independent with existing information (e.g. the
field/variable distinction is redundant if you have class-scope + static
attributes) but I don't think this is terribly important.

Depends on D77811

Differential Revision: https://reviews.llvm.org/D95701

Added: 


Modified: 
clang-tools-extra/clangd/SemanticHighlighting.cpp
clang-tools-extra/clangd/SemanticHighlighting.h
clang-tools-extra/clangd/test/initialize-params.test
clang-tools-extra/clangd/test/semantic-tokens.test
clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
clang-tools-extra/clangd/unittests/tweaks/AnnotateHighlightingsTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp 
b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index 00c6ed65d157..29afae3746f8 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -357,6 +357,46 @@ class HighlightingsBuilder {
   HighlightingToken Dummy; // returned from addToken(InvalidLoc)
 };
 
+llvm::Optional scopeModifier(const NamedDecl *D) {
+  const DeclContext *DC = D->getDeclContext();
+  // Injected "Foo" within the class "Foo" has file scope, not class scope.
+  if (auto *R = dyn_cast_or_null(D))
+if (R->isInjectedClassName())
+  DC = DC->getParent();
+  // Lambda captures are considered function scope, not class scope.
+  if (llvm::isa(D))
+if (const auto *RD = llvm::dyn_cast(DC))
+  if (RD->isLambda())
+return HighlightingModifier::FunctionScope;
+  // Walk up the DeclContext hierarchy until we find something interesting.
+  for (; !DC->isFileContext(); DC = DC->getParent()) {
+if (DC->isFunctionOrMethod())
+  return HighlightingModifier::FunctionScope;
+if (DC->isRecord())
+  return HighlightingModifier::ClassScope;
+  }
+  // Some template parameters (e.g. those for variable templates) don't have
+  // meaningful DeclContexts. That doesn't mean they're global!
+  if (DC->isTranslationUnit() && D->isTemplateParameter())
+return llvm::None;
+  // ExternalLinkage threshold could be tweaked, e.g. module-visible as global.
+  if (D->getLinkageInternal() < ExternalLinkage)
+return HighlightingModifier::FileScope;
+  return HighlightingModifier::GlobalScope;
+}
+
+llvm::Optional scopeModifier(const Type *T) {
+  if (!T)
+return llvm::None;
+  if (T->isBuiltinType())
+return HighlightingModifier::GlobalScope;
+  if (auto *TD = dyn_cast(T))
+return scopeModifier(TD->getDecl());
+  if (auto *TD = T->getAsTagDecl())
+return scopeModifier(TD);
+  return llvm::None;
+}
+
 /// Produces highlightings, which are not captured by findExplicitReferences,
 /// e.g. highlights dependent names and 'auto' as the underlying type.
 class CollectExtraHighlightings
@@ -365,9 +405,12 @@ class CollectExtraHighlightings
   CollectExtraHighlightings(HighlightingsBuilder ) : H(H) {}
 
   bool VisitDecltypeTypeLoc(DecltypeTypeLoc L) {
-if (auto K = kindForType(L.getTypePtr()))
-  H.addToken(L.getBeginLoc(), *K)
-  .addModifier(HighlightingModifier::Deduced);
+if (auto K = kindForType(L.getTypePtr())) {
+  auto  = H.addToken(L.getBeginLoc(), *K)
+  .addModifier(HighlightingModifier::Deduced);
+  if (auto Mod = scopeModifier(L.getTypePtr()))
+Tok.addModifier(*Mod);
+}
 return true;
   }
 
@@ -375,38 +418,47 @@ class CollectExtraHighlightings
 auto *AT = D->getType()->getContainedAutoType();
 if (!AT)
   return true;
-if (auto K = kindForType(AT->getDeducedType().getTypePtrOrNull()))
-  H.addToken(D->getTypeSpecStartLoc(), *K)
-  .addModifier(HighlightingModifier::Deduced);
+if (auto K = kindForType(AT->getDeducedType().getTypePtrOrNull())) {
+  auto  = H.addToken(D->getTypeSpecStartLoc(), *K)
+  .addModifier(HighlightingModifier::Deduced);
+  if (auto Mod = scopeModifier(AT->getDeducedType().getTypePtrOrNull()))
+Tok.addModifier(*Mod);
+}
 return true;
   }
 
   bool VisitOverloadExpr(OverloadExpr *E) {
 if (!E->decls().empty())
   return true; // handled by findExplicitReferences.
-H.addToken(E->getNameLoc(), HighlightingKind::DependentName);
+auto  = H.addToken(E->getNameLoc(), HighlightingKind::DependentName);
+if (llvm::isa(E))
+  Tok.addModifier(HighlightingModifier::ClassScope);
+// other case is 

[PATCH] D96248: [OpenMP][AMDGPU] Add support for linking libomptarget bitcode

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

Some background discussion about the diagnostic. This change means people using 
nvptx, where the library cannot be found, will now be advised to use 
libomptarget-device-bc-path instead of libomptarget-nvptx-bc-path. If they use 
libomptarget-nvptx-bc-path anyway, it'll work as intended. That avoids us 
adding libomptarget-amdgcn-bc-path and requiring some more conditional 
compilation for multiarch codebases.

@tianshilei1992 @jdoerfert can we agree on 'libomptarget-device-bc-path' being 
the better one to recommend in the error message, despite that being a minor 
change to the current behaviour? It'll slightly surprise people parsing error 
messages, such as our test, but shouldn't cause any confusion.

I'm personally OK with using libomptarget-nvptx-bc-path to indicate where to 
look for the amdgcn bitcode as well but can see that causing confusion. I'm 
assuming that both gpu runtimes will go in same directory - there may be a 
future where one invocation targets nvptx and amdgcn at the same time, but even 
then I'd prefer all the runtimes live in the same place in the filesystem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96248

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

[PATCH] D92808: [ObjC][ARC] Use operand bundle 'clang.arc.rv' instead of explicitly emitting retainRV or claimRV calls in the IR

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Here's a reduced repro:

  thakis@thakis:~/src/llvm-project$ cat 
GREYAssertDefaultConfiguration-9e1f3b.reduced.m
  inline id a() {}
  void b() {
a();
a();
  }
  thakis@thakis:~/src/llvm-project$ out/gn/bin/clang -cc1 -cc1 -triple 
arm64-apple-ios12.2.0 -Oz -fobjc-arc -emit-llvm 
GREYAssertDefaultConfiguration-9e1f3b.reduced.m
  GREYAssertDefaultConfiguration-9e1f3b.reduced.m:1:16: warning: non-void 
function does not return a value [-Wreturn-type]
  inline id a() {}
 ^
  clang: ../../llvm/include/llvm/IR/InstrTypes.h:1969: 
Optional llvm::CallBase::getOperandBundle(uint32_t) 
const: Assertion `countOperandBundlesOfType(ID) < 2 && "Precondition 
violated!"' failed.
  PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash 
backtrace, preprocessed source, and associated run script.
  Stack dump:
  0.Program arguments: out/gn/bin/clang -cc1 -cc1 -triple 
arm64-apple-ios12.2.0 -Oz -fobjc-arc -emit-llvm 
GREYAssertDefaultConfiguration-9e1f3b.reduced.m
  1. parser at end of file
  2.Per-module optimization passes
  3.Running pass 'CallGraph Pass Manager' on module 
'GREYAssertDefaultConfiguration-9e1f3b.reduced.m'.
   #0 0x034a054c llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) 
/usr/local/google/home/thakis/src/llvm-project/out/gn/../../llvm/lib/Support/Unix/Signals.inc:565:13
   #1 0x0349e45e llvm::sys::RunSignalHandlers() 
/usr/local/google/home/thakis/src/llvm-project/out/gn/../../llvm/lib/Support/Signals.cpp:72:18
   #2 0x034a089f SignalHandler(int) 
/usr/local/google/home/thakis/src/llvm-project/out/gn/../../llvm/lib/Support/Unix/Signals.inc:407:1
   #3 0x7f8505e34140 __restore_rt 
(/lib/x86_64-linux-gnu/libpthread.so.0+0x14140)
   #4 0x7f850594cce1 raise ./signal/../sysdeps/unix/sysv/linux/raise.c:51:1
   #5 0x7f8505936537 abort ./stdlib/abort.c:81:7
   #6 0x7f850593640f get_sysdep_segment_value ./intl/loadmsgcat.c:509:8
   #7 0x7f850593640f _nl_load_domain ./intl/loadmsgcat.c:970:34
   #8 0x7f8505945662 (/lib/x86_64-linux-gnu/libc.so.6+0x34662)
   #9 0x02be88bd (out/gn/bin/clang+0x2be88bd)
  #10 0x03afdd35 hasValue 
/usr/local/google/home/thakis/src/llvm-project/out/gn/../../llvm/include/llvm/ADT/Optional.h:194:53
  #11 0x03afdd35 hasValue 
/usr/local/google/home/thakis/src/llvm-project/out/gn/../../llvm/include/llvm/ADT/Optional.h:286:52
  #12 0x03afdd35 hasRVOpBundle 
/usr/local/google/home/thakis/src/llvm-project/out/gn/../../llvm/include/llvm/Analysis/ObjCARCUtil.h:42:61
  #13 0x03afdd35 llvm::InlineFunction(llvm::CallBase&, 
llvm::InlineFunctionInfo&, llvm::AAResults*, bool, llvm::Function*) 
/usr/local/google/home/thakis/src/llvm-project/out/gn/../../llvm/lib/Transforms/Utils/InlineFunction.cpp:1953:9

Given that the repro is so tiny, I've reverted this for now in 
de1966e5427985163f8e816834b3a0564b5e24cd 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92808

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

[clang] de1966e - Revert "[ObjC][ARC] Use operand bundle 'clang.arc.rv' instead of explicitly"

[Nico Weber via cfe-commits]

Author: Nico Weber
Date: 2021-02-09T11:06:32-05:00
New Revision: de1966e5427985163f8e816834b3a0564b5e24cd

URL: 
https://github.com/llvm/llvm-project/commit/de1966e5427985163f8e816834b3a0564b5e24cd
DIFF: 
https://github.com/llvm/llvm-project/commit/de1966e5427985163f8e816834b3a0564b5e24cd.diff

LOG: Revert "[ObjC][ARC] Use operand bundle 'clang.arc.rv' instead of 
explicitly"

This reverts commit 4a64d8fe392449b205e59031aad5424968cf7446.
Makes clang crash when buildling trivial iOS programs, see comment
after https://reviews.llvm.org/D92808#2551401

Added: 
llvm/test/Transforms/TailCallElim/deopt-bundle.ll

Modified: 
clang/lib/CodeGen/CGObjC.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/lib/CodeGen/CodeGenModule.h
clang/test/CodeGenObjC/arc-unsafeclaim.m
llvm/docs/LangRef.rst
llvm/include/llvm/IR/InstrTypes.h
llvm/include/llvm/IR/Intrinsics.td
llvm/include/llvm/IR/LLVMContext.h
llvm/lib/Analysis/ObjCARCInstKind.cpp
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
llvm/lib/IR/Instructions.cpp
llvm/lib/IR/LLVMContext.cpp
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
llvm/lib/Transforms/ObjCARC/ARCRuntimeEntryPoints.h
llvm/lib/Transforms/ObjCARC/ObjCARC.cpp
llvm/lib/Transforms/ObjCARC/ObjCARC.h
llvm/lib/Transforms/ObjCARC/ObjCARCContract.cpp
llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp
llvm/lib/Transforms/ObjCARC/PtrState.cpp
llvm/lib/Transforms/ObjCARC/PtrState.h
llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp
llvm/lib/Transforms/Utils/InlineFunction.cpp
llvm/test/Bitcode/operand-bundles-bc-analyzer.ll
llvm/test/CodeGen/AArch64/call-rv-marker.ll
llvm/test/Transforms/DeadArgElim/deadretval.ll
llvm/test/Transforms/ObjCARC/contract-marker-funclet.ll
llvm/test/Transforms/ObjCARC/contract.ll
llvm/test/Transforms/ObjCARC/intrinsic-use.ll
llvm/test/Transforms/ObjCARC/rv.ll

Removed: 
clang/test/CodeGenObjC/arc-rv-attr.m
llvm/include/llvm/Analysis/ObjCARCUtil.h
llvm/test/Transforms/Inline/inline-retainRV-call.ll
llvm/test/Transforms/ObjCARC/contract-rv-attr.ll
llvm/test/Transforms/TailCallElim/operand-bundles.ll



diff  --git a/clang/lib/CodeGen/CGObjC.cpp b/clang/lib/CodeGen/CGObjC.cpp
index ab3fba615335..3f930c76fe0a 100644
--- a/clang/lib/CodeGen/CGObjC.cpp
+++ b/clang/lib/CodeGen/CGObjC.cpp
@@ -23,7 +23,6 @@
 #include "clang/Basic/Diagnostic.h"
 #include "clang/CodeGen/CGFunctionInfo.h"
 #include "llvm/ADT/STLExtras.h"
-#include "llvm/Analysis/ObjCARCUtil.h"
 #include "llvm/BinaryFormat/MachO.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/InlineAsm.h"
@@ -2079,15 +2078,6 @@ void 
CodeGenFunction::EmitARCIntrinsicUse(ArrayRef values) {
   EmitNounwindRuntimeCall(fn, values);
 }
 
-/// Emit a call to "clang.arc.noop.use", which consumes the result of a call
-/// that has operand bundle "clang.arc.rv".
-void CodeGenFunction::EmitARCNoopIntrinsicUse(ArrayRef values) {
-  llvm::Function * = CGM.getObjCEntrypoints().clang_arc_noop_use;
-  if (!fn)
-fn = CGM.getIntrinsic(llvm::Intrinsic::objc_clang_arc_noop_use);
-  EmitNounwindRuntimeCall(fn, values);
-}
-
 static void setARCRuntimeFunctionLinkage(CodeGenModule , llvm::Value *RTF) 
{
   if (auto *F = dyn_cast(RTF)) {
 // If the target runtime doesn't naturally support ARC, emit weak
@@ -2314,11 +2304,10 @@ static void 
emitAutoreleasedReturnValueMarker(CodeGenFunction ) {
 // with this marker yet, so leave a breadcrumb for the ARC
 // optimizer to pick up.
 } else {
-  const char *retainRVMarkerKey = 
llvm::objcarc::getRVMarkerModuleFlagStr();
-  if (!CGF.CGM.getModule().getModuleFlag(retainRVMarkerKey)) {
+  const char *markerKey = "clang.arc.retainAutoreleasedReturnValueMarker";
+  if (!CGF.CGM.getModule().getModuleFlag(markerKey)) {
 auto *str = llvm::MDString::get(CGF.getLLVMContext(), assembly);
-CGF.CGM.getModule().addModuleFlag(llvm::Module::Error,
-  retainRVMarkerKey, str);
+CGF.CGM.getModule().addModuleFlag(llvm::Module::Error, markerKey, str);
   }
 }
   }
@@ -2328,46 +2317,6 @@ static void 
emitAutoreleasedReturnValueMarker(CodeGenFunction ) {
 CGF.Builder.CreateCall(marker, None, CGF.getBundlesForFunclet(marker));
 }
 
-static llvm::Value *emitOptimizedARCReturnCall(llvm::Value *value,
-   bool IsRetainRV,
-   CodeGenFunction ) {
-  emitAutoreleasedReturnValueMarker(CGF);
-
-  // Add operand bundle "clang.arc.rv" to the call instead of emitting retainRV
-  // or claimRV calls in the IR. We currently do this only when the 
optimization
-  // level isn't -O0 since global-isel, which is currently run at -O0, doesn't
-  // know about the operand bundle.
-
-  // FIXME: Do this when the target isn't aarch64.
-  if 

[PATCH] D96178: [OpenCL] Create VoidPtrTy with generic AS in C++ for OpenCL mode

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

@svenvh We haven't looked into address spaces in details when adding this. I 
wonder what behavior would make sense for new/delete? Do we plan to allow 
new/delete in named address spaces or only in generic? It might be more helpful 
to allow named address spaces since generic has not been intended for 
allocations, but however generic makes things easier because it avoids 
duplication and the concrete address space can always be cast to it. I think 
`constant` doesn't make sense at all since nothing can be done with a readonly 
chunk of memory?

Another aspect to consider - how will address spaces aligned with the concept 
of overloading i.e.

- Declaring multiple new operators with different address spaces in the same 
scope generates an error since overloading by the return type is not allowed.
- Declaring multiple delete operators differing by address spaces seems to fail 
because C++ doesn't allow to overload it yet.

While the test in the review works, if we start using other than generic 
address spaces it fails with various compile-time errors. For example, I get an 
error if I try to assign to `local` address space pointer even if the 
declaration of new has the address space too:

`class A {
public:

  __local void* operator new(size_t);

};
__local A* a = new A;`

`error: assigning 'A *' to '__local A *' changes address space of pointer`

Perhaps this deserves a PR at least?




Comment at: clang/lib/Sema/SemaDeclCXX.cpp:15287
+
+if (CanQual ExpectedPtrTy =
+ExpectedResultType->getAs()) {

I think `getAs` returns a `PointerType*` that we could just use straight away 
without the need for `CanQual`? but actually, you could just change to `auto*` 
that would make things easier.

The same below.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:15289
+ExpectedResultType->getAs()) {
+  ExpectedResultType = SemaRef.Context.getCanonicalType(
+  RemoveAddressSpaceFromPtr(SemaRef, ExpectedPtrTy->getTypePtr()));

FYI I think `ExpectedResultType` is going to be in always in the canonical form 
here by the way it is constructed so `getCanonicalType` should be redundant.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96178

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

[PATCH] D96347: Include function return type in JSON AST dumps.

[Rokas Kupstys via Phabricator via cfe-commits]

rokups created this revision.
rokups requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

For some reason return type was not included. Even though it is present
in full type dump, it is not good enough as C++ types can get quite
complex. Besides there is no way to get a desugared version of the type
from function type string. Patch uses "returnType" json key, just like
ObjCMethodDecl dumper. I also updated tests. If anything is missing -
please let me know.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96347

Files:
  clang/lib/AST/JSONNodeDumper.cpp
  clang/test/AST/ast-dump-decl-context-json.cpp
  clang/test/AST/ast-dump-decl-json.c
  clang/test/AST/ast-dump-decl-json.m
  clang/test/AST/ast-dump-expr-json.c
  clang/test/AST/ast-dump-expr-json.cpp
  clang/test/AST/ast-dump-expr-json.m
  clang/test/AST/ast-dump-funcs-json.cpp
  clang/test/AST/ast-dump-macro-json.c
  clang/test/AST/ast-dump-record-definition-data-json.cpp
  clang/test/AST/ast-dump-records-json.cpp
  clang/test/AST/ast-dump-stmt-json.cpp
  clang/test/AST/ast-dump-stmt-json.m
  clang/test/AST/ast-dump-template-decls-json.cpp

Index: clang/test/AST/ast-dump-template-decls-json.cpp
===
--- clang/test/AST/ast-dump-template-decls-json.cpp
+++ clang/test/AST/ast-dump-template-decls-json.cpp
@@ -287,6 +287,9 @@
 // CHECK-NEXT:  "type": {
 // CHECK-NEXT:   "qualType": "void (Ty)"
 // CHECK-NEXT:  },
+// CHECK-NEXT:  "returnType": {
+// CHECK-NEXT:   "qualType": "void"
+// CHECK-NEXT:  },
 // CHECK-NEXT:  "inner": [
 // CHECK-NEXT:   {
 // CHECK-NEXT:"id": "0x{{.*}}",
@@ -394,6 +397,9 @@
 // CHECK-NEXT:  "type": {
 // CHECK-NEXT:   "qualType": "void (Ty...)"
 // CHECK-NEXT:  },
+// CHECK-NEXT:  "returnType": {
+// CHECK-NEXT:   "qualType": "void"
+// CHECK-NEXT:  },
 // CHECK-NEXT:  "inner": [
 // CHECK-NEXT:   {
 // CHECK-NEXT:"id": "0x{{.*}}",
@@ -526,6 +532,9 @@
 // CHECK-NEXT:  "type": {
 // CHECK-NEXT:   "qualType": "void (Ty)"
 // CHECK-NEXT:  },
+// CHECK-NEXT:  "returnType": {
+// CHECK-NEXT:   "qualType": "void"
+// CHECK-NEXT:  },
 // CHECK-NEXT:  "inner": [
 // CHECK-NEXT:   {
 // CHECK-NEXT:"id": "0x{{.*}}",
@@ -592,6 +601,9 @@
 // CHECK-NEXT:"type": {
 // CHECK-NEXT: "qualType": "void (float)"
 // CHECK-NEXT:},
+// CHECK-NEXT:"returnType": {
+// CHECK-NEXT: "qualType": "void"
+// CHECK-NEXT:},
 // CHECK-NEXT:"inner": [
 // CHECK-NEXT: {
 // CHECK-NEXT:  "kind": "TemplateArgument",
@@ -776,6 +788,9 @@
 // CHECK-NEXT:  "type": {
 // CHECK-NEXT:   "qualType": "void (Ty, Uy)"
 // CHECK-NEXT:  },
+// CHECK-NEXT:  "returnType": {
+// CHECK-NEXT:   "qualType": "void"
+// CHECK-NEXT:  },
 // CHECK-NEXT:  "inner": [
 // CHECK-NEXT:   {
 // CHECK-NEXT:"id": "0x{{.*}}",
@@ -906,6 +921,9 @@
 // CHECK-NEXT:  "type": {
 // CHECK-NEXT:   "qualType": "void (Ty)"
 // CHECK-NEXT:  },
+// CHECK-NEXT:  "returnType": {
+// CHECK-NEXT:   "qualType": "void"
+// CHECK-NEXT:  },
 // CHECK-NEXT:  "inner": [
 // CHECK-NEXT:   {
 // CHECK-NEXT:"id": "0x{{.*}}",
@@ -1014,6 +1032,9 @@
 // CHECK-NEXT:  "type": {
 // CHECK-NEXT:   "qualType": "void (int)"
 // CHECK-NEXT:  },
+// CHECK-NEXT:  "returnType": {
+// CHECK-NEXT:   "qualType": "void"
+// CHECK-NEXT:  },
 // CHECK-NEXT:  "inner": [
 // CHECK-NEXT:   {
 // CHECK-NEXT:"id": "0x{{.*}}",
@@ -1176,6 +1197,9 @@
 // CHECK-NEXT:  "type": {
 // CHECK-NEXT:   "qualType": "void (Ty)"
 // CHECK-NEXT:  },
+// CHECK-NEXT:  "returnType": {
+// CHECK-NEXT:   "qualType": "void"
+// CHECK-NEXT:  },
 // CHECK-NEXT:  "inner": [
 // CHECK-NEXT:   {
 // CHECK-NEXT:"id": "0x{{.*}}",
@@ -1302,6 +1326,9 @@
 // CHECK-NEXT:  "name": "h",
 // CHECK-NEXT:  "type": {
 // CHECK-NEXT:   "qualType": "void ()"
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "returnType": {
+// CHECK-NEXT:   "qualType": "void"
 // CHECK-NEXT:  }
 // CHECK-NEXT: }
 // CHECK-NEXT:]
@@ -2468,6 +2495,9 @@
 // CHECK-NEXT:  "name": "f",
 // CHECK-NEXT:  "type": {
 // CHECK-NEXT:   "qualType": "void ()"
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "returnType": {
+// CHECK-NEXT:   "qualType": "void"
 // CHECK-NEXT:  }
 // CHECK-NEXT: }
 // CHECK-NEXT:]
@@ -2558,6 +2588,9 @@
 // CHECK-NEXT:  "type": {
 // CHECK-NEXT:   "qualType": "void ()"
 // CHECK-NEXT:  },
+// CHECK-NEXT:  "returnType": {
+// CHECK-NEXT:   "qualType": "void"
+// CHECK-NEXT:  },
 // CHECK-NEXT:  "inner": [
 // CHECK-NEXT:   {
 // CHECK-NEXT:"id": "0x{{.*}}",
Index: clang/test/AST/ast-dump-stmt-json.m

[PATCH] D96262: [clang][index] report references from unreslovedLookupExpr.

[Utkarsh Saxena via Phabricator via cfe-commits]

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

Thanks. LG.
Couple of comments about adding tests to clangd as well.




Comment at: clang/test/Index/Core/index-dependent-source.cpp:233
+// CHECK: [[@LINE-1]]:3 | function/C | foo | c:@FT@>1#Tfoo#v# |  | 
Ref,Call,RelCall,RelCont | rel: 1
+}

Maybe add more tests to clangd/XrefsTests like the ones specifically mentioned 
in https://github.com/clangd/clangd/issues/399

- Ref in uninstantiated `bar` is reported.
```
template  void $decl[[foo]](T t); 
template  void bar(T t) { [[foo]](t); }
void baz(int x) { [[f^oo]](x); }
```

What do you think about the behaviour ?
Should we report the ref in bar in the following example ?
- ADL: May be add a negative test documenting the behaviour. 
```
namespace ns {
struct S{};
void $decl[[foo]](S s);
}
template  void foo(T t); 
template  void bar(T t) { foo(t); } // FIXME: Maybe report this foo 
as a ref to ns::foo when bar is instantiated?
void baz(int x) { 
  ns::S s;
  bar(s);
  [[f^oo]](s); 
}

```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96262

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

[PATCH] D92808: [ObjC][ARC] Use operand bundle 'clang.arc.rv' instead of explicitly emitting retainRV or claimRV calls in the IR

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

This makes clang assert for some inputs. See 
https://bugs.chromium.org/p/chromium/issues/detail?id=1175886#c10 for a repro.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92808

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

[PATCH] D93446: [RISCV] Add vadd with mask and without mask builtin.

[Zakk Chen via Phabricator via cfe-commits]

khchen added inline comments.



Comment at: clang/lib/Basic/Targets/RISCV.cpp:89
+#define BUILTIN(ID, TYPE, ATTRS)   
\
+  {"__builtin_rvv_" #ID, TYPE, ATTRS, nullptr, ALL_LANGUAGES, nullptr},
+#include "clang/Basic/BuiltinsRISCV.def"

Jim wrote:
> Builtins for other extension don't have "__builtin_rvv_" prefix.
maybe we could rename BuiltinsRISCV.def as BuiltinsRVV.def, and other extension 
defines their own .def file?

@Jim do you have any suggestion?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93446

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