[PATCH] D59135: Add check for matching HeaderFilter before emitting Diagnostic

[Thorsten via Phabricator via cfe-commits]

thorsten-klein created this revision.
thorsten-klein added reviewers: aaron.ballman, klimek, alexfh.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fixed issue of not considering HeaderFilter which resulted in findings although 
they should be filtered out.
Patch by Thorsten Klein.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59135

Files:
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp


Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -449,8 +449,13 @@
   FullSourceLoc Loc;
   if (Info.getLocation().isValid() && Info.hasSourceManager())
 Loc = FullSourceLoc(Info.getLocation(), Info.getSourceManager());
-  Converter.emitDiagnostic(Loc, DiagLevel, Message, Info.getRanges(),
+
+  StringRef FileName(Loc.printToString(Loc.getManager()));
+  if(getHeaderFilter()->match(FileName)) // only emit if FileName is matching
+  {
+Converter.emitDiagnostic(Loc, DiagLevel, Message, Info.getRanges(),
Info.getFixItHints());
+  }
 
   if (Info.hasSourceManager())
 checkFilters(Info.getLocation(), Info.getSourceManager());


Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -449,8 +449,13 @@
   FullSourceLoc Loc;
   if (Info.getLocation().isValid() && Info.hasSourceManager())
 Loc = FullSourceLoc(Info.getLocation(), Info.getSourceManager());
-  Converter.emitDiagnostic(Loc, DiagLevel, Message, Info.getRanges(),
+
+  StringRef FileName(Loc.printToString(Loc.getManager()));
+  if(getHeaderFilter()->match(FileName)) // only emit if FileName is matching
+  {
+Converter.emitDiagnostic(Loc, DiagLevel, Message, Info.getRanges(),
Info.getFixItHints());
+  }
 
   if (Info.hasSourceManager())
 checkFilters(Info.getLocation(), Info.getSourceManager());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59135: Add check for matching HeaderFilter before emitting Diagnostic

[Thorsten via Phabricator via cfe-commits]

thorsten-klein updated this revision to Diff 189859.

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

https://reviews.llvm.org/D59135

Files:
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp


Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -449,8 +449,13 @@
   FullSourceLoc Loc;
   if (Info.getLocation().isValid() && Info.hasSourceManager())
 Loc = FullSourceLoc(Info.getLocation(), Info.getSourceManager());
-  Converter.emitDiagnostic(Loc, DiagLevel, Message, Info.getRanges(),
+
+  StringRef FileName(Loc.printToString(Loc.getManager()));
+  if(getHeaderFilter()->match(FileName))
+  {
+Converter.emitDiagnostic(Loc, DiagLevel, Message, Info.getRanges(),
Info.getFixItHints());
+  }
 
   if (Info.hasSourceManager())
 checkFilters(Info.getLocation(), Info.getSourceManager());


Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -449,8 +449,13 @@
   FullSourceLoc Loc;
   if (Info.getLocation().isValid() && Info.hasSourceManager())
 Loc = FullSourceLoc(Info.getLocation(), Info.getSourceManager());
-  Converter.emitDiagnostic(Loc, DiagLevel, Message, Info.getRanges(),
+
+  StringRef FileName(Loc.printToString(Loc.getManager()));
+  if(getHeaderFilter()->match(FileName))
+  {
+Converter.emitDiagnostic(Loc, DiagLevel, Message, Info.getRanges(),
Info.getFixItHints());
+  }
 
   if (Info.hasSourceManager())
 checkFilters(Info.getLocation(), Info.getSourceManager());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59135: Add check for matching HeaderFilter before emitting Diagnostic

[Thorsten via Phabricator via cfe-commits]

thorsten-klein added a comment.

Hello,
Can you please support how to do that? :-)


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

https://reviews.llvm.org/D59135



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

[PATCH] D59135: Add check for matching HeaderFilter before emitting Diagnostic

[Thorsten via Phabricator via cfe-commits]

thorsten-klein added a comment.

Sorry for delay. I am currently not able to build master (Error: "sort" is not 
a member of "llvm").
And if I try to build 6.0.1-rc3 I can build clang-tidy but I am not able to 
build corresponding unittests "ClangTidyTests".
I will try to solve as soon as possible.


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

https://reviews.llvm.org/D59135



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

[PATCH] D59135: Add check for matching HeaderFilter before emitting Diagnostic

[Thorsten via Phabricator via cfe-commits]

thorsten-klein added a comment.

Hello, 
can you please support with this Pull-Request? You really have better knowledge 
about source code. 
For our case this solution was working fine and I wanted to share with you. 
Maybe it will help you if I create an examplary project which reproduces the 
mentioned issue?




Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:454
+  StringRef FileName(Loc.printToString(Loc.getManager()));
+  if(getHeaderFilter()->match(FileName))
+  {

alexfh wrote:
> aaron.ballman wrote:
> > Formatting is incorrect here -- you should run the patch through 
> > clang-format 
> > (https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting),
> >  and elide the braces.
> I believe, this will break the handling of notes. If the notes attached to 
> the diagnostic are in the "interesting" part of the code (see the 
> checkFilters method below), the whole diagnostic should be treated as such.
Hello,
Unfortunately the emitDiagnostic method is called also for files, which should 
be actually excluded (since the FileName does not match the HeaderFilter 
regex). This results in diagnostic output, also if there should not be any. For 
this purpose I added this check here at this point. If the FileName is matching 
the HeaderFilter regex, then all diagnostic is emitted as before. 

Did you know about any issue regarding this? If this is not a working solution 
for you: do you have any other proposal?

PS: I have also read about this issue in another project: 
https://fuchsia.googlesource.com/peridot/+/HEAD/docs/lint.md
--> They say that they disable some clang-tidy checks as workaround


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

https://reviews.llvm.org/D59135



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

[PATCH] D59135: Add check for matching HeaderFilter before emitting Diagnostic

[Thorsten via Phabricator via cfe-commits]

thorsten-klein updated this revision to Diff 191415.

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

https://reviews.llvm.org/D59135

Files:
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp


Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -449,8 +449,12 @@
   FullSourceLoc Loc;
   if (Info.getLocation().isValid() && Info.hasSourceManager())
 Loc = FullSourceLoc(Info.getLocation(), Info.getSourceManager());
-  Converter.emitDiagnostic(Loc, DiagLevel, Message, Info.getRanges(),
+
+  StringRef FileName(Loc.printToString(Loc.getManager()));
+  if(getHeaderFilter()->match(FileName)) {
+Converter.emitDiagnostic(Loc, DiagLevel, Message, Info.getRanges(),
Info.getFixItHints());
+  }
 
   if (Info.hasSourceManager())
 checkFilters(Info.getLocation(), Info.getSourceManager());


Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -449,8 +449,12 @@
   FullSourceLoc Loc;
   if (Info.getLocation().isValid() && Info.hasSourceManager())
 Loc = FullSourceLoc(Info.getLocation(), Info.getSourceManager());
-  Converter.emitDiagnostic(Loc, DiagLevel, Message, Info.getRanges(),
+
+  StringRef FileName(Loc.printToString(Loc.getManager()));
+  if(getHeaderFilter()->match(FileName)) {
+Converter.emitDiagnostic(Loc, DiagLevel, Message, Info.getRanges(),
Info.getFixItHints());
+  }
 
   if (Info.hasSourceManager())
 checkFilters(Info.getLocation(), Info.getSourceManager());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55463: Introduce a source minimizer that reduces source to directives that might affect the dependency list for a compilation

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

Are you planning to do this recursively?
The minimizer does not help much for the following example, while Sema.h 
contains 10,000 lines of useless code.

  #include "clang/Sema/Sema.h"
  
  int foo() {}


Repository:
  rC Clang

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

https://reviews.llvm.org/D55463



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

[PATCH] D59135: Add check for matching HeaderFilter before emitting Diagnostic

[Thorsten via Phabricator via cfe-commits]

thorsten-klein added a comment.

Hello @alexfh ,
Let me extend your example

  $ cat a.cc 
  #include "b.h"
  #include "d.h"
  int main(){check(nullptr);}
  $ cat b.h 
  #include "c.h"
  inline void b() { c(/*y=*/42); }
  $ cat c.h 
  void c(int x);
  $ cat d.h 
  inline char* check(char* buffer)
  {
*buffer++=1; // Should be clang-analyzer-core.NullDereference
return buffer;
  }

Now an additional warning is found and shown (=not suppressed):

  $ clang-tidy -checks=-*,clang-*,bugprone-argument-comment a.cc --
  2 warnings generated.
  /home/default/Temp/clang-tidy-test/d.h:3:11: warning: Dereference of null 
pointer [clang-analyzer-core.NullDereference]
  *buffer++=1; // Should be clang-analyzer-core.NullDereference
   ^
  /home/default/Temp/clang-tidy-test/a.cc:3:12: note: Calling 'check'
  int main(){check(0);}
 ^
  /home/default/Temp/clang-tidy-test/d.h:3:3: note: Null pointer value stored 
to 'buffer'
  *buffer++=1; // Should be clang-analyzer-core.NullDereference
   ^
  /home/default/Temp/clang-tidy-test/d.h:3:11: note: Dereference of null pointer
  *buffer++=1; // Should be clang-analyzer-core.NullDereference
   ^
  Suppressed 1 warnings (1 in non-user code).
  Use -header-filter=.* to display errors from all non-system headers. Use 
-system-headers to display errors from system headers as well.

**How can I use -header-filter now?**

With -header-filter=b.h clang-tidy shows both warnings:

  $ clang-tidy -checks=-*,clang-*,bugprone-argument-comment a.cc 
-header-filter=b.h --
  2 warnings generated.
  /home/default/Temp/clang-tidy-test/b.h:2:21: warning: argument name 'y' in 
comment does not match parameter name 'x' [bugprone-argument-comment]
  inline void b() { c(/*y=*/42); }
  ^~
  /*x=*/
  /home/default/Temp/clang-tidy-test/c.h:1:12: note: 'x' declared here
  void c(int x);
 ^
  /home/default/Temp/clang-tidy-test/d.h:3:11: warning: Dereference of null 
pointer [clang-analyzer-core.NullDereference]
  *buffer++=1; // Should be clang-analyzer-core.NullDereference
   ^
  /home/default/Temp/clang-tidy-test/a.cc:3:12: note: Calling 'check'
  int main(){check(0);}
 ^
  /home/default/Temp/clang-tidy-test/d.h:3:3: note: Null pointer value stored 
to 'buffer'
  *buffer++=1; // Should be clang-analyzer-core.NullDereference
   ^
  /home/default/Temp/clang-tidy-test/d.h:3:11: note: Dereference of null pointer
  *buffer++=1; // Should be clang-analyzer-core.NullDereference
   ^

With -header-filter=c.h clang-tidy shows both warnings:

  $ clang-tidy -checks=-*,clang-*,bugprone-argument-comment a.cc 
-header-filter=c.h --
  2 warnings generated.
  /home/default/Temp/clang-tidy-test/b.h:2:21: warning: argument name 'y' in 
comment does not match parameter name 'x' [bugprone-argument-comment]
  inline void b() { c(/*y=*/42); }
  ^~
  /*x=*/
  /home/default/Temp/clang-tidy-test/c.h:1:12: note: 'x' declared here
  void c(int x);
 ^
  /home/default/Temp/clang-tidy-test/d.h:3:11: warning: Dereference of null 
pointer [clang-analyzer-core.NullDereference]
  *buffer++=1; // Should be clang-analyzer-core.NullDereference
   ^
  /home/default/Temp/clang-tidy-test/a.cc:3:12: note: Calling 'check'
  int main(){check(0);}
 ^
  /home/default/Temp/clang-tidy-test/d.h:3:3: note: Null pointer value stored 
to 'buffer'
  *buffer++=1; // Should be clang-analyzer-core.NullDereference
   ^
  /home/default/Temp/clang-tidy-test/d.h:3:11: note: Dereference of null pointer
  *buffer++=1; // Should be clang-analyzer-core.NullDereference
   ^

With -header-filter=c.h clang-tidy shows both warnings:

  $ clang-tidy -checks=-*,clang-*,bugprone-argument-comment a.cc 
-header-filter=d.h --
  2 warnings generated.
  /home/default/Temp/clang-tidy-test/d.h:3:11: warning: Dereference of null 
pointer [clang-analyzer-core.NullDereference]
  *buffer++=1; // Should be clang-analyzer-core.NullDereference
   ^
  /home/default/Temp/clang-tidy-test/a.cc:3:12: note: Calling 'check'
  int main(){check(nullptr);}
 ^
  /home/default/Temp/clang-tidy-test/d.h:3:3: note: Null pointer value stored 
to 'buffer'
  *buffer++=1; // Should be clang-analyzer-core.NullDereference
   ^
  /home/default/Temp/clang-tidy-test/d.h:3:11: note: Dereference of null pointer
  *buffer++=1; // Should be clang-analyzer-core.NullDereference
   ^
  Suppressed 1 warnings (1 in non-user code).
  Use -header-filter=.* to display errors from all non-system headers. Use 
-system-headers to display errors from system headers as well.

How can I suppress warning for my header file //**d.h**// so that only warning 
from //**b.h**// is shown?


CHANGES SINCE LAST 

[PATCH] D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

You could try:

  clangxx_host -Xclang -fdump-record-layouts %p/Inputs/layout.cpp 


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

https://reviews.llvm.org/D83008



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

[PATCH] D78903: [Driver] Add option -fproc-stat-report

[Thorsten via Phabricator via cfe-commits]

tschuett added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:83
 #include "llvm/Support/raw_ostream.h"
+#include 
 #include 

https://llvm.org/docs/CodingStandards.html#include-iostream-is-forbidden


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78903



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

[PATCH] D85102: [clang] improve diagnostics for misaligned and large atomics

[Thorsten via Phabricator via cfe-commits]

tschuett created this revision.
tschuett added a reviewer: rsmith.
Herald added a reviewer: jfb.
Herald added subscribers: cfe-commits, jfb.
Herald added a project: clang.
tschuett requested review of this revision.
Herald added a subscriber: dexonsmith.

"Listing the alignment and access size (== expected alignment) in the warning
seems like a good idea."

solves PR 46947

  struct Foo {
struct Bar {
  void * a;
  void * b;
};
Bar bar;
  };
  
  struct ThirtyTwo {
struct Large {
  void * a;
  void * b;
  void * c;
  void * d;
};
Large bar;
  };
  
  void braz(Foo *foo, ThirtyTwo *braz) {
Foo::Bar bar;
__atomic_load(&foo->bar, &bar, __ATOMIC_RELAXED);
  
ThirtyTwo::Large foobar;
__atomic_load(&braz->bar, &foobar, __ATOMIC_RELAXED);
  }

repro.cpp:21:3: warning: misaligned atomic operation may incur significant 
performance penalty (the expected (16 bytes) exceeds the actual alignment (8 
bytes) [-Watomic-alignment]

  __atomic_load(&foo->bar, &bar, __ATOMIC_RELAXED);
  ^

repro.cpp:24:3: warning: misaligned atomic operation may incur significant 
performance penalty (the expected (32 bytes) exceeds the actual alignment (8 
bytes) [-Watomic-alignment]

  __atomic_load(&braz->bar, &foobar, __ATOMIC_RELAXED);
  ^

repro.cpp:24:3: warning: large atomic operation may incur significant 
performance penalty (the access size (32 bytes) exceeds the max lock-free size 
(16  bytes)) [-Watomic-alignment]
3 warnings generated.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85102

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/lib/CodeGen/CGAtomic.cpp


Index: clang/lib/CodeGen/CGAtomic.cpp
===
--- clang/lib/CodeGen/CGAtomic.cpp
+++ clang/lib/CodeGen/CGAtomic.cpp
@@ -807,10 +807,20 @@
   bool Oversized = getContext().toBits(sizeChars) > MaxInlineWidthInBits;
   bool Misaligned = (Ptr.getAlignment() % sizeChars) != 0;
   bool UseLibcall = Misaligned | Oversized;
+  CharUnits MaxInlineWidth =
+  getContext().toCharUnitsFromBits(MaxInlineWidthInBits);
 
-  if (UseLibcall) {
-CGM.getDiags().Report(E->getBeginLoc(), diag::warn_atomic_op_misaligned)
-<< !Oversized;
+  DiagnosticsEngine &Diags = CGM.getDiags();
+
+  if (Misaligned) {
+Diags.Report(E->getBeginLoc(), diag::warn_atomic_op_misaligned)
+<< (int)sizeChars.getQuantity()
+<< (int)Ptr.getAlignment().getQuantity();
+  }
+
+  if (Oversized) {
+Diags.Report(E->getBeginLoc(), diag::warn_atomic_op_oversized)
+<< (int)sizeChars.getQuantity() << (int)MaxInlineWidth.getQuantity();
   }
 
   llvm::Value *Order = EmitScalarExpr(E->getOrder());
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -699,6 +699,7 @@
 def Reorder : DiagGroup<"reorder", [ReorderCtor, ReorderInitList]>;
 def UndeclaredSelector : DiagGroup<"undeclared-selector">;
 def ImplicitAtomic : DiagGroup<"implicit-atomic-properties">;
+def AtomicAlignment : DiagGroup<"atomic-alignment">;
 def CustomAtomic : DiagGroup<"custom-atomic-properties">;
 def AtomicProperties : DiagGroup<"atomic-properties",
  [ImplicitAtomic, CustomAtomic]>;
Index: clang/include/clang/Basic/DiagnosticFrontendKinds.td
===
--- clang/include/clang/Basic/DiagnosticFrontendKinds.td
+++ clang/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -270,8 +270,16 @@
   "ifunc resolver function must return a pointer">;
 
 def warn_atomic_op_misaligned : Warning<
-  "%select{large|misaligned}0 atomic operation may incur "
-  "significant performance penalty">, InGroup>;
+  "misaligned atomic operation may incur "
+  "significant performance penalty "
+  "(the expected (%0 bytes) exceeds the actual alignment (%1 bytes)">,
+  InGroup;
+
+def warn_atomic_op_oversized : Warning<
+  "large atomic operation may incur "
+  "significant performance penalty "
+  "(the access size (%0 bytes) exceeds the max lock-free size (%1  bytes))">,
+InGroup;
 
 def warn_alias_with_section : Warning<
   "%select{alias|ifunc}1 will not be in section '%0' but in the same section "


Index: clang/lib/CodeGen/CGAtomic.cpp
===
--- clang/lib/CodeGen/CGAtomic.cpp
+++ clang/lib/CodeGen/CGAtomic.cpp
@@ -807,10 +807,20 @@
   bool Oversized = getContext().toBits(sizeChars) > MaxInlineWidthInBits;
   bool Misaligned = (Ptr.getAlignment() % sizeChars) != 0;
   bool UseLibcall = Misaligned | Oversized;
+  CharUnits MaxInlineWidth =
+  getContext().toCharUnitsFromBits(MaxInlineWidthInBits);
 
-  if (UseLibcall) {
-CGM.getDiags().Report(E->getBeginLoc(), diag::warn_atomic_op_misaligned)
-   

[PATCH] D85102: [clang] improve diagnostics for misaligned and large atomics

[Thorsten via Phabricator via cfe-commits]

tschuett updated this revision to Diff 282483.
tschuett added a comment.

Replaced (...) by ;


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

https://reviews.llvm.org/D85102

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/lib/CodeGen/CGAtomic.cpp


Index: clang/lib/CodeGen/CGAtomic.cpp
===
--- clang/lib/CodeGen/CGAtomic.cpp
+++ clang/lib/CodeGen/CGAtomic.cpp
@@ -807,10 +807,20 @@
   bool Oversized = getContext().toBits(sizeChars) > MaxInlineWidthInBits;
   bool Misaligned = (Ptr.getAlignment() % sizeChars) != 0;
   bool UseLibcall = Misaligned | Oversized;
+  CharUnits MaxInlineWidth =
+  getContext().toCharUnitsFromBits(MaxInlineWidthInBits);
 
-  if (UseLibcall) {
-CGM.getDiags().Report(E->getBeginLoc(), diag::warn_atomic_op_misaligned)
-<< !Oversized;
+  DiagnosticsEngine &Diags = CGM.getDiags();
+
+  if (Misaligned) {
+Diags.Report(E->getBeginLoc(), diag::warn_atomic_op_misaligned)
+<< (int)sizeChars.getQuantity()
+<< (int)Ptr.getAlignment().getQuantity();
+  }
+
+  if (Oversized) {
+Diags.Report(E->getBeginLoc(), diag::warn_atomic_op_oversized)
+<< (int)sizeChars.getQuantity() << (int)MaxInlineWidth.getQuantity();
   }
 
   llvm::Value *Order = EmitScalarExpr(E->getOrder());
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -699,6 +699,7 @@
 def Reorder : DiagGroup<"reorder", [ReorderCtor, ReorderInitList]>;
 def UndeclaredSelector : DiagGroup<"undeclared-selector">;
 def ImplicitAtomic : DiagGroup<"implicit-atomic-properties">;
+def AtomicAlignment : DiagGroup<"atomic-alignment">;
 def CustomAtomic : DiagGroup<"custom-atomic-properties">;
 def AtomicProperties : DiagGroup<"atomic-properties",
  [ImplicitAtomic, CustomAtomic]>;
Index: clang/include/clang/Basic/DiagnosticFrontendKinds.td
===
--- clang/include/clang/Basic/DiagnosticFrontendKinds.td
+++ clang/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -270,8 +270,16 @@
   "ifunc resolver function must return a pointer">;
 
 def warn_atomic_op_misaligned : Warning<
-  "%select{large|misaligned}0 atomic operation may incur "
-  "significant performance penalty">, InGroup>;
+  "misaligned atomic operation may incur "
+  "significant performance penalty"
+  "; the expected (%0 bytes) exceeds the actual alignment (%1 bytes)">,
+  InGroup;
+
+def warn_atomic_op_oversized : Warning<
+  "large atomic operation may incur "
+  "significant performance penalty"
+  "; the access size (%0 bytes) exceeds the max lock-free size (%1  bytes)">,
+InGroup;
 
 def warn_alias_with_section : Warning<
   "%select{alias|ifunc}1 will not be in section '%0' but in the same section "


Index: clang/lib/CodeGen/CGAtomic.cpp
===
--- clang/lib/CodeGen/CGAtomic.cpp
+++ clang/lib/CodeGen/CGAtomic.cpp
@@ -807,10 +807,20 @@
   bool Oversized = getContext().toBits(sizeChars) > MaxInlineWidthInBits;
   bool Misaligned = (Ptr.getAlignment() % sizeChars) != 0;
   bool UseLibcall = Misaligned | Oversized;
+  CharUnits MaxInlineWidth =
+  getContext().toCharUnitsFromBits(MaxInlineWidthInBits);
 
-  if (UseLibcall) {
-CGM.getDiags().Report(E->getBeginLoc(), diag::warn_atomic_op_misaligned)
-<< !Oversized;
+  DiagnosticsEngine &Diags = CGM.getDiags();
+
+  if (Misaligned) {
+Diags.Report(E->getBeginLoc(), diag::warn_atomic_op_misaligned)
+<< (int)sizeChars.getQuantity()
+<< (int)Ptr.getAlignment().getQuantity();
+  }
+
+  if (Oversized) {
+Diags.Report(E->getBeginLoc(), diag::warn_atomic_op_oversized)
+<< (int)sizeChars.getQuantity() << (int)MaxInlineWidth.getQuantity();
   }
 
   llvm::Value *Order = EmitScalarExpr(E->getOrder());
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -699,6 +699,7 @@
 def Reorder : DiagGroup<"reorder", [ReorderCtor, ReorderInitList]>;
 def UndeclaredSelector : DiagGroup<"undeclared-selector">;
 def ImplicitAtomic : DiagGroup<"implicit-atomic-properties">;
+def AtomicAlignment : DiagGroup<"atomic-alignment">;
 def CustomAtomic : DiagGroup<"custom-atomic-properties">;
 def AtomicProperties : DiagGroup<"atomic-properties",
  [ImplicitAtomic, CustomAtomic]>;
Index: clang/include/clang/Basic/DiagnosticFrontendKinds.td
===
--- clang/include/clang/Basic/DiagnosticFrontendKinds.td
+++ clang/include/clang/Basic/DiagnosticFronten

[PATCH] D85102: [clang] improve diagnostics for misaligned and large atomics

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

introduced in https://reviews.llvm.org/D45319


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

https://reviews.llvm.org/D85102

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

[PATCH] D85102: [clang] improve diagnostics for misaligned and large atomics

[Thorsten via Phabricator via cfe-commits]

tschuett updated this revision to Diff 282485.
tschuett added a comment.

add/updated test


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

https://reviews.llvm.org/D85102

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/lib/CodeGen/CGAtomic.cpp
  clang/test/CodeGen/atomics-sema-alignment.c

Index: clang/test/CodeGen/atomics-sema-alignment.c
===
--- clang/test/CodeGen/atomics-sema-alignment.c
+++ clang/test/CodeGen/atomics-sema-alignment.c
@@ -12,10 +12,10 @@
 
 void func(IntPair *p) {
   IntPair res;
-  __atomic_load(p, &res, 0); // expected-warning {{misaligned atomic operation may incur significant performance penalty}}
-  __atomic_store(p, &res, 0); // expected-warning {{misaligned atomic operation may incur significant performance penalty}}
-  __atomic_fetch_add((unaligned_int *)p, 1, 2); // expected-warning {{misaligned atomic operation may incur significant performance penalty}}
-  __atomic_fetch_sub((unaligned_int *)p, 1, 3); // expected-warning {{misaligned atomic operation may incur significant performance penalty}}
+  __atomic_load(p, &res, 0);// expected-warning {{misaligned atomic operation may incur significant performance penalty; the expected (8 bytes) exceeds the actual alignment (4 bytes)}}
+  __atomic_store(p, &res, 0);   // expected-warning {{misaligned atomic operation may incur significant performance penalty; the expected (8 bytes) exceeds the actual alignment (4 bytes)}}
+  __atomic_fetch_add((unaligned_int *)p, 1, 2); // expected-warning {{misaligned atomic operation may incur significant performance penalty; the expected (4 bytes) exceeds the actual alignment (1 bytes)}}
+  __atomic_fetch_sub((unaligned_int *)p, 1, 3); // expected-warning {{misaligned atomic operation may incur significant performance penalty; the expected (4 bytes) exceeds the actual alignment (1 bytes)}}
 }
 
 void func1(LongStruct *p) {
@@ -25,3 +25,24 @@
   __atomic_fetch_add((int *)p, 1, 2);
   __atomic_fetch_sub((int *)p, 1, 3);
 }
+
+typedef struct {
+  void *a;
+  void *b;
+} Foo;
+
+typedef struct {
+  void *a;
+  void *b;
+  void *c;
+  void *d;
+} __attribute__((aligned(32))) ThirtyTwo;
+
+void braz(Foo *foo, ThirtyTwo *braz) {
+  Foo bar;
+  __atomic_load(foo, &bar, __ATOMIC_RELAXED); // expected-warning {{misaligned atomic operation may incur significant performance penalty; the expected (16 bytes) exceeds the actual alignment (8 bytes)}}
+
+  ThirtyTwo thirtyTwo1;
+  ThirtyTwo thirtyTwo2;
+  __atomic_load(&thirtyTwo1, &thirtyTwo2, __ATOMIC_RELAXED); // expected-warning {{large atomic operation may incur significant performance penalty; the access size (32 bytes) exceeds the max lock-free size (16  bytes)}}
+}
Index: clang/lib/CodeGen/CGAtomic.cpp
===
--- clang/lib/CodeGen/CGAtomic.cpp
+++ clang/lib/CodeGen/CGAtomic.cpp
@@ -807,10 +807,20 @@
   bool Oversized = getContext().toBits(sizeChars) > MaxInlineWidthInBits;
   bool Misaligned = (Ptr.getAlignment() % sizeChars) != 0;
   bool UseLibcall = Misaligned | Oversized;
+  CharUnits MaxInlineWidth =
+  getContext().toCharUnitsFromBits(MaxInlineWidthInBits);
 
-  if (UseLibcall) {
-CGM.getDiags().Report(E->getBeginLoc(), diag::warn_atomic_op_misaligned)
-<< !Oversized;
+  DiagnosticsEngine &Diags = CGM.getDiags();
+
+  if (Misaligned) {
+Diags.Report(E->getBeginLoc(), diag::warn_atomic_op_misaligned)
+<< (int)sizeChars.getQuantity()
+<< (int)Ptr.getAlignment().getQuantity();
+  }
+
+  if (Oversized) {
+Diags.Report(E->getBeginLoc(), diag::warn_atomic_op_oversized)
+<< (int)sizeChars.getQuantity() << (int)MaxInlineWidth.getQuantity();
   }
 
   llvm::Value *Order = EmitScalarExpr(E->getOrder());
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -699,6 +699,7 @@
 def Reorder : DiagGroup<"reorder", [ReorderCtor, ReorderInitList]>;
 def UndeclaredSelector : DiagGroup<"undeclared-selector">;
 def ImplicitAtomic : DiagGroup<"implicit-atomic-properties">;
+def AtomicAlignment : DiagGroup<"atomic-alignment">;
 def CustomAtomic : DiagGroup<"custom-atomic-properties">;
 def AtomicProperties : DiagGroup<"atomic-properties",
  [ImplicitAtomic, CustomAtomic]>;
Index: clang/include/clang/Basic/DiagnosticFrontendKinds.td
===
--- clang/include/clang/Basic/DiagnosticFrontendKinds.td
+++ clang/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -270,8 +270,16 @@
   "ifunc resolver function must return a pointer">;
 
 def warn_atomic_op_misaligned : Warning<
-  "%select{large|misaligned}0 atomic operation may incur

[PATCH] D85102: [clang] improve diagnostics for misaligned and large atomics

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

I wanted to save space. I know that the alignment is missing there, but the 
line is already too long.

I have no rights to commit.


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

https://reviews.llvm.org/D85102

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

[PATCH] D85102: [clang] improve diagnostics for misaligned and large atomics

[Thorsten via Phabricator via cfe-commits]

tschuett updated this revision to Diff 282637.
tschuett added a comment.

added missing "alignment"


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

https://reviews.llvm.org/D85102

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/lib/CodeGen/CGAtomic.cpp
  clang/test/CodeGen/atomics-sema-alignment.c

Index: clang/test/CodeGen/atomics-sema-alignment.c
===
--- clang/test/CodeGen/atomics-sema-alignment.c
+++ clang/test/CodeGen/atomics-sema-alignment.c
@@ -12,10 +12,10 @@
 
 void func(IntPair *p) {
   IntPair res;
-  __atomic_load(p, &res, 0); // expected-warning {{misaligned atomic operation may incur significant performance penalty}}
-  __atomic_store(p, &res, 0); // expected-warning {{misaligned atomic operation may incur significant performance penalty}}
-  __atomic_fetch_add((unaligned_int *)p, 1, 2); // expected-warning {{misaligned atomic operation may incur significant performance penalty}}
-  __atomic_fetch_sub((unaligned_int *)p, 1, 3); // expected-warning {{misaligned atomic operation may incur significant performance penalty}}
+  __atomic_load(p, &res, 0);// expected-warning {{misaligned atomic operation may incur significant performance penalty; the expected alignment (8 bytes) exceeds the actual alignment (4 bytes)}}
+  __atomic_store(p, &res, 0);   // expected-warning {{misaligned atomic operation may incur significant performance penalty; the expected alignment (8 bytes) exceeds the actual alignment (4 bytes)}}
+  __atomic_fetch_add((unaligned_int *)p, 1, 2); // expected-warning {{misaligned atomic operation may incur significant performance penalty; the expected alignment (4 bytes) exceeds the actual alignment (1 bytes)}}
+  __atomic_fetch_sub((unaligned_int *)p, 1, 3); // expected-warning {{misaligned atomic operation may incur significant performance penalty; the expected alignment (4 bytes) exceeds the actual alignment (1 bytes)}}
 }
 
 void func1(LongStruct *p) {
@@ -25,3 +25,24 @@
   __atomic_fetch_add((int *)p, 1, 2);
   __atomic_fetch_sub((int *)p, 1, 3);
 }
+
+typedef struct {
+  void *a;
+  void *b;
+} Foo;
+
+typedef struct {
+  void *a;
+  void *b;
+  void *c;
+  void *d;
+} __attribute__((aligned(32))) ThirtyTwo;
+
+void braz(Foo *foo, ThirtyTwo *braz) {
+  Foo bar;
+  __atomic_load(foo, &bar, __ATOMIC_RELAXED); // expected-warning {{misaligned atomic operation may incur significant performance penalty; the expected alignment (16 bytes) exceeds the actual alignment (8 bytes)}}
+
+  ThirtyTwo thirtyTwo1;
+  ThirtyTwo thirtyTwo2;
+  __atomic_load(&thirtyTwo1, &thirtyTwo2, __ATOMIC_RELAXED); // expected-warning {{large atomic operation may incur significant performance penalty; the access size (32 bytes) exceeds the max lock-free size (16  bytes)}}
+}
Index: clang/lib/CodeGen/CGAtomic.cpp
===
--- clang/lib/CodeGen/CGAtomic.cpp
+++ clang/lib/CodeGen/CGAtomic.cpp
@@ -807,10 +807,20 @@
   bool Oversized = getContext().toBits(sizeChars) > MaxInlineWidthInBits;
   bool Misaligned = (Ptr.getAlignment() % sizeChars) != 0;
   bool UseLibcall = Misaligned | Oversized;
+  CharUnits MaxInlineWidth =
+  getContext().toCharUnitsFromBits(MaxInlineWidthInBits);
 
-  if (UseLibcall) {
-CGM.getDiags().Report(E->getBeginLoc(), diag::warn_atomic_op_misaligned)
-<< !Oversized;
+  DiagnosticsEngine &Diags = CGM.getDiags();
+
+  if (Misaligned) {
+Diags.Report(E->getBeginLoc(), diag::warn_atomic_op_misaligned)
+<< (int)sizeChars.getQuantity()
+<< (int)Ptr.getAlignment().getQuantity();
+  }
+
+  if (Oversized) {
+Diags.Report(E->getBeginLoc(), diag::warn_atomic_op_oversized)
+<< (int)sizeChars.getQuantity() << (int)MaxInlineWidth.getQuantity();
   }
 
   llvm::Value *Order = EmitScalarExpr(E->getOrder());
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -699,6 +699,7 @@
 def Reorder : DiagGroup<"reorder", [ReorderCtor, ReorderInitList]>;
 def UndeclaredSelector : DiagGroup<"undeclared-selector">;
 def ImplicitAtomic : DiagGroup<"implicit-atomic-properties">;
+def AtomicAlignment : DiagGroup<"atomic-alignment">;
 def CustomAtomic : DiagGroup<"custom-atomic-properties">;
 def AtomicProperties : DiagGroup<"atomic-properties",
  [ImplicitAtomic, CustomAtomic]>;
Index: clang/include/clang/Basic/DiagnosticFrontendKinds.td
===
--- clang/include/clang/Basic/DiagnosticFrontendKinds.td
+++ clang/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -270,8 +270,16 @@
   "ifunc resolver function must return a pointer">;
 
 def warn_atomic_op_misaligned : Warning

[PATCH] D85128: [Prototype][SVE] Support arm_sve_vector_bits attribute

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

Stupid questions.

- Is it for convenience? You get arrays, global variables, structs, ... . 
Vectorization becomes easier ...
- Are there any potential performance benefits over scalable vectors?
- Is it compatible with GCC?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85128

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

[PATCH] D85102: [clang] improve diagnostics for misaligned and large atomics

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

I would appreciate it, if somebody could commit this patch on my behalf.
Thorsten Schuett
schu...@gmail.com

Thanks.


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

https://reviews.llvm.org/D85102

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

[PATCH] D85128: [Prototype][SVE] Support arm_sve_vector_bits attribute

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

Sorry. I meant ABI. Can link GCC .o files with Clang .o files using the 
attributes?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85128

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

[PATCH] D85191: [AST] Get field size in chars rather than bits in RecordLayoutBuilder.

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

I don't know whether the name of your downstream target is a secret. Wouldn't 
it help you to add a fake 16bit per char target and add units tests to prevent 
regressions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85191

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

[PATCH] D85977: [release][docs] Update contributions to LLVM 11 for SVE.

[Thorsten via Phabricator via cfe-commits]

tschuett added inline comments.



Comment at: llvm/docs/ReleaseNotes.rst:162
+ svfloat64_t vout = svadd_x(Pg, vx, vy);
+svst1(Pg, &out[i], vout);
+   }

Nit: indentation?


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

https://reviews.llvm.org/D85977

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

[PATCH] D68115: Zero initialize padding in unions

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

As an outsider: In Swift, reading an uninitialized variable is a compile-time 
error. Clang is not powerful enough to do this analysis. Aren't you really 
looking for the Clang Intermediate Language (CIL) ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68115



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

[PATCH] D68115: Zero initialize padding in unions

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

I watched the talk, but I still prefer compile-time errors over runtime crashes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68115



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

[PATCH] D132975: [clang][BOLT] Add clang-bolt target

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

Will there be eventually a way to build a fully optimised clang/lld with 
ThinLTO, PGO, and Bolt?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132975

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

[PATCH] D133392: [MTE] Add AArch64GlobalsTagging Pass

[Thorsten via Phabricator via cfe-commits]

tschuett added inline comments.



Comment at: llvm/lib/Target/AArch64/AArch64GlobalsTagging.cpp:93
+
+  uint64_t NewSize = alignTo(SizeInBytes, 16);
+  if (SizeInBytes != NewSize) {

If the `16` is the size of the granule, then it deserves to be named constant. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133392

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

[PATCH] D133436: Ground work for cuda-related checks in clang-tidy

[Thorsten via Phabricator via cfe-commits]

tschuett added inline comments.



Comment at: clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp:25
+
+// Register the GoogleTidyModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add

Is Google a copy and paste error?



Comment at: clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp:32
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the GoogleModule.
+volatile int CudaModuleAnchorSource = 0;

Is Google a copy and paste error?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133436

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

[PATCH] D133392: [MTE] Add AArch64GlobalsTagging Pass

[Thorsten via Phabricator via cfe-commits]

tschuett added inline comments.



Comment at: llvm/lib/Target/AArch64/AArch64GlobalsTagging.cpp:29
+
+namespace {
+

I believe you are going too far with the anonymous namespace. There is no need 
for static functions in anonymous namespaces.

https://llvm.org/docs/CodingStandards.html#anonymous-namespaces


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133392

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

[PATCH] D133771: Add a "Potentially Breaking Changes" section to the Clang release notes

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

The LLD release notes always refer to the Diff where the change was introduced. 
It is kind of sad that we loose all the discussions when we move to GitHub PRs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133771

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

[PATCH] D128415: [ARM] Add Support for Cortex-M85

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

The Clang release notes indicate that PACBTI is off by default. In several 
places, I can see PACBTI. Is the `ARM.td` missing something?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128415

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

[PATCH] D128415: [ARM] Add Support for Cortex-M85

[Thorsten via Phabricator via cfe-commits]

tschuett accepted this revision.
tschuett added a comment.

LGTM.

Please update the summary before you commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128415

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

[PATCH] D129135: [doc][ReleaseNotes] Document AArch64 SVE ABI fix from D127209

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

Could you cite a Diff where this was changed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129135

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

[PATCH] D118948: [MTE] Add -fsanitize=memtag* and friends.

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

Is `-fsanitize=memtag-heap` Android specific or target independent? It passes 
Android flags to the linker?!?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118948

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

[PATCH] D122008: [flang][driver] Add support for generating executables

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

https://reviews.llvm.org/D54604 used a really ugly flag:
`-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122008

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

[PATCH] D128930: [pseudo] Add ForestNode descendants iterator, print ambiguous/opaque node stats.

[Thorsten via Phabricator via cfe-commits]

tschuett added inline comments.



Comment at: clang-tools-extra/pseudo/include/clang-pseudo/Forest.h:202
+class ForestNode::RecursiveIterator
+: public std::iterator {
+  llvm::DenseSet Seen;

`std::iterator` is deprecated in C++17 and creates warnings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128930

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

[PATCH] D104854: Introduce intrinsic llvm.isnan

[Thorsten via Phabricator via cfe-commits]

tschuett added inline comments.



Comment at: llvm/docs/LangRef.rst:20983
+
+These functions get properties of floating point values.
+




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104854

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

[PATCH] D122160: [clang][extract-api] Refactor ExtractAPI and improve docs

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

Would you mind putting a high level description of the intent at the top of the 
main header.

Maybe even a link at https://github.com/apple/swift-docc-symbolkit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122160

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

[PATCH] D123167: [HLSL] Pointers are unsupported in HLSL

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

Do you need a SemaHLSL.cpp file similar to SemaSYCL.cpp?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123167

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

[PATCH] D123167: [HLSL] Pointers are unsupported in HLSL

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

In D123167#3434349 , @beanz wrote:

> In D123167#3434346 , @tschuett 
> wrote:
>
>> Do you need a SemaHLSL.cpp file similar to SemaSYCL.cpp?
>
> Yes absolutely. I haven't yet added any Sema-specific validations yet that 
> are complex enough to break out, but we will 100% get there soon.

Is the downstream compiler also relying on the parser?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123167

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

[PATCH] D123167: [HLSL] Pointers are unsupported in HLSL

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

Relying on the parser to find semantic misbehavior sounds strange ...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123167

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

[PATCH] D123167: [HLSL] Pointers are unsupported in HLSL

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

In D123167#3434372 , @beanz wrote:

> In D123167#3434364 , @tschuett 
> wrote:
>
>> Relying on the parser to find semantic misbehavior sounds strange ...
>
> I don't disagree. An alternative point: HLSL has no _syntax_ for pointers.

I got your point.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123167

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

[PATCH] D123278: [Clang] [Docs] Add HLSLSupport page

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

Thanks. I let Aaron go over the details.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123278

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

[PATCH] D122008: [flang][driver] Add support for generating executables

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

In my example, they even promised that the flag will removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122008

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

[PATCH] D123957: Update the developer policy to mention release notes

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

LGTM and thanks.

Feel free to ignore my comment. Maybe sort entries by importance. I have 
problems reading longish texts and documents.


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

https://reviews.llvm.org/D123957

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

[PATCH] D117924: [compiler_rt] Add a seperate runtime for Mac Catalyst

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

https://reviews.llvm.org/D118862
https://reviews.llvm.org/D118875


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117924

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

[PATCH] D124638: [clang] Track how headers get included generally during lookup time

[Thorsten via Phabricator via cfe-commits]

tschuett added inline comments.



Comment at: clang/lib/Lex/HeaderSearch.cpp:1036
+
+// This file is a system header or C++ unfriendly if the dir is.
 HFI.DirInfo = CurDir->getDirCharacteristic();

Why did the comment move?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124638

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

[PATCH] D124687: [Tooling/DependencyScanning & Preprocessor] Refactor dependency scanning to record and use pre-lexed preprocessor directive tokens, instead of minimized sources

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

Could you split this into smaller patches?
Does this support C++20 modules or is it limited to clang header modules?
Is there overhead in the non dependency scanning mode?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124687

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

[PATCH] D124998: [AArch64][SVE] Add aarch64_sve_pcs attribute to Clang

[Thorsten via Phabricator via cfe-commits]

tschuett added inline comments.



Comment at: clang/include/clang-c/Index.h:3448
+  CXCallingConv_AArch64SVEPcs= 17,
+  CXCallingConv_SwiftAsync = 18,
 

aaron.ballman wrote:
> peterwaller-arm wrote:
> > peterwaller-arm wrote:
> > > It shouldn't matter in principle (... "but in practice" ...) we should 
> > > probably avoid renumbering existing things in the enum and instead add to 
> > > the end of it.
> > > 
> > > Nit, this is missing a space before the equals.
> > > Nit, SVE is an acronym, so is PCS, so capitalization should be consistent 
> > > between the two. I see 'PCS' capitalized in AAPCS for example so probably 
> > > all upper case makes the sense.
> > > 
> > I retract my sloppy "it shouldn't matter in principle [at the source 
> > level]", of course it does matter, and it likely matters in this case (see 
> > 'alias for compatibility' comment above).
> > 
> > To be more specific, changing the enum is an ABI break, and breaks if these 
> > things are ever serialized and therefore not something you want to do.
> +1 -- I was just making that comment when you beat me to it.
Could you please add your CC as 18 and add a comment the next CC will 19 and do 
not touch the other ones.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124998

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

[PATCH] D124998: [AArch64][SVE] Add aarch64_sve_pcs attribute to Clang

[Thorsten via Phabricator via cfe-commits]

tschuett added inline comments.



Comment at: clang/include/clang-c/Index.h:3448
+  CXCallingConv_AArch64SVEPcs= 17,
+  CXCallingConv_SwiftAsync = 18,
 

tschuett wrote:
> aaron.ballman wrote:
> > peterwaller-arm wrote:
> > > peterwaller-arm wrote:
> > > > It shouldn't matter in principle (... "but in practice" ...) we should 
> > > > probably avoid renumbering existing things in the enum and instead add 
> > > > to the end of it.
> > > > 
> > > > Nit, this is missing a space before the equals.
> > > > Nit, SVE is an acronym, so is PCS, so capitalization should be 
> > > > consistent between the two. I see 'PCS' capitalized in AAPCS for 
> > > > example so probably all upper case makes the sense.
> > > > 
> > > I retract my sloppy "it shouldn't matter in principle [at the source 
> > > level]", of course it does matter, and it likely matters in this case 
> > > (see 'alias for compatibility' comment above).
> > > 
> > > To be more specific, changing the enum is an ABI break, and breaks if 
> > > these things are ever serialized and therefore not something you want to 
> > > do.
> > +1 -- I was just making that comment when you beat me to it.
> Could you please add your CC as 18 and add a comment the next CC will 19 and 
> do not touch the other ones.
Do you want to add a `static_assert` to prevent people from doing stupid things?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124998

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

[PATCH] D67025: Add .inl as valid C++ header type

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

Note that in this directory is an ,inl file:
https://github.com/openucx/ucx/tree/master/src/uct/ib/mlx5
It is a pure C library with C++ gtest.

I believe that .inl is about inlining, but it is not tied to a language.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67025

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

[PATCH] D124776: [SPIR-V] Allow setting SPIR-V version via target triple

[Thorsten via Phabricator via cfe-commits]

tschuett added inline comments.



Comment at: llvm/docs/SPIRVUsage.rst:14
+
+The SPIR-V target provides code generation for the SPIR-V binary format 
desribed
+in  `the official SPIR-V specification 
`_.

described


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

https://reviews.llvm.org/D124776

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

[PATCH] D66555: [driver] add a new option `-gen-cdb-fragment-path` to emit a fragment of a compilation database for each compilation

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

How does it relate to the -MJ option?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66555



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

[PATCH] D86895: [Modules] Add stats to measure performance of building and loading modules.

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

Where is the test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86895

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

[PATCH] D86688: [RecoveryExpr] Add 11.0.0 release note.

[Thorsten via Phabricator via cfe-commits]

tschuett added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:73
+
+ // clangd-11 produces a richer AST:
+ // VarDecl  col:5 x 'int' cinit

clangd-11 or clang-11?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86688

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

[PATCH] D86802: [Modules] Don't parse/load explicit module maps if modules are disabled

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.
Herald added a subscriber: danielkiss.

How about a malformed module map is not loaded and gives no errors?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86802

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

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

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

Whatever the final decision is, maybe add a doxygen comment explaining the 
semantics?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93179

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

[PATCH] D93597: [X86][SSE] Enable constexpr on some basic SSE intrinsics (RFC)

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

libcxx uses a function test and calls test() and static_assert(test()), see 
below
https://github.com/llvm/llvm-project/blob/d86a00d8febd0138a21f92d1420c4b62d7acb0ca/libcxx/test/std/utilities/memory/pointer.conversion/to_address.pass.cpp#L102-126


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93597

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

[PATCH] D93597: [X86][SSE] Enable constexpr on some basic SSE intrinsics (RFC)

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

  constexpr bool test() {
  float A = 1,0;
  float B = 2,0;
  float C = 3.0;
  float D = 4,0;
  __m128 AV;
  __m128 BV;
  __m128 result =  _mm_setr_ps(A, B, C, D);
  result =  _mm_setzero_ps();
  result = _mm_xor_ps(AV, BV);
  return true;
  }

It should help you to test, whether you can call all functions in constant 
evaluation contexts and normal evaluations. I don't know  whether you want to 
sprinkle some asserts into the test function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93597

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

[PATCH] D91055: [clang-tidy] Introduce misc No Integer To Pointer Cast check

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

Out of curiosity: why is this in clang-tidy and not in clang?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91055

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

[PATCH] D91055: [clang-tidy] Introduce misc No Integer To Pointer Cast check

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

I did not consider false positives, but it provides better visibility. It is 
always on.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91055

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

[PATCH] D91035: [NFC, Refactor] Convert FunctionDefinitionKind from DeclSpech.h to a scoped enum

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

I started with specifiers.h here: https://reviews.llvm.org/D91409, but it is 
not yet committed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91035

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

[PATCH] D91506: [NFC, Refactor] Convert TypeSpecifiersPipe from Specifiers.h to a scoped enum (tiny)

[Thorsten via Phabricator via cfe-commits]

tschuett created this revision.
tschuett added reviewers: aaron.ballman, faisalv, wchilders.
Herald added subscribers: cfe-commits, dexonsmith, usaxena95, kadircet.
Herald added a project: clang.
tschuett requested review of this revision.
Herald added a subscriber: ilya-biryukov.

> ninja clang clangd (with assertions)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91506

Files:
  clang/include/clang/Basic/Specifiers.h
  clang/lib/Sema/DeclSpec.cpp


Index: clang/lib/Sema/DeclSpec.cpp
===
--- clang/lib/Sema/DeclSpec.cpp
+++ clang/lib/Sema/DeclSpec.cpp
@@ -877,7 +877,7 @@
   }
 
   if (isPipe) {
-TypeSpecPipe = TSP_pipe;
+TypeSpecPipe = static_cast(TypeSpecifiersPipe::Pipe);
   }
   return false;
 }
Index: clang/include/clang/Basic/Specifiers.h
===
--- clang/include/clang/Basic/Specifiers.h
+++ clang/include/clang/Basic/Specifiers.h
@@ -46,10 +46,7 @@
 TSS_unsigned
   };
 
-  enum TypeSpecifiersPipe {
-TSP_unspecified,
-TSP_pipe
-  };
+  enum class TypeSpecifiersPipe { Unspecified, Pipe };
 
   /// Specifies the kind of type.
   enum TypeSpecifierType {


Index: clang/lib/Sema/DeclSpec.cpp
===
--- clang/lib/Sema/DeclSpec.cpp
+++ clang/lib/Sema/DeclSpec.cpp
@@ -877,7 +877,7 @@
   }
 
   if (isPipe) {
-TypeSpecPipe = TSP_pipe;
+TypeSpecPipe = static_cast(TypeSpecifiersPipe::Pipe);
   }
   return false;
 }
Index: clang/include/clang/Basic/Specifiers.h
===
--- clang/include/clang/Basic/Specifiers.h
+++ clang/include/clang/Basic/Specifiers.h
@@ -46,10 +46,7 @@
 TSS_unsigned
   };
 
-  enum TypeSpecifiersPipe {
-TSP_unspecified,
-TSP_pipe
-  };
+  enum class TypeSpecifiersPipe { Unspecified, Pipe };
 
   /// Specifies the kind of type.
   enum TypeSpecifierType {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91506: [NFC, Refactor] Convert TypeSpecifiersPipe from Specifiers.h to a scoped enum (tiny)

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

I am happy, if you could commit this for me. I am still learning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91506

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

[PATCH] D91506: [NFC, Refactor] Convert TypeSpecifiersPipe from Specifiers.h to a scoped enum (tiny)

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

Yes. Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91506

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

[PATCH] D91498: [NFC, Refactor] Convert TypeSpecifierSign from Specifiers.h to a scoped enum

[Thorsten via Phabricator via cfe-commits]

tschuett updated this revision to Diff 305490.
tschuett added a comment.

rebased


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

https://reviews.llvm.org/D91498

Files:
  clang/include/clang/AST/TypeLoc.h
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Sema/DeclSpec.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -198,7 +198,7 @@
   Record.AddSourceLocation(TL.getBuiltinLoc());
   if (TL.needsExtraLocalData()) {
 Record.push_back(TL.getWrittenTypeSpec());
-Record.push_back(TL.getWrittenSignSpec());
+Record.push_back(static_cast(TL.getWrittenSignSpec()));
 Record.push_back(static_cast(TL.getWrittenWidthSpec()));
 Record.push_back(TL.hasModeAttr());
   }
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -6457,7 +6457,7 @@
   TL.setBuiltinLoc(readSourceLocation());
   if (TL.needsExtraLocalData()) {
 TL.setWrittenTypeSpec(static_cast(Reader.readInt()));
-TL.setWrittenSignSpec(static_cast(Reader.readInt()));
+TL.setWrittenSignSpec(static_cast(Reader.readInt()));
 TL.setWrittenWidthSpec(static_cast(Reader.readInt()));
 TL.setModeAttr(Reader.readInt());
   }
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -1300,27 +1300,27 @@
 Result = Context.VoidTy;
 break;
   case DeclSpec::TST_char:
-if (DS.getTypeSpecSign() == DeclSpec::TSS_unspecified)
+if (DS.getTypeSpecSign() == TypeSpecifierSign::Unspecified)
   Result = Context.CharTy;
-else if (DS.getTypeSpecSign() == DeclSpec::TSS_signed)
+else if (DS.getTypeSpecSign() == TypeSpecifierSign::Signed)
   Result = Context.SignedCharTy;
 else {
-  assert(DS.getTypeSpecSign() == DeclSpec::TSS_unsigned &&
+  assert(DS.getTypeSpecSign() == TypeSpecifierSign::Unsigned &&
  "Unknown TSS value");
   Result = Context.UnsignedCharTy;
 }
 break;
   case DeclSpec::TST_wchar:
-if (DS.getTypeSpecSign() == DeclSpec::TSS_unspecified)
+if (DS.getTypeSpecSign() == TypeSpecifierSign::Unspecified)
   Result = Context.WCharTy;
-else if (DS.getTypeSpecSign() == DeclSpec::TSS_signed) {
+else if (DS.getTypeSpecSign() == TypeSpecifierSign::Signed) {
   S.Diag(DS.getTypeSpecSignLoc(), diag::ext_wchar_t_sign_spec)
 << DS.getSpecifierName(DS.getTypeSpecType(),
Context.getPrintingPolicy());
   Result = Context.getSignedWCharType();
 } else {
-  assert(DS.getTypeSpecSign() == DeclSpec::TSS_unsigned &&
-"Unknown TSS value");
+  assert(DS.getTypeSpecSign() == TypeSpecifierSign::Unsigned &&
+ "Unknown TSS value");
   S.Diag(DS.getTypeSpecSignLoc(), diag::ext_wchar_t_sign_spec)
 << DS.getSpecifierName(DS.getTypeSpecType(),
Context.getPrintingPolicy());
@@ -1328,19 +1328,19 @@
 }
 break;
   case DeclSpec::TST_char8:
-  assert(DS.getTypeSpecSign() == DeclSpec::TSS_unspecified &&
-"Unknown TSS value");
-  Result = Context.Char8Ty;
+assert(DS.getTypeSpecSign() == TypeSpecifierSign::Unspecified &&
+   "Unknown TSS value");
+Result = Context.Char8Ty;
 break;
   case DeclSpec::TST_char16:
-  assert(DS.getTypeSpecSign() == DeclSpec::TSS_unspecified &&
-"Unknown TSS value");
-  Result = Context.Char16Ty;
+assert(DS.getTypeSpecSign() == TypeSpecifierSign::Unspecified &&
+   "Unknown TSS value");
+Result = Context.Char16Ty;
 break;
   case DeclSpec::TST_char32:
-  assert(DS.getTypeSpecSign() == DeclSpec::TSS_unspecified &&
-"Unknown TSS value");
-  Result = Context.Char32Ty;
+assert(DS.getTypeSpecSign() == TypeSpecifierSign::Unspecified &&
+   "Unknown TSS value");
+Result = Context.Char32Ty;
 break;
   case DeclSpec::TST_unspecified:
 // If this is a missing declspec in a block literal return context, then it
@@ -1401,7 +1401,7 @@
 
 LLVM_FALLTHROUGH;
   case DeclSpec::TST_int: {
-if (DS.getTypeSpecSign() != DeclSpec::TSS_unsigned) {
+if (DS.getTypeSpecSign() != TypeSpecifierSign::Unsigned) {
   switch (DS.getTypeSpecWidth()) {
   case TypeSpecifierWidth::Unspecified:
 Result = Context.IntTy;
@@ -1458,8 +1458,9 @@
 if (!S.Context.getTargetInfo().hasExtIntType())
   S.Diag(DS.getTypeSpecTypeLoc(), diag::err_type_unsuppo

[PATCH] D78903: [Driver] Add option -fproc-stat-report

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

Could you add quotation marks around the executable name to make the CSV file 
easier to parse?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78903

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

[PATCH] D78903: [Driver] Add option -fproc-stat-report

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78903

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

[PATCH] D91695: [ARM][AArch64] Adding Neoverse N2 CPU support

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

In the first version N2 had FeatureTME. Did you remove that on purpose?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91695

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

[PATCH] D91695: [ARM][AArch64] Adding Neoverse N2 CPU support

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

The features still differ from gcc.
https://github.com/gcc-mirror/gcc/blob/cb1a4876a0e724ca3962ec14dce9e7819fa72ea5/gcc/config/aarch64/aarch64-cores.def#L146
It has SVE2_Bitperm.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91695

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

[PATCH] D91695: [ARM][AArch64] Adding Neoverse N2 CPU support

[Thorsten via Phabricator via cfe-commits]

tschuett added inline comments.



Comment at: llvm/lib/Target/AArch64/AArch64.td:909
+  FeatureMTE,
+  FeatureSVE2,
+  FeatureTRBE]>;

SVE2 Bitperm is missing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91695

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

[PATCH] D91979: [Clang][Attr] Introduce the `assume` function attribute

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

Swift uses a similar mechanism:
https://github.com/apple/swift/blob/887464b7b67d5202bfa7adc4e3f045ff1027a5a7/stdlib/public/core/Array.swift#L829


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91979

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

[PATCH] D89959: UBSAN: emit distinctive traps in trapping mode

[Thorsten via Phabricator via cfe-commits]

tschuett added inline comments.



Comment at: llvm/docs/LangRef.rst:19494
+
+  declare void @llvm.ubsantrap(i32 immarg) cold noreturn nounwind
+

This is still i32.


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

https://reviews.llvm.org/D89959

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

[PATCH] D102261: Introduce SYCL 2020 mode

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

My bad. My last comment:

  LangOptions::SYCL::Ver2017
  LangOptions::SYCL_2017


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

https://reviews.llvm.org/D102261

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

[PATCH] D110386: [clangd] Refactor IncludeStructure: use File (unsigned) for most computations

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

Do you want to wrap the `unsigned` in a struct to make it slightly safer, but 
more complicated to use?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110386

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

[PATCH] D96852: [clang][SVE] Remove inline keyword from arm_sve.h

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

See

  __STDC_VERSION__ 

in  https://gcc.gnu.org/onlinedocs/cpp/Standard-Predefined-Macros.html.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96852

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

[PATCH] D103807: [clang][deps] Ensure deterministic order of TU '-fmodule-file=' arguments

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

llvm discourages the use of std::map:
https://llvm.org/docs/ProgrammersManual.html#map


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103807

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

[PATCH] D105426: [clangd] IWYU as a library: Find all references to symbols in the file

[Thorsten via Phabricator via cfe-commits]

tschuett added a subscriber: kimgr.
tschuett added a comment.

@kimgr


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105426

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

[PATCH] D105263: [X86] AVX512FP16 instructions enabling 1/6

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

Could you add a link to a reference?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105263

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

[PATCH] D112890: headers: optionalise some generated resource headers

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

I am undecided between this may break something and I cannot use the risc-v 
headers on an x86 machine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112890

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

[PATCH] D112890: headers: optionalise some generated resource headers

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

If I understand you correctly, I would need to pass something ala `-target 
riscv-xx` to  enable `__riscv_vector`. However, this is impossible because the 
risk target is disabled. So I thing it is save what you are doing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112890

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

[PATCH] D112890: headers: optionalise some generated resource headers

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

In D112890#3099258 , @etcwilde wrote:

> Right, the header is only necessary when you're targeting riscv, so if 
> compiling for riscv isn't supported by the compiler noted in the 
> `LLVM_TARGETS_TO_BUILD`, you don't need it and can't use it anyway.
> This is separate from the host platform that the compiler is running on.

I would instead say that the header is only necessary and available when you're 
targeting riscv. If you are targeting ARM, the header is neither available nor 
useful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112890

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

[PATCH] D112941: [clang] Add support for the new pointer authentication builtins.

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

The original `ptrauth.h` has the same comment style. Would doxygen style be an 
improvement?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112941

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

[PATCH] D112941: [clang] Add support for the new pointer authentication builtins.

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

The `avxintrin.h` header has more structured documentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112941

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

[PATCH] D112941: [clang] Add support for the new pointer authentication builtins.

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

If you look at the `immintrin.h` header, the access too many builtins is 
guarded by ifdefs.
` #if defined(__SSSE3__)`

The builtin `__builtin_ia32_reduce_smin_d512` is useless on aarch64.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112941

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

[PATCH] D113391: [Modules] Don't support clang module and c++20 module at the same time

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

It would enforce that there is exactly module mode live.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113391

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

[PATCH] D113776: [Clang][SVE] Properly enable/disable dependant SVE target features based upon +(no)sve.* options

[Thorsten via Phabricator via cfe-commits]

tschuett added inline comments.



Comment at: clang/lib/Driver/ToolChains/Arch/AArch64.cpp:66
 
+typedef enum { Unset, True, False } OptState;
+bool isUnset(OptState State) { return State == OptState::Unset; }

This is almost an enum class.
```
enum class OptState { Unset, True, False };
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113776

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

[PATCH] D114099: Enable `_Float16` type support on X86 without the avx512fp16 flag

[Thorsten via Phabricator via cfe-commits]

tschuett added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:676
 * SPIR
-* X86 (Only available under feature AVX512-FP16)
+* X86
 

Would something like SSE2 and up help understanding?


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

https://reviews.llvm.org/D114099

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

[PATCH] D114379: [OMPIRBuilder] Add support for simd (loop) directive.

[Thorsten via Phabricator via cfe-commits]

tschuett added inline comments.



Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:520
 
+  /// Add metadata to simd-ise a loop.
+  ///

I believe LLVM uses American English.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114379

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

[PATCH] D114379: [OMPIRBuilder] Add support for simd (loop) directive.

[Thorsten via Phabricator via cfe-commits]

tschuett added inline comments.



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:2121
+static void addSIMDMetadata(BasicBlock *block,
+ArrayRef Properties) {
+  for (auto &I : *block) {

Properties is unused in the function?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114379

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

[PATCH] D114677: [AArch64] Avoid crashing on invalid -Wa,-march= values

[Thorsten via Phabricator via cfe-commits]

tschuett added inline comments.



Comment at: clang/lib/Driver/ToolChains/Arch/AArch64.cpp:263
   if (!success)
-D.Diag(diag::err_drv_clang_unsupported) << A->getAsString(Args);
+D.Diag(diag::err_drv_clang_unsupported)
+<< (WaMArch.size() ? "-march=" + WaMArch.str() : A->getAsString(Args));

Would an if statement be more readable and actually show what the issue is?

Bonus points for a comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114677

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

[PATCH] D114677: [AArch64] Avoid crashing on invalid -Wa,-march= values

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

There is precedence for checking `WaMArch.size()` in the file. I would expected 
`!WaMArch.empty()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114677

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

[PATCH] D114677: [AArch64] Avoid crashing on invalid -Wa,-march= values

[Thorsten via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114677

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

[PATCH] D114971: [clang][deps] Handle symlinks in minimizing FS

[Thorsten via Phabricator via cfe-commits]

tschuett added inline comments.



Comment at: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h:229
   /// The information requested from \c getOrCreateFileSystemEntry.
   enum RequestedInformation {
+RI_UniqueID,

Do you want to modernise `RequestedInformation` to an enum class?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114971

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

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Thorsten via Phabricator via cfe-commits]

tschuett added inline comments.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2120
+Optional SDKName = None;
+if (getTriple().isWatchOS()) {
+  if (getTriple().isSimulatorEnvironment())

Will there be an enum over the Apple variants to make this less error prone and 
future proof? I want a switch statement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136315

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

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

[Thorsten via Phabricator via cfe-commits]

tschuett added inline comments.



Comment at: clang/tools/clang-stat-cache/clang-stat-cache.cpp:61
+
+namespace {
+

Sorry, but you misuse anonymous namespaces. You want static instead.
https://llvm.org/docs/CodingStandards.html#anonymous-namespaces


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136651

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

[PATCH] D136953: [C++20] Diagnose invalid and reserved module names

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

Thx.




Comment at: clang/test/Modules/reserved-names-1.cpp:42
+
+// We can still use a reserved name on imoport.
+import std; // expected-error {{module 'std' not found}}

import?


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

https://reviews.llvm.org/D136953

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

[PATCH] D136953: [C++20] Diagnose invalid and reserved module names

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

Are malformed imports an issue or are they already covered? There is limit test 
coverage for import. Am I missing something?

  import import;
  import module;


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

https://reviews.llvm.org/D136953

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

[PATCH] D136953: [C++20] Diagnose invalid and reserved module names

[Thorsten via Phabricator via cfe-commits]

tschuett added inline comments.



Comment at: clang/lib/Sema/SemaModule.cpp:148
+/// Tests whether the given identifier is reserved as a module name and
+/// diagnoses if it is. Returs true if a diagnostic is emitted and false
+/// otherwise.

Returns


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

https://reviews.llvm.org/D136953

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

[PATCH] D134410: [clang][CodeGen] Add noundef metadata to load instructions (preliminary 1 or 2)

[Thorsten via Phabricator via cfe-commits]

tschuett added inline comments.



Comment at: clang/lib/CodeGen/CGExpr.cpp:676
+namespace {
+void applyNoundefToLoadInst(bool enable, const clang::QualType &Ty,
+llvm::LoadInst *Load) {

Nit: You meant static.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134410

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

[PATCH] D134410: [clang][CodeGen] Add noundef metadata to load instructions (preliminary 1 or 2)

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

Almost all flags are off by default. That's why they are called flags, i.e., 
`-Weverything`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134410

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

[PATCH] D136589: [AArch64] Add support for the Cortex-X3 CPU

[Thorsten via Phabricator via cfe-commits]

tschuett added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:696
-- Add driver and tuning support for Neoverse V2 via the flag 
``-mcpu=neoverse-v2``.
-  Native detection is also supported via ``-mcpu=native``.
 

What happened with native detection?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136589

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