[PATCH] D95860: [clang][Frontend] Fix a crash in DiagnosticRenderer.

[Balázs Kéri 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 rG085dcc82178f: [clang][Frontend] Fix a crash in 
DiagnosticRenderer. (authored by balazske).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95860

Files:
  clang/lib/Frontend/DiagnosticRenderer.cpp
  
clang/test/Analysis/copypaste/Inputs/clone-begin-end-different-file-begin-1.inc
  
clang/test/Analysis/copypaste/Inputs/clone-begin-end-different-file-begin-2.inc
  
clang/test/Analysis/copypaste/Inputs/clone-begin-end-different-file-begin-macro-1.inc
  
clang/test/Analysis/copypaste/Inputs/clone-begin-end-different-file-begin-macro-2.inc
  
clang/test/Analysis/copypaste/Inputs/clone-begin-end-different-file-end-macro.inc
  clang/test/Analysis/copypaste/Inputs/clone-begin-end-different-file-end.inc
  clang/test/Analysis/copypaste/clone-begin-end-different-file.cpp
  clang/test/Frontend/crash-diagnostic-renderer.cpp

Index: clang/test/Frontend/crash-diagnostic-renderer.cpp
===
--- /dev/null
+++ clang/test/Frontend/crash-diagnostic-renderer.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -verify %s
+
+// This case reproduces a problem that is shown here:
+// https://bugs.llvm.org/show_bug.cgi?id=46540
+// No assertion should happen during printing of diagnostic messages.
+
+// expected-error@13{{'b' does not refer to a type name in pseudo-destructor expression; expected the name of type 'volatile long'}}
+// expected-error@13{{expected ')'}}
+// expected-note@13{{to match this '('}}
+// expected-error@13{{reference to pseudo-destructor must be called; did you mean to call it with no arguments?}}
+// expected-error@13{{cannot initialize a variable of type 'volatile long' with an rvalue of type 'void'}}
+// expected-error@13{{expected ';' after top level declarator}}
+volatile long a ( a .~b
Index: clang/test/Analysis/copypaste/clone-begin-end-different-file.cpp
===
--- /dev/null
+++ clang/test/Analysis/copypaste/clone-begin-end-different-file.cpp
@@ -0,0 +1,37 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.clone.CloneChecker -analyzer-config alpha.clone.CloneChecker:MinimumCloneComplexity=5 -verify %s
+
+// This test should verify that there is no crash if the detected clone range
+// starts in a file and ends in a different file.
+
+void f_end(int i) {
+  if (i == 10) // expected-warning{{Duplicate code detected}}
+#include "Inputs/clone-begin-end-different-file-end.inc"
+  if (i == 10) // expected-note{{Similar code here}}
+#include "Inputs/clone-begin-end-different-file-end.inc"
+}
+
+void f_begin(int i) {
+#include "Inputs/clone-begin-end-different-file-begin-1.inc"
+if (true) {}
+#include "Inputs/clone-begin-end-different-file-begin-2.inc"
+if (true) {}
+}
+
+#define X while (true) {}
+
+void f1m(int i) {
+  if (i == 10) // expected-warning{{Duplicate code detected}}
+#include "Inputs/clone-begin-end-different-file-end-macro.inc"
+  if (i == 10) // expected-note{{Similar code here}}
+#include "Inputs/clone-begin-end-different-file-end-macro.inc"
+}
+
+#undef X
+#define X if (i == 10)
+
+void f2m(int i) {
+#include "Inputs/clone-begin-end-different-file-begin-macro-1.inc"
+while (true) { i = 1; }
+#include "Inputs/clone-begin-end-different-file-begin-macro-2.inc"
+while (true) { i = 1; }
+}
Index: clang/test/Analysis/copypaste/Inputs/clone-begin-end-different-file-end.inc
===
--- /dev/null
+++ clang/test/Analysis/copypaste/Inputs/clone-begin-end-different-file-end.inc
@@ -0,0 +1 @@
+while (true) {}
Index: clang/test/Analysis/copypaste/Inputs/clone-begin-end-different-file-end-macro.inc
===
--- /dev/null
+++ clang/test/Analysis/copypaste/Inputs/clone-begin-end-different-file-end-macro.inc
@@ -0,0 +1 @@
+X
Index: clang/test/Analysis/copypaste/Inputs/clone-begin-end-different-file-begin-macro-2.inc
===
--- /dev/null
+++ clang/test/Analysis/copypaste/Inputs/clone-begin-end-different-file-begin-macro-2.inc
@@ -0,0 +1 @@
+X // expected-note{{Similar code here}}
Index: clang/test/Analysis/copypaste/Inputs/clone-begin-end-different-file-begin-macro-1.inc
===
--- /dev/null
+++ clang/test/Analysis/copypaste/Inputs/clone-begin-end-different-file-begin-macro-1.inc
@@ -0,0 +1 @@
+X // expected-warning{{Duplicate code detected}}
Index: clang/test/Analysis/copypaste/Inputs/clone-begin-end-different-file-begin-2.inc
===
--- /dev/null
+++ clang/test/Analysis/copypaste/Inputs/clone-begin-end-different-file-begin-2.inc
@@ -0,0 +1 @@
+while 

[clang] 085dcc8 - [clang][Frontend] Fix a crash in DiagnosticRenderer.

[Balázs Kéri via cfe-commits]

Author: Balázs Kéri
Date: 2021-02-17T09:02:49+01:00
New Revision: 085dcc82178f94b99783c5730e70a953e4105c00

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

LOG: [clang][Frontend] Fix a crash in DiagnosticRenderer.

Displaying the problem range could crash if the begin and end of a
range is in different files or macros. After the change such range
is displayed only as the beginning location.

There is a bug for this problem:
https://bugs.llvm.org/show_bug.cgi?id=46540

Reviewed By: steakhal

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

Added: 

clang/test/Analysis/copypaste/Inputs/clone-begin-end-different-file-begin-1.inc

clang/test/Analysis/copypaste/Inputs/clone-begin-end-different-file-begin-2.inc

clang/test/Analysis/copypaste/Inputs/clone-begin-end-different-file-begin-macro-1.inc

clang/test/Analysis/copypaste/Inputs/clone-begin-end-different-file-begin-macro-2.inc

clang/test/Analysis/copypaste/Inputs/clone-begin-end-different-file-end-macro.inc
clang/test/Analysis/copypaste/Inputs/clone-begin-end-different-file-end.inc
clang/test/Analysis/copypaste/clone-begin-end-different-file.cpp
clang/test/Frontend/crash-diagnostic-renderer.cpp

Modified: 
clang/lib/Frontend/DiagnosticRenderer.cpp

Removed: 




diff  --git a/clang/lib/Frontend/DiagnosticRenderer.cpp 
b/clang/lib/Frontend/DiagnosticRenderer.cpp
index 22b957988f46..0afc8f3b1dab 100644
--- a/clang/lib/Frontend/DiagnosticRenderer.cpp
+++ b/clang/lib/Frontend/DiagnosticRenderer.cpp
@@ -394,6 +394,13 @@ mapDiagnosticRanges(FullSourceLoc CaretLoc, 
ArrayRef Ranges,
   }
 }
 
+// There is a chance that begin or end is invalid here, for example if
+// specific compile error is reported.
+// It is possible that the FileID's do not match, if one comes from an
+// included file. In this case we can not produce a meaningful source 
range.
+if (Begin.isInvalid() || End.isInvalid() || BeginFileID != EndFileID)
+  continue;
+
 // Do the backtracking.
 SmallVector CommonArgExpansions;
 computeCommonMacroArgExpansionFileIDs(Begin, End, SM, CommonArgExpansions);

diff  --git a/clang/test/Analysis/copypaste/Inputs/clone-begin-end-
diff erent-file-begin-1.inc 
b/clang/test/Analysis/copypaste/Inputs/clone-begin-end-
diff erent-file-begin-1.inc
new file mode 100644
index ..a6b44fd87073
--- /dev/null
+++ b/clang/test/Analysis/copypaste/Inputs/clone-begin-end-
diff erent-file-begin-1.inc
@@ -0,0 +1 @@
+while (i == 10) // expected-warning{{Duplicate code detected}}

diff  --git a/clang/test/Analysis/copypaste/Inputs/clone-begin-end-
diff erent-file-begin-2.inc 
b/clang/test/Analysis/copypaste/Inputs/clone-begin-end-
diff erent-file-begin-2.inc
new file mode 100644
index ..7aa4a3dcdc86
--- /dev/null
+++ b/clang/test/Analysis/copypaste/Inputs/clone-begin-end-
diff erent-file-begin-2.inc
@@ -0,0 +1 @@
+while (i == 10) // expected-note{{Similar code here}}

diff  --git a/clang/test/Analysis/copypaste/Inputs/clone-begin-end-
diff erent-file-begin-macro-1.inc 
b/clang/test/Analysis/copypaste/Inputs/clone-begin-end-
diff erent-file-begin-macro-1.inc
new file mode 100644
index ..76637bbd8733
--- /dev/null
+++ b/clang/test/Analysis/copypaste/Inputs/clone-begin-end-
diff erent-file-begin-macro-1.inc
@@ -0,0 +1 @@
+X // expected-warning{{Duplicate code detected}}

diff  --git a/clang/test/Analysis/copypaste/Inputs/clone-begin-end-
diff erent-file-begin-macro-2.inc 
b/clang/test/Analysis/copypaste/Inputs/clone-begin-end-
diff erent-file-begin-macro-2.inc
new file mode 100644
index ..9f03aeac4e83
--- /dev/null
+++ b/clang/test/Analysis/copypaste/Inputs/clone-begin-end-
diff erent-file-begin-macro-2.inc
@@ -0,0 +1 @@
+X // expected-note{{Similar code here}}

diff  --git a/clang/test/Analysis/copypaste/Inputs/clone-begin-end-
diff erent-file-end-macro.inc 
b/clang/test/Analysis/copypaste/Inputs/clone-begin-end-
diff erent-file-end-macro.inc
new file mode 100644
index ..62d8fe9f6db6
--- /dev/null
+++ b/clang/test/Analysis/copypaste/Inputs/clone-begin-end-
diff erent-file-end-macro.inc
@@ -0,0 +1 @@
+X

diff  --git a/clang/test/Analysis/copypaste/Inputs/clone-begin-end-
diff erent-file-end.inc b/clang/test/Analysis/copypaste/Inputs/clone-begin-end-
diff erent-file-end.inc
new file mode 100644
index ..1fa180e56531
--- /dev/null
+++ b/clang/test/Analysis/copypaste/Inputs/clone-begin-end-
diff erent-file-end.inc
@@ -0,0 +1 @@
+while (true) {}

diff  --git a/clang/test/Analysis/copypaste/clone-begin-end-
diff erent-file.cpp b/clang/test/Analysis/copypaste/clone-begin-end-
diff erent-file.cpp
new file mode 100644
index ..a5f1b668feff
--- /dev/null
+++ 

[PATCH] D96807: Modify TypePrinter to differentiate between anonymous struct and unnamed struct

[Raphael Isemann via Phabricator via cfe-commits]

teemperor added a comment.

LGTM.

Regarding the LLDB example: Given that the LLDB API is in theory not bound to 
the semantics a specific language, I think one can argue that `IsAnonymousType` 
could also return true for unnamed classes. The use case that triggered this 
whole discussion was someone who wanted to know whether the type has a name, so 
calling this function seems logical. Given that we don't really have a valid 
type name for unnamed classes, we might want to change the implementation of 
`IsAnonymousType` to also return true for unnamed classes. But all that is out 
of scope for this patch which is about fixing the type printer in Clang. Just 
bringing this up as it's the example in the patch description.


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

https://reviews.llvm.org/D96807

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

[PATCH] D96845: [clangd] IndexedFiles should include Fils from RefSlab and RelationSlab.

[Haojian Wu via Phabricator via cfe-commits]

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

This looks like an oversight.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96845

Files:
  clang-tools-extra/clangd/index/FileIndex.cpp


Index: clang-tools-extra/clangd/index/FileIndex.cpp
===
--- clang-tools-extra/clangd/index/FileIndex.cpp
+++ clang-tools-extra/clangd/index/FileIndex.cpp
@@ -280,11 +280,14 @@
 }
 for (const auto  : RefsSnapshot) {
   RefSlabs.push_back(FileAndRefs.second.Slab);
+  Files.insert(FileAndRefs.first());
   if (FileAndRefs.second.CountReferences)
 MainFileRefs.push_back(RefSlabs.back().get());
 }
-for (const auto  : RelationsSnapshot)
+for (const auto  : RelationsSnapshot) {
+  Files.insert(FileAndRelations.first());
   RelationSlabs.push_back(FileAndRelations.second);
+}
 
 if (Version)
   *Version = this->Version;


Index: clang-tools-extra/clangd/index/FileIndex.cpp
===
--- clang-tools-extra/clangd/index/FileIndex.cpp
+++ clang-tools-extra/clangd/index/FileIndex.cpp
@@ -280,11 +280,14 @@
 }
 for (const auto  : RefsSnapshot) {
   RefSlabs.push_back(FileAndRefs.second.Slab);
+  Files.insert(FileAndRefs.first());
   if (FileAndRefs.second.CountReferences)
 MainFileRefs.push_back(RefSlabs.back().get());
 }
-for (const auto  : RelationsSnapshot)
+for (const auto  : RelationsSnapshot) {
+  Files.insert(FileAndRelations.first());
   RelationSlabs.push_back(FileAndRelations.second);
+}
 
 if (Version)
   *Version = this->Version;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] ab8a620 - [OpenMP] Fix a warning on an unused variable

[Kazu Hirata via cfe-commits]

Author: Kazu Hirata
Date: 2021-02-16T23:46:21-08:00
New Revision: ab8a62057384e1bbfd528a4012cd898511c83b02

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

LOG: [OpenMP] Fix a warning on an unused variable

Added: 


Modified: 
clang/lib/Sema/SemaOpenMP.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index bc7e8b55c757..f2c9fe644d96 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -18109,6 +18109,7 @@ static void checkMappableExpressionList(
   return MC.getAssociatedDeclaration();
 });
 assert(I != CurComponents.end() && "Null decl on map clause.");
+(void)I;
 QualType Type;
 auto *ASE = dyn_cast(VE->IgnoreParens());
 auto *OASE = dyn_cast(VE->IgnoreParens());



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

[PATCH] D96844: [clangd] Pass file when possible to resolve URI.

[Haojian Wu via Phabricator via cfe-commits]

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

Some URI scheme needs the hint path to do a correct resolution, we pass
one of the open files as hint path.

This is not perfect, and it might not work for opening files across
project, but it would fix a bug with our internal scheme.

in the long run, removing URIs from all the index internals is a more proper 
fix.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96844

Files:
  clang-tools-extra/clangd/index/MemIndex.cpp
  clang-tools-extra/clangd/index/dex/Dex.cpp


Index: clang-tools-extra/clangd/index/dex/Dex.cpp
===
--- clang-tools-extra/clangd/index/dex/Dex.cpp
+++ clang-tools-extra/clangd/index/dex/Dex.cpp
@@ -316,9 +316,11 @@
 llvm::unique_function
 Dex::indexedFiles() const {
   return [this](llvm::StringRef FileURI) {
-auto Path = URI::resolve(FileURI);
+if (Files.empty())
+  return IndexContents::None;
+auto Path = URI::resolve(FileURI, Files.begin()->first());
 if (!Path) {
-  llvm::consumeError(Path.takeError());
+  vlog("Failed to resolve the URI {0} : {1}", FileURI, Path.takeError());
   return IndexContents::None;
 }
 return Files.contains(*Path) ? IdxContents : IndexContents::None;
Index: clang-tools-extra/clangd/index/MemIndex.cpp
===
--- clang-tools-extra/clangd/index/MemIndex.cpp
+++ clang-tools-extra/clangd/index/MemIndex.cpp
@@ -112,9 +112,11 @@
 llvm::unique_function
 MemIndex::indexedFiles() const {
   return [this](llvm::StringRef FileURI) {
-auto Path = URI::resolve(FileURI);
+if (Files.empty())
+  return IndexContents::None;
+auto Path = URI::resolve(FileURI, Files.begin()->first());
 if (!Path) {
-  llvm::consumeError(Path.takeError());
+  vlog("Failed to resolve the URI {0} : {1}", FileURI, Path.takeError());
   return IndexContents::None;
 }
 return Files.contains(*Path) ? IdxContents : IndexContents::None;


Index: clang-tools-extra/clangd/index/dex/Dex.cpp
===
--- clang-tools-extra/clangd/index/dex/Dex.cpp
+++ clang-tools-extra/clangd/index/dex/Dex.cpp
@@ -316,9 +316,11 @@
 llvm::unique_function
 Dex::indexedFiles() const {
   return [this](llvm::StringRef FileURI) {
-auto Path = URI::resolve(FileURI);
+if (Files.empty())
+  return IndexContents::None;
+auto Path = URI::resolve(FileURI, Files.begin()->first());
 if (!Path) {
-  llvm::consumeError(Path.takeError());
+  vlog("Failed to resolve the URI {0} : {1}", FileURI, Path.takeError());
   return IndexContents::None;
 }
 return Files.contains(*Path) ? IdxContents : IndexContents::None;
Index: clang-tools-extra/clangd/index/MemIndex.cpp
===
--- clang-tools-extra/clangd/index/MemIndex.cpp
+++ clang-tools-extra/clangd/index/MemIndex.cpp
@@ -112,9 +112,11 @@
 llvm::unique_function
 MemIndex::indexedFiles() const {
   return [this](llvm::StringRef FileURI) {
-auto Path = URI::resolve(FileURI);
+if (Files.empty())
+  return IndexContents::None;
+auto Path = URI::resolve(FileURI, Files.begin()->first());
 if (!Path) {
-  llvm::consumeError(Path.takeError());
+  vlog("Failed to resolve the URI {0} : {1}", FileURI, Path.takeError());
   return IndexContents::None;
 }
 return Files.contains(*Path) ? IdxContents : IndexContents::None;
___
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.

[Hsiangkai Wang via Phabricator via cfe-commits]

HsiangKai added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11127
+def err_riscvv_builtin_not_useable : Error<
+   "builtin requires 'V' extension support to be enabled">;
 } // end of sema component.

Jim wrote:
> Add blank line.
> Is it "err_riscvv_builtin_not_enabled"?
I use the name that is defined in BSC version. It makes sense to me that it 
means "the risc-v V builtins are not useable."

@rogfer01, do you have any opinion on it?


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] D96843: [Clang][RISCV] Add vsetvl and vsetvlmax.

[Zakk Chen via Phabricator via cfe-commits]

khchen created this revision.
khchen added reviewers: craig.topper, jrtc27, rogfer01, frasercrmck, HsiangKai, 
evandro.
Herald added subscribers: vkmr, dexonsmith, luismarques, apazos, 
sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, 
brucehoult, MartinMosbeck, edward-jones, zzheng, shiva0217, kito-cheng, niosHD, 
sabuasal, simoncook, johnrusso, rbar, asb.
khchen requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96843

Files:
  clang/include/clang/Basic/riscv_vector.td
  clang/lib/Basic/Targets/RISCV.cpp
  clang/test/CodeGen/RISCV/rvv-intrinsics/vsetvl.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vsetvlmax.c
  clang/utils/TableGen/RISCVVEmitter.cpp

Index: clang/utils/TableGen/RISCVVEmitter.cpp
===
--- clang/utils/TableGen/RISCVVEmitter.cpp
+++ clang/utils/TableGen/RISCVVEmitter.cpp
@@ -132,6 +132,9 @@
   bool HasMaskedOffOperand;
   bool HasVL;
   bool HasGeneric;
+  bool HasAutoDef;
+  bool HasManualCodegen;
+  std::string ManualCodegen;
   RVVTypes Types;  // Include output and other input
   std::vector IntrinsicTypes; // Type name in LLVM IR intrinsic suffix
   uint8_t RISCV_Extensions = 0;
@@ -140,8 +143,9 @@
   RVVIntrinsic(StringRef Name, StringRef Suffix, StringRef MangledName,
StringRef IRName, bool HasSideEffects, bool IsMask,
bool HasMaskedOffOperand, bool HasVL, bool HasGeneric,
+   bool HasAutoDef, bool HasManualCodegen, StringRef ManualCodegen,
const RVVTypes ,
-   const std::vector );
+   const std::vector );
   ~RVVIntrinsic() = default;
 
   StringRef getName() const { return Name; }
@@ -150,6 +154,9 @@
   bool hasMaskedOffOperand() const { return HasMaskedOffOperand; }
   bool hasVL() const { return HasVL; }
   bool hasGeneric() const { return HasGeneric; }
+  bool hasAutoDef() const { return HasAutoDef; }
+  bool hasManualCodegen() const { return HasManualCodegen; }
+  StringRef getManualCodegen() const { return ManualCodegen; }
   size_t getNumOperand() const { return Types.size() - 1; }
   // Get output and input types
   ArrayRef getTypes() const { return Types; }
@@ -176,6 +183,7 @@
 class RVVEmitter {
 private:
   RecordKeeper 
+  std::string HeaderCode;
   // Concat BasicType, LMUL and Proto as key
   StringMap LegalTypes;
   StringSet<> IllegalTypes;
@@ -604,11 +612,15 @@
StringRef NewMangledName, StringRef IRName,
bool HasSideEffects, bool IsMask,
bool HasMaskedOffOperand, bool HasVL,
-   bool HasGeneric, const RVVTypes ,
+   bool HasGeneric, bool HasAutoDef,
+   bool HasManualCodegen, StringRef ManualCodegen,
+   const RVVTypes ,
const std::vector )
 : IRName(IRName), HasSideEffects(HasSideEffects), IsMask(IsMask),
   HasMaskedOffOperand(HasMaskedOffOperand), HasVL(HasVL),
-  HasGeneric(HasGeneric), Types(Types), IntrinsicTypes(IntrinsicTypes) {
+  HasGeneric(HasGeneric), HasAutoDef(HasAutoDef),
+  HasManualCodegen(HasManualCodegen), ManualCodegen(ManualCodegen.str()),
+  Types(Types), IntrinsicTypes(IntrinsicTypes) {
 
   // Init Name and MangledName
   Name = NewName.str();
@@ -645,8 +657,6 @@
 
   auto getIntrinsicTypesString =
   [this](const std::vector , raw_ostream ) {
-OS << "  ID = Intrinsic::riscv_" + getIRName() + ";\n";
-
 OS << "  IntrinsicTypes = {";
 for (const auto  : IntrinsicTypes) {
   if (Idx == -1)
@@ -663,6 +673,11 @@
 OS << "};\n";
   };
 
+  OS << "  ID = Intrinsic::riscv_" + getIRName() + ";\n";
+  if (hasManualCodegen()) {
+OS << getManualCodegen().str();
+return;
+  }
   if (!IsMask) {
 getIntrinsicTypesString(getIntrinsicTypes(), OS);
 return;
@@ -756,6 +771,11 @@
   std::vector> Defs;
   createRVVIntrinsics(Defs);
 
+  // Dump header
+  if (HeaderCode.size()) {
+OS << HeaderCode;
+  }
+
   // Dump RVV boolean types.
   auto dumpType = [&](auto T) {
 OS << "typedef " << T->clang_builtin_str() << " " << T->type_str() << ";\n";
@@ -875,7 +895,6 @@
 
 void RVVEmitter::createRVVIntrinsics(
 std::vector> ) {
-
   std::vector RV = Records.getAllDerivedDefinitions("RVVBuiltin");
   for (auto *R : RV) {
 StringRef Name = R->getValueAsString("Name");
@@ -889,11 +908,19 @@
 bool HasGeneric = R->getValueAsBit("HasGeneric");
 bool HasSideEffects = R->getValueAsBit("HasSideEffects");
 std::vector Log2LMULList = R->getValueAsListOfInts("Log2LMUL");
+bool HasManualCodegen = R->getValueAsBit("HasManualCodegen");
+StringRef ManualCodegen = R->getValueAsString("ManualCodegen");
+StringRef ManualCodegenMask = 

[PATCH] D96783: [Driver] Support -gdwarf64 for assembly files

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin marked 2 inline comments as done.
ikudrin added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3757-3771
+  if (DwarfFormatArg &&
+  DwarfFormatArg->getOption().matches(options::OPT_gdwarf64)) {
+if (DwarfVersion < 3)
+  D.Diag(diag::err_drv_argument_only_allowed_with)
+  << DwarfFormatArg->getAsString(Args) << "DWARFv3 or greater";
+else if (!T.isArch64Bit())
+  D.Diag(diag::err_drv_argument_only_allowed_with)

dblaikie wrote:
> Could simplify this with an early return:
> ```
> if (!DwarfFormatArg)
>   return;
> ...
> ```
> 
> So DwarfFormatArg isn't tested twice/reduces indentation/etc.
Thanks! I'll update that on commit.



Comment at: clang/tools/driver/cc1as_main.cpp:237
+  if (auto *DwarfFormatArg = Args.getLastArg(OPT_gdwarf64, OPT_gdwarf32))
+Opts.Dwarf64 = DwarfFormatArg->getOption().matches(OPT_gdwarf64) ? 1 : 0;
   Opts.DwarfVersion = getLastArgIntValue(Args, OPT_dwarf_version_EQ, 2, Diags);

dblaikie wrote:
> No need for the ` ? 1 : 0`, that's what booleans convert to anyway. (unless 
> there's prior art/other instances of this idiom in this file that this is 
> consistent with)
Thanks! I'll apply the suggestion on commit, too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96783

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

[PATCH] D96838: CodeGen: Set !retain metadata on UsedAttr definitions for Linux/FreeBSD

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
Herald added subscribers: krytarowski, arichardson, emaste.
MaskRay requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

GCC 11 will set SHF_GNU_RETAIN on the section of a `__attribute__((used))`
function/variable, if the configure-time GNU as supports SHF_GNU_RETAIN
(binutils 2.36 or above on Linux or FreeBSD).

This patch sets !retain metadata on UsedAttr definitions.
We let the backend to figure out whether the ELF section flag SHF_GNU_RETAIN
should be used (integrated assembler or `-fno-integrated-as 
-fbinutils-version=2.36`).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96838

Files:
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGen/attr-used.c
  clang/test/CodeGenCXX/attr-used.cpp

Index: clang/test/CodeGenCXX/attr-used.cpp
===
--- clang/test/CodeGenCXX/attr-used.cpp
+++ clang/test/CodeGenCXX/attr-used.cpp
@@ -1,10 +1,13 @@
 // RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -o - %s | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-linux -o - %s | FileCheck %s --check-prefix=RETAIN
 
 // : clang++ not respecting __attribute__((used)) on destructors
 struct X0 {
   // CHECK-DAG: define linkonce_odr {{.*}} @_ZN2X0C1Ev
+  // RETAIN-DAG: define linkonce_odr {{.*}} @_ZN2X0C1Ev({{.*}} !retain
   __attribute__((used)) X0() {}
   // CHECK-DAG: define linkonce_odr {{.*}} @_ZN2X0D1Ev
+  // RETAIN-DAG: define linkonce_odr {{.*}} @_ZN2X0D1Ev({{.*}} !retain
   __attribute__((used)) ~X0() {}
 };
 
@@ -12,6 +15,7 @@
 struct X1 {
   struct Nested {
 // CHECK-DAG: define linkonce_odr {{.*}} @_ZN2X16Nested1fEv
+// RETAIN-DAG: define linkonce_odr {{.*}} @_ZN2X16Nested1fEv({{.*}} !retain
 void __attribute__((used)) f() {}
   };
 };
@@ -24,4 +28,5 @@
 
   // CHECK-DAG: define linkonce_odr {{.*}} @_ZN2X23barEv
   // CHECK-DAG: define linkonce_odr {{.*}} @_ZN2X23fooEv
+  // RETAIN-DAG: define linkonce_odr {{.*}} @_ZN2X23barEv({{.*}} !retain
 };
Index: clang/test/CodeGen/attr-used.c
===
--- clang/test/CodeGen/attr-used.c
+++ clang/test/CodeGen/attr-used.c
@@ -1,10 +1,23 @@
-// RUN: %clang_cc1 -emit-llvm -o %t %s
-// RUN: grep '@llvm.used = .*@a0' %t
-// RUN: grep '@llvm.used = .*@g0' %t
-// RUN: grep '@llvm.used = .*@f0' %t
-// RUN: grep '@llvm.used = .*@f1.l0' %t
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-linux %s -o - | FileCheck %s --check-prefixes=CHECK,RETAIN
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-freebsd %s -o - | FileCheck %s --check-prefixes=CHECK,RETAIN
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-apple-darwin %s -o - | FileCheck %s --check-prefixes=CHECK,NORETAIN
 
+// RETAIN:  @r0 ={{.*}} constant i32 {{.*}} !retain ![[#EMPTY:]]
+// RETAIN:  @f1.l0 = internal global i32 {{.*}} !retain ![[#EMPTY]]
+// RETAIN:  @a0 ={{.*}} global i32 {{.*}} !retain ![[#EMPTY]]
+// RETAIN:  @g0 ={{.*}} global i32 {{.*}} !retain ![[#EMPTY]]
 
+/// Set !retain only on Linux and FreeBSD.
+// NORETAIN-NOT: !retain
+
+// CHECK:   @llvm.used ={{.*}} @f0
+// CHECK-SAME:@f1.l0
+// CHECK-SAME:@g0
+// CHECK-SAME:@a0
+
+// RETAIN:  [[#EMPTY]] = !{}
+
+const int r0 __attribute__((used)) = 42;
 int g0 __attribute__((used));
 
 static void __attribute__((used)) f0(void) {
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -1048,6 +1048,9 @@
   template
   void MaybeHandleStaticInExternC(const SomeDecl *D, llvm::GlobalValue *GV);
 
+  /// Set !retain metadata if the target supports.
+  void maybeSetRetain(llvm::GlobalObject *GO);
+
   /// Add a global to a list to be added to the llvm.used metadata.
   void addUsedGlobal(llvm::GlobalValue *GV);
 
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1883,6 +1883,8 @@
 
   if (D) {
 if (auto *GV = dyn_cast(GO)) {
+  if (D->hasAttr())
+maybeSetRetain(GV);
   if (auto *SA = D->getAttr())
 GV->addAttribute("bss-section", SA->getName());
   if (auto *SA = D->getAttr())
@@ -1894,6 +1896,8 @@
 }
 
 if (auto *F = dyn_cast(GO)) {
+  if (D->hasAttr())
+maybeSetRetain(F);
   if (auto *SA = D->getAttr())
 if (!D->getAttr())
   F->addFnAttr("implicit-section-name", SA->getName());
@@ -2062,6 +2066,13 @@
   }
 }
 
+void CodeGenModule::maybeSetRetain(llvm::GlobalObject *GO) {
+  const llvm::Triple  = getTarget().getTriple();
+  if (T.isOSFreeBSD() || T.isOSLinux())
+GO->setMetadata(llvm::LLVMContext::MD_retain,
+llvm::MDNode::get(getLLVMContext(), None));
+}
+
 

[PATCH] D96835: [HIP] Support device sanitizer

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added a reviewer: tra.
Herald added subscribers: jansvoboda11, dang, kerbowa, tpr, nhaehnle, jvesely.
yaxunl requested review of this revision.

Add option -fgpu-sanitize to enable sanitizer for AMDGPU target.

Since it is experimental, it is off by default.


https://reviews.llvm.org/D96835

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/lib/Driver/ToolChains/HIP.cpp
  clang/lib/Driver/ToolChains/HIP.h
  clang/lib/Driver/ToolChains/ROCm.h
  clang/test/Driver/Inputs/rocm-invalid/amdgcn/bitcode/hip.bc
  clang/test/Driver/Inputs/rocm-invalid/amdgcn/bitcode/ockl.bc
  
clang/test/Driver/Inputs/rocm-invalid/amdgcn/bitcode/oclc_correctly_rounded_sqrt_off.bc
  
clang/test/Driver/Inputs/rocm-invalid/amdgcn/bitcode/oclc_correctly_rounded_sqrt_on.bc
  clang/test/Driver/Inputs/rocm-invalid/amdgcn/bitcode/oclc_daz_opt_off.bc
  clang/test/Driver/Inputs/rocm-invalid/amdgcn/bitcode/oclc_daz_opt_on.bc
  clang/test/Driver/Inputs/rocm-invalid/amdgcn/bitcode/oclc_finite_only_off.bc
  clang/test/Driver/Inputs/rocm-invalid/amdgcn/bitcode/oclc_finite_only_on.bc
  clang/test/Driver/Inputs/rocm-invalid/amdgcn/bitcode/oclc_isa_version_1010.bc
  clang/test/Driver/Inputs/rocm-invalid/amdgcn/bitcode/oclc_isa_version_1011.bc
  clang/test/Driver/Inputs/rocm-invalid/amdgcn/bitcode/oclc_isa_version_1012.bc
  clang/test/Driver/Inputs/rocm-invalid/amdgcn/bitcode/oclc_isa_version_803.bc
  clang/test/Driver/Inputs/rocm-invalid/amdgcn/bitcode/oclc_isa_version_900.bc
  clang/test/Driver/Inputs/rocm-invalid/amdgcn/bitcode/oclc_isa_version_908.bc
  clang/test/Driver/Inputs/rocm-invalid/amdgcn/bitcode/oclc_unsafe_math_off.bc
  clang/test/Driver/Inputs/rocm-invalid/amdgcn/bitcode/oclc_unsafe_math_on.bc
  
clang/test/Driver/Inputs/rocm-invalid/amdgcn/bitcode/oclc_wavefrontsize64_off.bc
  
clang/test/Driver/Inputs/rocm-invalid/amdgcn/bitcode/oclc_wavefrontsize64_on.bc
  clang/test/Driver/Inputs/rocm-invalid/amdgcn/bitcode/ocml.bc
  clang/test/Driver/Inputs/rocm-invalid/amdgcn/bitcode/opencl.bc
  clang/test/Driver/Inputs/rocm-invalid/bin/.hipVersion
  clang/test/Driver/Inputs/rocm-invalid/include/hip/hip_runtime.h
  clang/test/Driver/Inputs/rocm/amdgcn/bitcode/asanrtl.bc
  clang/test/Driver/hip-sanitize-options.hip

Index: clang/test/Driver/hip-sanitize-options.hip
===
--- clang/test/Driver/hip-sanitize-options.hip
+++ clang/test/Driver/hip-sanitize-options.hip
@@ -1,9 +1,40 @@
 // REQUIRES: clang-driver, x86-registered-target, amdgpu-registered-target
 
-// RUN: %clang -### -target x86_64-unknown-linux-gnu --offload-arch=gfx906 \
+// RUN: %clang -### -target x86_64-unknown-linux-gnu --offload-arch=gfx900 \
 // RUN:   -fsanitize=address \
-// RUN:   -nogpuinc -nogpulib \
+// RUN:   -nogpuinc --rocm-path=%S/Inputs/rocm \
 // RUN:   %s 2>&1 | FileCheck %s
 
+// RUN: %clang -### -target x86_64-unknown-linux-gnu --offload-arch=gfx900 \
+// RUN:   -fsanitize=address -fno-gpu-sanitize \
+// RUN:   -nogpuinc --rocm-path=%S/Inputs/rocm \
+// RUN:   %s 2>&1 | FileCheck %s
+
+// RUN: %clang -### -target x86_64-unknown-linux-gnu --offload-arch=gfx900 \
+// RUN:   -fsanitize=address -fgpu-sanitize \
+// RUN:   -nogpuinc --rocm-path=%S/Inputs/rocm \
+// RUN:   %s 2>&1 | FileCheck -check-prefixes=NORDC %s
+
+// RUN: %clang -### -target x86_64-unknown-linux-gnu --offload-arch=gfx900 \
+// RUN:   -fsanitize=address -fgpu-sanitize -fgpu-rdc \
+// RUN:   -nogpuinc --rocm-path=%S/Inputs/rocm \
+// RUN:   %s 2>&1 | FileCheck -check-prefixes=RDC %s
+
+// RUN: %clang -### -target x86_64-unknown-linux-gnu --offload-arch=gfx900 \
+// RUN:   -fsanitize=address -fgpu-sanitize \
+// RUN:   -nogpuinc --rocm-path=%S/Inputs/rocm-invalid \
+// RUN:   %s 2>&1 | FileCheck -check-prefixes=FAIL %s
+
 // CHECK-NOT: {{"[^"]*clang[^"]*".* "-fcuda-is-device".* "-fsanitize=address"}}
+// CHECK-NOT: {{"[^"]*lld[^"]*".* ".*hip.bc"}}
 // CHECK: {{"[^"]*clang[^"]*".* "-triple" "x86_64-unknown-linux-gnu".* "-fsanitize=address"}}
+
+// NORDC: {{"[^"]*clang[^"]*".* "-fcuda-is-device".* "-fsanitize=address".*}} "-o" "[[OUT:[^"]*.bc]]"
+// NORDC: {{"[^"]*lld[^"]*".*}} "[[OUT]]" {{".*asanrtl.bc" ".*hip.bc"}}
+// NORDC: {{"[^"]*clang[^"]*".* "-triple" "x86_64-unknown-linux-gnu".* "-fsanitize=address"}}
+
+// RDC: {{"[^"]*clang[^"]*".* "-triple" "x86_64-unknown-linux-gnu".* "-fsanitize=address"}}
+// RDC: {{"[^"]*clang[^"]*".* "-emit-llvm-bc".* "-fcuda-is-device".* "-fsanitize=address".*}} "-o" "[[OUT:[^"]*.bc]]"
+// RDC: {{"[^"]*lld[^"]*".*}} "[[OUT]]" {{".*asanrtl.bc" ".*hip.bc"}}
+
+// FAIL: AMDGPU address sanitizer runtime library (asanrtl) is not found. Please install ROCm device library which supports address sanitizer
Index: clang/test/Driver/Inputs/rocm-invalid/bin/.hipVersion

[PATCH] D96456: [ThinLTO, NewPM] Register sanitizers with OptimizerLastPassBuilderHook

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added inline comments.



Comment at: clang/lib/CodeGen/BackendUtil.cpp:1070-1071
+// ThinLTOIndexFile is provideds so we must be in ThinLTO PostLink.
+// For -O0 ThinLTO PreLink does basic optimization and triggers
+// OptimizerLastEPCallbacks. PostLink will not
+// run optimizations and this callback should

tejohnson wrote:
> vitalybuka wrote:
> > aeubanks wrote:
> > > We could fix this. It doesn't make sense for only -O0 to run 
> > > OptimizerLastEPCallbacks prelink. Would that help simplify this?
> > Do you mean move optimizer of -O0 into PostLink as well?
> > Yes, this would be more consistent.
> > Still I guess it should a separate patch.
> > 
> Or does it make sense to go the other way, i.e. find a way to add the 
> sanitizer passes to the end of the pre-link of ThinLTO like for regular LTO? 
> The ThinLTO pre-link doesn't execute the module optimization pipeline, but 
> presumably at the end of buildThinLTOPreLinkDefaultPipeline we could invoke 
> the OptimizerLastEPCallbacks? That avoids the issue of trying to get this 
> working separately for in-process ThinLTO. And would be more consistent with 
> regular LTO.
I started from PreLink approach but @aeubanks advised the PostLink. At this 
point I slightly prefer it over PreLink. 

Sanitizers In PostLink pros:
+ instrumentation of optimized code produces smaller binary
+ optimizer called after instrumentation may removes some checks and we miss 
some bugs (It's the case for non-LTO for at least msan. It stops reporting some 
bugs. Maybe just a bug in msan.)
+ StackSafetyAnalysis supports ThinLTO and can be used to optimize sanitizers.
+ consistent with non-LTO pipeline
+ OptimizerLastEPCallbacks called once per module (I guess not true for full 
LTO).


Sanitizers In PreLink pros:
+ in-process ThinLTO. does not need special handling
+ Simpler patch
+ Can keep inconsistent -O0 pipeline (not sure why)
+ consistent with regular LTO (but why regular LTO is not consistent with 
ThinLTO here?)

WDYT if ThinLTO PreLink with sanitizer use buildO0DefaultPipeline?



Comment at: clang/lib/CodeGen/BackendUtil.cpp:1079
+  if (CodeGenOpts.PrepareForThinLTO &&
+  CodeGenOpts.ThinLTOIndexFile.empty()) {
+// We are here if we in ThinLTO PreLink.

tejohnson wrote:
> You should only have to check PrepareForThinLTO to see if we are in the 
> pre-link.
It needed for clang/test/CodeGen/cspgo-instrumentation_thinlto.c:26:53


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96456

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

[PATCH] D96456: [ThinLTO, NewPM] Register sanitizers with OptimizerLastPassBuilderHook

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka updated this revision to Diff 324173.
vitalybuka added a comment.

undo CodeGenOpts.ThinLTOIndexFile.empty()


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96456

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/thinlto-distributed-sanitizers.ll
  clang/test/Driver/asan.c
  clang/test/Driver/dfsan.c
  clang/test/Driver/msan.c
  clang/test/Driver/sancov.c
  clang/test/Driver/tsan.c
  llvm/include/llvm/LTO/Config.h
  llvm/lib/LTO/LTOBackend.cpp

Index: llvm/lib/LTO/LTOBackend.cpp
===
--- llvm/lib/LTO/LTOBackend.cpp
+++ llvm/lib/LTO/LTOBackend.cpp
@@ -258,6 +258,9 @@
   PB.registerLoopAnalyses(LAM);
   PB.crossRegisterProxies(LAM, FAM, CGAM, MAM);
 
+  if (Conf.OptimizerLastPassBuilderHook)
+Conf.OptimizerLastPassBuilderHook(PB);
+
   ModulePassManager MPM(Conf.DebugPassManager);
 
   if (!Conf.DisableVerify)
Index: llvm/include/llvm/LTO/Config.h
===
--- llvm/include/llvm/LTO/Config.h
+++ llvm/include/llvm/LTO/Config.h
@@ -48,6 +48,8 @@
   TargetOptions Options;
   std::vector MAttrs;
   std::vector PassPlugins;
+  /// For adding passes that run by optimizer (NewPM only).
+  std::function OptimizerLastPassBuilderHook;
   /// For adding passes that run right before codegen.
   std::function PreCodeGenPassesHook;
   Optional RelocModel = Reloc::PIC_;
Index: clang/test/Driver/tsan.c
===
--- clang/test/Driver/tsan.c
+++ clang/test/Driver/tsan.c
@@ -19,9 +19,10 @@
 // RUN: %clang -O3 -fexperimental-new-pass-manager -target x86_64-unknown-linux -fsanitize=thread %s -S -emit-llvm -o - | FileCheck %s
 // RUN: %clang -fexperimental-new-pass-manager -target x86_64-unknown-linux -fsanitize=thread %s -S -emit-llvm -o - | FileCheck %s
 // RUN: %clang -fexperimental-new-pass-manager -target x86_64-unknown-linux -fsanitize=thread %s -S -emit-llvm -flto=thin -o - | FileCheck %s
-// FIX: %clang -O2 -fexperimental-new-pass-manager -target x86_64-unknown-linux -fsanitize=thread %s -S -emit-llvm -flto=thin -o - | FileCheck %s
+// RUN: %clang -O2 -fexperimental-new-pass-manager -target x86_64-unknown-linux -fsanitize=thread %s -S -emit-llvm -flto=thin -o - | FileCheck %s --check-prefixes=NOTSAN
 // RUN: %clang -fexperimental-new-pass-manager -target x86_64-unknown-linux -fsanitize=thread %s -S -emit-llvm -flto -o - | FileCheck %s
 // RUN: %clang -O2 -fexperimental-new-pass-manager -target x86_64-unknown-linux -fsanitize=thread %s -S -emit-llvm -flto -o - | FileCheck %s
 
 int foo(int *a) { return *a; }
 // CHECK: __tsan_init
+// NOTSAN-NOT: __tsan
Index: clang/test/Driver/sancov.c
===
--- clang/test/Driver/sancov.c
+++ clang/test/Driver/sancov.c
@@ -3,7 +3,7 @@
 // RUN: %clang -O2 -fexperimental-new-pass-manager -target x86_64-unknown-linux -fsanitize-coverage=trace-pc-guard %s -S -emit-llvm -o - | FileCheck %s
 // RUN: %clang -O3 -fexperimental-new-pass-manager -target x86_64-unknown-linux -fsanitize-coverage=trace-pc-guard %s -S -emit-llvm -o - | FileCheck %s
 // RUN: %clang -fexperimental-new-pass-manager -target x86_64-unknown-linux -fsanitize-coverage=trace-pc-guard %s -S -emit-llvm -flto=thin -o - | FileCheck %s
-// FIX: %clang -O2 -fexperimental-new-pass-manager -target x86_64-unknown-linux -fsanitize-coverage=trace-pc-guard %s -S -emit-llvm -flto=thin -o - | FileCheck %s
+// RUN: %clang -O2 -fexperimental-new-pass-manager -target x86_64-unknown-linux -fsanitize-coverage=trace-pc-guard %s -S -emit-llvm -flto=thin -o - | FileCheck %s --check-prefixes=NOSANCOV
 // RUN: %clang -fexperimental-new-pass-manager -target x86_64-unknown-linux -fsanitize-coverage=trace-pc-guard %s -S -emit-llvm -flto -o - | FileCheck %s
 // RUN: %clang -O2 -fexperimental-new-pass-manager -target x86_64-unknown-linux -fsanitize-coverage=trace-pc-guard %s -S -emit-llvm -flto -o - | FileCheck %s
 
@@ -20,3 +20,4 @@
 
 int foo(int *a) { return *a; }
 // CHECK: _sancov_
+// NOSANCOV-NOT: _sancov
Index: clang/test/Driver/msan.c
===
--- clang/test/Driver/msan.c
+++ clang/test/Driver/msan.c
@@ -1,51 +1,55 @@
 // REQUIRES: x86-registered-target
 
-// RUN: %clang -fno-experimental-new-pass-manager -target x86_64-unknown-linux -fsanitize=memory %s -S -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,O0
-// RUN: %clang -O1 -fno-experimental-new-pass-manager -target x86_64-unknown-linux -fsanitize=memory %s -S -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,O1
-// RUN: %clang -O2 -fno-experimental-new-pass-manager -target x86_64-unknown-linux -fsanitize=memory %s -S -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,O1
-// RUN: %clang -O3 -fno-experimental-new-pass-manager -target 

[PATCH] D96456: [ThinLTO, NewPM] Register sanitizers with OptimizerLastPassBuilderHook

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka updated this revision to Diff 324171.
vitalybuka marked 4 inline comments as done.
vitalybuka added a comment.

fix language


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96456

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/thinlto-distributed-sanitizers.ll
  clang/test/Driver/asan.c
  clang/test/Driver/dfsan.c
  clang/test/Driver/msan.c
  clang/test/Driver/sancov.c
  clang/test/Driver/tsan.c
  llvm/include/llvm/LTO/Config.h
  llvm/lib/LTO/LTOBackend.cpp

Index: llvm/lib/LTO/LTOBackend.cpp
===
--- llvm/lib/LTO/LTOBackend.cpp
+++ llvm/lib/LTO/LTOBackend.cpp
@@ -258,6 +258,9 @@
   PB.registerLoopAnalyses(LAM);
   PB.crossRegisterProxies(LAM, FAM, CGAM, MAM);
 
+  if (Conf.OptimizerLastPassBuilderHook)
+Conf.OptimizerLastPassBuilderHook(PB);
+
   ModulePassManager MPM(Conf.DebugPassManager);
 
   if (!Conf.DisableVerify)
Index: llvm/include/llvm/LTO/Config.h
===
--- llvm/include/llvm/LTO/Config.h
+++ llvm/include/llvm/LTO/Config.h
@@ -48,6 +48,8 @@
   TargetOptions Options;
   std::vector MAttrs;
   std::vector PassPlugins;
+  /// For adding passes that run by optimizer (NewPM only).
+  std::function OptimizerLastPassBuilderHook;
   /// For adding passes that run right before codegen.
   std::function PreCodeGenPassesHook;
   Optional RelocModel = Reloc::PIC_;
Index: clang/test/Driver/tsan.c
===
--- clang/test/Driver/tsan.c
+++ clang/test/Driver/tsan.c
@@ -19,9 +19,10 @@
 // RUN: %clang -O3 -fexperimental-new-pass-manager -target x86_64-unknown-linux -fsanitize=thread %s -S -emit-llvm -o - | FileCheck %s
 // RUN: %clang -fexperimental-new-pass-manager -target x86_64-unknown-linux -fsanitize=thread %s -S -emit-llvm -o - | FileCheck %s
 // RUN: %clang -fexperimental-new-pass-manager -target x86_64-unknown-linux -fsanitize=thread %s -S -emit-llvm -flto=thin -o - | FileCheck %s
-// FIX: %clang -O2 -fexperimental-new-pass-manager -target x86_64-unknown-linux -fsanitize=thread %s -S -emit-llvm -flto=thin -o - | FileCheck %s
+// RUN: %clang -O2 -fexperimental-new-pass-manager -target x86_64-unknown-linux -fsanitize=thread %s -S -emit-llvm -flto=thin -o - | FileCheck %s --check-prefixes=NOTSAN
 // RUN: %clang -fexperimental-new-pass-manager -target x86_64-unknown-linux -fsanitize=thread %s -S -emit-llvm -flto -o - | FileCheck %s
 // RUN: %clang -O2 -fexperimental-new-pass-manager -target x86_64-unknown-linux -fsanitize=thread %s -S -emit-llvm -flto -o - | FileCheck %s
 
 int foo(int *a) { return *a; }
 // CHECK: __tsan_init
+// NOTSAN-NOT: __tsan
Index: clang/test/Driver/sancov.c
===
--- clang/test/Driver/sancov.c
+++ clang/test/Driver/sancov.c
@@ -3,7 +3,7 @@
 // RUN: %clang -O2 -fexperimental-new-pass-manager -target x86_64-unknown-linux -fsanitize-coverage=trace-pc-guard %s -S -emit-llvm -o - | FileCheck %s
 // RUN: %clang -O3 -fexperimental-new-pass-manager -target x86_64-unknown-linux -fsanitize-coverage=trace-pc-guard %s -S -emit-llvm -o - | FileCheck %s
 // RUN: %clang -fexperimental-new-pass-manager -target x86_64-unknown-linux -fsanitize-coverage=trace-pc-guard %s -S -emit-llvm -flto=thin -o - | FileCheck %s
-// FIX: %clang -O2 -fexperimental-new-pass-manager -target x86_64-unknown-linux -fsanitize-coverage=trace-pc-guard %s -S -emit-llvm -flto=thin -o - | FileCheck %s
+// RUN: %clang -O2 -fexperimental-new-pass-manager -target x86_64-unknown-linux -fsanitize-coverage=trace-pc-guard %s -S -emit-llvm -flto=thin -o - | FileCheck %s --check-prefixes=NOSANCOV
 // RUN: %clang -fexperimental-new-pass-manager -target x86_64-unknown-linux -fsanitize-coverage=trace-pc-guard %s -S -emit-llvm -flto -o - | FileCheck %s
 // RUN: %clang -O2 -fexperimental-new-pass-manager -target x86_64-unknown-linux -fsanitize-coverage=trace-pc-guard %s -S -emit-llvm -flto -o - | FileCheck %s
 
@@ -20,3 +20,4 @@
 
 int foo(int *a) { return *a; }
 // CHECK: _sancov_
+// NOSANCOV-NOT: _sancov
Index: clang/test/Driver/msan.c
===
--- clang/test/Driver/msan.c
+++ clang/test/Driver/msan.c
@@ -1,51 +1,55 @@
 // REQUIRES: x86-registered-target
 
-// RUN: %clang -fno-experimental-new-pass-manager -target x86_64-unknown-linux -fsanitize=memory %s -S -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,O0
-// RUN: %clang -O1 -fno-experimental-new-pass-manager -target x86_64-unknown-linux -fsanitize=memory %s -S -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,O1
-// RUN: %clang -O2 -fno-experimental-new-pass-manager -target x86_64-unknown-linux -fsanitize=memory %s -S -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,O1
-// RUN: %clang -O3 -fno-experimental-new-pass-manager -target 

[PATCH] D96832: [Clang][Attributes] Allow not_tail_called attribute to be applied to virtual function.

[Zequan Wu via Phabricator via cfe-commits]

zequanwu created this revision.
zequanwu added reviewers: ahatanak, aaron.ballman, rnk.
zequanwu requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

It would be beneficial to allow not_tail_called attribute to be applied to
virtual functions. I don't see any drawback of allowing this.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96832

Files:
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CodeGenCXX/attr-notail.cpp
  clang/test/SemaCXX/attr-notail.cpp

Index: clang/test/SemaCXX/attr-notail.cpp
===
--- clang/test/SemaCXX/attr-notail.cpp
+++ clang/test/SemaCXX/attr-notail.cpp
@@ -1,8 +1,9 @@
 // RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+// expected-no-diagnostics
 
 class Base {
 public:
-  [[clang::not_tail_called]] virtual int foo1(); // expected-error {{'not_tail_called' attribute cannot be applied to virtual functions}}
+  [[clang::not_tail_called]] virtual int foo1();
   virtual int foo2();
   [[clang::not_tail_called]] int foo3();
   virtual ~Base() {}
@@ -11,6 +12,6 @@
 class Derived1 : public Base {
 public:
   int foo1() override;
-  [[clang::not_tail_called]] int foo2() override; // expected-error {{'not_tail_called' attribute cannot be applied to virtual functions}}
+  [[clang::not_tail_called]] int foo2() override;
   [[clang::not_tail_called]] int foo4();
 };
Index: clang/test/CodeGenCXX/attr-notail.cpp
===
--- clang/test/CodeGenCXX/attr-notail.cpp
+++ clang/test/CodeGenCXX/attr-notail.cpp
@@ -4,14 +4,17 @@
 public:
   [[clang::not_tail_called]] int m1();
   int m2();
+  [[clang::not_tail_called]] virtual int m3();
 };
 
 int foo1(int a, Class1 *c1) {
   if (a)
 return c1->m1();
+  c1->m3();
   return c1->m2();
 }
 
 // CHECK-LABEL: define{{.*}} i32 @_Z4foo1iP6Class1(
 // CHECK: %{{[a-z0-9]+}} = notail call i32 @_ZN6Class12m1Ev(%class.Class1*
+// CHECK: %{{[a-z0-9]+}} = notail call i32 %{{[0-9]+}}(%class.Class1* 
 // CHECK: %{{[a-z0-9]+}} = call i32 @_ZN6Class12m2Ev(%class.Class1*
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -6432,16 +6432,6 @@
 }
   }
 
-  // Virtual functions cannot be marked as 'notail'.
-  if (auto *Attr = ND.getAttr())
-if (auto *MD = dyn_cast())
-  if (MD->isVirtual()) {
-S.Diag(ND.getLocation(),
-   diag::err_invalid_attribute_on_virtual_function)
-<< Attr;
-ND.dropAttr();
-  }
-
   // Check the attributes on the function type, if any.
   if (const auto *FD = dyn_cast()) {
 // Don't declare this variable in the second operand of the for-statement;
Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -4057,9 +4057,8 @@
   let Category = DocCatFunction;
   let Content = [{
 The ``not_tail_called`` attribute prevents tail-call optimization on statically
-bound calls. It has no effect on indirect calls. Virtual functions, objective-c
-methods, and functions marked as ``always_inline`` cannot be marked as
-``not_tail_called``.
+bound calls. Objective-c methods, and functions marked as ``always_inline`` 
+cannot be marked as ``not_tail_called``.
 
 For example, it prevents tail-call optimization in the following case:
 
@@ -4070,43 +4069,6 @@
 int foo2(int a) {
   return foo1(a); // No tail-call optimization on direct calls.
 }
-
-However, it doesn't prevent tail-call optimization in this case:
-
-  .. code-block:: c
-
-int __attribute__((not_tail_called)) foo1(int);
-
-int foo2(int a) {
-  int (*fn)(int) = 
-
-  // not_tail_called has no effect on an indirect call even if the call can
-  // be resolved at compile time.
-  return (*fn)(a);
-}
-
-Marking virtual functions as ``not_tail_called`` is an error:
-
-  .. code-block:: c++
-
-class Base {
-public:
-  // not_tail_called on a virtual function is an error.
-  [[clang::not_tail_called]] virtual int foo1();
-
-  virtual int foo2();
-
-  // Non-virtual functions can be marked ``not_tail_called``.
-  [[clang::not_tail_called]] int foo3();
-};
-
-class Derived1 : public Base {
-public:
-  int foo1() override;
-
-  // not_tail_called on a virtual function is an error.
-  [[clang::not_tail_called]] int foo2() override;
-};
   }];
 }
 
___
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.

[Jim Lin via Phabricator via cfe-commits]

Jim 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"

craig.topper wrote:
> 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?
Most of targets pass the full name with the prefix to the BUILTIN macro.
It can define the full name with the prefix in BuiltinsRISCV.def


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] D96825: [AArch64] Adding Polynomial vadd Intrinsics support

[Ryan Santhirarajan via Phabricator via cfe-commits]

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

rebased due to merge issues


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

https://reviews.llvm.org/D96825

Files:
  clang/include/clang/Basic/arm_neon.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/aarch64-poly-add.c

Index: clang/test/CodeGen/aarch64-poly-add.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-poly-add.c
@@ -0,0 +1,85 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// REQUIRES: aarch64-registered-target
+// RUN: %clang_cc1 -triple arm64-none-linux-gnu -target-feature +neon \
+// RUN: -disable-O0-optnone -ffp-contract=fast -emit-llvm -o - %s | opt -S -mem2reg \
+// RUN:  | FileCheck %s
+
+#include 
+
+// CHECK-LABEL: @test_vadd_p8(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = xor <8 x i8> [[A:%.*]], [[B:%.*]]
+// CHECK-NEXT:ret <8 x i8> [[TMP0]]
+//
+poly8x8_t test_vadd_p8(poly8x8_t a, poly8x8_t b) {
+  return vadd_p8 (a, b);
+}
+
+// CHECK-LABEL: @test_vadd_p16(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = bitcast <4 x i16> [[A:%.*]] to <8 x i8>
+// CHECK-NEXT:[[TMP1:%.*]] = bitcast <4 x i16> [[B:%.*]] to <8 x i8>
+// CHECK-NEXT:[[TMP2:%.*]] = xor <8 x i8> [[TMP0]], [[TMP1]]
+// CHECK-NEXT:[[TMP3:%.*]] = bitcast <8 x i8> [[TMP2]] to <4 x i16>
+// CHECK-NEXT:ret <4 x i16> [[TMP3]]
+//
+poly16x4_t test_vadd_p16(poly16x4_t a, poly16x4_t b) {
+  return vadd_p16 (a, b);
+}
+
+// CHECK-LABEL: @test_vadd_p64(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = bitcast <1 x i64> [[A:%.*]] to <8 x i8>
+// CHECK-NEXT:[[TMP1:%.*]] = bitcast <1 x i64> [[B:%.*]] to <8 x i8>
+// CHECK-NEXT:[[TMP2:%.*]] = xor <8 x i8> [[TMP0]], [[TMP1]]
+// CHECK-NEXT:[[TMP3:%.*]] = bitcast <8 x i8> [[TMP2]] to <1 x i64>
+// CHECK-NEXT:ret <1 x i64> [[TMP3]]
+//
+poly64x1_t test_vadd_p64(poly64x1_t a, poly64x1_t b) {
+  return vadd_p64(a, b);
+}
+
+// CHECK-LABEL: @test_vaddq_p8(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = xor <16 x i8> [[A:%.*]], [[B:%.*]]
+// CHECK-NEXT:ret <16 x i8> [[TMP0]]
+//
+poly8x16_t test_vaddq_p8(poly8x16_t a, poly8x16_t b){
+  return vaddq_p8(a, b);
+}
+
+// CHECK-LABEL: @test_vaddq_p16(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = bitcast <8 x i16> [[A:%.*]] to <16 x i8>
+// CHECK-NEXT:[[TMP1:%.*]] = bitcast <8 x i16> [[B:%.*]] to <16 x i8>
+// CHECK-NEXT:[[TMP2:%.*]] = xor <16 x i8> [[TMP0]], [[TMP1]]
+// CHECK-NEXT:[[TMP3:%.*]] = bitcast <16 x i8> [[TMP2]] to <8 x i16>
+// CHECK-NEXT:ret <8 x i16> [[TMP3]]
+//
+poly16x8_t test_vaddq_p16(poly16x8_t a, poly16x8_t b){
+  return vaddq_p16(a, b);
+}
+
+// CHECK-LABEL: @test_vaddq_p64(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = bitcast <2 x i64> [[A:%.*]] to <16 x i8>
+// CHECK-NEXT:[[TMP1:%.*]] = bitcast <2 x i64> [[B:%.*]] to <16 x i8>
+// CHECK-NEXT:[[TMP2:%.*]] = xor <16 x i8> [[TMP0]], [[TMP1]]
+// CHECK-NEXT:[[TMP3:%.*]] = bitcast <16 x i8> [[TMP2]] to <2 x i64>
+// CHECK-NEXT:ret <2 x i64> [[TMP3]]
+//
+poly64x2_t test_vaddq_p64(poly64x2_t a, poly64x2_t b){
+  return vaddq_p64(a, b);
+}
+
+// CHECK-LABEL: @test_vaddq_p128(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = bitcast i128 [[A:%.*]] to <16 x i8>
+// CHECK-NEXT:[[TMP1:%.*]] = bitcast i128 [[B:%.*]] to <16 x i8>
+// CHECK-NEXT:[[TMP2:%.*]] = xor <16 x i8> [[TMP0]], [[TMP1]]
+// CHECK-NEXT:[[TMP3:%.*]] = bitcast <16 x i8> [[TMP2]] to i128
+// CHECK-NEXT:ret i128 [[TMP3]]
+//
+poly128_t test_vaddq_p128 (poly128_t a, poly128_t b){
+  return vaddq_p128(a, b);
+}
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -5368,7 +5368,10 @@
   NEONMAP2(vabdq_v, arm_neon_vabdu, arm_neon_vabds, Add1ArgType | UnsignedAlts),
   NEONMAP1(vabs_v, arm_neon_vabs, 0),
   NEONMAP1(vabsq_v, arm_neon_vabs, 0),
+  NEONMAP0(vadd_v),
   NEONMAP0(vaddhn_v),
+  NEONMAP0(vaddq_p128),
+  NEONMAP0(vaddq_v),
   NEONMAP1(vaesdq_v, arm_neon_aesd, 0),
   NEONMAP1(vaeseq_v, arm_neon_aese, 0),
   NEONMAP1(vaesimcq_v, arm_neon_aesimc, 0),
@@ -6299,6 +6302,14 @@
 if (VTy->getElementType()->isFloatingPointTy())
   return EmitNeonCall(CGM.getIntrinsic(Intrinsic::fabs, Ty), Ops, "vabs");
 return EmitNeonCall(CGM.getIntrinsic(LLVMIntrinsic, Ty), Ops, "vabs");
+  case NEON::BI__builtin_neon_vadd_v:
+  case NEON::BI__builtin_neon_vaddq_v: {
+llvm::Type *VTy = llvm::FixedVectorType::get(Int8Ty, Quad ? 16 : 8);
+Ops[0] = Builder.CreateBitCast(Ops[0], VTy);
+Ops[1] = Builder.CreateBitCast(Ops[1], VTy);
+Ops[0] =  Builder.CreateXor(Ops[0], Ops[1]);
+return Builder.CreateBitCast(Ops[0], Ty);
+  }
   case NEON::BI__builtin_neon_vaddhn_v: {
 llvm::FixedVectorType *SrcTy =
 

[PATCH] D94355: [SimplifyCFG] Add relative switch lookup tables

[Gulfem Savrun Yeniceri via Phabricator via cfe-commits]

gulfem added a comment.

@lebedev.ri based on your feedback, I added it as a separate pass and added 
support for user-defined lookup tables.
Please let me know if you have any comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94355

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

[PATCH] D94355: [SimplifyCFG] Add relative switch lookup tables

[Gulfem Savrun Yeniceri via Phabricator via cfe-commits]

gulfem updated this revision to Diff 324144.
gulfem marked an inline comment as not done.
gulfem added a comment.
Herald added a subscriber: mgorny.

Implement it as a separate pass and apply it to user-defined lookup tables as 
well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94355

Files:
  clang/test/CodeGen/switch-to-lookup-table.c
  llvm/docs/Passes.rst
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/Transforms/Scalar.h
  llvm/include/llvm/Transforms/Utils/RelLookupTableGenerator.h
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Passes/PassRegistry.def
  llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
  llvm/lib/Transforms/Utils/CMakeLists.txt
  llvm/lib/Transforms/Utils/RelLookupTableGenerator.cpp
  llvm/lib/Transforms/Utils/Utils.cpp
  llvm/test/Other/pass-pipelines.ll
  llvm/test/Transforms/SimplifyCFG/X86/relative_lookup_table.ll
  llvm/utils/gn/secondary/llvm/lib/Transforms/Utils/BUILD.gn

Index: llvm/utils/gn/secondary/llvm/lib/Transforms/Utils/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/lib/Transforms/Utils/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/lib/Transforms/Utils/BUILD.gn
@@ -60,6 +60,7 @@
 "NameAnonGlobals.cpp",
 "PredicateInfo.cpp",
 "PromoteMemoryToRegister.cpp",
+"RelLookupTableGenerator.cpp"
 "SSAUpdater.cpp",
 "SSAUpdaterBulk.cpp",
 "SampleProfileLoaderBaseUtil.cpp",
Index: llvm/test/Transforms/SimplifyCFG/X86/relative_lookup_table.ll
===
--- /dev/null
+++ llvm/test/Transforms/SimplifyCFG/X86/relative_lookup_table.ll
@@ -0,0 +1,187 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -rel-lookup-table-generator -S -mtriple=x86_64-unknown-linux-gnu | FileCheck %s
+; RUN: opt < %s -passes=rel-lookup-table-generator -S -mtriple=x86_64-unknown-linux-gnu | FileCheck %s
+target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@.str = private unnamed_addr constant [5 x i8] c"zero\00", align 1
+@.str.1 = private unnamed_addr constant [4 x i8] c"one\00", align 1
+@.str.2 = private unnamed_addr constant [4 x i8] c"two\00", align 1
+@.str.3 = private unnamed_addr constant [8 x i8] c"default\00", align 1
+@.str.4 = private unnamed_addr constant [6 x i8] c"three\00", align 1
+@.str.5 = private unnamed_addr constant [5 x i8] c"str1\00", align 1
+@.str.6 = private unnamed_addr constant [5 x i8] c"str2\00", align 1
+@.str.7 = private unnamed_addr constant [12 x i8] c"singlevalue\00", align 1
+
+@switch.table.string_table = private unnamed_addr constant [3 x i8*]
+ [
+  i8* getelementptr inbounds ([5 x i8], [5 x i8]* @.str, i64 0, i64 0),
+  i8* getelementptr inbounds ([4 x i8], [4 x i8]* @.str.1, i64 0, i64 0),
+  i8* getelementptr inbounds ([4 x i8], [4 x i8]* @.str.2, i64 0, i64 0)
+ ], align 8
+
+@switch.table.string_table_holes = private unnamed_addr constant [4 x i8*]
+  [
+i8* getelementptr inbounds ([5 x i8], [5 x i8]* @.str, i64 0, i64 0),
+i8* getelementptr inbounds ([8 x i8], [8 x i8]* @.str.3, i64 0, i64 0),
+i8* getelementptr inbounds ([4 x i8], [4 x i8]* @.str.2, i64 0, i64 0),
+i8* getelementptr inbounds ([6 x i8], [6 x i8]* @.str.4, i64 0, i64 0)
+  ], align 8
+
+@switch.table.no_dso_local = private unnamed_addr constant [3 x i32*] [i32* @a, i32* @b, i32* @c], align 8
+
+@switch.table.single_value = private unnamed_addr constant [3 x i8*]
+  [
+i8* getelementptr inbounds ([5 x i8], [5 x i8]* @.str, i64 0, i64 0),
+i8* getelementptr inbounds ([4 x i8], [4 x i8]* @.str.1, i64 0, i64 0),
+i8* getelementptr inbounds ([4 x i8], [4 x i8]* @.str.2, i64 0, i64 0)
+  ], align 8
+
+; Integer pointer table lookup
+; CHECK: @switch.table.no_dso_local = private unnamed_addr constant [3 x i32*] [i32* @a, i32* @b, i32* @c], align
+
+; Relative string table lookup
+; CHECK: @reltable.string_table = private unnamed_addr constant [3 x i32]
+; CHECK-SAME: [
+; CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint ([5 x i8]* @.str to i64), i64 ptrtoint ([3 x i32]* @reltable.string_table to i64)) to i32),
+; CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint ([4 x i8]* @.str.1 to i64), i64 ptrtoint ([3 x i32]* @reltable.string_table to i64)) to i32),
+; CHECK-SAME: i32 trunc (i64 

[PATCH] D96751: [clangd] Populate detail field in document symbols

[WangWei via Phabricator via cfe-commits]

lightmelodies added a comment.

Thanks for your response. Yes decl is too verbose in many cases.  In fact I 
have tried another concise version, but I can not decide which one is better. 
Unit test in FindSymbolsTests.cpp is also done, but I would like upload them 
after we reach an agreement on what behavior is prefered.
F15557452: image.png 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96751

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

[PATCH] D96751: [clangd] Populate detail field in document symbols

[WangWei via Phabricator via cfe-commits]

lightmelodies updated this revision to Diff 324143.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96751

Files:
  clang-tools-extra/clangd/FindSymbols.cpp
  clang-tools-extra/clangd/test/symbols.test

Index: clang-tools-extra/clangd/test/symbols.test
===
--- clang-tools-extra/clangd/test/symbols.test
+++ clang-tools-extra/clangd/test/symbols.test
@@ -1,5 +1,5 @@
 # RUN: clangd --index-file=%S/Inputs/symbols.test.yaml -lit-test < %s | FileCheck %s
-{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"workspace":{"symbol":{"symbolKind":{"valueSet": [1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26],"trace":"off"}}
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument":{"documentSymbol":{"hierarchicalDocumentSymbolSupport":true}},"workspace":{"symbol":{"symbolKind":{"valueSet": [1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26],"trace":"off"}}
 ---
 {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"void foo(); int main() { foo(); }\n"}}}
 ---
@@ -34,40 +34,54 @@
 # CHECK-NEXT:  "jsonrpc": "2.0",
 # CHECK-NEXT:"result": [
 # CHECK-NEXT:  {
-# CHECK-NEXT:"containerName": "",
+# CHECK-NEXT:"detail": "void ()",
 # CHECK-NEXT:"kind": 12,
-# CHECK-NEXT:"location": {
-# CHECK-NEXT:  "range": {
-# CHECK-NEXT:"end": {
-# CHECK-NEXT:  "character": {{.*}},
-# CHECK-NEXT:  "line": {{.*}}
-# CHECK-NEXT:},
-# CHECK-NEXT:"start": {
-# CHECK-NEXT:  "character": {{.*}},
-# CHECK-NEXT:  "line": {{.*}}
-# CHECK-NEXT:}
+# CHECK-NEXT:"name": "foo",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": {{.*}},
+# CHECK-NEXT:"line": {{.*}}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file://{{.*}}/main.cpp"
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": {{.*}},
+# CHECK-NEXT:"line": {{.*}}
+# CHECK-NEXT:  }
 # CHECK-NEXT:},
-# CHECK-NEXT:"name": "foo"
-# CHECK-NEXT:  }
+# CHECK-NEXT:"selectionRange": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": {{.*}},
+# CHECK-NEXT:"line": {{.*}}
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": {{.*}},
+# CHECK-NEXT:"line": {{.*}}
+# CHECK-NEXT:  }
+# CHECK-NEXT:}
+# CHECK-NEXT:  },
 # CHECK-NEXT:  {
-# CHECK-NEXT:"containerName": "",
+# CHECK-NEXT:"detail": "int ()",
 # CHECK-NEXT:"kind": 12,
-# CHECK-NEXT:"location": {
-# CHECK-NEXT:  "range": {
-# CHECK-NEXT:"end": {
-# CHECK-NEXT:  "character": {{.*}},
-# CHECK-NEXT:  "line": {{.*}}
-# CHECK-NEXT:},
-# CHECK-NEXT:"start": {
-# CHECK-NEXT:  "character": {{.*}},
-# CHECK-NEXT:  "line": {{.*}}
-# CHECK-NEXT:}
+# CHECK-NEXT:"name": "main",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": {{.*}},
+# CHECK-NEXT:"line": {{.*}}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file://{{.*}}/main.cpp"
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": {{.*}},
+# CHECK-NEXT:"line": {{.*}}
+# CHECK-NEXT:  }
 # CHECK-NEXT:},
-# CHECK-NEXT:"name": "main"
+# CHECK-NEXT:"selectionRange": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": {{.*}},
+# CHECK-NEXT:"line": {{.*}}
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": {{.*}},
+# CHECK-NEXT:"line": {{.*}}
+# CHECK-NEXT:  }
+# CHECK-NEXT:}
 # CHECK-NEXT:  }
 # CHECK-NEXT:]
 # CHECK-NEXT:}
Index: clang-tools-extra/clangd/FindSymbols.cpp
===
--- clang-tools-extra/clangd/FindSymbols.cpp
+++ clang-tools-extra/clangd/FindSymbols.cpp
@@ -172,6 +172,31 @@
 }
 
 namespace {
+
+std::string getSymbolDetail(ASTContext , const NamedDecl ) {
+  PrintingPolicy P(Ctx.getPrintingPolicy());
+  P.SuppressScope = true;
+  P.SuppressUnwrittenScope = true;
+  P.AnonymousTagLocations = false;
+  P.PolishForDeclaration = true;
+  std::string Detail;
+  llvm::raw_string_ostream OS(Detail);
+  if (const auto *TP = ND.getDescribedTemplateParams()) {
+TP->print(OS, Ctx, P);
+  }
+  if (const auto *VD = dyn_cast()) {
+

[PATCH] D96825: [AArch64] Adding Polynomial vadd Intrinsics support

[Ryan Santhirarajan via Phabricator via cfe-commits]

rsanthir.quic added a comment.

This is the final 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/D96825/new/

https://reviews.llvm.org/D96825

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

[PATCH] D96825: [AArch64] Adding Polynomial vadd Intrinsics support

[Ryan Santhirarajan via Phabricator via cfe-commits]

rsanthir.quic created this revision.
rsanthir.quic added reviewers: t.p.northover, kevin.qin, labrinea, pbarrio, 
apazos.
Herald added subscribers: danielkiss, kristof.beyls.
rsanthir.quic requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch adds the following intrinsics:

  vadd_p8
  vadd_p16
  vadd_p64
  vaddq_p8
  vaddq_p16
  vaddq_p64
  vaddq_p128


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96825

Files:
  clang/include/clang/Basic/arm_neon.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/aarch64-poly-add.c

Index: clang/test/CodeGen/aarch64-poly-add.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-poly-add.c
@@ -0,0 +1,85 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// REQUIRES: aarch64-registered-target
+// RUN: %clang_cc1 -triple arm64-none-linux-gnu -target-feature +neon \
+// RUN: -disable-O0-optnone -ffp-contract=fast -emit-llvm -o - %s | opt -S -mem2reg \
+// RUN:  | FileCheck %s
+
+#include 
+
+// CHECK-LABEL: @test_vadd_p8(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = xor <8 x i8> [[A:%.*]], [[B:%.*]]
+// CHECK-NEXT:ret <8 x i8> [[TMP0]]
+//
+poly8x8_t test_vadd_p8(poly8x8_t a, poly8x8_t b) {
+  return vadd_p8 (a, b);
+}
+
+// CHECK-LABEL: @test_vadd_p16(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = bitcast <4 x i16> [[A:%.*]] to <8 x i8>
+// CHECK-NEXT:[[TMP1:%.*]] = bitcast <4 x i16> [[B:%.*]] to <8 x i8>
+// CHECK-NEXT:[[TMP2:%.*]] = xor <8 x i8> [[TMP0]], [[TMP1]]
+// CHECK-NEXT:[[TMP3:%.*]] = bitcast <8 x i8> [[TMP2]] to <4 x i16>
+// CHECK-NEXT:ret <4 x i16> [[TMP3]]
+//
+poly16x4_t test_vadd_p16(poly16x4_t a, poly16x4_t b) {
+  return vadd_p16 (a, b);
+}
+
+// CHECK-LABEL: @test_vadd_p64(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = bitcast <1 x i64> [[A:%.*]] to <8 x i8>
+// CHECK-NEXT:[[TMP1:%.*]] = bitcast <1 x i64> [[B:%.*]] to <8 x i8>
+// CHECK-NEXT:[[TMP2:%.*]] = xor <8 x i8> [[TMP0]], [[TMP1]]
+// CHECK-NEXT:[[TMP3:%.*]] = bitcast <8 x i8> [[TMP2]] to <1 x i64>
+// CHECK-NEXT:ret <1 x i64> [[TMP3]]
+//
+poly64x1_t test_vadd_p64(poly64x1_t a, poly64x1_t b) {
+  return vadd_p64(a, b);
+}
+
+// CHECK-LABEL: @test_vaddq_p8(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = xor <16 x i8> [[A:%.*]], [[B:%.*]]
+// CHECK-NEXT:ret <16 x i8> [[TMP0]]
+//
+poly8x16_t test_vaddq_p8(poly8x16_t a, poly8x16_t b){
+  return vaddq_p8(a, b);
+}
+
+// CHECK-LABEL: @test_vaddq_p16(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = bitcast <8 x i16> [[A:%.*]] to <16 x i8>
+// CHECK-NEXT:[[TMP1:%.*]] = bitcast <8 x i16> [[B:%.*]] to <16 x i8>
+// CHECK-NEXT:[[TMP2:%.*]] = xor <16 x i8> [[TMP0]], [[TMP1]]
+// CHECK-NEXT:[[TMP3:%.*]] = bitcast <16 x i8> [[TMP2]] to <8 x i16>
+// CHECK-NEXT:ret <8 x i16> [[TMP3]]
+//
+poly16x8_t test_vaddq_p16(poly16x8_t a, poly16x8_t b){
+  return vaddq_p16(a, b);
+}
+
+// CHECK-LABEL: @test_vaddq_p64(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = bitcast <2 x i64> [[A:%.*]] to <16 x i8>
+// CHECK-NEXT:[[TMP1:%.*]] = bitcast <2 x i64> [[B:%.*]] to <16 x i8>
+// CHECK-NEXT:[[TMP2:%.*]] = xor <16 x i8> [[TMP0]], [[TMP1]]
+// CHECK-NEXT:[[TMP3:%.*]] = bitcast <16 x i8> [[TMP2]] to <2 x i64>
+// CHECK-NEXT:ret <2 x i64> [[TMP3]]
+//
+poly64x2_t test_vaddq_p64(poly64x2_t a, poly64x2_t b){
+  return vaddq_p64(a, b);
+}
+
+// CHECK-LABEL: @test_vaddq_p128(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = bitcast i128 [[A:%.*]] to <16 x i8>
+// CHECK-NEXT:[[TMP1:%.*]] = bitcast i128 [[B:%.*]] to <16 x i8>
+// CHECK-NEXT:[[TMP2:%.*]] = xor <16 x i8> [[TMP0]], [[TMP1]]
+// CHECK-NEXT:[[TMP3:%.*]] = bitcast <16 x i8> [[TMP2]] to i128
+// CHECK-NEXT:ret i128 [[TMP3]]
+//
+poly128_t test_vaddq_p128 (poly128_t a, poly128_t b){
+  return vaddq_p128(a, b);
+}
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -5662,7 +5662,10 @@
   NEONMAP0(splatq_laneq_v),
   NEONMAP1(vabs_v, aarch64_neon_abs, 0),
   NEONMAP1(vabsq_v, aarch64_neon_abs, 0),
+  NEONMAP0(vadd_v),
   NEONMAP0(vaddhn_v),
+  NEONMAP0(vaddq_p128),
+  NEONMAP0(vaddq_v),
   NEONMAP1(vaesdq_v, aarch64_crypto_aesd, 0),
   NEONMAP1(vaeseq_v, aarch64_crypto_aese, 0),
   NEONMAP1(vaesimcq_v, aarch64_crypto_aesimc, 0),
@@ -6290,6 +6293,14 @@
 if (VTy->getElementType()->isFloatingPointTy())
   return EmitNeonCall(CGM.getIntrinsic(Intrinsic::fabs, Ty), Ops, "vabs");
 return EmitNeonCall(CGM.getIntrinsic(LLVMIntrinsic, Ty), Ops, "vabs");
+  case NEON::BI__builtin_neon_vadd_v:
+  case NEON::BI__builtin_neon_vaddq_v: {
+llvm::Type *VTy = llvm::FixedVectorType::get(Int8Ty, Quad ? 16 : 8);
+Ops[0] = Builder.CreateBitCast(Ops[0], 

[PATCH] D96568: [CFE, SystemZ] Emit s390.tdc instrincic for __builtin_isnan in Constrained FP mode.

[Jonas Paulsson via Phabricator via cfe-commits]

jonpa added a comment.

In D96568#2559476 , @thopre wrote:

> In D96568#2559475 , @uweigand wrote:
>
>> In D96568#2559336 , @thopre wrote:
>>
>>> That's interesting. I presume that can be used to implement isinf as well? 
>>> Perhaps better call the hook fpclassify or similar?
>>
>> Hmm, the instruction doesn't really implement fpclassify in itself, it is 
>> more like a combined check for fpclassify() == .   
>> Specifically, the TEST DATA CLASS instruction takes an immediate operand 
>> that represents a bit mask, which each bit standing for one type of 
>> floating-point value (zero, normal, subnormal, infinity, QNaN, SNaN -- each 
>> in positive and negative versions).  The instruction sets the condition code 
>> depending on whether the input FP number is in one of the classes selected 
>> by the bit mask, or not.
>>
>> This is why Jonas' patch uses a bit mask of 0x0F -- this has the bits for 
>> the four types of NaN set (pos/neg QNaN/SNan).   The instruction could 
>> indeed also be used to implement an isinf check (bit mask 0x30) or many 
>> other checks.   We actually have a SystemZ back-end pass that tries to 
>> multiple combine FP checks into a single TEST DATA CLASS instruction.
>>
>> However, the instruction does not directly implement the fpclassify 
>> semantics.  To implement fpclassify, you'd still have to use multiple 
>> invocations of the instruction with different flags to determine the 
>> fpclassify output value.
>
> I see. I'm not sure whether it's better to have several target hooks or a 
> single one like testFPKind that would take a flag saying what do we want to 
> test (NaN, Inf, etc.).

Personally I think testFPKind should work well - it gives a single target hook 
for related purposes which should be more readable, and it is also natural for 
SystemZ since we will be using the same (Test Data Class) instruction for 
differnet checks only with different flag bits... Any one else has an opinion? 
Should I go ahead and change the patch to this end?


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

https://reviews.llvm.org/D96568

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

[PATCH] D96082: [clang-tidy] Add 'readability-useless-return-value' check

[Lukas Hänel via Phabricator via cfe-commits]

LukasHanel added a comment.

In D96082#2566984 , @steveire wrote:

> In D96082#2565339 , @aaron.ballman 
> wrote:
>
>> A somewhat similar check that would be interesting is a function that 
>> returns the same value on all control paths
>
> I think we shouldn't try to design a new, different check in the comments of 
> this MR.
>
> I think it would be better to finalize what to do with this one.
>
> Request further work to also change expressions in all affected TUs? Or close?

Hi,
I finally managed to run this checker on my own code base and I run into all 
the above problems:

1. There are some sloppy areas in the code that this checker nicely highlights.
2. Many false-positives for example with callbacks or involving preprocessor
3. Some questionable suggestions, like where it is breaking code-symmetry with 
a set of handlers that have the same signatures but some always return 0.

In conclusion for this checker:

- it's a good way to spot sloppy areas in the code.
- You couldn't run it in -Werror mode to enforce clean code.
- Yes, I could filter more FP/noise, e.g. functions that have a single return 
statement in the body; plus maybe assert.
- However, if you have a policy to use the return value of all functions, this 
checker can be a good way to clean the code first.

I am ok to close this one, I will park it somewhere.

In the meantime, I try to spin up some fixes for the false positives that I see 
with other checkers, as mentioned above.

As a follow-up to this checker, clang -Wdocumentation does not understand the 
`@retval` command.
I was thinking of adding a new clang-tidy checker for this to verify/complete 
the list of documented return values.
In this case where the documentation does not say `@retval 0 always`, than I 
will come back to the checker here, suggest make the function void or add the 
"always" to the text :).
The work with the useless-return-value was a study towards this new @retval 
checker.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96082

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

[PATCH] D93003: [libunwind] unw_* alias fixes for ELF and Mach-O

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

In D93003#2566906 , @rprichard wrote:

> Maybe this is blocked on someone from Apple reviewing the Mach-O parts?

This is fine with me for how Apple builds libunwind. I am not sure if the open 
source libunwind build for MachO do expect different behavior but I don't think 
there are other reasonable behaviors.

I can give my LGTM if @ldionne has no problem with that.


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] D96082: [clang-tidy] Add 'readability-useless-return-value' check

[Lukas Hänel via Phabricator via cfe-commits]

LukasHanel updated this revision to Diff 324132.
LukasHanel added a comment.

Handle the case of a global variable being the "return value"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96082

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/clang-tidy/readability/UselessReturnValueCheck.cpp
  clang-tools-extra/clang-tidy/readability/UselessReturnValueCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-useless-return-value.rst
  clang-tools-extra/test/clang-tidy/checkers/readability-useless-return-value.c
  
clang-tools-extra/test/clang-tidy/checkers/readability-useless-return-value.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability-useless-return-value.h

Index: clang-tools-extra/test/clang-tidy/checkers/readability-useless-return-value.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-useless-return-value.h
@@ -0,0 +1,3 @@
+int f11(void);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f11' returns always the same value [readability-useless-return-value]
+// CHECK-FIXES: {{^}}void f11(void);{{$}}
Index: clang-tools-extra/test/clang-tidy/checkers/readability-useless-return-value.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-useless-return-value.cpp
@@ -0,0 +1,32 @@
+// RUN: %check_clang_tidy %s readability-useless-return-value %t
+
+int f() {
+  return 0;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:5: warning: function 'f' returns always the same value [readability-useless-return-value]
+// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: returns 0 here [readability-useless-return-value]
+
+// CHECK-FIXES: {{^}}void f() {{{$}}
+// CHECK-FIXES: {{^}}  return;{{$}}
+
+struct Foo {
+  Foo(int i);
+  Foo =(int &);
+};
+Foo tie(int i) {
+  return Foo(i);
+}
+
+int demangleUnsigned() {
+  int Number = 0;
+  tie(Number) = 1;
+  return Number;
+}
+
+class Foo1 {
+  virtual unsigned g(void) const;
+};
+
+unsigned Foo1::g(void) const {
+  return 0;
+}
Index: clang-tools-extra/test/clang-tidy/checkers/readability-useless-return-value.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-useless-return-value.c
@@ -0,0 +1,252 @@
+// RUN: %check_clang_tidy %s readability-useless-return-value %t
+
+int f() {
+  return 0;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:5: warning: function 'f' returns always the same value [readability-useless-return-value]
+// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: returns 0 here [readability-useless-return-value]
+
+// CHECK-FIXES: {{^}}void f() {{{$}}
+// CHECK-FIXES: {{^}}  return;{{$}}
+
+unsigned int f2() {
+  return 0U;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:14: warning: function 'f2' returns always the same value [readability-useless-return-value]
+// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: returns 0 here [readability-useless-return-value]
+
+// CHECK-FIXES: {{^}}void f2() {{{$}}
+// CHECK-FIXES: {{^}}  return;{{$}}
+
+typedef unsigned int mytype_t;
+mytype_t f3() {
+  return 0U;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:10: warning: function 'f3' returns always the same value [readability-useless-return-value]
+// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: returns 0 here [readability-useless-return-value]
+
+// CHECK-FIXES: {{^}}void f3() {{{$}}
+// CHECK-FIXES: {{^}}  return;{{$}}
+
+const int f4() {
+  return 0;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:11: warning: function 'f4' returns always the same value [readability-useless-return-value]
+// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: returns 0 here [readability-useless-return-value]
+
+// CHECK-FIXES: {{^}}const void f4() {{{$}}
+// CHECK-FIXES: {{^}}  return;{{$}}
+
+static int f5() {
+  return 0;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:12: warning: function 'f5' returns always the same value [readability-useless-return-value]
+// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: returns 0 here [readability-useless-return-value]
+
+// CHECK-FIXES: {{^}}static void f5() {{{$}}
+// CHECK-FIXES: {{^}}  return;{{$}}
+
+#define EXIT_SUCCESS 0
+
+int f6() {
+  return EXIT_SUCCESS; //EXIT_SUCCESS
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:5: warning: function 'f6' returns always the same value [readability-useless-return-value]
+// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: returns 0 here [readability-useless-return-value]
+
+// CHECK-FIXES: {{^}}void f6() {{{$}}
+// CHECK-FIXES: {{^}}  return; //EXIT_SUCCESS{{$}}
+
+#define NULL 0
+
+int f7() {
+  return NULL; //NULL
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:5: warning: function 'f7' returns always the same value [readability-useless-return-value]
+// 

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

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

jdenny added inline comments.



Comment at: clang/lib/CodeGen/CodeGenFunction.h:292
+  /// nest would extend.
+  SmallVector OMPLoopNestStack;
+

Unless I missed something, the only accesses to `OMPLoopNestStack` are 
`push_back`, `clear`, `back`, and `size`.  It's never popped.  That is, every 
push appears to mean that all existing elements will never be accessed again.  
So why is it a stack?  Is that purely for the sake of the assertion that calls 
`size`?



Comment at: clang/lib/Sema/SemaOpenMP.cpp:5355
+  Expr *Cond, *Inc;
+  VarDecl *CounterDecl, *LVDecl;
+  if (auto *For = dyn_cast(AStmt)) {

`CounterDecl` is the declaration of the "loop iteration variable" based on the 
comments now on `OMPCanonicalLoop`, right?  If so, can we update the variable 
names here?  One possibility is `LIVDecl` and `LUVDecl`.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:5139
+/// Used to capture variable references if already parsed 
statements/expressions
+/// into a CapturedStatement.
+class CaptureVars : public TreeTransform {

Meinersbur wrote:
> jdenny wrote:
> > I think this means:
> > 
> > > If already parsed statements/expressions, used to capture variable 
> > > references into a CapturedStatement.
> > 
> > But it reads like it means:
> > 
> > > If already parsed statements/expressions into a CapturedStatement, used 
> > > to capture variable references.
> if -> of
> 
> I reformulated the sentence which I hope is now more clear.
That's better.  Thanks.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:5215
+  Expr *NegIncAmount =
+  AssertSuccess(Actions.BuildUnaryOp(nullptr, {}, UO_Minus, NewStep));
+  Expr *BackwardDist = AssertSuccess(

Meinersbur wrote:
> Meinersbur wrote:
> > jdenny wrote:
> > > It looks like the AST nodes `NewStart`, `NewStop`, and `NewStep` are 
> > > shared by multiple parent nodes here.  Does that ever happen anywhere 
> > > else in a Clang AST?
> > Yes. For instance, when instantiating a template, nodes form the 
> > TemplatedDecl are reused unless they need to change. Therefore multiple 
> > template instantiation can also share entire AST subtrees. Why rebuild them 
> > if they are identical?
> > 
> > However, I also found the following description of 
> > TreeTransform::AlwaysRebuild():
> > ```
> >   /// We must always rebuild all AST nodes when performing variadic template
> >   /// pack expansion, in order to avoid violating the AST invariant that 
> > each
> >   /// statement node appears at most once in its containing declaration.
> > ```
> > It was added by @rsmith in rG2aa81a718e64fb4ca651ea12ab7afaeffc11e463 to 
> > fix llvm.org/PR17800 . The assertion doesn't seem to exist in the current 
> > code base.
> > 
> > Does this apply here? 
> I think the invariant exists because codegen has a map from ast expression 
> nodes to its temporary memory location. This means that reusing the same AST 
> expression node indeed should not be done.
> 
> I think the assertion that failed in llvm.org/PR17800 is now here:
> https://github.com/llvm/llvm-project/blob/7ba2e1c6011eeb1b91ce3f5d8fa7187b7518e77a/clang/lib/AST/ExprConstant.cpp#L1872
> 
> Assertion message was changed in this commit:
> https://github.com/llvm/llvm-project/commit/f7f2e4261a98b2da519d58e7f6794b013cda7a4b
Thanks for the links.

I guess reuse is ok across different instantiations of a template because this 
map is cleared across different functions.

It might not be worth much at this point, but I also saw this comment as more 
evidence for this invariant: [[ 
https://clang.llvm.org/doxygen/classclang_1_1ASTContext.html#ad6ff7b541a77cea65d8f72ed3f903fb3
 | "expression's are never shared"]].



Comment at: clang/lib/Sema/TreeTransform.h:8321
+TreeTransform::TransformOMPCanonicalLoop(OMPCanonicalLoop *L) {
+  // The OMPCanonicalLoop will be recreated when transforming the 
loop-associted
+  // directive.

I'm used to seeing `TransformX` call `RebuildX` call `ActOnX`.  Why not do that 
for `X=OMPCanonicalLoop`?  Does `TransformOMPExecutableDirective` really need a 
special case for `OMPCanonicalLoop`?



Comment at: clang/tools/libclang/CIndex.cpp:2839
+  VisitStmt(L);
+  EnqueueChildren(L);
+}

Doesn't `VisitStmt(L)` already call `EnqueueChildren(L)`?


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] D96816: [ObjC] Encode pointers to C++ classes as "^v" if the encoded string would otherwise include template specialization types

[John McCall via Phabricator via cfe-commits]

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

You might want to test nested classes of class templates.  I don't know if we 
encode nested classes in any reasonable way in the first place.

I'm generally okay with being aggressive about stripping this kind of 
information from encodings, since encodings are not good enough to do type 
reflection in ObjC++ anyway.  If you did decide you wanted to preserve more 
information from the encoding, though, you could consider only applying this at 
the innermost level that mentions a template specialization type, rather than 
the outermost.  That is, the new logic seems to not expand the `X` in `X*` if 
`X` recursively has a field that's a pointer to a template specialization, 
rather than expanding `X` and then not expanding the type of that one field.  
Definitely not something I think you need to do now, though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96816

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

[PATCH] D96142: [clang-tidy] Simplify too-small loop variable check

[Stephen Kelly via Phabricator via cfe-commits]

steveire abandoned this revision.
steveire added a comment.

In D96142#2564265 , @njames93 wrote:

> In D96142#2547418 , @steveire wrote:
>
>> In D96142#2545078 , @njames93 wrote:
>>
>>> I'm not sure about this. The warning is good and addresses a real problem.
>>
>> Well, I've made the diagnostic better anyway.
>
> I'm not sure its an improvement. If the template is never instantiated with 
> something that would trigger this, we shouldn't really warn on it.
> There are plenty of times when certain template definitions will error if 
> certain template parameters are passed to them.
> This is well defined and no compiler will ever warn on those situations.

Yes, fair point. I still don't think the existing diagnostic is valuable, but 
removing the diagnostic would be the inevitable result of a change like this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96142

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

[PATCH] D96082: [clang-tidy] Add 'readability-useless-return-value' check

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

In D96082#2565339 , @aaron.ballman 
wrote:

> A somewhat similar check that would be interesting is a function that returns 
> the same value on all control paths

I think we shouldn't try to design a new, different check in the comments of 
this MR.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96082

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

[PATCH] D88220: [C++20] P1825R0: More implicit moves

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

In D88220#2566628 , @Quuxplusone wrote:

> My concern was more that I'm much less of an expert on Clang than "most of 
> us."

Understood, but Clang tries to closely follow the C++ standard in its 
implementation, and I'd say that the APIs used in this change do exactly what 
one would naively think they do. And we haven't introduced any new functions so 
we can't even discuss where to put them.

I'm sometimes reading up about implicit moving but the area seemed a bit in 
flux so I was hoping for things to settle down before I start to remember what 
might soon be obsolete. Let's hope this settles it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88220

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

[PATCH] D96751: [clangd] Populate detail field in document symbols

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Thanks for doing this!

I think this behavior is a great starting point.

I suspect printing the decl is more verbose than desired in a lot of cases, at 
least avoiding repeating the identifier would be nice. e.g.
x | int x --> x | int
foo | void foo() --> foo | void()
bar | struct bar {} --> bar | struct
This can be tweaked later, the simple implementation seems like a big 
improvement already.
If you feel like it though, I think this is mostly just printing the type 
instead of the decl for ValueDecls.

When you upload snapshots, please prepare the diffs with full context `-U` 
to make review easier. (the Arcanist `arc` tool will do this automatically)




Comment at: clang-tools-extra/clangd/FindSymbols.cpp:201
+  P.SuppressUnwrittenScope = true;
+  llvm::raw_string_ostream OS(SI.detail);
+  if (const auto *TP = ND.getDescribedTemplateParams()) {

nit: limit the scope of raw_string_ostream by putting it in an anonymous block.
It does not flush until destroyed (or explicitly flushed)



Comment at: clang-tools-extra/clangd/FindSymbols.cpp:201
+  P.SuppressUnwrittenScope = true;
+  llvm::raw_string_ostream OS(SI.detail);
+  if (const auto *TP = ND.getDescribedTemplateParams()) {

sammccall wrote:
> nit: limit the scope of raw_string_ostream by putting it in an anonymous 
> block.
> It does not flush until destroyed (or explicitly flushed)
Can we pull out a helper function for this? getSymbolDetail or so?

For now it's just a few lines but I imagine it could grow as we refine the 
behavior.



Comment at: clang-tools-extra/clangd/Hover.h:20
 
+PrintingPolicy getPrintingPolicy(PrintingPolicy Base);
 /// Contains detailed information about a Symbol. Especially useful when

this isn't a reasonable thing to expose & depend on from FindSymbols.cpp.

It's fine to add a copy into FindSymbols.cpp, especially since you want a 
slightly different policy anyway



Comment at: clang-tools-extra/clangd/test/symbols.test:1
 # RUN: clangd --index-file=%S/Inputs/symbols.test.yaml -lit-test < %s | 
FileCheck %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument":{"documentSymbol":{"hierarchicalDocumentSymbolSupport":true}},"workspace":{"symbol":{"symbolKind":{"valueSet":
 
[1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26],"trace":"off"}}

Can you add unit-tests for this feature in FindSymbolsTests.cpp?
Don't need to assert the detail for every symbol we test there, but adding a 
WithDetail matcher and adding it to a few nodes in the testcases would be 
useful. In particular, at least one that shows the getDescribedTemplateParams 
bit.

(It's nice to have a basic smoke-test here but unit-tests are where we try to 
cover more of the cases)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96751

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

[PATCH] D64146: [Clang Interpreter] Initial patch for the constexpr interpreter

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.
Herald added a subscriber: dang.

So I'm running into issues with this patch, specifically the line annotated 
below. I'm trying to compile our CHERI-LLVM fork as a native pure-capability 
CHERI[1] binary (which eventually will mean compiling the Morello-LLVM fork of 
our CHERI-LLVM to run as a native pure-capability CHERI binary on Arm's 
prototype Morello[2] board when it ships late this year/early next year), and I 
have LLVM (other than Orc JIT which is a whole beast and can't work without 
in-tree backend support) and LLD working (albeit only very lightly tested), and 
if I stub out that single function I am able to get Clang itself compiling and 
working (at least, well enough to compile a hello world C program all the way 
down to an executable), so this is the only technical blocker that I know which 
is rather frustrating.

CHERI capabilities can be used to implement C language pointers, providing 
strict fine-grained spatial safety and pointer provenance guarantees (i.e. it's 
not just that pointers have bounds, you also can't go and synthesise a pointer 
with the bounds you want if you don't already have one for that region), plus 
heap temporal safety on an experimental branch. This is done using tagged 
memory, where every capability-sized word in memory has a single tag bit 
indicating whether it has a valid capability or not. This means that, although 
you can memcpy capabilities around, they must remain aligned, otherwise their 
tags will become cleared and no longer be dereferenceable.

So, for us, that line _has_ to be able to be written as `auto Punned = 
*(uintptr_t *)Ptr;` (although it can be written in equivalent ways), as byte 
swapping uintptr_t is not something we can ever support (though with 
endianness::native you could at least make the templated version specialised to 
not call swapByteOrder, though the one taking a function argument can't be, but 
why bother going through all that indirection), nor is storing pointers to 
unaligned addresses if you want to be able to use them as pointers later.

I know nothing about this recently(ish)-added Clang constexpr interpreter, but 
it seems like this should be a solvable problem with some changes to the design 
to be less lax about alignment. Is there any documentation you could provide on 
how `CodePtr` is meant to work? "Pointer into the code segment." isn't exactly 
helpful, especially given it's actually storing pointers, which is not what you 
normally find in code segments when talking instead about compiled code. Do you 
have ideas for how this problem can be avoided?

[1] http://cheri-cpu.org/
[2] https://developer.arm.com/architectures/cpu-architecture/a-profile/morello




Comment at: cfe/trunk/lib/AST/Interp/Source.h:69
+using namespace llvm::support;
+auto Punned = endian::read(Ptr);
+return reinterpret_cast(Punned);

^^^ this line ^^^


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64146

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

[PATCH] D96612: [clangd] Improve printing of Objective-C categories and methods

[David Goldman via Phabricator via cfe-commits]

dgoldman marked 2 inline comments as done.
dgoldman added inline comments.



Comment at: clang-tools-extra/clangd/AST.cpp:228
+Out << (Method->isInstanceMethod() ? '-' : '+');
+Method->getSelector().print(Out);
+return Out.str();

sammccall wrote:
> in the other patch this was with `-[brackets]`
We don't want to put the full form (e.g. including the container name) here - 
that would repeat the class name in the document symbols when the methods are 
already nested under the class.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96612

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

[PATCH] D96612: [clangd] Improve printing of Objective-C categories and methods

[David Goldman via Phabricator via cfe-commits]

dgoldman marked 2 inline comments as done.
dgoldman added inline comments.



Comment at: clang-tools-extra/clangd/AST.cpp:224
 
+  if (const auto *C = dyn_cast())
+return printObjCContainer(*C);

sammccall wrote:
> I'm not sure this fits with the contract of printName, which really is to 
> print the name of the symbol rather than describe it.
> 
> Examples:
>  - the title of a hovercard for a method will be "instance-method `-[foo]`" 
> instead of "instance-method `foo`", which is inconsistent with c/c++
>  - the title of a hovercard for a category will be "extension 
> `PotentiallyLongClassName(CatName)`" instead of `extension ClassName`
>  - the documentSymbol response will include the extra details in the `name` 
> field rather than the `detail` field
> 
> We don't actually populate `detail` in documentSymbol, so maybe we do need a 
> separate function that gives that level of detail.
> Though I wonder whether it should mosty just print the decl (e.g. `class 
> X{};`, `@implementation PotentiallyLongClassName(CatName)` etc. WDYT?
> 
> See also https://github.com/clangd/clangd/issues/520 
> https://github.com/clangd/clangd/issues/601 though both appear to have 
> stalled.
Maybe instead this should be split somehow into a separate specialization for 
Document Symbols?

e.g.

Hovercards:
`instance-method foo`
`extension (anonymous)` (current) or `class-extension BTNButtonModel` or 
`BTNButtonModel()`

Document Symbols:
`-method`
`ClassName(Ext)`

Since I think for the case here we would prefer to keep them different. Maybe 
if the LSP spec differentiated between instance methods and class/static 
methods `-method` that wouldn't be necessary? I guess I could revert the Method 
change here and just keep a special case for categories/class extensions?

The `detail` work seems worth doing too, but I think we would similarly just 
put the type information in there (for methods), for categories/class 
extensions I feel like it would be odd since it would at the very least repeat 
the name



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96612

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

[PATCH] D96755: [clangd] Shutdown sequence for modules, and doc threading requirements

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:180
+for (auto  : *Modules)
+  Mod.blockUntilIdle(Deadline::infinity());
+  }

kadircet wrote:
> why is our contract saying that just calling `stop` is not enough?
> i think clangdserver should just signal shutdown to modules, and our contract 
> should say that server facilities will be undefined from this point forward.
> that way modules accessing the facilities, could block stop until they are 
> done, and never make use of it afterwards? it'll make modules a little more 
> complicated, at the very least they would need some stricter control whenever 
> they are accessing facilities, but I think it is worth for keeping 
> clangdserver shutdown cleaner. wdyt?
We need Module::blockUntilIdle anyway for tests and stuff. And in practice 
requesting shutdown of background work doesn't naturally block until that work 
is complete IME.

So we could ask every module to call blockUntilIdle() in stop(), or make stop() 
non-virtual and have it wrap requestStop()+blockUntilIdle().

Neither really seems simpler/cleaner to me overall, and it means modules shut 
down in serial instead of parallel.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96755

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

[PATCH] D96755: [clangd] Shutdown sequence for modules, and doc threading requirements

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 324120.
sammccall marked 2 inline comments as done.
sammccall added a comment.

trim tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96755

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Module.h
  clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp

Index: clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
===
--- clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -16,6 +16,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/JSON.h"
+#include "llvm/Testing/Support/Error.h"
 #include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -23,6 +24,8 @@
 namespace clang {
 namespace clangd {
 namespace {
+using llvm::HasValue;
+using llvm::Succeeded;
 
 MATCHER_P(DiagMessage, M, "") {
   if (const auto *O = arg.getAsObject()) {
@@ -39,6 +42,7 @@
 Base = ClangdServer::optsForTest();
 // This is needed to we can test index-based operations like call hierarchy.
 Base.BuildDynamicSymbolIndex = true;
+Base.Modules = 
   }
 
   LSPClient () {
@@ -66,6 +70,7 @@
 
   MockFS FS;
   ClangdLSPServer::Options Opts;
+  ModuleSet Modules;
 
 private:
   // Color logs so we can distinguish them from test output.
@@ -239,16 +244,112 @@
   [Reply(std::move(Reply)), Value(Value)]() mutable { Reply(Value); });
 }
   };
-  ModuleSet Mods;
-  Mods.add(std::make_unique());
-  Opts.Modules = 
+  Modules.add(std::make_unique());
 
   auto  = start();
   Client.notify("add", 2);
-  Mods.get()->add(8);
+  Client.notify("add", 8);
   EXPECT_EQ(10, Client.call("get", nullptr).takeValue());
 }
 
+// Creates a Callback that writes its received value into an Optional.
+template 
+llvm::unique_function)>
+capture(llvm::Optional> ) {
+  Out.reset();
+  return [](llvm::Expected V) { Out.emplace(std::move(V)); };
+}
+
+TEST_F(LSPTest, ModulesThreadingTest) {
+  // A module that does its work on a background thread, and so exercises the
+  // block/shutdown protocol.
+  class AsyncCounter final : public Module {
+bool ShouldStop = false;
+int State = 0;
+std::deque> Queue; // null = increment, non-null = read.
+std::condition_variable CV;
+std::mutex Mu;
+std::thread Thread;
+
+void run() {
+  std::unique_lock Lock(Mu);
+  while (true) {
+CV.wait(Lock, [&] { return ShouldStop || !Queue.empty(); });
+if (ShouldStop) {
+  Queue.clear();
+  CV.notify_all();
+  return;
+}
+Callback  = Queue.front();
+if (Task)
+  Task(State);
+else
+  ++State;
+Queue.pop_front();
+CV.notify_all();
+  }
+}
+
+bool blockUntilIdle(Deadline D) override {
+  std::unique_lock Lock(Mu);
+  return clangd::wait(Lock, CV, D, [this] { return Queue.empty(); });
+}
+
+void stop() override {
+  {
+std::lock_guard Lock(Mu);
+ShouldStop = true;
+  }
+  CV.notify_all();
+}
+
+  public:
+AsyncCounter() : Thread([this] { run(); }) {}
+~AsyncCounter() {
+  // Verify shutdown sequence was performed.
+  // Real modules would not do this, to be robust to no ClangdServer.
+  EXPECT_TRUE(ShouldStop) << "ClangdServer should request shutdown";
+  EXPECT_EQ(Queue.size(), 0u) << "ClangdServer should block until idle";
+  Thread.join();
+}
+
+void initializeLSP(LSPBinder , const llvm::json::Object ,
+   llvm::json::Object ) override {
+  Bind.notification("increment", this, ::increment);
+}
+
+// Get the current value, bypassing the queue.
+// Used to verify that sync->blockUntilIdle avoids races in tests.
+int getSync() {
+  std::lock_guard Lock(Mu);
+  return State;
+}
+
+// Increment the current value asynchronously.
+void increment(const std::nullptr_t &) {
+  {
+std::lock_guard Lock(Mu);
+Queue.push_back(nullptr);
+  }
+  CV.notify_all();
+}
+  };
+
+  Modules.add(std::make_unique());
+  auto  = start();
+
+  Client.notify("increment", nullptr);
+  Client.notify("increment", nullptr);
+  Client.notify("increment", nullptr);
+  EXPECT_THAT_EXPECTED(Client.call("sync", nullptr).take(), Succeeded());
+  EXPECT_EQ(3, Modules.get()->getSync());
+  // Throw some work on the queue to make sure shutdown blocks on it.
+  Client.notify("increment", nullptr);
+  Client.notify("increment", nullptr);
+  Client.notify("increment", nullptr);
+  // And immediately shut down. Module destructor verifies that we blocked.
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: 

[PATCH] D96816: [ObjC] Encode pointers to C++ classes as "^v" if the encoded string would otherwise include template specialization types

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

This patch focuses only on reducing the encoded size of classes with template 
specialization, but if the information about pointed-to types isn't used at 
all, clang can always emit "^v" for C++ pointer types. Or it might make more 
sense to base the decision on whether the type is POD rather than on whether 
the string includes template specialization.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96816

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

[PATCH] D93003: [libunwind] unw_* alias fixes for ELF and Mach-O

[Ryan Prichard via Phabricator via cfe-commits]

rprichard added a comment.

Maybe this is blocked on someone from Apple reviewing the Mach-O parts?


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] D88220: [C++20] P1825R0: More implicit moves

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfbee4a0c79cc: [C++20] [P1825] More implicit moves (authored 
by nullptr.cpp, committed by arthur.j.odwyer).

Changed prior to commit:
  https://reviews.llvm.org/D88220?vs=318420=324115#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88220

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
  clang/test/SemaCXX/P1155.cpp
  clang/test/SemaCXX/warn-return-std-move.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -79,7 +79,7 @@
   Clang 2.9

 https://wg21.link/p1825r0;>P1825R0 (DR)
-No
+Clang 13
   
 
 
@@ -1203,6 +1203,11 @@
   https://wg21.link/p0593r6;>P0593R6 (DR)
   Clang 11
 
+
+  More implicit moves
+  https://wg21.link/p1825r0;>P1825R0 (DR)
+  Clang 13
+
 
 
 
Index: clang/test/SemaCXX/warn-return-std-move.cpp
===
--- clang/test/SemaCXX/warn-return-std-move.cpp
+++ clang/test/SemaCXX/warn-return-std-move.cpp
@@ -1,12 +1,17 @@
-// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -Wreturn-std-move-in-c++11 -std=c++14 -verify %s
-// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -Wreturn-std-move-in-c++11 -std=c++14 -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++20 -verify=cxx20 %s
+// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++17 -verify=expected %s
+// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++14 -verify=expected %s
+// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++11 -verify=expected %s
+// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++17 -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefix=CHECK
+// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++14 -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefix=CHECK
+// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++11 -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefix=CHECK
 
 // definitions for std::move
 namespace std {
 inline namespace foo {
 template  struct remove_reference { typedef T type; };
-template  struct remove_reference { typedef T type; };
-template  struct remove_reference { typedef T type; };
+template  struct remove_reference { typedef T type; };
+template  struct remove_reference { typedef T type; };
 
 template  typename remove_reference::type &(T &);
 } // namespace foo
@@ -76,11 +81,8 @@
 // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:"std::move(d2)"
 }
 ConstructFromDerived test3() {
-Derived d3;
-return d3;  // e2-cxx11
-// expected-warning@-1{{would have been copied despite being returned by name}}
-// expected-note@-2{{to avoid copying on older compilers}}
-// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:"std::move(d3)"
+  Derived d3;
+  return d3; // ok
 }
 ConstructFromBase test4() {
 Derived d4;
@@ -153,10 +155,7 @@
 // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:13}:"std::move(d)"
 }
 ConstructFromDerived testParam3(Derived d) {
-return d;  // e7-cxx11
-// expected-warning@-1{{would have been copied despite being returned by name}}
-// expected-note@-2{{to avoid copying on older compilers}}
-// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:13}:"std::move(d)"
+  return d; // ok
 }
 ConstructFromBase testParam4(Derived d) {
 return d;  // e8
@@ -218,8 +217,10 @@
 // But if the return type is a reference type, then moving would be wrong.
 Derived& testRetRef1(Derived&& d) { return d; }
 Base& testRetRef2(Derived&& d) { return d; }
+#if __cplusplus >= 201402L
 auto&& testRetRef3(Derived&& d) { return d; }
 decltype(auto) testRetRef4(Derived&& d) { return (d); }
+#endif
 
 // As long as we're checking parentheses, make sure parentheses don't disable the warning.
 Base testParens1() {
@@ -230,14 +231,10 @@
 // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:15}:"std::move(d)"
 }
 ConstructFromDerived testParens2() {
-Derived d;
-return (d);  // e18-cxx11
-// expected-warning@-1{{would have been copied despite being returned by name}}
-// expected-note@-2{{to avoid copying}}
-// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:15}:"std::move(d)"
+  Derived d;
+  return (d); // ok
 }
 
-
 // If the target is a catch-handler 

[clang] fbee4a0 - [C++20] [P1825] More implicit moves

[Arthur O'Dwyer via cfe-commits]

Author: Yang Fan
Date: 2021-02-16T17:24:20-05:00
New Revision: fbee4a0c79cc4ee87c34e51342742a5bc6fcf872

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

LOG: [C++20] [P1825] More implicit moves

Implement all of P1825R0:

- implicitly movable entity can be an rvalue reference to non-volatile
automatic object.
- operand of throw-expression can be a function or catch-clause parameter
(support for function parameter has already been implemented).
- in the first overload resolution, the selected function no need to be
a constructor.
- in the first overload resolution, the first parameter of the selected
function no need to be an rvalue reference to the object's type.

This patch also removes the diagnostic `-Wreturn-std-move-in-c++11`.

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

Added: 
clang/test/SemaCXX/P1155.cpp

Modified: 
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaCoroutine.cpp
clang/lib/Sema/SemaStmt.cpp
clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
clang/test/SemaCXX/warn-return-std-move.cpp
clang/www/cxx_status.html

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index fcac6f44aa9a..21fc7d4cb82b 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -499,7 +499,6 @@ def Packed : DiagGroup<"packed">;
 def Padded : DiagGroup<"padded">;
 
 def PessimizingMove : DiagGroup<"pessimizing-move">;
-def ReturnStdMoveInCXX11 : DiagGroup<"return-std-move-in-c++11">;
 def ReturnStdMove : DiagGroup<"return-std-move">;
 
 def PointerArith : DiagGroup<"pointer-arith">;

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 782140f1d62e..f69ef3286975 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6370,13 +6370,6 @@ def warn_return_std_move : Warning<
   InGroup, DefaultIgnore;
 def note_add_std_move : Note<
   "call 'std::move' explicitly to avoid copying">;
-def warn_return_std_move_in_cxx11 : Warning<
-  "prior to the resolution of a defect report against ISO C++11, "
-  "local variable %0 would have been copied despite being returned by name, "
-  "due to its not matching the function return type%
diff { ($ vs $)|}1,2">,
-  InGroup, DefaultIgnore;
-def note_add_std_move_in_cxx11 : Note<
-  "call 'std::move' explicitly to avoid copying on older compilers">;
 
 def warn_string_plus_int : Warning<
   "adding %0 to a string does not append to the string">,

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 162c0b472bd3..b6fd6640a835 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -4724,10 +4724,12 @@ class Sema final {
 CES_AllowParameters = 1,
 CES_AllowDifferentTypes = 2,
 CES_AllowExceptionVariables = 4,
-CES_FormerDefault = (CES_AllowParameters),
-CES_Default = (CES_AllowParameters | CES_AllowDifferentTypes),
-CES_AsIfByStdMove = (CES_AllowParameters | CES_AllowDifferentTypes |
- CES_AllowExceptionVariables),
+CES_AllowRValueReferenceType = 8,
+CES_ImplicitlyMovableCXX11CXX14CXX17 =
+(CES_AllowParameters | CES_AllowDifferentTypes),
+CES_ImplicitlyMovableCXX20 =
+(CES_AllowParameters | CES_AllowDifferentTypes |
+ CES_AllowExceptionVariables | CES_AllowRValueReferenceType),
   };
 
   VarDecl *getCopyElisionCandidate(QualType ReturnType, Expr *E,

diff  --git a/clang/lib/Sema/SemaCoroutine.cpp 
b/clang/lib/Sema/SemaCoroutine.cpp
index 7a48bfa429e9..cd3b0db6b824 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -996,7 +996,8 @@ StmtResult Sema::BuildCoreturnStmt(SourceLocation Loc, Expr 
*E,
 
   // Move the return value if we can
   if (E) {
-auto NRVOCandidate = this->getCopyElisionCandidate(E->getType(), E, 
CES_AsIfByStdMove);
+const VarDecl *NRVOCandidate = this->getCopyElisionCandidate(
+E->getType(), E, CES_ImplicitlyMovableCXX20);
 if (NRVOCandidate) {
   InitializedEntity Entity =
   InitializedEntity::InitializeResult(Loc, E->getType(), 
NRVOCandidate);

diff  --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index b24a8ab110b2..e25d69931538 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -3083,12 +3083,20 @@ bool Sema::isCopyElisionCandidate(QualType ReturnType, 
const VarDecl *VD,
   // Return false if VD is a __block variable. We don't want to implicitly move
   // 

[PATCH] D95246: [test] Use host platform specific error message substitution in lit tests

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

In D95246#2566417 , 
@abhina.sreeskantharajan wrote:

> I was curious if all of these modified testcases are now failing or only the 
> three you mentioned.

Here is the list of the tests which fail after the patch (I haven't build LLD 
so don't have tests as well):

  Clang :: CodeGen/basic-block-sections.c
  Clang :: CodeGen/ubsan-blacklist-vfs.c
  Clang :: Frontend/output-paths.c
  Clang :: Frontend/stats-file.c  
  LLVM :: DebugInfo/symbolize-missing-file.test
  LLVM :: Object/archive-extract-dir.test
  LLVM :: Object/directory.ll
  LLVM :: tools/llvm-ar/error-opening-directory.test
  LLVM :: tools/llvm-ar/missing-thin-archive-member.test
  LLVM :: tools/llvm-ar/move.test
  LLVM :: tools/llvm-ar/print.test
  LLVM :: tools/llvm-ar/quick-append.test
  LLVM :: tools/llvm-ar/replace.test
  LLVM :: tools/llvm-ar/response.test
  LLVM :: tools/llvm-cxxdump/trivial.test
  LLVM :: tools/llvm-libtool-darwin/filelist.test
  LLVM :: tools/llvm-libtool-darwin/invalid-input-output-args.test
  LLVM :: tools/llvm-lipo/create-arch.test
  LLVM :: tools/llvm-lipo/replace-invalid-input.test
  LLVM :: tools/llvm-lto/error.ll
  LLVM :: tools/llvm-lto2/X86/stats-file-option.ll
  LLVM :: tools/llvm-mc/basic.test
  LLVM :: tools/llvm-mca/invalid_input_file_name.test
  LLVM :: tools/llvm-ml/basic.test
  LLVM :: tools/llvm-objcopy/COFF/add-section.test
  LLVM :: tools/llvm-objcopy/ELF/add-section.test
  LLVM :: tools/llvm-objcopy/ELF/error-format.test
  LLVM :: tools/llvm-objcopy/MachO/add-section-error.test
  LLVM :: tools/llvm-objcopy/redefine-symbols.test
  LLVM :: tools/llvm-objcopy/wasm/dump-section.test
  LLVM :: tools/llvm-profdata/weight-instr.test
  LLVM :: tools/llvm-profdata/weight-sample.test
  LLVM :: tools/llvm-readobj/ELF/thin-archive-paths.test
  LLVM :: tools/llvm-readobj/basic.test
  LLVM :: tools/llvm-readobj/thin-archive.test
  LLVM :: tools/llvm-size/no-input.test
  LLVM :: tools/llvm-xray/X86/no-such-file.txt
  LLVM :: tools/obj2yaml/invalid_input_file.test
  LLVM :: tools/yaml2obj/output-file.yaml

> I also don't know how this change can cause a pop-up to occur, is there more 
> information you can provide?

I got this. The popups is not the fault of this patch. They happened earlier. 
I'll investigate it later.

>   llvm/test/tools/llvm-elfabi/fail-file-write-windows.test
>
> I did not modify this but it requires system-windows and has a lower-case 
> error message. I expect if my change causes failures for you, then this 
> testcase should also fail.

Yes, this fails for me too. Didn't check it earlier.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95246

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

[PATCH] D96816: [ObjC] Encode pointers to C++ classes as "^v" if the encoded string would otherwise include template specialization types

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak created this revision.
ahatanak added reviewers: rjmccall, theraven.
ahatanak added a project: clang.
Herald added subscribers: jansvoboda11, dexonsmith, dang, jkorous.
ahatanak requested review of this revision.

This helps reduce the size of the encoded C++ type strings in the binary.

I'm assuming most of the code that uses the encoded type strings doesn't care 
about the pointed-to-type, but I've added a command line option to re-enable 
the existing behavior in case someone is relying on that information.

rdar://63288571


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96816

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/AST/ASTContext.cpp
  clang/test/CodeGenObjCXX/encode.mm

Index: clang/test/CodeGenObjCXX/encode.mm
===
--- clang/test/CodeGenObjCXX/encode.mm
+++ clang/test/CodeGenObjCXX/encode.mm
@@ -1,5 +1,6 @@
-// RUN: %clang_cc1 -Wno-objc-root-class -std=gnu++98 %s -triple=x86_64-apple-darwin10 -emit-llvm -o - | FileCheck --check-prefixes CHECK,CHECKCXX98 %s
-// RUN: %clang_cc1 -Wno-objc-root-class -std=gnu++20 %s -triple=x86_64-apple-darwin10 -emit-llvm -o - | FileCheck --check-prefixes CHECK,CHECKCXX20 %s
+// RUN: %clang_cc1 -Wno-objc-root-class -std=gnu++98 %s -triple=x86_64-apple-darwin10 -emit-llvm -o - | FileCheck --check-prefixes CHECK,CHECKCXX98,CHECK-NO-TEMP-SPEC %s
+// RUN: %clang_cc1 -Wno-objc-root-class -std=gnu++20 %s -triple=x86_64-apple-darwin10 -emit-llvm -o - | FileCheck --check-prefixes CHECK,CHECKCXX20,CHECK-NO-TEMP-SPEC %s
+// RUN: %clang_cc1 -Wno-objc-root-class -std=gnu++20 %s -triple=x86_64-apple-darwin10 -fencode-cxx-class-template-spec -emit-llvm -o - | FileCheck --check-prefixes CHECK,CHECKCXX20,CHECK-TEMP-SPEC %s
 
 // CHECK: v17@0:8{vector=}16
 // CHECK: {vector=}
@@ -260,3 +261,63 @@
   extern const char x[] = @encode(I);
 }
 #endif
+
+namespace test_cxx_template_specialization {
+template 
+struct B0 {
+  T a;
+};
+struct D0 : B0 {};
+struct D1 : D0 {};
+struct D2 : virtual B0 {};
+struct S0 {
+  B0 a;
+};
+struct S1 {
+  B0 *a;
+};
+struct S2 {
+  S1 *a;
+};
+template 
+union U0 {
+  T a;
+};
+typedef B0 TD0;
+typedef B0 *Array0[4];
+
+// CHECK: @[[STR22:.*]] = {{.*}} [12 x i8] c"{B0=i}\00"
+// CHECK: @_ZN32test_cxx_template_specialization2b0E = {{.*}} ([12 x i8], [12 x i8]* @[[STR22]], i32 0, i32 0)
+// CHECK-NO-TEMP-SPEC: @[[STR23:.*]] = {{.*}} [3 x i8] c"^v\00"
+// CHECK-NO-TEMP-SPEC: @_ZN32test_cxx_template_specialization3b01E = {{.*}} ([3 x i8], [3 x i8]* @[[STR23]], i32 0, i32 0)
+// CHECK-TEMP-SPEC: @[[STR23:.*]] = {{.*}} [13 x i8] c"^{B0=i}\00"
+// CHECK-TEMP-SPEC: @_ZN32test_cxx_template_specialization3b01E = {{.*}} ([13 x i8], [13 x i8]* @[[STR23]], i32 0, i32 0)
+// CHECK-NO-TEMP-SPEC: @_ZN32test_cxx_template_specialization3b02E = {{.*}} ([3 x i8], [3 x i8]* @[[STR23]], i32 0, i32 0)
+// CHECK-NO-TEMP-SPEC: @_ZN32test_cxx_template_specialization2d0E = {{.*}} ([3 x i8], [3 x i8]* @[[STR23]], i32 0, i32 0)
+// CHECK-NO-TEMP-SPEC: @_ZN32test_cxx_template_specialization2d1E = {{.*}} ([3 x i8], [3 x i8]* @[[STR23]], i32 0, i32 0)
+// CHECK-NO-TEMP-SPEC: @_ZN32test_cxx_template_specialization2d2E = {{.*}} ([3 x i8], [3 x i8]* @[[STR23]], i32 0, i32 0)
+// CHECK: @[[STR24:.*]] = {{.*}} [7 x i8] c"^^{D2}\00"
+// CHECK: @_ZN32test_cxx_template_specialization3d21E = {{.*}} ([7 x i8], [7 x i8]* @[[STR24]], i32 0, i32 0)
+// CHECK-NO-TEMP-SPEC: @_ZN32test_cxx_template_specialization2s0E = {{.*}} ([3 x i8], [3 x i8]* @[[STR23]], i32 0, i32 0)
+// CHECK-NO-TEMP-SPEC: @_ZN32test_cxx_template_specialization2s1E = {{.*}} ([3 x i8], [3 x i8]* @[[STR23]], i32 0, i32 0)
+// CHECK: @[[STR25:.*]] = {{.*}} [12 x i8] c"^{S2=^{S1}}\00"
+// CHECK: @_ZN32test_cxx_template_specialization2s2E = {{.*}} ([12 x i8], [12 x i8]* @[[STR25]], i32 0, i32 0)
+// CHECK-NO-TEMP-SPEC: @_ZN32test_cxx_template_specialization2u0E = {{.*}} ([3 x i8], [3 x i8]* @[[STR23]], i32 0, i32 0)
+// CHECK-NO-TEMP-SPEC: @_ZN32test_cxx_template_specialization3td0E = {{.*}} ([3 x i8], [3 x i8]* @[[STR23]], i32 0, i32 0)
+// CHECK-NO-TEMP-SPEC: @[[STR26:.*]] = {{.*}} [6 x i8] c"[4^v]\00"
+// CHECK-NO-TEMP-SPEC: @_ZN32test_cxx_template_specialization2a0E = {{.*}} ([6 x i8], [6 x i8]* @[[STR26]], i32 0, i32 0)
+
+const char *b0 = @encode(B0);
+const char *b01 = @encode(B0 *);
+const char *b02 = @encode(B0 &);
+const char *d0 = @encode(D0 *);
+const char *d1 = @encode(D1 *);
+const char *d2 = @encode(D2 *);
+const char *d21 = @encode(D2 **);
+const char *s0 = @encode(S0 *);
+const char *s1 = @encode(S1 *);
+const char *s2 = @encode(S2 *);
+const char *u0 = @encode(U0 *);
+const char *td0 = @encode(TD0 *);
+const char *a0 = @encode(Array0);
+}
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -7305,6 +7305,40 @@
   S += 

[PATCH] D94627: [PowerPC][PC Rel] Implement option to omit Power10 instructions from stubs

[Amy Kwan via Phabricator via cfe-commits]

amyk added a comment.

Thanks for answering the questions I previously had on this patch.




Comment at: lld/ELF/Driver.cpp:763
 
+// Parses --power10-stubs= flags, to disable or enable Power 10
+// instructions in stubs.

nit: remove the space after `=` and before `flags`.


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

https://reviews.llvm.org/D94627

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

[PATCH] D95458: [PowerPC] Exploit xxsplti32dx (constant materialization) for scalars

[Amy Kwan via Phabricator via cfe-commits]

amyk added a comment.

In addition to the nit comments, I also have the same question as Stefan for 
`getFPAs64BitIntHi`/`getFPAs64BitIntLo`.




Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:8591
+  // Only convert if it loses info, since XXSPLTIDP should
+  // handle the other case
+  return !ArgAPFloat.isDenormal() &&

nit: end with period.



Comment at: llvm/lib/Target/PowerPC/PPCInstrPrefix.td:2609
 
+// To replace constant pool with XXSPLI32DX for scalars.
+def : Pat<(f32 nzFPImmAsi64





Comment at: llvm/lib/Target/PowerPC/PPCInstrPrefix.td:2611
+def : Pat<(f32 nzFPImmAsi64
+   : $A),
+  (COPY_TO_REGCLASS(XXSPLTI32DX(XXSPLTI32DX(IMPLICIT_DEF), 0,

Have this `: $A` on the line above? Same as the one below.
Also a minor nit, but add a space to separate the `(`.


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

https://reviews.llvm.org/D95458

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

[PATCH] D96717: [clangd] Bind outgoing calls through LSPBinder too. NFC

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 324110.
sammccall added a comment.

fix nit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96717

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/LSPBinder.h
  clang-tools-extra/clangd/Module.h
  clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
  clang-tools-extra/clangd/unittests/LSPBinderTests.cpp

Index: clang-tools-extra/clangd/unittests/LSPBinderTests.cpp
===
--- clang-tools-extra/clangd/unittests/LSPBinderTests.cpp
+++ clang-tools-extra/clangd/unittests/LSPBinderTests.cpp
@@ -15,12 +15,15 @@
 namespace clangd {
 namespace {
 
+using testing::ElementsAre;
 using testing::HasSubstr;
+using testing::IsEmpty;
 using testing::UnorderedElementsAre;
 
 // JSON-serializable type for testing.
 struct Foo {
   int X;
+  friend bool operator==(Foo A, Foo B) { return A.X == B.X; }
 };
 bool fromJSON(const llvm::json::Value , Foo , llvm::json::Path P) {
   return fromJSON(V, F.X, P.field("X"));
@@ -35,9 +38,31 @@
   return [](llvm::Expected V) { Out.emplace(std::move(V)); };
 }
 
+struct OutgoingRecorder : public LSPBinder::RawOutgoing {
+  llvm::StringMap> Received;
+
+  void callMethod(llvm::StringRef Method, llvm::json::Value Params,
+  Callback Reply) override {
+Received[Method].push_back(Params);
+if (Method == "fail")
+  return Reply(error("Params={0}", Params));
+Reply(Params); // echo back the request
+  }
+  void notify(llvm::StringRef Method, llvm::json::Value Params) override {
+Received[Method].push_back(std::move(Params));
+  }
+
+  std::vector take(llvm::StringRef Method) {
+std::vector Result = Received.lookup(Method);
+Received.erase(Method);
+return Result;
+  }
+};
+
 TEST(LSPBinderTest, IncomingCalls) {
   LSPBinder::RawHandlers RawHandlers;
-  LSPBinder Binder{RawHandlers};
+  OutgoingRecorder RawOutgoing;
+  LSPBinder Binder{RawHandlers, RawOutgoing};
   struct Handler {
 void plusOne(const Foo , Callback Reply) {
   Reply(Foo{Params.X + 1});
@@ -94,7 +119,45 @@
   RawCmdPlusOne(1, capture(Reply));
   ASSERT_TRUE(Reply.hasValue());
   EXPECT_THAT_EXPECTED(Reply.getValue(), llvm::HasValue(2));
+
+  // None of this generated any outgoing traffic.
+  EXPECT_THAT(RawOutgoing.Received, IsEmpty());
+}
+
+TEST(LSPBinderTest, OutgoingCalls) {
+  LSPBinder::RawHandlers RawHandlers;
+  OutgoingRecorder RawOutgoing;
+  LSPBinder Binder{RawHandlers, RawOutgoing};
+
+  LSPBinder::OutgoingMethod Echo;
+  Echo = Binder.outgoingMethod("echo");
+  LSPBinder::OutgoingMethod WrongSignature;
+  WrongSignature = Binder.outgoingMethod("wrongSignature");
+  LSPBinder::OutgoingMethod Fail;
+  Fail = Binder.outgoingMethod("fail");
+
+  llvm::Optional> Reply;
+  Echo(Foo{2}, capture(Reply));
+  EXPECT_THAT(RawOutgoing.take("echo"), ElementsAre(llvm::json::Value(2)));
+  ASSERT_TRUE(Reply.hasValue());
+  EXPECT_THAT_EXPECTED(Reply.getValue(), llvm::HasValue(Foo{2}));
+
+  // JSON response is integer, can't be parsed as string.
+  llvm::Optional> WrongTypeReply;
+  WrongSignature(Foo{2}, capture(WrongTypeReply));
+  EXPECT_THAT(RawOutgoing.take("wrongSignature"),
+  ElementsAre(llvm::json::Value(2)));
+  ASSERT_TRUE(Reply.hasValue());
+  EXPECT_THAT_EXPECTED(WrongTypeReply.getValue(),
+   llvm::FailedWithMessage(
+   HasSubstr("failed to decode wrongSignature reply")));
+
+  Fail(Foo{2}, capture(Reply));
+  EXPECT_THAT(RawOutgoing.take("fail"), ElementsAre(llvm::json::Value(2)));
+  ASSERT_TRUE(Reply.hasValue());
+  EXPECT_THAT_EXPECTED(Reply.getValue(), llvm::FailedWithMessage("Params=2"));
 }
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
===
--- clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -23,6 +23,7 @@
 namespace clang {
 namespace clangd {
 namespace {
+using testing::ElementsAre;
 
 MATCHER_P(DiagMessage, M, "") {
   if (const auto *O = arg.getAsObject()) {
@@ -223,16 +224,20 @@
 
 TEST_F(LSPTest, ModulesTest) {
   class MathModule final : public Module {
+OutgoingNotification Changed;
 void initializeLSP(LSPBinder , const llvm::json::Object ,
llvm::json::Object ) override {
   Bind.notification("add", this, ::add);
   Bind.method("get", this, ::get);
+  Changed = Bind.outgoingNotification("changed");
 }
 
 int Value = 0;
 
-  public:
-void add(const int ) { Value += X; }
+void add(const int ) {
+  Value += X;
+  Changed(Value);
+}
 void get(const std::nullptr_t &, Callback Reply) {
   scheduler().runQuick(
   "get", 

[PATCH] D96717: [clangd] Bind outgoing calls through LSPBinder too. NFC

[Sam McCall via Phabricator via cfe-commits]

sammccall marked 2 inline comments as done.
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/LSPBinder.h:88
+  template 
+  void outgoingMethod(llvm::StringLiteral Method,
+  OutgoingMethod ) {

kadircet wrote:
> sammccall wrote:
> > kadircet wrote:
> > > well this one filling an out-parameter definitely tripped me over while 
> > > reading the code a couple times, and once more on the tests.
> > > 
> > > what about making this return the handler instead? E.g. `Edit = 
> > > Bind.outgoingMethod("edit")`, I know it is ugly 
> > > that we duplicate template params on declaration + here now. Maybe we can 
> > > get away by making `OutgoingMethod` a class with a call operator and a 
> > > `RawOutgoing *Out` member that can be bound later on ? e.g:
> > > 
> > > ```
> > > template 
> > > class OutgoingMethod {
> > >   RawOutgoing *Out = nullptr;
> > > public:
> > >   void operator()(const P& Params, Callback CB) {
> > > assert(Out && "Method haven't bound");
> > > Out->callMethod(Method, JSON(Params), );
> > >   }
> > > };
> > > ```
> > > 
> > > then we either make LSPBinder a friend of OutgoingMethod and set Out 
> > > through it, or the other way around and have a 
> > > `OutgoingMethod::bindOut(LSPBinder&)` ?
> > > 
> > > not sure if it is any better though, as we still construct a somewhat 
> > > invalid object :/
> > > well this one filling an out-parameter definitely tripped me over while 
> > > reading the code a couple times, and once more on the tests.
> > 
> > Yup. My feeling is the wins are:
> >  - avoid writing the types over and over (we can't really avoid it on the 
> > instance variable)
> >  - resembles syntax for binding incoming things
> > 
> > And the main loss is definitely that we're not using assignment syntax for 
> > assignment.
> > 
> > > what about making this return the handler instead? E.g. Edit = 
> > > Bind.outgoingMethod("edit")
> > 
> > This is a reasonable alternative, as you say the duplication is the 
> > downside.
> > 
> > > Maybe we can get away by making OutgoingMethod a class with a call 
> > > operator
> > 
> > Yeah, I don't really understand what this achieves - the usage looks the 
> > same, just now the logical assignment doesn't use assignment syntax *or* 
> > assignment semantics :-)
> > 
> > Other ideas I had:
> >  - the object is untyped, and the call operator is templated/typed - no 
> > type-safety at callsite, yuck
> >  - `outgoingMethod` returns an untyped proxy object with a template 
> > conversion operator to unique_function. This gives us assignment 
> > syntax... and horrible compiler errors on mismatch.
> > 
> > I'm going to try the latter out to see how bad it is.
> thanks! this looks a whole lot better!
Glad you like it... personally I felt a little ill after writing it :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96717

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

[PATCH] D96244: [clangd] Introduce Modules

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D96244#2566225 , @dblaikie wrote:

> "modules" is a pretty overloaded term in Clang (between llvm Modules and 
> Clang Modules and now C++ standard Modules) - any chance of some other term 
> that might be less overloaded in this space?

You're right, and I'm not sure why we didn't bikeshed this name harder :-\
(FWIW Module is also a nod to Guice and its terrible DSLs for binding things 
things, which we borrow a bit here...)

ideas...
Component isn't terrible: I don't like it quite as much as i'm not sure it 
suggests the uniformity-of-interface as well.
Feature is what VSCode calls this on the client side. Personally I find this 
terribly confusing.
Plugin I don't think is great, because there's no dynamic loading or injection 
going on (clang plugins are well-named, but a different concept)
@kadircet any thoughts on these or more?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96244

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

[PATCH] D96512: [PowerPC] Add option for ROP Protection

[Amy Kwan via Phabricator via cfe-commits]

amyk accepted this revision as: amyk.
amyk added a comment.
This revision is now accepted and ready to land.

I just have a small question about the patch but overall I think this LGTM.




Comment at: llvm/lib/Target/PowerPC/PPC.td:317
   // Power8
   list P8AdditionalFeatures =
 [DirectivePwr8,

Does `FeatureROPProtection` need to be here and in the P9/P10 features?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96512

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

[PATCH] D88220: [C++20] P1825R0: More implicit moves

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

Quuxplusone added a comment.

In D88220#2566525 , @aaronpuchert 
wrote:

> I think you're much more of an expert on implicit moves than most of us

Short answer, yes. ;) My concern was more that I'm much less of an expert on 
Clang than "most of us." I'd rather trust a Clang expert to learn and implement 
the couple of pages of P1825 , than you should 
trust a P1825  expert to learn and implement 
the couple hundred lines of Clang in this patch!

But as it's been months, I'm finally ready to WP:BOLD and let someone revert it 
later if they need to.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88220

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

[PATCH] D95458: [PowerPC] Exploit xxsplti32dx (constant materialization) for scalars

[Stefan Pintilie via Phabricator via cfe-commits]

stefanp requested changes to this revision.
stefanp added inline comments.
This revision now requires changes to proceed.



Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.h:1321
   bool convertToNonDenormSingle(APFloat );
+  bool checkNonDenormCannotConvertToSingle(APInt );
+  bool checkNonDenormCannotConvertToSingle(APFloat );

Is the APInt version of this function used anywere?




Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.td:412
+  APFloat APFloatOfN = N->getValueAPF();
+  checkNonDenormCannotConvertToSingle(APFloatOfN);
+  uint32_t Hi = (uint32_t)((APFloatOfN.bitcastToAPInt().getZExtValue() &

Why are we running this here?
We don't check the return of the function so we must assume that it returns 
true.
In that case the value of `APFloatOfN` won't change because 
`convertToNonDenormSingle` will only change the value of the parameter if it 
returns true. But `checkNonDenormCannotConvertToSingle` only returns true if 
`convertToNonDenormSingle` return false.



Comment at: llvm/test/CodeGen/PowerPC/constant-pool.ll:44
+; CHECK-NEXT:xxsplti32dx vs1, 0, 56623104
+; CHECK-NEXT:xxsplti32dx vs1, 1, -609716532
+; CHECK-NEXT:# kill: def $f1 killed $f1 killed $vsl1

I'm looking to understand this test case.
We are trying to materialize a special PowerPC long double (double-double). It 
seems that we have materialized one half of it and not the other half.

Is it because the first half is a denormal?
Why are we avoiding denormals anyway? It seems like we can completely specify a 
64 bit double with two `xxsplti32dx` instructions. 



Comment at: llvm/test/CodeGen/PowerPC/constant-pool.ll:363
+; CHECK-NEXT:stxv vs3, 32(r1) # 16-byte Folded Spill
+; CHECK-NEXT:xxsplti32dx vs3, 1, -343597384
+; CHECK-NEXT:# kill: def $f3 killed $f3 killed $vsl3

What is going on here?
It almost looks like we are spilling `vs3` half way through materializing a 
constant.


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

https://reviews.llvm.org/D95458

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

[PATCH] D96807: Modify TypePrinter to differentiate between anonymous struct and unnamed struct

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thank you for working on this! In general, I like the change.




Comment at: clang/lib/AST/TypePrinter.cpp:1308
+} else if ((isa(D) && 
cast(D)->isAnonymousStructOrUnion()) ||
+isa(D)) {
   OS << "anonymous";

I think `EnumDecl` should probably be `unnamed` rather than `anonymous` because 
there's no term of art for an anonymous enumeration (there's only anonymous 
structures and anonymous unions). WDYT?


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

https://reviews.llvm.org/D96807

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

[PATCH] D88220: [C++20] P1825R0: More implicit moves

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

In D88220#2565768 , @Quuxplusone wrote:

>> either way I'm a dead end — I don't consider myself authorized to "accept" 
>> Clang changes.
>> You're still going to have to attract the attention of someone with the 
>> authority and/or force-of-will to say "okay ship it."
>
> Well, it's been a month since I said that. I'll land this tonight, unless 
> someone yells "stop" before then. If it causes any problems, we can revert it 
> easy enough.

I think you're much more of an expert on implicit moves than most of us (except 
@rsmith perhaps). I've done a superficial review and didn't find anything 
objectionable, also we're early in the release cycle, so I think this approach 
is fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88220

___
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 added a comment.

In D96354#2566511 , @dblaikie wrote:

> In D96354#2566502 , @hoy wrote:
>
>> In D96354#2554129 , @probinson 
>> wrote:
>>
 the driver had a redundant pass-through of the option
>>>
>>> I could've sworn it worked to remove that, but it didn't when I rebased, so 
>>> that's gone from the final patch (i.e, the explicit pass-through in the 
>>> driver is still there).  It's a functionally separate topic anyway.
>>
>> Soung
>>
>> In D96354#2566273 , @dblaikie wrote:
>>
>>> @hoy Could you explain a bit further why these two features are 
>>> incompatible/what the crash looks like? At first glance I wouldn't expect 
>>> any debug info mode to be incompatible with any non-debug-info mode (maybe 
>>> less useful, but not crashy levels of incompatible).
>>
>> Both debug-info mode and the pseudo-probe mode use the Dwarf discriminators 
>> but for different purposes. Therefore the passes that populate the Dwarf 
>> discriminators should not be scheduled at the same time. The crash was like 
>> compiler fatal error before this change. It
>
> Oh, that's subtle and seems somewhat unfortunate. Is that documented 
> somewhere I could read more about?

The summary of this diff has some information https://reviews.llvm.org/D91756 . 
Unfortunately we haven't rolled out a formal document yet.


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] D96354: Avoid conflicts between debug-info and pseudo-probe profiling

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D96354#2566502 , @hoy wrote:

> In D96354#2554129 , @probinson wrote:
>
>>> the driver had a redundant pass-through of the option
>>
>> I could've sworn it worked to remove that, but it didn't when I rebased, so 
>> that's gone from the final patch (i.e, the explicit pass-through in the 
>> driver is still there).  It's a functionally separate topic anyway.
>
> Soung
>
> In D96354#2566273 , @dblaikie wrote:
>
>> @hoy Could you explain a bit further why these two features are 
>> incompatible/what the crash looks like? At first glance I wouldn't expect 
>> any debug info mode to be incompatible with any non-debug-info mode (maybe 
>> less useful, but not crashy levels of incompatible).
>
> Both debug-info mode and the pseudo-probe mode use the Dwarf discriminators 
> but for different purposes. Therefore the passes that populate the Dwarf 
> discriminators should not be scheduled at the same time. The crash was like 
> compiler fatal error before this change. It

Oh, that's subtle and seems somewhat unfortunate. Is that documented somewhere 
I could read more about?


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] D95754: [clang] Print 32 candidates on the first failure, with -fshow-overloads=best.

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added inline comments.



Comment at: clang/lib/Sema/Sema.cpp:2310
 
+  S.Diags.noteNumOverloadCandidatesShown(ShownOverloads);
+

jlebar wrote:
> aaronpuchert wrote:
> > jlebar wrote:
> > > aaronpuchert wrote:
> > > > Why not in the following `if`? I assume we want to show a long list not 
> > > > necessarily once, but only if it comes with the first error?
> > > My intent was to show the long list once, even if it's not the very first 
> > > error.  My thought process:
> > > 
> > > All things being equal, it's better to show more information to the user 
> > > than less.  The problem is, at some point, the amount of information we 
> > > show becomes overwhelming and spammy.  Particularly problematic are 
> > > multiline errors, because then you get O(nm) error lines across the whole 
> > > TU.  We prevent the O(nm) overwhelm by limiting the number of lines a 
> > > particular error can produce (using the mechanism in question here, or 
> > > the template backtrace limit, etc), and then also limiting the total 
> > > number of individual errors before we stop printing those.
> > > 
> > > With this change, we display the full(ish) error the first time it occurs 
> > > and then the truncated error every other time.  So in total it's O(n + m) 
> > > rather than O(nm).
> > Got it, but currently you'd not be exhausting the option to print a long 
> > list once: when the first overload resolution error has fewer candidates 
> > than the limit you'd still say we stop printing long lists of notes from 
> > now on. That confused me because you're calling 
> > `noteNumOverloadCandidatesShown` but you might not actually have printed 
> > that note.
> > 
> > If you're going by the //O(n+m)// argument, you could put this call into 
> > the `if (SuppressedOverloads)` and still stay under that limit.
> > currently you'd not be exhausting the option to print a long list once: 
> > when the first overload resolution error has fewer candidates than the 
> > limit you'd still say we stop printing long lists of notes from now on.
> 
> But if we print <= 4 overloads then noteNumOverloadCandidatesShown has no 
> effect?
> 
> OTOH if we move it into the `if` then imagine a case where NumOverloads 
> starts at 32 and we print 16 overloads.  In that case we don't suppress any 
> overloads.  But the *next* time we still want to limit it to only 4.  So 
> moving it into the `if` would do the wrong thing.
> 
> Unless I'm misunderstanding?
You're absolutely right. I guess I was confused by the function name 
`noteNumOverloadCandidatesShown`, thinking that calling it would indicate that 
we've shown the note below.

Perhaps we can just drop the `note` or find another verb? Or you could explain 
how the name is intended to be read.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95754

___
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 added a comment.

In D96354#2554129 , @probinson wrote:

>> the driver had a redundant pass-through of the option
>
> I could've sworn it worked to remove that, but it didn't when I rebased, so 
> that's gone from the final patch (i.e, the explicit pass-through in the 
> driver is still there).  It's a functionally separate topic anyway.

Soung

In D96354#2566273 , @dblaikie wrote:

> @hoy Could you explain a bit further why these two features are 
> incompatible/what the crash looks like? At first glance I wouldn't expect any 
> debug info mode to be incompatible with any non-debug-info mode (maybe less 
> useful, but not crashy levels of incompatible).

Both debug-info mode and the pseudo-probe mode use the Dwarf discriminators but 
for different purposes. Therefore the passes that populate the Dwarf 
discriminators should not be scheduled at the same time. The crash was like 
compiler fatal error before this change. It


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] D76342: [OpenMP] Implement '#pragma omp tile'

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

A fix is ready hereL https://reviews.llvm.org/D96808


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76342

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

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

Hi Stefan,

Thanks a lot for the details you shared. They are really helpful to me.

In D96033#2565708 , @sgraenitz wrote:

> Hi Vassil, thanks for upstreaming this! I think it goes into a good direction.
>
> The last time I looked at the Cling sources downstream, it was based on LLVM 
> release 5.0. The IncrementalJIT class was based on what we call OrcV1 today. 
> OrcV1 is long deprecated and even though it's still in tree today, it will 
> very likely be removed in the upcoming release cycle. So I guess, one of the 
> challenges will be porting the Cling internals to OrcV2 -- a lot has changed, 
> mostly to the better :) Not all of this is relevant for this patch, but maybe 
> it's worth mentioning for upcoming additions.

Cling is currently being upgraded to llvm9 
(https://github.com/vgvassilev/cling/tree/upgrade_llvm90). I expect by the end 
of this year to be upgraded again to llvm12 and the big challenge is making 
good use of OrcV2. We need to support code removal of the kind:

  [cling] struct Adder { double Add(double a, double b) {return a - b; }; // 
comes in a "script" file.
  [cling] Adder adder; printf("%f\n", adder.Add(1, 2)); //realize that we have 
a mistake.
  [cling] .undo 2
  [cling] struct Adder { double Add(double a, double b) {return a + b; }; // 
comes in a "script" file.
  [cling] Adder adder; printf("%f\n", adder.Add(1, 2));

Some more details can be seen here -- 
https://blog.llvm.org/posts/2020-11-30-interactive-cpp-with-cling/

In the example above the JIT will need to remove objects from its state 
(including already created machine code). Until recently that was not entirely 
possible with the OrcV2. Lang was actively working on this and maybe this now 
works well.

The other aspect of this is that upon unloading of these pieces of code we need 
to run the destructors (that's why we need some non-canonical handling of when 
we run the `atexit` handlers).

I'd very much want to jump on the OrcV2 with this patch but that would cause me 
a longer term problem as now the initial version of minimal cling (clang-repl) 
is already substantially different than cling itself. My upstream strategy was 
to make a minimal patch with design as close as possible to cling. Then 
depending on the comments we will start evolving both systems in such a way 
that cling is not left substantially behind as it still has a majority of 
interactive C++ cases which we care about.

> OrcV2 works with Dylibs, basically symbol namespaces. When you add a module 
> to a Dylib, all its symbols will be added. Respectively, if you want to 
> remove something from a Dylib, you have to remove the symbols (for 
> fine-tuning it you can reach for a Dylib's ResourceTracker). Symbols won't be 
> materialized until you look them up. I guess for incremental compilation you 
> would keep on adding symbols, one increment at a time.

All this sounds super nice and I am eager to start gradually using it.

>   int var1 = 42; int f() { return var1; }
>   int var2 = f();
>
> Let's say these are two inputs. The first line only adds definitions for 
> symbols `var1` and `f()` but won't materialize anything. The second line 
> would have to lookup `f()`, execute it and emit a new definition for `var2`. 
> I never got into Cling deep enough to find out how it works, but I assume 
> it's high-level enough that it won't require large changes. One thing I'd 
> recommend to double-check: if there is a third line that adds a static 
> constructor, will LLJIT only run this one or will it run all previous static 
> ctors again when calling `initialize()`? I assume the former but I wouldn't 
> bet on it.

Would that capture your concern?

  ./bin/clang-repl 
  clang-repl> extern "C" int printf(const char*,...);
  clang-repl> int var1 = 42; int f() { return printf("init_once\n"); }
  clang-repl> int var2 = f();
  init_once
  clang-repl> int var3 = f();
  init_once
  clang-repl> struct S{ S(const char*) {} } s("");
  clang-repl> 

I think the reason we do not rerun the static constructors is this tweak we 
have in codegen https://github.com/llvm/llvm-project/commit/188ad3ac02d06

> Another aspect is that downstream Cling is based on RuntimeDyld for linking 
> Orc's output object files. I remember RemovableObjectLinkingLayer adding some 
> object file removal code. Upstream OrcV2 grew it's own linker in the 
> meantime. It's called JITLink and gets pulled into LLJIT via 
> `ObjectLinkingLayer`. RuntimeDyld-based linking is still supported with the 
> `RTDyldObjectLinkingLayer`. JITLink is not complete for all platforms yet. 
> Thus, LLJITBuilder defaults to JITLink on macOS and RuntimeDyld otherwise. 
> Chances are that JITLink gets good enough for ELF to enable it by default on 
> Linux (at least x86-64). I guess that's your platform of concern?

We also care about COFF.

> The related question is whether you are aiming for JITLink straight away or 
> staying with 

[PATCH] D76342: [OpenMP] Implement '#pragma omp tile'

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

@Meinersbur This patch is making Flang buildbots failing.

Since you have added a clause in `OMP.td` you have to add lines here as well -> 
https://github.com/llvm/llvm-project/blob/adfd3c7083f9808d145239153c10f72eece485d8/flang/lib/Semantics/check-omp-structure.cpp#L681

  CHECK_SIMPLE_CLAUSE(Sizes, OMPC_sizes)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76342

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

[clang-tools-extra] cdef5a7 - [clangd] Fix windows buildbots after ecea7218fb9b994b26471e9877851cdb51a5f1d4

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2021-02-16T20:57:08+01:00
New Revision: cdef5a7161767c2c4b3b7cb2542cf1d29b6d4a09

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

LOG: [clangd] Fix windows buildbots after 
ecea7218fb9b994b26471e9877851cdb51a5f1d4

Added: 


Modified: 
clang-tools-extra/clangd/support/Path.cpp
clang-tools-extra/clangd/unittests/support/PathTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/support/Path.cpp 
b/clang-tools-extra/clangd/support/Path.cpp
index 6fc74b92fc7a..a7907cffe60c 100644
--- a/clang-tools-extra/clangd/support/Path.cpp
+++ b/clang-tools-extra/clangd/support/Path.cpp
@@ -21,8 +21,8 @@ bool pathEqual(PathRef A, PathRef B) { return A == B; }
 
 bool pathStartsWith(PathRef Ancestor, PathRef Path,
 llvm::sys::path::Style Style) {
-  assert(llvm::sys::path::is_absolute(Ancestor, Style) &&
- llvm::sys::path::is_absolute(Path, Style));
+  assert(llvm::sys::path::is_absolute(Ancestor) &&
+ llvm::sys::path::is_absolute(Path));
   // If ancestor ends with a separator drop that, so that we can match /foo/ as
   // a parent of /foo.
   if (llvm::sys::path::is_separator(Ancestor.back(), Style))

diff  --git a/clang-tools-extra/clangd/unittests/support/PathTests.cpp 
b/clang-tools-extra/clangd/unittests/support/PathTests.cpp
index 26b999d103a0..599c76926d30 100644
--- a/clang-tools-extra/clangd/unittests/support/PathTests.cpp
+++ b/clang-tools-extra/clangd/unittests/support/PathTests.cpp
@@ -6,6 +6,7 @@
 //
 
//===--===//
 
+#include "TestFS.h"
 #include "support/Path.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -14,21 +15,21 @@ namespace clang {
 namespace clangd {
 namespace {
 TEST(PathTests, IsAncestor) {
-  EXPECT_TRUE(pathStartsWith("/foo", "/foo"));
-  EXPECT_TRUE(pathStartsWith("/foo/", "/foo"));
+  EXPECT_TRUE(pathStartsWith(testPath("foo"), testPath("foo")));
+  EXPECT_TRUE(pathStartsWith(testPath("foo/"), testPath("foo")));
 
-  EXPECT_FALSE(pathStartsWith("/foo", "/fooz"));
-  EXPECT_FALSE(pathStartsWith("/foo/", "/fooz"));
+  EXPECT_FALSE(pathStartsWith(testPath("foo"), testPath("fooz")));
+  EXPECT_FALSE(pathStartsWith(testPath("foo/"), testPath("fooz")));
 
-  EXPECT_TRUE(pathStartsWith("/foo", "/foo/bar"));
-  EXPECT_TRUE(pathStartsWith("/foo/", "/foo/bar"));
+  EXPECT_TRUE(pathStartsWith(testPath("foo"), testPath("foo/bar")));
+  EXPECT_TRUE(pathStartsWith(testPath("foo/"), testPath("foo/bar")));
 
 #ifdef CLANGD_PATH_CASE_INSENSITIVE
-  EXPECT_TRUE(pathStartsWith("/fOo", "/foo/bar"));
-  EXPECT_TRUE(pathStartsWith("/foo", "/fOo/bar"));
+  EXPECT_TRUE(pathStartsWith(testPath("fOo"), testPath("foo/bar")));
+  EXPECT_TRUE(pathStartsWith(testPath("foo"), testPath("fOo/bar")));
 #else
-  EXPECT_FALSE(pathStartsWith("/fOo", "/foo/bar"));
-  EXPECT_FALSE(pathStartsWith("/foo", "/fOo/bar"));
+  EXPECT_FALSE(pathStartsWith(testPath("fOo"), testPath("foo/bar")));
+  EXPECT_FALSE(pathStartsWith(testPath("foo"), testPath("fOo/bar")));
 #endif
 }
 } // namespace



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

[PATCH] D96807: Modify TypePrinter to differentiate between anonymous struct and unnamed struct

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

Note: I am not fixing how we treat anonymous and unnamed enums, I could not 
find a way to figure out if an enum was anonymous or not.


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

https://reviews.llvm.org/D96807

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

[PATCH] D95246: [test] Use host platform specific error message substitution in lit tests

[Abhina Sree via Phabricator via cfe-commits]

abhina.sreeskantharajan added a comment.

In D95246#2566029 , @ASDenysPetrov 
wrote:

> In D95246#2565351 , 
> @abhina.sreeskantharajan wrote:
>
>> 
>
>
>
>> Do you know what is different between your environments which is causing the 
>> spelling to be capitalized? This might help me determine how to fix the 
>> current host platform check in llvm/utils/lit/lit/llvm/config.py.
>
> I'd love to help you. I'm using ordinary Win10. Here is my python output:
>
>   >>> os.name
>   'nt'
>   >>> sys.platform
>   'win32'
>   >>> platform.system()
>   'Windows'
>   >>> platform.release()
>   '10'
>   >>>
>
> If you would say what exact info you need I would execute and send you.
> BTW my python3 uses //Sentence case capitalization// in errors as you can see:
>
>   >>> open('some')
>   Traceback (most recent call last):
> File "", line 1, in 
>   FileNotFoundError: [Errno 2] No such file or directory: 'some'
>   >>>

Thanks! The following Windows bot (clang-x64-windows-msvc) is also passing 
http://lab.llvm.org:8011/#/builders/123, I expect it would have the same output 
in python as your machine.  James mentioned that this issue might be caused by 
a difference in standard library.

I was curious if all of these modified testcases are now failing or only the 
three you mentioned. I also don't know how this change can cause a pop-up to 
occur, is there more information you can provide? 
And lastly, if you have the time, does this testcase pass for you?

  llvm/test/tools/llvm-elfabi/fail-file-write-windows.test

I did not modify this but it requires system-windows and has a lower-case error 
message. I expect if my change causes failures for you, then this testcase 
should also fail.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95246

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

[PATCH] D96807: Modify TypePrinter to differentiate between anonymous struct and unnamed struct

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik created this revision.
shafik added reviewers: aaron.ballman, rsmith, teemperor.
Herald added a subscriber: arphaman.
shafik requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added a subscriber: sstefan1.

Currently `TypePrinter` lumps anonymous classes and unnamed classes in one 
group `"anonymous"` this is not correct and can be confusing in some contexts. 
e.g.

LLDB SBType  
provides an `IsAnonymousType()` function but when displaying the type we see 
conflicting information, given:

struct A { 
 struct { int x; }; // A
 struct { int y; } B;
  };
  
  int main() {
 A a1; 
//...
  }

we get the following in LLDB:

  (lldb) script var = lldb.frame.FindVariable("a1")
  (lldb) script print(var.GetChildAtIndex(0).GetType().IsAnonymousType())
  True
  (lldb) script print(var.GetChildAtIndex(1).GetType().IsAnonymousType())
  False
  (lldb) script print(var.GetChildAtIndex(0).GetType().GetName())
  A::(anonymous struct)
  (lldb) script print(var.GetChildAtIndex(1).GetType().GetName())
  A::(anonymous struct)


https://reviews.llvm.org/D96807

Files:
  clang/lib/AST/TypePrinter.cpp
  clang/test/AST/ast-dump-decl-json.c
  clang/test/AST/ast-dump-openmp-cancel.c
  clang/test/AST/ast-dump-openmp-cancellation-point.c
  clang/test/AST/ast-dump-openmp-distribute-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-distribute-parallel-for.c
  clang/test/AST/ast-dump-openmp-distribute-simd.c
  clang/test/AST/ast-dump-openmp-distribute.c
  clang/test/AST/ast-dump-openmp-for-simd.c
  clang/test/AST/ast-dump-openmp-for.c
  clang/test/AST/ast-dump-openmp-ordered.c
  clang/test/AST/ast-dump-openmp-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-parallel-for.c
  clang/test/AST/ast-dump-openmp-parallel-sections.c
  clang/test/AST/ast-dump-openmp-parallel.c
  clang/test/AST/ast-dump-openmp-section.c
  clang/test/AST/ast-dump-openmp-sections.c
  clang/test/AST/ast-dump-openmp-simd.c
  clang/test/AST/ast-dump-openmp-single.c
  clang/test/AST/ast-dump-openmp-target-data.c
  clang/test/AST/ast-dump-openmp-target-enter-data.c
  clang/test/AST/ast-dump-openmp-target-exit-data.c
  clang/test/AST/ast-dump-openmp-target-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-target-parallel-for.c
  clang/test/AST/ast-dump-openmp-target-parallel.c
  clang/test/AST/ast-dump-openmp-target-simd.c
  clang/test/AST/ast-dump-openmp-target-teams-distribute-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-target-teams-distribute-parallel-for.c
  clang/test/AST/ast-dump-openmp-target-teams-distribute-simd.c
  clang/test/AST/ast-dump-openmp-target-teams-distribute.c
  clang/test/AST/ast-dump-openmp-target-teams.c
  clang/test/AST/ast-dump-openmp-target-update.c
  clang/test/AST/ast-dump-openmp-target.c
  clang/test/AST/ast-dump-openmp-task.c
  clang/test/AST/ast-dump-openmp-taskgroup.c
  clang/test/AST/ast-dump-openmp-taskloop-simd.c
  clang/test/AST/ast-dump-openmp-taskloop.c
  clang/test/AST/ast-dump-openmp-teams-distribute-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-teams-distribute-parallel-for.c
  clang/test/AST/ast-dump-openmp-teams-distribute-simd.c
  clang/test/AST/ast-dump-openmp-teams-distribute.c
  clang/test/AST/ast-dump-openmp-teams.c
  clang/test/AST/ast-dump-records-json.cpp
  clang/test/AST/ast-dump-records.c
  clang/test/AST/ast-dump-records.cpp
  clang/test/AST/ast-dump-stmt-json.m
  clang/test/ASTMerge/struct/test.c
  clang/test/Analysis/cfg.cpp
  clang/test/Analysis/padding_c.c
  clang/test/Index/print-type.c
  clang/test/Layout/ms-x86-alias-avoidance-padding.cpp
  clang/test/PCH/stmt-openmp_structured_block-bit.cpp
  clang/test/Sema/assign.c
  clang/test/SemaCXX/condition.cpp
  
lldb/test/Shell/SymbolFile/DWARF/clang-ast-from-dwarf-unamed-and-anon-structs.cpp
  lldb/test/Shell/SymbolFile/DWARF/debug-types-missing-signature.test

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

[PATCH] D46443: Add missing cstdalign header

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.
Herald added a subscriber: wingo.

The resolution of LWG 2828 
 means that `` 
exists in C++11 and C++14. The rationale from 
https://reviews.llvm.org/D96786#2566110 can be taken as saying that adding this 
header for C++11 conformance is reasonable.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D46443

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

[PATCH] D96802: [Clang] Add proper target checks for SwiftAsync convention.

[Arnold Schwaighofer via Phabricator via cfe-commits]

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

Looks good to me


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96802

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

[PATCH] D96690: [clangd] Treat paths case-insensitively depending on the platform

[Kadir Cetinkaya 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 rGecea7218fb9b: [clangd] Treat paths case-insensitively 
depending on the platform (authored by kadircet).

Changed prior to commit:
  https://reviews.llvm.org/D96690?vs=323997=324065#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96690

Files:
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/support/Path.cpp
  clang-tools-extra/clangd/support/Path.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
  clang-tools-extra/clangd/unittests/RenameTests.cpp
  clang-tools-extra/clangd/unittests/support/PathTests.cpp

Index: clang-tools-extra/clangd/unittests/support/PathTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/support/PathTests.cpp
@@ -0,0 +1,36 @@
+//===-- PathTests.cpp ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "support/Path.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+TEST(PathTests, IsAncestor) {
+  EXPECT_TRUE(pathStartsWith("/foo", "/foo"));
+  EXPECT_TRUE(pathStartsWith("/foo/", "/foo"));
+
+  EXPECT_FALSE(pathStartsWith("/foo", "/fooz"));
+  EXPECT_FALSE(pathStartsWith("/foo/", "/fooz"));
+
+  EXPECT_TRUE(pathStartsWith("/foo", "/foo/bar"));
+  EXPECT_TRUE(pathStartsWith("/foo/", "/foo/bar"));
+
+#ifdef CLANGD_PATH_CASE_INSENSITIVE
+  EXPECT_TRUE(pathStartsWith("/fOo", "/foo/bar"));
+  EXPECT_TRUE(pathStartsWith("/foo", "/fOo/bar"));
+#else
+  EXPECT_FALSE(pathStartsWith("/fOo", "/foo/bar"));
+  EXPECT_FALSE(pathStartsWith("/foo", "/fOo/bar"));
+#endif
+}
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -1125,7 +1125,7 @@
   EXPECT_THAT(Results.GlobalChanges.keys(),
   UnorderedElementsAre(Main, testPath("other.cc")));
 
-#if defined(_WIN32) || defined(__APPLE__)
+#ifdef CLANGD_PATH_CASE_INSENSITIVE
   // On case-insensitive systems, no duplicates if AST vs index case differs.
   // https://github.com/clangd/clangd/issues/665
   TU.Filename = "MAIN.CC";
Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
===
--- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -99,6 +99,25 @@
   Frag.If.PathMatch.emplace_back("ba*r");
   EXPECT_FALSE(compileAndApply());
   EXPECT_THAT(Diags.Diagnostics, IsEmpty());
+
+  // Only matches case-insensitively.
+  Frag = {};
+  Frag.If.PathMatch.emplace_back("B.*R");
+  EXPECT_THAT(Diags.Diagnostics, IsEmpty());
+#ifdef CLANGD_PATH_CASE_INSENSITIVE
+  EXPECT_TRUE(compileAndApply());
+#else
+  EXPECT_FALSE(compileAndApply());
+#endif
+
+  Frag = {};
+  Frag.If.PathExclude.emplace_back("B.*R");
+  EXPECT_THAT(Diags.Diagnostics, IsEmpty());
+#ifdef CLANGD_PATH_CASE_INSENSITIVE
+  EXPECT_FALSE(compileAndApply());
+#else
+  EXPECT_TRUE(compileAndApply());
+#endif
 }
 
 TEST_F(ConfigCompileTests, CompileCommands) {
@@ -406,6 +425,23 @@
   ASSERT_THAT(Diags.Diagnostics, IsEmpty());
   ASSERT_TRUE(Conf.Index.External);
   EXPECT_THAT(Conf.Index.External->MountPoint, FooPath);
+
+  // Only matches case-insensitively.
+  BazPath = testPath("fOo/baz.h", llvm::sys::path::Style::posix);
+  BazPath = llvm::sys::path::convert_to_slash(BazPath);
+  Parm.Path = BazPath;
+
+  FooPath = testPath("FOO/", llvm::sys::path::Style::posix);
+  FooPath = llvm::sys::path::convert_to_slash(FooPath);
+  Frag = GetFrag("", FooPath.c_str());
+  compileAndApply();
+  ASSERT_THAT(Diags.Diagnostics, IsEmpty());
+#ifdef CLANGD_PATH_CASE_INSENSITIVE
+  ASSERT_TRUE(Conf.Index.External);
+  EXPECT_THAT(Conf.Index.External->MountPoint, FooPath);
+#else
+  ASSERT_FALSE(Conf.Index.External);
+#endif
 }
 } // namespace
 } // namespace config
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -105,6 +105,7 @@
   support/FunctionTests.cpp
   support/MarkupTests.cpp
   support/MemoryTreeTests.cpp
+  support/PathTests.cpp
   support/ThreadingTests.cpp
 

[clang-tools-extra] ecea721 - [clangd] Treat paths case-insensitively depending on the platform

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2021-02-16T20:20:53+01:00
New Revision: ecea7218fb9b994b26471e9877851cdb51a5f1d4

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

LOG: [clangd] Treat paths case-insensitively depending on the platform

Path{Match,Exclude} and MountPoint were checking paths case-sensitively
on all platforms, as with other features, this was causing problems on
windows. Since users can have capital drive letters on config files, but
editors might lower-case them.

This patch addresses that issue by:
- Creating regexes with case-insensitive matching on those platforms.
- Introducing a new pathIsAncestor helper, which performs checks in a
  case-correct manner where needed.

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

Added: 
clang-tools-extra/clangd/unittests/support/PathTests.cpp

Modified: 
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/support/Path.cpp
clang-tools-extra/clangd/support/Path.h
clang-tools-extra/clangd/unittests/CMakeLists.txt
clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
clang-tools-extra/clangd/unittests/RenameTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ConfigCompile.cpp 
b/clang-tools-extra/clangd/ConfigCompile.cpp
index 8682cae36f26..dadc578c3a81 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -31,6 +31,7 @@
 #include "Features.inc"
 #include "TidyProvider.h"
 #include "support/Logger.h"
+#include "support/Path.h"
 #include "support/Trace.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
@@ -101,9 +102,11 @@ struct FragmentCompiler {
   // Normalized Fragment::SourceInfo::Directory.
   std::string FragmentDirectory;
 
-  llvm::Optional compileRegex(const Located ) {
+  llvm::Optional
+  compileRegex(const Located ,
+   llvm::Regex::RegexFlags Flags = llvm::Regex::NoFlags) {
 std::string Anchored = "^(" + *Text + ")$";
-llvm::Regex Result(Anchored);
+llvm::Regex Result(Anchored, Flags);
 std::string RegexError;
 if (!Result.isValid(RegexError)) {
   diag(Error, "Invalid regex " + Anchored + ": " + RegexError, Text.Range);
@@ -195,9 +198,15 @@ struct FragmentCompiler {
 if (F.HasUnrecognizedCondition)
   Out.Conditions.push_back([&](const Params &) { return false; });
 
+#ifdef CLANGD_PATH_CASE_INSENSITIVE
+llvm::Regex::RegexFlags Flags = llvm::Regex::IgnoreCase;
+#else
+llvm::Regex::RegexFlags Flags = llvm::Regex::NoFlags;
+#endif
+
 auto PathMatch = std::make_unique>();
 for (auto  : F.PathMatch) {
-  if (auto RE = compileRegex(Entry))
+  if (auto RE = compileRegex(Entry, Flags))
 PathMatch->push_back(std::move(*RE));
 }
 if (!PathMatch->empty()) {
@@ -218,7 +227,7 @@ struct FragmentCompiler {
 
 auto PathExclude = std::make_unique>();
 for (auto  : F.PathExclude) {
-  if (auto RE = compileRegex(Entry))
+  if (auto RE = compileRegex(Entry, Flags))
 PathExclude->push_back(std::move(*RE));
 }
 if (!PathExclude->empty()) {
@@ -349,7 +358,8 @@ struct FragmentCompiler {
   return;
 Spec.MountPoint = std::move(*AbsPath);
 Out.Apply.push_back([Spec(std::move(Spec))](const Params , Config ) {
-  if (!P.Path.startswith(Spec.MountPoint))
+  if (P.Path.empty() || !pathStartsWith(Spec.MountPoint, P.Path,
+llvm::sys::path::Style::posix))
 return;
   C.Index.External = Spec;
   // Disable background indexing for the files under the mountpoint.

diff  --git a/clang-tools-extra/clangd/support/Path.cpp 
b/clang-tools-extra/clangd/support/Path.cpp
index f72d00070f34..6fc74b92fc7a 100644
--- a/clang-tools-extra/clangd/support/Path.cpp
+++ b/clang-tools-extra/clangd/support/Path.cpp
@@ -7,24 +7,33 @@
 
//===--===//
 
 #include "support/Path.h"
+#include "llvm/Support/Path.h"
 namespace clang {
 namespace clangd {
 
-std::string maybeCaseFoldPath(PathRef Path) {
-#if defined(_WIN32) || defined(__APPLE__)
-  return Path.lower();
-#else
-  return std::string(Path);
-#endif
-}
+#ifdef CLANGD_PATH_CASE_INSENSITIVE
+std::string maybeCaseFoldPath(PathRef Path) { return Path.lower(); }
+bool pathEqual(PathRef A, PathRef B) { return A.equals_lower(B); }
+#else  // NOT CLANGD_PATH_CASE_INSENSITIVE
+std::string maybeCaseFoldPath(PathRef Path) { return Path.str(); }
+bool pathEqual(PathRef A, PathRef B) { return A == B; }
+#endif // CLANGD_PATH_CASE_INSENSITIVE
 
-bool pathEqual(PathRef A, PathRef B) {
-#if defined(_WIN32) || defined(__APPLE__)
-  return A.equals_lower(B);
-#else
-  return A == B;
-#endif
+bool pathStartsWith(PathRef Ancestor, PathRef Path,
+ 

[PATCH] D96803: EntryExitInstrumenter: Move to a module pass and enable at all optimization levels (PR49143)

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks added a comment.

why is this now a module pass?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96803

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

[libunwind] cddc53e - libunwind: Don't attempt to authenticate a null return address.

[Peter Collingbourne via cfe-commits]

Author: Peter Collingbourne
Date: 2021-02-16T11:18:02-08:00
New Revision: cddc53ef088b68586094c9841a76b41bee3994a4

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

LOG: libunwind: Don't attempt to authenticate a null return address.

Null return addresses can appear at the bottom of the stack (i.e. the
frame corresponding to the entry point). Authenticating these addresses
will set the error code in the address, which will lead to a segfault
in the sigreturn trampoline detection code. Fix this problem by not
authenticating null addresses.

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

Added: 


Modified: 
libunwind/src/DwarfInstructions.hpp

Removed: 




diff  --git a/libunwind/src/DwarfInstructions.hpp 
b/libunwind/src/DwarfInstructions.hpp
index c39cabe1f783..bd13d1623153 100644
--- a/libunwind/src/DwarfInstructions.hpp
+++ b/libunwind/src/DwarfInstructions.hpp
@@ -213,7 +213,8 @@ int DwarfInstructions::stepWithDwarf(A , 
pint_t pc,
   // restored. autia1716 is used instead of autia as autia1716 assembles
   // to a NOP on pre-v8.3a architectures.
   if ((R::getArch() == REGISTERS_ARM64) &&
-  prolog.savedRegisters[UNW_ARM64_RA_SIGN_STATE].value) {
+  prolog.savedRegisters[UNW_ARM64_RA_SIGN_STATE].value &&
+  returnAddress != 0) {
 #if !defined(_LIBUNWIND_IS_NATIVE_ONLY)
 return UNW_ECROSSRASIGNING;
 #else



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

[PATCH] D96803: EntryExitInstrumenter: Move to a module pass and enable at all optimization levels (PR49143)

[Adhemerval Zanella via Phabricator via cfe-commits]

zatrazz created this revision.
zatrazz added reviewers: lenary, ostannard, aeubanks.
Herald added a subscriber: hiraditya.
zatrazz requested review of this revision.
Herald added projects: clang, LLVM.

The function passes are disabled by default for optnone with the new
pass manager.  The pass is now enabled in clang backend if the
-finstrument-functions, -finstrument-function-entry-bare, or
-finstrument-functions-after-inlining instead of just for
optimization level different than -O0.

It fixes PR49143 and allows re-enable the eeprof-1.c test from
test-suite (disabled by D96521 ).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96803

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  llvm/include/llvm/Transforms/Utils/EntryExitInstrumenter.h
  llvm/lib/Passes/PassRegistry.def
  llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
  llvm/test/Transforms/EntryExitInstrumenter/debug-info.ll
  llvm/test/Transforms/EntryExitInstrumenter/mcount.ll

Index: llvm/test/Transforms/EntryExitInstrumenter/mcount.ll
===
--- llvm/test/Transforms/EntryExitInstrumenter/mcount.ll
+++ llvm/test/Transforms/EntryExitInstrumenter/mcount.ll
@@ -1,7 +1,10 @@
-; RUN: opt -passes="function(ee-instrument),cgscc(inline),function(post-inline-ee-instrument)" -S < %s | FileCheck %s
+; RUN: opt --O0 --ee-instrument --inline --post-inline-ee-instrument -S < %s | FileCheck %s
+
+; Check if the pass is enabbled on both -O0 and higher optimization levels
+; RUN: opt --O2 --ee-instrument --post-inline-ee-instrument -S < %s | FileCheck --check-prefix=OPT %s
 
 ; Running the passes twice should not result in more instrumentation.
-; RUN: opt -passes="function(ee-instrument),function(ee-instrument),cgscc(inline),function(post-inline-ee-instrument),function(post-inline-ee-instrument)" -S < %s | FileCheck %s
+; RUN: opt -passes="ee-instrument,ee-instrument,cgscc(inline),post-inline-ee-instrument,post-inline-ee-instrument" -S < %s | FileCheck %s
 
 target datalayout = "E-m:e-i64:64-n32:64"
 target triple = "powerpc64le-unknown-linux"
@@ -18,6 +21,15 @@
 ; CHECK-NEXT: %1 = call i8* @llvm.returnaddress(i32 0)
 ; CHECK-NEXT: call void @__cyg_profile_func_exit(i8* bitcast (void ()* @leaf_function to i8*), i8* %1)
 ; CHECK-NEXT: ret void
+
+; OPT-LABEL: define void @leaf_function()
+; OPT: entry:
+; OPT-NEXT: call void @mcount()
+; OPT-NEXT: %0 = call i8* @llvm.returnaddress(i32 0)
+; OPT-NEXT: call void @__cyg_profile_func_enter(i8* bitcast (void ()* @leaf_function to i8*), i8* %0)
+; OPT-NEXT: %1 = call i8* @llvm.returnaddress(i32 0)
+; OPT-NEXT: call void @__cyg_profile_func_exit(i8* bitcast (void ()* @leaf_function to i8*), i8* %1)
+; OPT-NEXT: ret void
 }
 
 
@@ -42,6 +54,16 @@
 
 ; CHECK-NEXT: call void @__cyg_profile_func_exit(i8* bitcast (void ()* @root_function to i8*), i8* %3)
 ; CHECK-NEXT: ret void
+
+; OPT-LABEL: define void @root_function()
+; OPT: entry:
+; OPT-NEXT: call void @mcount()
+
+; OPT-NEXT: %0 = call i8* @llvm.returnaddress(i32 0)
+; OPT-NEXT: call void @__cyg_profile_func_enter(i8* bitcast (void ()* @root_function to i8*), i8* %0)
+; OPT-NEXT: %1 = call i8* @llvm.returnaddress(i32 0)
+; OPT-NEXT: call void @__cyg_profile_func_exit(i8* bitcast (void ()* @root_function to i8*), i8* %1)
+; OPT-NEXT: ret void
 }
 
 
Index: llvm/test/Transforms/EntryExitInstrumenter/debug-info.ll
===
--- llvm/test/Transforms/EntryExitInstrumenter/debug-info.ll
+++ llvm/test/Transforms/EntryExitInstrumenter/debug-info.ll
@@ -1,4 +1,4 @@
-; RUN: opt -passes="function(ee-instrument),cgscc(inline),function(post-inline-ee-instrument)" -S < %s | FileCheck %s
+; RUN: opt -passes="ee-instrument,cgscc(inline),post-inline-ee-instrument" -S < %s | FileCheck %s
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
Index: llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
===
--- llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
+++ llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
@@ -176,9 +176,12 @@
   return new PostInlineEntryExitInstrumenter();
 }
 
-PreservedAnalyses
-llvm::EntryExitInstrumenterPass::run(Function , FunctionAnalysisManager ) {
-  runOnFunction(F, PostInlining);
+PreservedAnalyses llvm::EntryExitInstrumenterPass::run(Module ,
+   ModuleAnalysisManager ) {
+  for (Function  : M) {
+runOnFunction(F, PostInlining);
+  }
+
   PreservedAnalyses PA;
   PA.preserveSet();
   return PA;
Index: llvm/lib/Passes/PassRegistry.def
===
--- llvm/lib/Passes/PassRegistry.def
+++ llvm/lib/Passes/PassRegistry.def
@@ -120,6 +120,8 @@
 MODULE_PASS("memprof-module", ModuleMemProfilerPass())
 MODULE_PASS("poison-checking", PoisonCheckingPass())
 

[PATCH] D93110: [analyzer] Implement fine-grained suppressions via attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D93110#2560095 , @vsavchenko wrote:

> In D93110#2534690 , @aaron.ballman 
> wrote:
>
>> In D93110#2529917 , @NoQ wrote:
>>
 What I want to avoid is having a static analyzer-specific suppression 
 attribute, and a different one for clang-tidy, and a third one for the 
 frontend.
>>>
>>> Given that there are already multiple suppression mechanisms circulating 
>>> around (clang-tidy's `// NOLINT`, frontend pragmas, static analyzer's 
>>> `#ifdef __clang_analyzer__`, now also attribute), i guess a path forward 
>>> would be to eventually add support for multiple mechanisms everywhere.  
>>> This may be tedious to implement but benefits may be well worth it. 
>>> Consistency across tools is important when tools are used together; for 
>>> instance, when clang-tidy integrates static analyzer, static analyzer 
>>> automatically starts supporting `// NOLINT` through clang-tidy's diagnostic 
>>> consumer magic. This would also be amazing for discoverability: users can 
>>> keep using their preferred mechanism and it'd just work out of the box when 
>>> they add a new tool to their arsenal.
>>
>> To be clear, I'm fine with multiple mechanisms, but not fine with multiple 
>> of the same mechanisms. e.g., I don't think it's an issue that we can 
>> suppress via the command line, pragmas, NOLINT comments, and attribute 
>> usage, but I do think it's a problem if there's one attribute used by 
>> clang-tidy and a different attribute used by the static analyzer, and a 
>> third attribute for clang itself. Using different attributes causes users to 
>> have to consider what basically amount to implementation details of their 
>> toolchain and makes it harder to do things like integrate clang-tidy and the 
>> clang static analyzer together.
>
> We can integrate these solutions together for clang-tidy and for clang static 
> analyzer, it's not a problem at all.  I would've used the existing Suppress 
> attribute if it was without `gsl` part.

Sorry, I failed at being clear. Let me take another stab at it.


I don't want multiple unrelated semantic attributes to handle this concept. We 
can have a single semantic attribute with multiple ways of spelling it (e.g., 
`[[gsl::suppress]]` and `[[clang::suppress]]`) easily enough. I want to avoid 
code that looks like `if (const auto *A = D->getAttr()) { ... } 
else if (const auto *A = D->getAttr()) { ... }` because 
this causes maintenance problems where someone invariably forgets the less-used 
suppression attribute somewhere. (I would be fine if we had a common base class 
used by multiple suppression semantic attributes, if that was necessary to 
avoid this problem. )

I also don't want multiple attribute spellings for different parts of the 
toolchain. e.g., a user should not have to write `[[clang::suppress(...)]]` for 
a frontend suppression and `[[tidy::suppress(...)]]` for a tidy suppression, 
and `[[csa::suppress(...)]]` for a static analyzer suppression. This leaks 
implementation details out to users that they're not likely to have insights 
into anyway and it causes problems when we shift a diagnostic around or combine 
tools (like the static analyzer and tidy being able to run one another's 
checks).

I don't think it's an issue to have multiple attribute spellings to support 
different coding style guidelines or to be compatible with another compiler 
because users will be able to reason about those annotations more easily than 
which part of the toolchain a diagnostic comes from. So I don't think it's an 
issue to have `[[gsl::suppress(...)]]` and `[[clang::suppress(...)]]` as 
spellings so long as we wind up with a single semantic attribute interface 
under the hood.


> As I mentioned in the summary, at the moment, we want to use it ONLY inside 
> functions.  I can see that `SuppressAttr` is not used anywhere in the whole, 
> but I can't be sure that it's not used somewhere externally.  So, I couldn't 
> just reduce the scope of it.

We have the notion of attribute accessors to help with this sort of thing. It's 
reasonable to add an accessor to see whether the attribute was spelled with the 
gsl spelling, and if so, make alternative diagnostic decisions from it. e.g. 
https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/Attr.td#L650


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93110

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

[PATCH] D96597: [DebugInfo] Keep the DWARF64 flag in the module metadata

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:396
+  bool Dwarf64 =
+  (Asm->TM.Options.MCOptions.Dwarf64 || MMI->getModule()->isDwarf64()) &&
+  DwarfVersion >= 3 &&   // DWARF64 was introduced in DWARFv3.

ikudrin wrote:
> dblaikie wrote:
> > Once this patch is accepted, it's probably worth going back and removing 
> > the MCOption? (I don't think we have MCOptions for other debug related 
> > module metadata, like the DWARF version?)
> Exactly for the DWARF version, there is a similar option, please take a look 
> at line 388.
> As for the DWARF64 option here, it only simplifies testing with tools like 
> `llc`, `opt`, etc. Originally, `MCTargetOptions::Dwarf64` was aimed to adjust 
> generating in the assembler, but the corresponding command-line flag is 
> visible in other tools, too. Do you think it is worth it to move the command 
> line option from the common library code to specific tools?
Oh, nah, that's fine to keep around like the DWARF version for LLVM testing 
purposes (easier to write a test with a single .ll file and different RUN lines 
to test both 32 and 64 behavior, without having to have two IR Files, or 
rewrite the IR file to switch its format, etc)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96597

___
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

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

@hoy Could you explain a bit further why these two features are 
incompatible/what the crash looks like? At first glance I wouldn't expect any 
debug info mode to be incompatible with any non-debug-info mode (maybe less 
useful, but not crashy levels of incompatible).


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] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

[Abhinav Gaba via Phabricator via cfe-commits]

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

Thanks for the changes, Alexey.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86119

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

[PATCH] D96244: [clangd] Introduce Modules

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

"modules" is a pretty overloaded term in Clang (between llvm Modules and Clang 
Modules and now C++ standard Modules) - any chance of some other term that 
might be less overloaded in this space?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96244

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

[PATCH] D96783: [Driver] Support -gdwarf64 for assembly files

[Alexander Yermolovich via Phabricator via cfe-commits]

ayermolo added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96783

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

[PATCH] D96802: [Clang] Add proper target checks for SwiftAsync convention.

[Varun Gandhi via Phabricator via cfe-commits]

varungandhi-apple created this revision.
varungandhi-apple added a reviewer: rjmccall.
Herald added subscribers: dexonsmith, pengfei, kbarton, kristof.beyls, 
jgravelle-google, sbc100, nemanjai, dschuff.
varungandhi-apple requested review of this revision.
Herald added subscribers: cfe-commits, aheejin.
Herald added a project: clang.

Previously, we erroneously claimed that all targets were supported.
However, at the moment, only x86_64 and ARM are known to be working.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96802

Files:
  clang/include/clang/Basic/Features.def
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/ARM.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Basic/Targets/SystemZ.h
  clang/lib/Basic/Targets/WebAssembly.h
  clang/lib/Basic/Targets/X86.h

Index: clang/lib/Basic/Targets/X86.h
===
--- clang/lib/Basic/Targets/X86.h
+++ clang/lib/Basic/Targets/X86.h
@@ -344,11 +344,12 @@
 case CC_C:
 case CC_PreserveMost:
 case CC_Swift:
-case CC_SwiftAsync:
 case CC_X86Pascal:
 case CC_IntelOclBicc:
 case CC_OpenCLKernel:
   return CCCR_OK;
+case CC_SwiftAsync:
+  return checkSwiftAsyncCCSupported();
 default:
   return CCCR_Warning;
 }
@@ -703,7 +704,6 @@
 switch (CC) {
 case CC_C:
 case CC_Swift:
-case CC_SwiftAsync:
 case CC_X86VectorCall:
 case CC_IntelOclBicc:
 case CC_Win64:
@@ -712,6 +712,8 @@
 case CC_X86RegCall:
 case CC_OpenCLKernel:
   return CCCR_OK;
+case CC_SwiftAsync:
+  return checkSwiftAsyncCCSupported();
 default:
   return CCCR_Warning;
 }
@@ -785,10 +787,11 @@
 case CC_PreserveAll:
 case CC_X86_64SysV:
 case CC_Swift:
-case CC_SwiftAsync:
 case CC_X86RegCall:
 case CC_OpenCLKernel:
   return CCCR_OK;
+case CC_SwiftAsync:
+  return checkSwiftAsyncCCSupported();
 default:
   return CCCR_Warning;
 }
Index: clang/lib/Basic/Targets/WebAssembly.h
===
--- clang/lib/Basic/Targets/WebAssembly.h
+++ clang/lib/Basic/Targets/WebAssembly.h
@@ -129,8 +129,9 @@
 switch (CC) {
 case CC_C:
 case CC_Swift:
-case CC_SwiftAsync:
   return CCCR_OK;
+case CC_SwiftAsync:
+  return checkSwiftAsyncCCSupported();
 default:
   return CCCR_Warning;
 }
Index: clang/lib/Basic/Targets/SystemZ.h
===
--- clang/lib/Basic/Targets/SystemZ.h
+++ clang/lib/Basic/Targets/SystemZ.h
@@ -141,9 +141,10 @@
 switch (CC) {
 case CC_C:
 case CC_Swift:
-case CC_SwiftAsync:
 case CC_OpenCLKernel:
   return CCCR_OK;
+case CC_SwiftAsync:
+  return checkSwiftAsyncCCSupported();
 default:
   return CCCR_Warning;
 }
Index: clang/lib/Basic/Targets/PPC.h
===
--- clang/lib/Basic/Targets/PPC.h
+++ clang/lib/Basic/Targets/PPC.h
@@ -450,8 +450,9 @@
   CallingConvCheckResult checkCallingConvention(CallingConv CC) const override {
 switch (CC) {
 case CC_Swift:
-case CC_SwiftAsync:
   return CCCR_OK;
+case CC_SwiftAsync:
+  return checkSwiftAsyncCCSupported();
 default:
   return CCCR_Warning;
 }
Index: clang/lib/Basic/Targets/ARM.cpp
===
--- clang/lib/Basic/Targets/ARM.cpp
+++ clang/lib/Basic/Targets/ARM.cpp
@@ -1120,9 +1120,10 @@
   case CC_AAPCS:
   case CC_AAPCS_VFP:
   case CC_Swift:
-  case CC_SwiftAsync:
   case CC_OpenCLKernel:
 return CCCR_OK;
+  case CC_SwiftAsync:
+return checkSwiftAsyncCCSupported();
   default:
 return CCCR_Warning;
   }
@@ -1200,8 +1201,9 @@
   case CC_PreserveMost:
   case CC_PreserveAll:
   case CC_Swift:
-  case CC_SwiftAsync:
 return CCCR_OK;
+  case CC_SwiftAsync:
+return checkSwiftAsyncCCSupported();
   default:
 return CCCR_Warning;
   }
Index: clang/lib/Basic/Targets/AArch64.cpp
===
--- clang/lib/Basic/Targets/AArch64.cpp
+++ clang/lib/Basic/Targets/AArch64.cpp
@@ -537,13 +537,14 @@
   switch (CC) {
   case CC_C:
   case CC_Swift:
-  case CC_SwiftAsync:
   case CC_PreserveMost:
   case CC_PreserveAll:
   case CC_OpenCLKernel:
   case CC_AArch64VectorCall:
   case CC_Win64:
 return CCCR_OK;
+  case CC_SwiftAsync:
+return checkSwiftAsyncCCSupported();
   default:
 return CCCR_Warning;
   }
@@ -811,9 +812,10 @@
   case CC_PreserveMost:
   case CC_PreserveAll:
   case CC_Swift:
-  case CC_SwiftAsync:
   case CC_Win64:
 return CCCR_OK;
+  case CC_SwiftAsync:
+return checkSwiftAsyncCCSupported();
   default:
 return CCCR_Warning;
   }
Index: clang/include/clang/Basic/TargetInfo.h

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

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

This looks like a straightforward implementation. The only caveat is that the 
XAR immediate does not represent a lane, and hence the need for a custom 
immediate range check. Looks sensible to me.
@labrinea   and others at ARM, do have any other comment before this is merged?


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] D96783: [Driver] Support -gdwarf64 for assembly files

[David Blaikie via Phabricator via cfe-commits]

dblaikie accepted this revision.
dblaikie added a comment.

Looks good to me - thanks




Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3757-3771
+  if (DwarfFormatArg &&
+  DwarfFormatArg->getOption().matches(options::OPT_gdwarf64)) {
+if (DwarfVersion < 3)
+  D.Diag(diag::err_drv_argument_only_allowed_with)
+  << DwarfFormatArg->getAsString(Args) << "DWARFv3 or greater";
+else if (!T.isArch64Bit())
+  D.Diag(diag::err_drv_argument_only_allowed_with)

Could simplify this with an early return:
```
if (!DwarfFormatArg)
  return;
...
```

So DwarfFormatArg isn't tested twice/reduces indentation/etc.



Comment at: clang/tools/driver/cc1as_main.cpp:237
+  if (auto *DwarfFormatArg = Args.getLastArg(OPT_gdwarf64, OPT_gdwarf32))
+Opts.Dwarf64 = DwarfFormatArg->getOption().matches(OPT_gdwarf64) ? 1 : 0;
   Opts.DwarfVersion = getLastArgIntValue(Args, OPT_dwarf_version_EQ, 2, Diags);

No need for the ` ? 1 : 0`, that's what booleans convert to anyway. (unless 
there's prior art/other instances of this idiom in this file that this is 
consistent with)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96783

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

[PATCH] D88631: [X86] Support customizing stack protector guard

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.
Herald added a subscriber: jansvoboda11.

Looks like there's also a `-mstack-protector-guard-symbol=` which Linux kernel 
developers are looking to use for 32b X86 kernels: 
https://bugs.llvm.org/show_bug.cgi?id=49209.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88631

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

[PATCH] D67429: Improve code generation for thread_local variables:

[Wolfgang Pieb via Phabricator via cfe-commits]

wolfgangp added a comment.

Ping. Just wondering if there are any new insights on the issue reported in 
PR48030 .


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67429

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

[PATCH] D92024: [clang] Implement P0692R1 from C++20 (access checking on specializations and instantiations)

[Alex Orlov via Phabricator via cfe-commits]

aorlov added a comment.

Ping! Please, don't pass by this patch. I need your competent evaluation to 
load it!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92024

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

[PATCH] D95246: [test] Use host platform specific error message substitution in lit tests

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

In D95246#2565351 , 
@abhina.sreeskantharajan wrote:

> 



> Do you know what is different between your environments which is causing the 
> spelling to be capitalized? This might help me determine how to fix the 
> current host platform check in llvm/utils/lit/lit/llvm/config.py.

I'd love to help you. I'm using ordinary Win10. Here is my python output:

  >>> os.name
  'nt'
  >>> sys.platform
  'win32'
  >>> platform.system()
  'Windows'
  >>> platform.release()
  '10'
  >>>

If you would say what exact info you need I would execute and send you.
BTW my python3 uses //Sentence case capitalization// in errors as you can see:

  >>> open('some')
  Traceback (most recent call last):
File "", line 1, in 
  FileNotFoundError: [Errno 2] No such file or directory: 'some'
  >>>




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95246

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

[PATCH] D96717: [clangd] Bind outgoing calls through LSPBinder too. NFC

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks! can't wait for the unique_function sfinae fix :)




Comment at: clang-tools-extra/clangd/LSPBinder.h:185
+
+LSPBinder::UntypedOutgoingNotification inline LSPBinder::outgoingNotification(
+llvm::StringLiteral Method) {

maybe put `inline` at the start of the declaration ? :D



Comment at: clang-tools-extra/clangd/LSPBinder.h:88
+  template 
+  void outgoingMethod(llvm::StringLiteral Method,
+  OutgoingMethod ) {

sammccall wrote:
> kadircet wrote:
> > well this one filling an out-parameter definitely tripped me over while 
> > reading the code a couple times, and once more on the tests.
> > 
> > what about making this return the handler instead? E.g. `Edit = 
> > Bind.outgoingMethod("edit")`, I know it is ugly 
> > that we duplicate template params on declaration + here now. Maybe we can 
> > get away by making `OutgoingMethod` a class with a call operator and a 
> > `RawOutgoing *Out` member that can be bound later on ? e.g:
> > 
> > ```
> > template 
> > class OutgoingMethod {
> >   RawOutgoing *Out = nullptr;
> > public:
> >   void operator()(const P& Params, Callback CB) {
> > assert(Out && "Method haven't bound");
> > Out->callMethod(Method, JSON(Params), );
> >   }
> > };
> > ```
> > 
> > then we either make LSPBinder a friend of OutgoingMethod and set Out 
> > through it, or the other way around and have a 
> > `OutgoingMethod::bindOut(LSPBinder&)` ?
> > 
> > not sure if it is any better though, as we still construct a somewhat 
> > invalid object :/
> > well this one filling an out-parameter definitely tripped me over while 
> > reading the code a couple times, and once more on the tests.
> 
> Yup. My feeling is the wins are:
>  - avoid writing the types over and over (we can't really avoid it on the 
> instance variable)
>  - resembles syntax for binding incoming things
> 
> And the main loss is definitely that we're not using assignment syntax for 
> assignment.
> 
> > what about making this return the handler instead? E.g. Edit = 
> > Bind.outgoingMethod("edit")
> 
> This is a reasonable alternative, as you say the duplication is the downside.
> 
> > Maybe we can get away by making OutgoingMethod a class with a call operator
> 
> Yeah, I don't really understand what this achieves - the usage looks the 
> same, just now the logical assignment doesn't use assignment syntax *or* 
> assignment semantics :-)
> 
> Other ideas I had:
>  - the object is untyped, and the call operator is templated/typed - no 
> type-safety at callsite, yuck
>  - `outgoingMethod` returns an untyped proxy object with a template 
> conversion operator to unique_function. This gives us assignment 
> syntax... and horrible compiler errors on mismatch.
> 
> I'm going to try the latter out to see how bad it is.
thanks! this looks a whole lot better!



Comment at: clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp:237
+  Value += X;
+  Changed(Value);
+}

sammccall wrote:
> whoops, after rebasing against 40cc63ea6eec7874d3a358f9fa549ef2f6543512 this 
> uncovered a bug - we're testing the public API from the wrong thread (server 
> thread != main thread).
> 
> So I've had to remove that test in this patch. We can test public APIs again 
> in D96755.
yikes, i see what you mean now.. (i was searching for the failure in the 
previous commit you mentioned)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96717

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

[clang] 1a323c8 - [analyzer] Fix a warning

[Kazu Hirata via cfe-commits]

Author: Kazu Hirata
Date: 2021-02-16T09:12:07-08:00
New Revision: 1a323c8a96afc53ef965a4268cd419cfde2f1890

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

LOG: [analyzer] Fix a warning

This patch fixes a warning from -Wcovered-switch-default.  The switch
statement in question handles all the enum values.

Added: 


Modified: 
clang/lib/StaticAnalyzer/Core/SValBuilder.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp 
b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
index 4b146d83c194..a47a28e1e866 100644
--- a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -560,9 +560,9 @@ SVal SValBuilder::evalCast(SVal V, QualType CastTy, 
QualType OriginalTy) {
 return evalCastKind(V.castAs(), CastTy, OriginalTy);
   case SVal::NonLocKind:
 return evalCastKind(V.castAs(), CastTy, OriginalTy);
-  default:
-llvm_unreachable("Unknown SVal kind");
   }
+
+  llvm_unreachable("Unknown SVal kind");
 }
 
 SVal SValBuilder::evalCastKind(UndefinedVal V, QualType CastTy,



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

[PATCH] D76342: [OpenMP] Implement '#pragma omp tile'

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D76342#2565945 , @Meinersbur wrote:

> In D76342#2565686 , @ABataev wrote:
>
>> I can commit it as soon as you accepted it if you don't mind of course.
>
> I do not mind

Ok, will commit it in a few minutes, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76342

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

[PATCH] D76342: [OpenMP] Implement '#pragma omp tile'

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added a comment.

In D76342#2565686 , @ABataev wrote:

> I can commit it as soon as you accepted it if you don't mind of course.

I do not mind


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76342

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

[PATCH] D87519: [analyzer][Liveness][NFC] Enqueue the CFGBlocks post-order

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

What about analyzing a Sema translation unit? That should be beefy enough to 
have a longer runtime.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87519

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

[PATCH] D96717: [clangd] Bind outgoing calls through LSPBinder too. NFC

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 324025.
sammccall added a comment.

OK, the awful proxy object thing works :-)
It requires a SFINAE fix to unique_function though, which i'll send separately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96717

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/LSPBinder.h
  clang-tools-extra/clangd/Module.h
  clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
  clang-tools-extra/clangd/unittests/LSPBinderTests.cpp

Index: clang-tools-extra/clangd/unittests/LSPBinderTests.cpp
===
--- clang-tools-extra/clangd/unittests/LSPBinderTests.cpp
+++ clang-tools-extra/clangd/unittests/LSPBinderTests.cpp
@@ -15,12 +15,15 @@
 namespace clangd {
 namespace {
 
+using testing::ElementsAre;
 using testing::HasSubstr;
+using testing::IsEmpty;
 using testing::UnorderedElementsAre;
 
 // JSON-serializable type for testing.
 struct Foo {
   int X;
+  friend bool operator==(Foo A, Foo B) { return A.X == B.X; }
 };
 bool fromJSON(const llvm::json::Value , Foo , llvm::json::Path P) {
   return fromJSON(V, F.X, P.field("X"));
@@ -35,9 +38,31 @@
   return [](llvm::Expected V) { Out.emplace(std::move(V)); };
 }
 
+struct OutgoingRecorder : public LSPBinder::RawOutgoing {
+  llvm::StringMap> Received;
+
+  void callMethod(llvm::StringRef Method, llvm::json::Value Params,
+  Callback Reply) override {
+Received[Method].push_back(Params);
+if (Method == "fail")
+  return Reply(error("Params={0}", Params));
+Reply(Params); // echo back the request
+  }
+  void notify(llvm::StringRef Method, llvm::json::Value Params) override {
+Received[Method].push_back(std::move(Params));
+  }
+
+  std::vector take(llvm::StringRef Method) {
+std::vector Result = Received.lookup(Method);
+Received.erase(Method);
+return Result;
+  }
+};
+
 TEST(LSPBinderTest, IncomingCalls) {
   LSPBinder::RawHandlers RawHandlers;
-  LSPBinder Binder{RawHandlers};
+  OutgoingRecorder RawOutgoing;
+  LSPBinder Binder{RawHandlers, RawOutgoing};
   struct Handler {
 void plusOne(const Foo , Callback Reply) {
   Reply(Foo{Params.X + 1});
@@ -94,7 +119,45 @@
   RawCmdPlusOne(1, capture(Reply));
   ASSERT_TRUE(Reply.hasValue());
   EXPECT_THAT_EXPECTED(Reply.getValue(), llvm::HasValue(2));
+
+  // None of this generated any outgoing traffic.
+  EXPECT_THAT(RawOutgoing.Received, IsEmpty());
+}
+
+TEST(LSPBinderTest, OutgoingCalls) {
+  LSPBinder::RawHandlers RawHandlers;
+  OutgoingRecorder RawOutgoing;
+  LSPBinder Binder{RawHandlers, RawOutgoing};
+
+  LSPBinder::OutgoingMethod Echo;
+  Echo = Binder.outgoingMethod("echo");
+  LSPBinder::OutgoingMethod WrongSignature;
+  WrongSignature = Binder.outgoingMethod("wrongSignature");
+  LSPBinder::OutgoingMethod Fail;
+  Fail = Binder.outgoingMethod("fail");
+
+  llvm::Optional> Reply;
+  Echo(Foo{2}, capture(Reply));
+  EXPECT_THAT(RawOutgoing.take("echo"), ElementsAre(llvm::json::Value(2)));
+  ASSERT_TRUE(Reply.hasValue());
+  EXPECT_THAT_EXPECTED(Reply.getValue(), llvm::HasValue(Foo{2}));
+
+  // JSON response is integer, can't be parsed as string.
+  llvm::Optional> WrongTypeReply;
+  WrongSignature(Foo{2}, capture(WrongTypeReply));
+  EXPECT_THAT(RawOutgoing.take("wrongSignature"),
+  ElementsAre(llvm::json::Value(2)));
+  ASSERT_TRUE(Reply.hasValue());
+  EXPECT_THAT_EXPECTED(WrongTypeReply.getValue(),
+   llvm::FailedWithMessage(
+   HasSubstr("failed to decode wrongSignature reply")));
+
+  Fail(Foo{2}, capture(Reply));
+  EXPECT_THAT(RawOutgoing.take("fail"), ElementsAre(llvm::json::Value(2)));
+  ASSERT_TRUE(Reply.hasValue());
+  EXPECT_THAT_EXPECTED(Reply.getValue(), llvm::FailedWithMessage("Params=2"));
 }
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
===
--- clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -23,6 +23,7 @@
 namespace clang {
 namespace clangd {
 namespace {
+using testing::ElementsAre;
 
 MATCHER_P(DiagMessage, M, "") {
   if (const auto *O = arg.getAsObject()) {
@@ -223,16 +224,20 @@
 
 TEST_F(LSPTest, ModulesTest) {
   class MathModule final : public Module {
+OutgoingNotification Changed;
 void initializeLSP(LSPBinder , const llvm::json::Object ,
llvm::json::Object ) override {
   Bind.notification("add", this, ::add);
   Bind.method("get", this, ::get);
+  Changed = Bind.outgoingNotification("changed");
 }
 
 int Value = 0;
 
-  public:
-void add(const int ) { Value += X; }
+void add(const int ) {
+  Value += X;
+  

[PATCH] D96771: [OpenCL] Add distinct file extension for C++ for OpenCL

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh added inline comments.



Comment at: clang/lib/Driver/Types.cpp:265
.Case("cl", TY_CL)
+   .Case("clcpp", TY_CLCXX)
.Case("cp", TY_CXX)

mantognini wrote:
> I'm not sure we want that -- I'm actually fine if we don't -- but I see below 
> `c++` and `cxx` are supported in addition to `cpp`. Should we therefore also 
> have `clc++` and `clcxx` as file valid extensions for consistency? I wonder 
> what the general opinion is.
And there is also `.cc` above.  Since you're asking for opinions, my personal 
opinion is that one extension should be enough, and providing a CL counterpart 
for all existing C++ file extensions does not bring more value.


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

https://reviews.llvm.org/D96771

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

[PATCH] D95790: [clang][cli] Documentation of CompilerInvocation parsing/generation

[Michael Spencer via Phabricator via cfe-commits]

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

Needs an example in the "Creating new Command Line Option" section, but with 
that it looks good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95790

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

[PATCH] D96769: [OpenMP][AMDGPU] Skip backend and assemble phases for amdgcn

[Pushpinder Singh via Phabricator via cfe-commits]

pdhaliwal added a comment.

It is because of how addClangTargetOptions is invoked. In case of save-temps, 
it is being invoked for all the actions resulting in target cc1 call. That's 
why all these invocations have -emit-llvm-bc. I guess we need Action as an 
argument to addClangTargetOptions.

Also, it does not make sense for having assemble and backend action for amdgcn 
as linker is dependent directly on llvm IR. They will also come up redundantly 
in the -ccc-print-phases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96769

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

[PATCH] D94673: [analyzer][CTU] API for CTU macro expansions

[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.

LGTM! You may wanna wait for someone more knowledgeable about CTU to give it 
another accept.


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

https://reviews.llvm.org/D94673

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

[PATCH] D96771: [OpenCL] Add distinct file extension for C++ for OpenCL

[Marco Antognini via Phabricator via cfe-commits]

mantognini added a comment.

I feel this would be a valuable addition, indeed. Only a minor question from me.




Comment at: clang/lib/Driver/Types.cpp:265
.Case("cl", TY_CL)
+   .Case("clcpp", TY_CLCXX)
.Case("cp", TY_CXX)

I'm not sure we want that -- I'm actually fine if we don't -- but I see below 
`c++` and `cxx` are supported in addition to `cpp`. Should we therefore also 
have `clc++` and `clcxx` as file valid extensions for consistency? I wonder 
what the general opinion is.


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

https://reviews.llvm.org/D96771

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

[PATCH] D96716: [flang][driver] Add debug dump options

[Faris Rehman via Phabricator via cfe-commits]

FarisRehman added inline comments.



Comment at: flang/include/flang/Frontend/FrontendActions.h:54
+public:
+  std::unique_ptr semantics_;
 };

awarzynski wrote:
> Do we need to expose it? Shouldn't this be private?
It will not compile if the variable is private as it needs to be referenced in 
the implementation of ExecuteAction, e.g. 
`DebugDumpParseTreeAction::ExecuteAction()`
It will compile if set to `protected` or `public`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96716

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

[PATCH] D88220: [C++20] P1825R0: More implicit moves

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

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

> either way I'm a dead end — I don't consider myself authorized to "accept" 
> Clang changes.
> You're still going to have to attract the attention of someone with the 
> authority and/or force-of-will to say "okay ship it."

Well, it's been a month since I said that. I'll land this tonight, unless 
someone yells "stop" before then. If it causes any problems, we can revert it 
easy enough.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88220

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

[PATCH] D96769: [OpenMP][AMDGPU] Skip backend and assemble phases for amdgcn

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D96769#2565751 , @pdhaliwal wrote:

> emit-llvm-bc does not correctly solve the problem. It works because [input, 
> compile, assemble, backend] actions collapse to a single action by driver. 
> This single command handles emit-llvm-bc properly. But when save-temps is 
> specified, this collapsing does not happen which messes up command line flags 
> of the jobs and hence the output, for e.g., preprocessor command also has 
> -emit-llvm-bc.

Is this the intended behavior of -save-temps + -emit-llvm-bc or an accident? 
What exactly happens when both are specified and what is the problem. Maybe we 
should fix that instead of working around it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96769

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

[PATCH] D96719: [clang-tidy] Add new check 'bugprone-thread-canceltype-asynchronous' and alias 'cert-pos47-c'.

[Balázs Kéri via Phabricator via cfe-commits]

balazske updated this revision to Diff 324013.
balazske added a comment.

Adding missed alias to CERT module.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96719

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/ThreadCanceltypeAsynchronousCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/ThreadCanceltypeAsynchronousCheck.h
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-thread-canceltype-asynchronous.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-pos47-c.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-thread-canceltype-asynchronous.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-thread-canceltype-asynchronous.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-thread-canceltype-asynchronous.cpp
@@ -0,0 +1,32 @@
+// RUN: %check_clang_tidy %s bugprone-thread-canceltype-asynchronous %t
+
+#define PTHREAD_CANCEL_DEFERRED 0
+#define PTHREAD_CANCEL_ASYNCHRONOUS 1
+
+int pthread_setcanceltype(int type, int *oldtype);
+
+int main() {
+  int result, oldtype;
+
+  if ((result = pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, )) != 0) {
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: asynchronous cancelability type should not be used [bugprone-thread-canceltype-asynchronous]
+return 1;
+  }
+
+  if ((result = pthread_setcanceltype(PTHREAD_CANCEL_DEFERRED, )) != 0) {
+return 1;
+  }
+
+  return 0;
+}
+
+int f() {
+  int result, oldtype;
+
+  if ((result = pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, )) != 0) {
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: asynchronous cancelability type should not be used [bugprone-thread-canceltype-asynchronous]
+return 1;
+  }
+
+  return 0;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -95,6 +95,7 @@
`bugprone-suspicious-string-compare `_, "Yes"
`bugprone-swapped-arguments `_, "Yes"
`bugprone-terminating-continue `_, "Yes"
+   `bugprone-thread-canceltype-asynchronous `_, "No"
`bugprone-throw-keyword-missing `_,
`bugprone-too-small-loop-variable `_,
`bugprone-undefined-memory-manipulation `_,
@@ -334,6 +335,7 @@
`cert-oop11-cpp `_, `performance-move-constructor-init `_, "Yes"
`cert-oop54-cpp `_, `bugprone-unhandled-self-assignment `_,
`cert-pos44-c `_, `bugprone-bad-signal-to-kill-thread `_,
+   `cert-pos47-c `_, `bugprone-thread-canceltype-asynchronous `_,
`cert-str34-c `_, `bugprone-signed-char-misuse `_,
`clang-analyzer-core.CallAndMessage `_, `Clang Static Analyzer `_,
`clang-analyzer-core.DivideZero `_, `Clang Static Analyzer `_,
Index: clang-tools-extra/docs/clang-tidy/checks/cert-pos47-c.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/cert-pos47-c.rst
@@ -0,0 +1,9 @@
+.. title:: clang-tidy - cert-pos47-c
+.. meta::
+   :http-equiv=refresh: 5;URL=bugprone-thread-canceltype-asynchronous.html
+
+cert-pos47-c
+
+
+The cert-pos47-c check is an alias, please see
+`bugprone-thread-canceltype-asynchronous `_ for more information.
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-thread-canceltype-asynchronous.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-thread-canceltype-asynchronous.rst
@@ -0,0 +1,19 @@
+.. title:: clang-tidy - bugprone-thread-canceltype-asynchronous
+
+bugprone-thread-canceltype-asynchronous
+===
+
+Finds ``pthread_setcanceltype`` function calls where a thread's cancellation
+type is set to asynchronous. Asynchronous cancellation type
+(``PTHREAD_CANCEL_ASYNCHRONOUS``) is generally unsafe, use type
+``PTHREAD_CANCEL_DEFERRED`` instead which is the default. Even with deferred
+cancellation, a cancellation point in an asynchronous signal handler may still
+be acted upon and the effect is as if it was an asynchronous cancellation.
+
+.. code-block: c++
+
+  pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, );
+
+This check corresponds to the CERT C Coding Standard rule
+`POS47-C. Do not use threads that can be canceled asynchronously
+`_.
Index: clang-tools-extra/docs/ReleaseNotes.rst