[PATCH] D127379: [Lex] Keep track of skipped preprocessor blocks and advance the lexer directly if they are revisited

2022-06-08 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi created this revision.
Herald added a project: All.
akyrtzi requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This speeds up preprocessing, specifically for preprocessing the clang sources 
time is reduced by about -36%,
using measurements on M1Pro with a release+thinLTO build.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127379

Files:
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/PPDirectives.cpp

Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -493,10 +493,53 @@
   CurPPLexer->LexingRawMode = true;
   Token Tok;
   SourceLocation endLoc;
+
+  /// Keeps track and caches skipped ranges and also retrieves a prior skipped
+  /// range if the same block is re-visited.
+  struct SkippingRangeStateTy {
+Preprocessor 
+
+const char *BeginPtr = nullptr;
+unsigned *SkipRangePtr = nullptr;
+
+SkippingRangeStateTy(Preprocessor ) : PP(PP) {}
+
+void beginLexPass() {
+  if (BeginPtr)
+return; // continue skipping a block.
+
+  // Initiate a skipping block and adjust the lexer if we already skipped it
+  // before.
+  BeginPtr = PP.CurLexer->getBufferLocation();
+  SkipRangePtr = [BeginPtr];
+  if (*SkipRangePtr) {
+PP.CurLexer->seek(PP.CurLexer->getCurrentBufferOffset() + *SkipRangePtr,
+  /*IsAtStartOfLine*/ true);
+  }
+}
+
+void endLexPass(const char *Hashptr) {
+  if (!BeginPtr) {
+// Not doing normal lexing.
+assert(PP.CurLexer->isDependencyDirectivesLexer());
+return;
+  }
+
+  // Finished skipping a block, record the range if it's first time visited.
+  if (!*SkipRangePtr) {
+*SkipRangePtr = Hashptr - BeginPtr;
+  }
+  assert(*SkipRangePtr == Hashptr - BeginPtr);
+  BeginPtr = nullptr;
+  SkipRangePtr = nullptr;
+}
+  } SkippingRangeState(*this);
+
   while (true) {
 if (CurLexer->isDependencyDirectivesLexer()) {
   CurLexer->LexDependencyDirectiveTokenWhileSkipping(Tok);
 } else {
+  SkippingRangeState.beginLexPass();
   while (true) {
 CurLexer->Lex(Tok);
 
@@ -535,6 +578,9 @@
 CurPPLexer->ParsingPreprocessorDirective = true;
 if (CurLexer) CurLexer->SetKeepWhitespaceMode(false);
 
+assert(Tok.is(tok::hash));
+const char *Hashptr = CurLexer->getBufferLocation() - Tok.getLength();
+assert(CurLexer->getSourceLocation(Hashptr) == Tok.getLocation());
 
 // Read the next token, the directive flavor.
 LexUnexpandedToken(Tok);
@@ -609,6 +655,7 @@
 
 // If we popped the outermost skipping block, we're done skipping!
 if (!CondInfo.WasSkipping) {
+  SkippingRangeState.endLexPass(Hashptr);
   // Restore the value of LexingRawMode so that trailing comments
   // are handled correctly, if we've reached the outermost block.
   CurPPLexer->LexingRawMode = false;
@@ -626,6 +673,9 @@
 // as a non-skipping conditional.
 PPConditionalInfo  = CurPPLexer->peekConditionalLevel();
 
+if (!CondInfo.WasSkipping)
+  SkippingRangeState.endLexPass(Hashptr);
+
 // If this is a #else with a #else before it, report the error.
 if (CondInfo.FoundElse)
   Diag(Tok, diag::pp_err_else_after_else);
@@ -651,6 +701,9 @@
   } else if (Sub == "lif") {  // "elif".
 PPConditionalInfo  = CurPPLexer->peekConditionalLevel();
 
+if (!CondInfo.WasSkipping)
+  SkippingRangeState.endLexPass(Hashptr);
+
 // If this is a #elif with a #else before it, report the error.
 if (CondInfo.FoundElse)
   Diag(Tok, diag::pp_err_elif_after_else) << PED_Elif;
@@ -693,6 +746,9 @@
 PPConditionalInfo  = CurPPLexer->peekConditionalLevel();
 Token DirectiveToken = Tok;
 
+if (!CondInfo.WasSkipping)
+  SkippingRangeState.endLexPass(Hashptr);
+
 // Warn if using `#elifdef` & `#elifndef` in not C2x & C++2b mode even
 // if this branch is in a skipping block.
 unsigned DiagID;
Index: clang/include/clang/Lex/Preprocessor.h
===
--- clang/include/clang/Lex/Preprocessor.h
+++ clang/include/clang/Lex/Preprocessor.h
@@ -2595,6 +2595,12 @@
   void emitMacroDeprecationWarning(const Token ) const;
   void emitRestrictExpansionWarning(const Token ) const;
   void emitFinalMacroWarning(const Token , bool IsUndef) const;
+
+  /// Keeps track of skipped range mappings that were recorded while skipping
+  /// excluded conditional directives. It maps the source buffer pointer at
+  /// the beginning of a skipped block, to the number of bytes that should be
+  /// skipped.
+  llvm::DenseMap RecordedSkippedRanges;
 };
 
 /// Abstract base class that describes a handler that will 

[PATCH] D127310: [clang][driver] fix to correctly set devtoolset on RHEL

2022-06-08 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added a comment.

In D127310#3568777 , @nemanjai wrote:

> In D127310#3567472 , @MaskRay wrote:
>
>> Do you have more authoritative answer when /root/usr is used and when it 
>> isn't?
>
> These suffixes were always part of the code and the mentioned changeset 
> removed it without any justification. The burden of providing this answer 
> should lie with the author of the change that removed this (i.e. @tbaeder). 
> Here at IBM, we are not aware of any Redhat release that does not have this 
> suffix. Namely, all of our RH PowerPC machines with all the toolset versions 
> and distro versions we have available have the suffix. So we are ill equipped 
> to answer this question. I assume that Timm is aware of situations where this 
> isn't part of the path.

I did not remove that on purpose, so adding it back makes sense to me.

>> This change also needs a unit test.
>
> +1

Does the existing unit test not break with this change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127310

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


[PATCH] D126495: [clang-tidy] Organize test docs into subdirectories by module (NFC)

2022-06-08 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment.

In D126495#3568998 , 
@LegalizeAdulthood wrote:

> Gentle ping

My previous point about the links in the header files not being updated hasn't 
been addressed.

It would also be nice if there was a redirect that would dynamically translate 
the old links to new links. A regex like this would do the job, but I have no 
idea on the inner workings of sphinx in order to set this up: 
-`clang-tidy/checks/(\w+)(-\w+)+` -> `clang-tidy/checks/$1/$2`


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

https://reviews.llvm.org/D126495

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


[PATCH] D123493: Support the min of module flags when linking, use for AArch64 BTI/PAC-RET

2022-06-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

This behavior requires that all participating modules have the module flag. In 
the absence of the module flag, what should be behavior be?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123493

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


[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2022-06-08 Thread Sunil K via Phabricator via cfe-commits
koops added a comment.

Is it possible to move this back from "Closed" state to "Needs Review"? The 
merge has been rolled back due to a defect in the code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123235

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


[PATCH] D126495: [clang-tidy] Organize test docs into subdirectories by module (NFC)

2022-06-08 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment.

Gentle ping


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

https://reviews.llvm.org/D126495

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


[PATCH] D127271: [pseudo] Fix link time undefined reference to llvm::EnableABIBreakingChecks

2022-06-08 Thread John McIver via Phabricator via cfe-commits
jmciver closed this revision.
jmciver added a comment.

Obsoleted by D127269 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127271

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


[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

2022-06-08 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments.



Comment at: clang/lib/Sema/SemaConcept.cpp:442-444
+}
+  } else if (FD->getTemplatedKind() == FunctionDecl::TK_MemberSpecialization ||
+ FD->getTemplatedKind() == FunctionDecl::TK_DependentNonTemplate) {

erichkeane wrote:
> ChuanqiXu wrote:
> > nit: This is not important but I prefer the style. It makes each logical 
> > section shorter and indentation shorter.
> I THINK i got this suggestion right?  Phab makes it not particularly clear.
Yes, this is what I want.


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

https://reviews.llvm.org/D126907

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


[PATCH] D127293: [clang-tidy] Ignore other members in a union if any member of it is initialized in cppcoreguidelines-pro-type-member-init

2022-06-08 Thread gehry via Phabricator via cfe-commits
Sockke updated this revision to Diff 435409.
Sockke edited the summary of this revision.
Sockke added a comment.

Fix test file.


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

https://reviews.llvm.org/D127293

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp
@@ -552,3 +552,19 @@
   int A;
   // CHECK-FIXES-NOT: int A{};
 };
+
+struct S1 {
+  S1() {}
+  union {
+int a = 0;
+int b;
+  };
+};
+
+struct S2 {
+  S2() : a{} {}
+  union {
+int a;
+int b;
+  };
+};
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
@@ -61,6 +61,17 @@
   }
 }
 
+void removeFieldInitialized(const FieldDecl *M,
+SmallPtrSetImpl ) {
+  const RecordDecl *R = M->getParent();
+  if (R && R->isUnion()) {
+// Erase all members in a union if any member of it is initialized.
+for (const auto *F : R->fields())
+  FieldDecls.erase(F);
+  } else
+FieldDecls.erase(M);
+}
+
 void removeFieldsInitializedInBody(
 const Stmt , ASTContext ,
 SmallPtrSetImpl ) {
@@ -70,7 +81,7 @@
 hasLHS(memberExpr(member(fieldDecl().bind("fieldDecl")),
 Stmt, Context);
   for (const auto  : Matches)
-FieldDecls.erase(Match.getNodeAs("fieldDecl"));
+removeFieldInitialized(Match.getNodeAs("fieldDecl"), 
FieldDecls);
 }
 
 StringRef getName(const FieldDecl *Field) { return Field->getName(); }
@@ -418,13 +429,18 @@
 
   // Gather all fields (direct and indirect) that need to be initialized.
   SmallPtrSet FieldsToInit;
-  forEachField(ClassDecl, ClassDecl.fields(), [&](const FieldDecl *F) {
+  bool AnyMemberHasInitPerUnion = false;
+  forEachFieldWithFilter(ClassDecl, ClassDecl.fields(),
+ AnyMemberHasInitPerUnion, [&](const FieldDecl *F) {
 if (IgnoreArrays && F->getType()->isArrayType())
   return;
+if (F->hasInClassInitializer() && F->getParent()->isUnion())
+  AnyMemberHasInitPerUnion = true;
 if (!F->hasInClassInitializer() &&
 utils::type_traits::isTriviallyDefaultConstructible(F->getType(),
 Context) &&
-!isEmpty(Context, F->getType()) && !F->isUnnamedBitfield())
+!isEmpty(Context, F->getType()) && !F->isUnnamedBitfield() &&
+!AnyMemberHasInitPerUnion)
   FieldsToInit.insert(F);
   });
   if (FieldsToInit.empty())
@@ -437,7 +453,7 @@
   if (Init->isAnyMemberInitializer() && Init->isWritten()) {
 if (IsUnion)
   return; // We can only initialize one member of a union.
-FieldsToInit.erase(Init->getAnyMember());
+removeFieldInitialized(Init->getAnyMember(), FieldsToInit);
   }
 }
 removeFieldsInitializedInBody(*Ctor->getBody(), Context, FieldsToInit);
@@ -478,7 +494,7 @@
   // Collect all fields but only suggest a fix for the first member of unions,
   // as initializing more than one union member is an error.
   SmallPtrSet FieldsToFix;
-  bool AnyMemberHasInitPerUnion = false;
+  AnyMemberHasInitPerUnion = false;
   forEachFieldWithFilter(ClassDecl, ClassDecl.fields(),
  AnyMemberHasInitPerUnion, [&](const FieldDecl *F) {
 if (!FieldsToInit.count(F))


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp
@@ -552,3 +552,19 @@
   int A;
   // CHECK-FIXES-NOT: int A{};
 };
+
+struct S1 {
+  S1() {}
+  union {
+int a = 0;
+int b;
+  };
+};
+
+struct S2 {
+  S2() : a{} {}
+  union {
+int a;
+int b;
+  };
+};
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
@@ -61,6 +61,17 @@
   }
 }
 
+void removeFieldInitialized(const FieldDecl *M,
+SmallPtrSetImpl ) {
+  const RecordDecl *R = M->getParent();
+  if (R && R->isUnion()) {
+// Erase all 

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

2022-06-08 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment.

In D126907#3566633 , @erichkeane 
wrote:

> All of the comments from @ChuanqiXu done, thank you so much!
>
> As far as the crash you're seeing: That is typically a 'depth mismatch' kinda 
> thing.  They sadly only show up when the types of the template arguments are 
> different (that is, pack vs integral vs type), but I thought I got all of 
> them with examples.
>
> I'll keep hunting for more during the day today, but any amount of reproducer 
> you can provide (even if not minimized) would be incredibly helpful!

Oh, if a not minimized reproducer is desired, you could find it at: 
https://github.com/alibaba/async_simple. It should crash when we run 'make' in 
build directory. (The version of libstdc++ is 10.2 in my environment)

I would try to make a reduced example in the next week if we don't go on 
smoothly.


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

https://reviews.llvm.org/D126907

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


[PATCH] D127142: [HIP] Link with clang_rt.builtins

2022-06-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

> These functions are not available in libgcc but in libclang_rt.builtins. 
> Therefore --hip-link needs to link with libclang_rt.builtins by default.

I think this is problematic.

The current link sequence is `... "-lamdhip64" 
"/tmp/Debug/lib/clang/15.0.0/lib/linux/libclang_rt.builtins-x86_64.a" "-lgcc" 
"--as-needed" "-lgcc_s" "--no-as-needed" "-lc" "-lgcc" "--as-needed" "-lgcc_s" 
"--no-as-needed" "/usr/lib/gcc/x86_64-linux-gnu/12/crtendS.o" 
"/lib/x86_64-linux-gnu/crtn.o"`

Linking in libclang_rt.builtins essentially does --rtlib=compiler-rt. The 
default rtlib for Linux is libgcc. I don't think silently changing the behavior 
is desired.
Perhaps `--rocm-path` should require `--rtlib=compiler-rt`. See the 
`--rtlib=libgcc requires --unwindlib=libgcc` diagnostic.




Comment at: clang/test/Driver/hip-runtime-libs-linux.hip:6
+// Get compiler-rt library full path.
+// RUN: %clang -target x86_64-linux-gnu -rtlib=compiler-rt \
+// RUN:   -print-libgcc-file-name >%t.1

Note: `-target ` is legacy spelling. `--target=` is the recommended name.


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

https://reviews.llvm.org/D127142

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


[PATCH] D127093: [clang][pseudo] Add missing Support lib to cxx

2022-06-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

Thanks! Obsoleted by D127269 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127093

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


[PATCH] D127310: [clang][driver] fix to correctly set devtoolset on RHEL

2022-06-08 Thread Nemanja Ivanovic via Phabricator via cfe-commits
nemanjai added a comment.

In D127310#3567472 , @MaskRay wrote:

> Do you have more authoritative answer when /root/usr is used and when it 
> isn't?

These suffixes were always part of the code and the mentioned changeset removed 
it without any justification. The burden of providing this answer should lie 
with the author of the change that removed this (i.e. @tbaeder). Here at IBM, 
we are not aware of any Redhat release that does not have this suffix. Namely, 
all of our RH PowerPC machines with all the toolset versions and distro 
versions we have available have the suffix. So we are ill equipped to answer 
this question. I assume that Timm is aware of situations where this isn't part 
of the path.

> I'd wish that we have comments for them.

I agree 100%. Timm, do you know the answer to this and if so, can you propose a 
comment to that end?

> This change also needs a unit test.

+1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127310

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


[PATCH] D127351: clang: fix early substitution of alias templates

2022-06-08 Thread Matheus Izvekov via Phabricator via cfe-commits
mizvekov added inline comments.



Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:1060
   LocalInstantiationScope Scope(SemaRef);
+  auto EarlySubstitutionScope = getEarlySubstitutionRAII();
 

mark


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127351

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


[PATCH] D127366: [clang-format][NFC] Format lib/Format and unittests/Format in clang

2022-06-08 Thread Owen Pan via Phabricator via cfe-commits
owenpan created this revision.
owenpan added reviewers: curdeius, HazardyKnusperkeks, MyDeveloperDay.
owenpan added a project: clang-format.
Herald added a project: All.
owenpan requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Run clang-format 5ead1f1 
 with 
`InsertBraces` and `RemoveBracesLLVM` in clang/lib/Format and 
clang/unittests/Format.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127366

Files:
  clang/lib/Format/BreakableToken.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -25439,8 +25439,7 @@
   verifyFormat("do {\n"
"  ++I;\n"
"} while (hasMore() && Filter(*I));",
-   "do { ++I; } while (hasMore() && Filter(*I));",
-   Style);
+   "do { ++I; } while (hasMore() && Filter(*I));", Style);
 
   verifyFormat("if (a)\n"
"  if (b)\n"
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -230,12 +230,10 @@
 if (Change.Tok->is(tok::comment)) {
   if (Change.Tok->is(TT_LineComment) || !Change.IsInsideToken) {
 LastBlockComment = 
-  } else {
-if ((Change.StartOfBlockComment = LastBlockComment)) {
-  Change.IndentationOffset =
-  Change.StartOfTokenColumn -
-  Change.StartOfBlockComment->StartOfTokenColumn;
-}
+  } else if ((Change.StartOfBlockComment = LastBlockComment)) {
+Change.IndentationOffset =
+Change.StartOfTokenColumn -
+Change.StartOfBlockComment->StartOfTokenColumn;
   }
 } else {
   LastBlockComment = nullptr;
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -705,11 +705,9 @@
   // contain a trailing whitespace.
   Prefix = Prefix.substr(0, 1);
 }
-  } else {
-if (ContentColumn[LineIndex] == 1) {
-  // This line starts immediately after the decorating *.
-  Prefix = Prefix.substr(0, 1);
-}
+  } else if (ContentColumn[LineIndex] == 1) {
+// This line starts immediately after the decorating *.
+Prefix = Prefix.substr(0, 1);
   }
   // This is the offset of the end of the last line relative to the start of 
the
   // token text in the token.


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -25439,8 +25439,7 @@
   verifyFormat("do {\n"
"  ++I;\n"
"} while (hasMore() && Filter(*I));",
-   "do { ++I; } while (hasMore() && Filter(*I));",
-   Style);
+   "do { ++I; } while (hasMore() && Filter(*I));", Style);
 
   verifyFormat("if (a)\n"
"  if (b)\n"
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -230,12 +230,10 @@
 if (Change.Tok->is(tok::comment)) {
   if (Change.Tok->is(TT_LineComment) || !Change.IsInsideToken) {
 LastBlockComment = 
-  } else {
-if ((Change.StartOfBlockComment = LastBlockComment)) {
-  Change.IndentationOffset =
-  Change.StartOfTokenColumn -
-  Change.StartOfBlockComment->StartOfTokenColumn;
-}
+  } else if ((Change.StartOfBlockComment = LastBlockComment)) {
+Change.IndentationOffset =
+Change.StartOfTokenColumn -
+Change.StartOfBlockComment->StartOfTokenColumn;
   }
 } else {
   LastBlockComment = nullptr;
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -705,11 +705,9 @@
   // contain a trailing whitespace.
   Prefix = Prefix.substr(0, 1);
 }
-  } else {
-if (ContentColumn[LineIndex] == 1) {
-  // This line starts immediately after the decorating *.
-  Prefix = Prefix.substr(0, 1);
-}
+  } else if (ContentColumn[LineIndex] == 1) {
+// This line starts immediately after the decorating *.
+Prefix = Prefix.substr(0, 1);
   }
   // This is the offset of the end of the last line relative to the start of the
   // token text in the token.
___
cfe-commits 

[PATCH] D126308: cmake: use llvm dir variables for clang/utils/hmaptool

2022-06-08 Thread Matheus Izvekov via Phabricator via cfe-commits
mizvekov added a comment.

@nikic ping, can you confirm this current patch works for you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126308

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


[PATCH] D127351: clang: fix early substitution of alias templates

2022-06-08 Thread Matheus Izvekov via Phabricator via cfe-commits
mizvekov added a comment.

@rsmith , I have marked the few spots where the change is meaningful.




Comment at: clang/include/clang/Sema/Template.h:488-497
+bool EarlySubstitution;
+
   public:
 TemplateDeclInstantiator(Sema , DeclContext *Owner,
- const MultiLevelTemplateArgumentList 
)
+ const MultiLevelTemplateArgumentList 
,
+ bool EarlySubstitution)
 : SemaRef(SemaRef),

mark



Comment at: clang/include/clang/Sema/Template.h:629-648
+
+unsigned getNewDepth(unsigned Depth) const {
+  return Depth -
+ (EarlySubstitution ? 0 : TemplateArgs.getNumSubstitutedLevels());
+}
+
+class EarlySubstitutionRAII {

mark



Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:931-941
+bool EarlySubstitution;
 
   public:
 typedef TreeTransform inherited;
 
 TemplateInstantiator(Sema ,
  const MultiLevelTemplateArgumentList ,

mark



Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:1194-1215
+  TemplateDeclInstantiator DeclInstantiator(
+  getSema(),
+  /* DeclContext *Owner */ Owner, TemplateArgs, EarlySubstitution);
   return DeclInstantiator.SubstTemplateParams(OrigTPL);
 }
 
 concepts::TypeRequirement *

mark



Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:1872
   QualType Result = getSema().Context.getTemplateTypeParmType(
-  T->getDepth() - TemplateArgs.getNumSubstitutedLevels(), T->getIndex(),
-  T->isParameterPack(), NewTTPDecl);
+  getNewDepth(T->getDepth()), T->getIndex(), T->isParameterPack(),
+  NewTTPDecl);

mark



Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:2807
   SemaRef.Context, Owner, D->getBeginLoc(), D->getLocation(),
-  D->getDepth() - TemplateArgs.getNumSubstitutedLevels(), D->getIndex(),
-  D->getIdentifier(), D->wasDeclaredWithTypename(), D->isParameterPack(),
+  getNewDepth(D->getDepth()), D->getIndex(), D->getIdentifier(),
+  D->wasDeclaredWithTypename(), D->isParameterPack(),

mark



Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:2965-2970
+getNewDepth(D->getDepth()), D->getPosition(), D->getIdentifier(), T, 
DI,
+ExpandedParameterPackTypes, ExpandedParameterPackTypesAsWritten);
   else
 Param = NonTypeTemplateParmDecl::Create(
 SemaRef.Context, Owner, D->getInnerLocStart(), D->getLocation(),
+getNewDepth(D->getDepth()), D->getPosition(), D->getIdentifier(), T,

mark



Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:3105-3109
+SemaRef.Context, Owner, D->getLocation(), getNewDepth(D->getDepth()),
 D->getPosition(), D->getIdentifier(), InstParams, ExpandedParams);
   else
 Param = TemplateTemplateParmDecl::Create(
+SemaRef.Context, Owner, D->getLocation(), getNewDepth(D->getDepth()),

mark



Comment at: clang/test/AST/ast-dump-template-decls.cpp:124
 // CHECK-NEXT: SubstTemplateTypeParmType 0x{{[^ ]*}} 'void' sugar
-// CHECK-NEXT: TemplateTypeParmType 0x{{[^ ]*}} 'U' dependent depth 0 index 0
+// CHECK-NEXT: TemplateTypeParmType 0x{{[^ ]*}} 'U' dependent depth 1 index 0
 // CHECK-NEXT: TemplateTypeParm 0x{{[^ ]*}} 'U'

mark


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127351

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


[PATCH] D127338: [Clang][CoverageMapping] Fix compile time explosions by adjusting only appropriated skipped ranges

2022-06-08 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno updated this revision to Diff 435388.
bruno added a comment.

Empty lines handling should also make it skipped, fix the 3 failures.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127338

Files:
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/lib/CodeGen/CoverageMappingGen.h

Index: clang/lib/CodeGen/CoverageMappingGen.h
===
--- clang/lib/CodeGen/CoverageMappingGen.h
+++ clang/lib/CodeGen/CoverageMappingGen.h
@@ -31,15 +31,29 @@
 class Stmt;
 
 struct SkippedRange {
+  enum Kind {
+PPIfElse, // Preprocessor #if/#else ...
+EmptyLine,
+Comment,
+  };
+
   SourceRange Range;
   // The location of token before the skipped source range.
   SourceLocation PrevTokLoc;
   // The location of token after the skipped source range.
   SourceLocation NextTokLoc;
+  // The nature of this skipped range
+  Kind RangeKind;
+
+  bool isComment() { return RangeKind == Comment; }
+  bool isEmptyLine() { return RangeKind == EmptyLine; }
+  bool isPPIfElse() { return RangeKind == PPIfElse; }
 
-  SkippedRange(SourceRange Range, SourceLocation PrevTokLoc = SourceLocation(),
+  SkippedRange(SourceRange Range, Kind K,
+   SourceLocation PrevTokLoc = SourceLocation(),
SourceLocation NextTokLoc = SourceLocation())
-  : Range(Range), PrevTokLoc(PrevTokLoc), NextTokLoc(NextTokLoc) {}
+  : Range(Range), PrevTokLoc(PrevTokLoc), NextTokLoc(NextTokLoc),
+RangeKind(K) {}
 };
 
 /// Stores additional source code information like skipped ranges which
@@ -62,7 +76,7 @@
 
   std::vector () { return SkippedRanges; }
 
-  void AddSkippedRange(SourceRange Range);
+  void AddSkippedRange(SourceRange Range, SkippedRange::Kind RangeKind);
 
   void SourceRangeSkipped(SourceRange Range, SourceLocation EndifLoc) override;
 
Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -60,26 +60,27 @@
   return CoverageInfo;
 }
 
-void CoverageSourceInfo::AddSkippedRange(SourceRange Range) {
+void CoverageSourceInfo::AddSkippedRange(SourceRange Range,
+ SkippedRange::Kind RangeKind) {
   if (EmptyLineCommentCoverage && !SkippedRanges.empty() &&
   PrevTokLoc == SkippedRanges.back().PrevTokLoc &&
   SourceMgr.isWrittenInSameFile(SkippedRanges.back().Range.getEnd(),
 Range.getBegin()))
 SkippedRanges.back().Range.setEnd(Range.getEnd());
   else
-SkippedRanges.push_back({Range, PrevTokLoc});
+SkippedRanges.push_back({Range, RangeKind, PrevTokLoc});
 }
 
 void CoverageSourceInfo::SourceRangeSkipped(SourceRange Range, SourceLocation) {
-  AddSkippedRange(Range);
+  AddSkippedRange(Range, SkippedRange::PPIfElse);
 }
 
 void CoverageSourceInfo::HandleEmptyline(SourceRange Range) {
-  AddSkippedRange(Range);
+  AddSkippedRange(Range, SkippedRange::EmptyLine);
 }
 
 bool CoverageSourceInfo::HandleComment(Preprocessor , SourceRange Range) {
-  AddSkippedRange(Range);
+  AddSkippedRange(Range, SkippedRange::Comment);
   return false;
 }
 
@@ -335,6 +336,8 @@
   /// This shrinks the skipped range if it spans a line that contains a
   /// non-comment token. If shrinking the skipped range would make it empty,
   /// this returns None.
+  /// Note this function can potentially be expensive because
+  /// getSpellingLineNumber uses getLineNumber, which is expensive.
   Optional adjustSkippedRange(SourceManager ,
   SourceLocation LocStart,
   SourceLocation LocEnd,
@@ -382,8 +385,13 @@
   auto CovFileID = getCoverageFileID(LocStart);
   if (!CovFileID)
 continue;
-  Optional SR =
-  adjustSkippedRange(SM, LocStart, LocEnd, I.PrevTokLoc, I.NextTokLoc);
+  Optional SR;
+  if (I.isComment())
+SR = adjustSkippedRange(SM, LocStart, LocEnd, I.PrevTokLoc,
+I.NextTokLoc);
+  else if (I.isPPIfElse() || I.isEmptyLine())
+SR = {SM, LocStart, LocEnd};
+
   if (!SR.hasValue())
 continue;
   auto Region = CounterMappingRegion::makeSkipped(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D127363: [Lex] Fix for char32_t literal truncation on 16 bit architectures

2022-06-08 Thread Sebastian Perta via Phabricator via cfe-commits
SebastianPerta created this revision.
SebastianPerta added reviewers: aaron.ballman, sammccall, DaanDeMeyer.
Herald added a subscriber: dylanmckay.
Herald added a project: All.
SebastianPerta requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

On 16 bit architectures char32_t literals are truncated, for example 
U'\U00064321' will be truncated to  0x4321.
The issue can be seen using the RL78 backend which I announced a while ago 
(https://lists.llvm.org/pipermail/llvm-dev/2020-April/140546.html) and I'm 
ready to upstream.
Upstream, the problem can be observed on MSP430, however this patch is not 
sufficient in case of MSP430 since Char32Type is left to the default type 
UnsignedInt which is 16 bit in case of MSP430 (set in TargetInfo.cpp). On RL78 
I set it to UnsignedLong just like in case of AVR (see AVR.h).

Regarding testing, I found the problem using the following test from the GCC 
regression:
gcc/testsuite/g++.dg/ext/utf32-1.C
I'm happy to write a new test if I can get any pointers where and how to write 
it (the test fails at execution so not sure how to test it without executing 
it).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127363

Files:
  clang/lib/Lex/LiteralSupport.cpp


Index: clang/lib/Lex/LiteralSupport.cpp
===
--- clang/lib/Lex/LiteralSupport.cpp
+++ clang/lib/Lex/LiteralSupport.cpp
@@ -1597,7 +1597,7 @@
 IsMultiChar = false;
   }
 
-  llvm::APInt LitVal(PP.getTargetInfo().getIntWidth(), 0);
+  llvm::APInt LitVal(PP.getTargetInfo().getChar32Width(), 0);
 
   // Narrow character literals act as though their value is concatenated
   // in this implementation, but warn on overflow.


Index: clang/lib/Lex/LiteralSupport.cpp
===
--- clang/lib/Lex/LiteralSupport.cpp
+++ clang/lib/Lex/LiteralSupport.cpp
@@ -1597,7 +1597,7 @@
 IsMultiChar = false;
   }
 
-  llvm::APInt LitVal(PP.getTargetInfo().getIntWidth(), 0);
+  llvm::APInt LitVal(PP.getTargetInfo().getChar32Width(), 0);
 
   // Narrow character literals act as though their value is concatenated
   // in this implementation, but warn on overflow.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] d6f6cd5 - [docs][clang] Fixing minor typo

2022-06-08 Thread Jose Manuel Monsalve Diaz via cfe-commits

Author: Jose Manuel Monsalve Diaz
Date: 2022-06-08T23:35:11Z
New Revision: d6f6cd5cd52bbfe91444226ef8a9687288d939b1

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

LOG: [docs][clang] Fixing minor typo

Changing "tot the" to "to the"

Added: 


Modified: 
clang/docs/OffloadingDesign.rst

Removed: 




diff  --git a/clang/docs/OffloadingDesign.rst b/clang/docs/OffloadingDesign.rst
index 43a0c0d2c29f7..5df8a320b4da7 100644
--- a/clang/docs/OffloadingDesign.rst
+++ b/clang/docs/OffloadingDesign.rst
@@ -258,7 +258,7 @@ create an executable device image. This is done using a 
Clang tool, see
 :doc:`ClangLinkerWrapper` for more information. This tool works as a wrapper
 over the host linking job. It scans the input object files for the offloading
 section ``.llvm.offloading``. The device files stored in this section are then
-extracted and passed tot he appropriate linking job. The linked device image is
+extracted and passed to the appropriate linking job. The linked device image is
 then :ref:`wrapped ` to create the symbols used to load
 the device image and link it with the host.
 



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


[PATCH] D127312: [clang][dataflow] Remove IndirectionValue class, moving PointeeLoc field into PointerValue and ReferenceValue

2022-06-08 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa1b2b7d9790b: [clang][dataflow] Remove IndirectionValue 
class, moving PointeeLoc field into… (authored by wyt, committed by gribozavr).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127312

Files:
  clang/include/clang/Analysis/FlowSensitive/Value.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -122,24 +122,24 @@
   // [[p]]
 }
   )";
-  runDataflow(
-  Code, [](llvm::ArrayRef<
-   std::pair>>
-   Results,
-   ASTContext ) {
-ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
-const Environment  = Results[0].second.Env;
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext ) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment  = Results[0].second.Env;
 
-const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
-ASSERT_THAT(FooDecl, NotNull());
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
 
-const StorageLocation *FooLoc =
-Env.getStorageLocation(*FooDecl, SkipPast::None);
-ASSERT_TRUE(isa_and_nonnull(FooLoc));
+const StorageLocation *FooLoc =
+Env.getStorageLocation(*FooDecl, SkipPast::None);
+ASSERT_TRUE(isa_and_nonnull(FooLoc));
 
-const Value *FooVal = Env.getValue(*FooLoc);
-EXPECT_TRUE(isa_and_nonnull(FooVal));
-  });
+const Value *FooVal = Env.getValue(*FooLoc);
+EXPECT_TRUE(isa_and_nonnull(FooVal));
+  });
 }
 
 TEST_F(TransferTest, StructVarDecl) {
@@ -293,29 +293,29 @@
   // [[p]]
 }
   )";
-  runDataflow(
-  Code, [](llvm::ArrayRef<
-   std::pair>>
-   Results,
-   ASTContext ) {
-ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
-const Environment  = Results[0].second.Env;
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext ) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment  = Results[0].second.Env;
 
-const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
-ASSERT_THAT(FooDecl, NotNull());
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
 
-const StorageLocation *FooLoc =
-Env.getStorageLocation(*FooDecl, SkipPast::None);
-ASSERT_TRUE(isa_and_nonnull(FooLoc));
+const StorageLocation *FooLoc =
+Env.getStorageLocation(*FooDecl, SkipPast::None);
+ASSERT_TRUE(isa_and_nonnull(FooLoc));
 
-const ReferenceValue *FooVal =
-cast(Env.getValue(*FooLoc));
-const StorageLocation  = FooVal->getPointeeLoc();
-EXPECT_TRUE(isa());
+const ReferenceValue *FooVal =
+cast(Env.getValue(*FooLoc));
+const StorageLocation  = FooVal->getPointeeLoc();
+EXPECT_TRUE(isa());
 
-const Value *FooPointeeVal = Env.getValue(FooPointeeLoc);
-EXPECT_TRUE(isa_and_nonnull(FooPointeeVal));
-  });
+const Value *FooPointeeVal = Env.getValue(FooPointeeLoc);
+EXPECT_TRUE(isa_and_nonnull(FooPointeeVal));
+  });
 }
 
 TEST_F(TransferTest, SelfReferentialReferenceVarDecl) {
@@ -3327,23 +3327,23 @@
 ASSERT_THAT(LDecl, NotNull());
 
 // Inner.
-auto *LVal = dyn_cast(
-InnerEnv.getValue(*LDecl, SkipPast::None));
+auto *LVal =
+dyn_cast(InnerEnv.getValue(*LDecl, SkipPast::None));
 ASSERT_THAT(LVal, NotNull());
 
 EXPECT_EQ(>getPointeeLoc(),
   InnerEnv.getStorageLocation(*ValDecl, SkipPast::Reference));
 
 // Outer.
-LVal = dyn_cast(
-OuterEnv.getValue(*LDecl, SkipPast::None));
+LVal =
+dyn_cast(OuterEnv.getValue(*LDecl, SkipPast::None));
 ASSERT_THAT(LVal, NotNull());
 
 // The loop body may not have been executed, so we should not conclude
 // that `l` points to `val`.
 EXPECT_NE(>getPointeeLoc(),
   OuterEnv.getStorageLocation(*ValDecl, SkipPast::Reference));
-});
+ 

[clang] a1b2b7d - [clang][dataflow] Remove IndirectionValue class, moving PointeeLoc field into PointerValue and ReferenceValue

2022-06-08 Thread Dmitri Gribenko via cfe-commits

Author: Wei Yi Tee
Date: 2022-06-09T01:29:16+02:00
New Revision: a1b2b7d9790b8a150d798fcc672387607986dbe0

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

LOG: [clang][dataflow] Remove IndirectionValue class, moving PointeeLoc field 
into PointerValue and ReferenceValue

This patch precedes a future patch to make PointeeLoc for PointerValue possibly 
empty (for nullptr), by using a pointer instead of a reference type.
ReferenceValue should maintain a non-empty PointeeLoc reference.

Reviewed By: gribozavr2

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

Added: 


Modified: 
clang/include/clang/Analysis/FlowSensitive/Value.h
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Removed: 




diff  --git a/clang/include/clang/Analysis/FlowSensitive/Value.h 
b/clang/include/clang/Analysis/FlowSensitive/Value.h
index 1aedd8a300dd5..3ac4b8c60996b 100644
--- a/clang/include/clang/Analysis/FlowSensitive/Value.h
+++ b/clang/include/clang/Analysis/FlowSensitive/Value.h
@@ -166,15 +166,15 @@ class IntegerValue : public Value {
   }
 };
 
-/// Base class for values that refer to storage locations.
-class IndirectionValue : public Value {
+/// Models a dereferenced pointer. For example, a reference in C++ or an lvalue
+/// in C.
+class ReferenceValue final : public Value {
 public:
-  /// Constructs a value that refers to `PointeeLoc`.
-  explicit IndirectionValue(Kind ValueKind, StorageLocation )
-  : Value(ValueKind), PointeeLoc(PointeeLoc) {}
+  explicit ReferenceValue(StorageLocation )
+  : Value(Kind::Reference), PointeeLoc(PointeeLoc) {}
 
   static bool classof(const Value *Val) {
-return Val->getKind() == Kind::Reference || Val->getKind() == 
Kind::Pointer;
+return Val->getKind() == Kind::Reference;
   }
 
   StorageLocation () const { return PointeeLoc; }
@@ -183,27 +183,20 @@ class IndirectionValue : public Value {
   StorageLocation 
 };
 
-/// Models a dereferenced pointer. For example, a reference in C++ or an lvalue
-/// in C.
-class ReferenceValue final : public IndirectionValue {
-public:
-  explicit ReferenceValue(StorageLocation )
-  : IndirectionValue(Kind::Reference, PointeeLoc) {}
-
-  static bool classof(const Value *Val) {
-return Val->getKind() == Kind::Reference;
-  }
-};
-
 /// Models a symbolic pointer. Specifically, any value of type `T*`.
-class PointerValue final : public IndirectionValue {
+class PointerValue final : public Value {
 public:
   explicit PointerValue(StorageLocation )
-  : IndirectionValue(Kind::Pointer, PointeeLoc) {}
+  : Value(Kind::Pointer), PointeeLoc(PointeeLoc) {}
 
   static bool classof(const Value *Val) {
 return Val->getKind() == Kind::Pointer;
   }
+
+  StorageLocation () const { return PointeeLoc; }
+
+private:
+  StorageLocation 
 };
 
 /// Models a value of `struct` or `class` type, with a flat map of fields to

diff  --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp 
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 13fef0d4286cf..033ef6afbeb2f 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -50,22 +50,25 @@ llvm::DenseMap intersectDenseMaps(const 
llvm::DenseMap ,
   return Result;
 }
 
+static bool areEquivalentIndirectionValues(Value *Val1, Value *Val2) {
+  if (auto *IndVal1 = dyn_cast(Val1)) {
+auto *IndVal2 = cast(Val2);
+return >getPointeeLoc() == >getPointeeLoc();
+  }
+  if (auto *IndVal1 = dyn_cast(Val1)) {
+auto *IndVal2 = cast(Val2);
+return >getPointeeLoc() == >getPointeeLoc();
+  }
+  return false;
+}
+
 /// Returns true if and only if `Val1` is equivalent to `Val2`.
 static bool equivalentValues(QualType Type, Value *Val1,
  const Environment , Value *Val2,
  const Environment ,
  Environment::ValueModel ) {
-  if (Val1 == Val2)
-return true;
-
-  if (auto *IndVal1 = dyn_cast(Val1)) {
-auto *IndVal2 = cast(Val2);
-assert(IndVal1->getKind() == IndVal2->getKind());
-if (>getPointeeLoc() == >getPointeeLoc())
-  return true;
-  }
-
-  return Model.compareEquivalent(Type, *Val1, Env1, *Val2, Env2);
+  return Val1 == Val2 || areEquivalentIndirectionValues(Val1, Val2) ||
+ Model.compareEquivalent(Type, *Val1, Env1, *Val2, Env2);
 }
 
 /// Attempts to merge distinct values `Val1` and `Val2` in `Env1` and `Env2`,
@@ -89,12 +92,8 @@ static Value *mergeDistinctValues(QualType Type, Value *Val1,
   }
 
   // FIXME: add unit tests that cover this statement.
-  if (auto *IndVal1 = dyn_cast(Val1)) {
-auto *IndVal2 = cast(Val2);
-assert(IndVal1->getKind() == 

[PATCH] D127361: [NFC] clang: add test for PR55886

2022-06-08 Thread Matheus Izvekov via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGcfda534b9944: [NFC] clang: add test for PR55886 (authored by 
mizvekov).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127361

Files:
  clang/test/AST/ast-dump-template-decls.cpp


Index: clang/test/AST/ast-dump-template-decls.cpp
===
--- clang/test/AST/ast-dump-template-decls.cpp
+++ clang/test/AST/ast-dump-template-decls.cpp
@@ -108,3 +108,22 @@
 // CHECK: FunctionTemplateDecl 0x{{[^ ]*}} parent 0x{{[^ ]*}} prev 0x{{[^ ]*}} 
 col:13 f
 // CHECK-NEXT: TemplateTypeParmDecl 0x{{[^ ]*}}  
col:20 typename depth 1 index 0 Uy
 void V::f() {}
+
+namespace PR55886 {
+template  struct C {
+  template  using type1 = U(T);
+};
+using type2 = typename C::type1;
+// CHECK:  TypeAliasDecl 0x{{[^ ]*}}  col:7 
type2 'typename C::type1':'void (int)'
+// CHECK-NEXT: ElaboratedType 0x{{[^ ]*}} 'typename C::type1' sugar
+// CHECK-NEXT: TemplateSpecializationType 0x{{[^ ]*}} 'type1' sugar 
alias type1
+// CHECK-NEXT: TemplateArgument type 'void'
+// CHECK-NEXT: BuiltinType 0x{{[^ ]*}} 'void'
+// CHECK-NEXT: FunctionProtoType 0x{{[^ ]*}} 'void (int)' cdecl
+// CHECK-NEXT: SubstTemplateTypeParmType 0x{{[^ ]*}} 'void' sugar
+// CHECK-NEXT: TemplateTypeParmType 0x{{[^ ]*}} 'U' dependent depth 0 index 0
+// CHECK-NEXT: TemplateTypeParm 0x{{[^ ]*}} 'U'
+// CHECK-NEXT: BuiltinType 0x{{[^ ]*}} 'void'
+// CHECK-NEXT: SubstTemplateTypeParmType 0x{{[^ ]*}} 'int' sugar
+// CHECK-NEXT: TemplateTypeParmType 0x{{[^ ]*}} 'T' dependent depth 0 index 0
+} // namespace PR55886


Index: clang/test/AST/ast-dump-template-decls.cpp
===
--- clang/test/AST/ast-dump-template-decls.cpp
+++ clang/test/AST/ast-dump-template-decls.cpp
@@ -108,3 +108,22 @@
 // CHECK: FunctionTemplateDecl 0x{{[^ ]*}} parent 0x{{[^ ]*}} prev 0x{{[^ ]*}}  col:13 f
 // CHECK-NEXT: TemplateTypeParmDecl 0x{{[^ ]*}}  col:20 typename depth 1 index 0 Uy
 void V::f() {}
+
+namespace PR55886 {
+template  struct C {
+  template  using type1 = U(T);
+};
+using type2 = typename C::type1;
+// CHECK:  TypeAliasDecl 0x{{[^ ]*}}  col:7 type2 'typename C::type1':'void (int)'
+// CHECK-NEXT: ElaboratedType 0x{{[^ ]*}} 'typename C::type1' sugar
+// CHECK-NEXT: TemplateSpecializationType 0x{{[^ ]*}} 'type1' sugar alias type1
+// CHECK-NEXT: TemplateArgument type 'void'
+// CHECK-NEXT: BuiltinType 0x{{[^ ]*}} 'void'
+// CHECK-NEXT: FunctionProtoType 0x{{[^ ]*}} 'void (int)' cdecl
+// CHECK-NEXT: SubstTemplateTypeParmType 0x{{[^ ]*}} 'void' sugar
+// CHECK-NEXT: TemplateTypeParmType 0x{{[^ ]*}} 'U' dependent depth 0 index 0
+// CHECK-NEXT: TemplateTypeParm 0x{{[^ ]*}} 'U'
+// CHECK-NEXT: BuiltinType 0x{{[^ ]*}} 'void'
+// CHECK-NEXT: SubstTemplateTypeParmType 0x{{[^ ]*}} 'int' sugar
+// CHECK-NEXT: TemplateTypeParmType 0x{{[^ ]*}} 'T' dependent depth 0 index 0
+} // namespace PR55886
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] cfda534 - [NFC] clang: add test for PR55886

2022-06-08 Thread Matheus Izvekov via cfe-commits

Author: Matheus Izvekov
Date: 2022-06-09T01:20:58+02:00
New Revision: cfda534b9944571bf55f0b275982d578361ab7d7

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

LOG: [NFC] clang: add test for PR55886

Signed-off-by: Matheus Izvekov 

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

Added: 


Modified: 
clang/test/AST/ast-dump-template-decls.cpp

Removed: 




diff  --git a/clang/test/AST/ast-dump-template-decls.cpp 
b/clang/test/AST/ast-dump-template-decls.cpp
index 51ec673ab8f3..616a96902eff 100644
--- a/clang/test/AST/ast-dump-template-decls.cpp
+++ b/clang/test/AST/ast-dump-template-decls.cpp
@@ -108,3 +108,22 @@ template 
 // CHECK: FunctionTemplateDecl 0x{{[^ ]*}} parent 0x{{[^ ]*}} prev 0x{{[^ ]*}} 
 col:13 f
 // CHECK-NEXT: TemplateTypeParmDecl 0x{{[^ ]*}}  
col:20 typename depth 1 index 0 Uy
 void V::f() {}
+
+namespace PR55886 {
+template  struct C {
+  template  using type1 = U(T);
+};
+using type2 = typename C::type1;
+// CHECK:  TypeAliasDecl 0x{{[^ ]*}}  col:7 
type2 'typename C::type1':'void (int)'
+// CHECK-NEXT: ElaboratedType 0x{{[^ ]*}} 'typename C::type1' sugar
+// CHECK-NEXT: TemplateSpecializationType 0x{{[^ ]*}} 'type1' sugar 
alias type1
+// CHECK-NEXT: TemplateArgument type 'void'
+// CHECK-NEXT: BuiltinType 0x{{[^ ]*}} 'void'
+// CHECK-NEXT: FunctionProtoType 0x{{[^ ]*}} 'void (int)' cdecl
+// CHECK-NEXT: SubstTemplateTypeParmType 0x{{[^ ]*}} 'void' sugar
+// CHECK-NEXT: TemplateTypeParmType 0x{{[^ ]*}} 'U' dependent depth 0 index 0
+// CHECK-NEXT: TemplateTypeParm 0x{{[^ ]*}} 'U'
+// CHECK-NEXT: BuiltinType 0x{{[^ ]*}} 'void'
+// CHECK-NEXT: SubstTemplateTypeParmType 0x{{[^ ]*}} 'int' sugar
+// CHECK-NEXT: TemplateTypeParmType 0x{{[^ ]*}} 'T' dependent depth 0 index 0
+} // namespace PR55886



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


[PATCH] D127312: [clang][dataflow] Remove IndirectionValue class, moving PointeeLoc field into PointerValue and ReferenceValue

2022-06-08 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision.
gribozavr2 added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:63
-auto *IndVal2 = cast(Val2);
-assert(IndVal1->getKind() == IndVal2->getKind());
-if (>getPointeeLoc() == >getPointeeLoc())

wyt wrote:
> gribozavr2 wrote:
> > This assert was lost in the new implementation.
> The assertion was to ensure that the values were either both Reference or 
> both Pointer, as an IndirectionValue could be either. 
> Since we've removed IndirectionVal and have separate casts for 
> ReferenceVal/PointerVal, the assertion is not neccessary.
Ah I see, the `cast` now plays the role of the assert.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127312

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


[PATCH] D127260: [clang-format] Remove braces of else blocks that embody an if block

2022-06-08 Thread Owen Pan via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5ead1f13a2d8: [clang-format] Remove braces of else blocks 
that embody an if block (authored by owenpan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127260

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -25466,6 +25466,117 @@
"  h;",
Style);
 
+  verifyFormat("if (a) {\n"
+   "  b;\n"
+   "} else if (c) {\n"
+   "  d;\n"
+   "  e;\n"
+   "}",
+   "if (a) {\n"
+   "  b;\n"
+   "} else {\n"
+   "  if (c) {\n"
+   "d;\n"
+   "e;\n"
+   "  }\n"
+   "}",
+   Style);
+
+  verifyFormat("if (a) {\n"
+   "  b;\n"
+   "  c;\n"
+   "} else if (d) {\n"
+   "  e;\n"
+   "  f;\n"
+   "}",
+   "if (a) {\n"
+   "  b;\n"
+   "  c;\n"
+   "} else {\n"
+   "  if (d) {\n"
+   "e;\n"
+   "f;\n"
+   "  }\n"
+   "}",
+   Style);
+
+  verifyFormat("if (a) {\n"
+   "  b;\n"
+   "} else if (c) {\n"
+   "  d;\n"
+   "} else {\n"
+   "  e;\n"
+   "  f;\n"
+   "}",
+   "if (a) {\n"
+   "  b;\n"
+   "} else {\n"
+   "  if (c) {\n"
+   "d;\n"
+   "  } else {\n"
+   "e;\n"
+   "f;\n"
+   "  }\n"
+   "}",
+   Style);
+
+  verifyFormat("if (a) {\n"
+   "  b;\n"
+   "} else if (c) {\n"
+   "  d;\n"
+   "} else if (e) {\n"
+   "  f;\n"
+   "  g;\n"
+   "}",
+   "if (a) {\n"
+   "  b;\n"
+   "} else {\n"
+   "  if (c) {\n"
+   "d;\n"
+   "  } else if (e) {\n"
+   "f;\n"
+   "g;\n"
+   "  }\n"
+   "}",
+   Style);
+
+  verifyFormat("if (a) {\n"
+   "  if (b)\n"
+   "c;\n"
+   "  else if (d) {\n"
+   "e;\n"
+   "f;\n"
+   "  }\n"
+   "} else {\n"
+   "  g;\n"
+   "}",
+   "if (a) {\n"
+   "  if (b)\n"
+   "c;\n"
+   "  else {\n"
+   "if (d) {\n"
+   "  e;\n"
+   "  f;\n"
+   "}\n"
+   "  }\n"
+   "} else {\n"
+   "  g;\n"
+   "}",
+   Style);
+
+  verifyFormat("if (a)\n"
+   "  if (b)\n"
+   "c;\n"
+   "  else {\n"
+   "if (d) {\n"
+   "  e;\n"
+   "  f;\n"
+   "}\n"
+   "  }\n"
+   "else\n"
+   "  g;",
+   Style);
+
   verifyFormat("if (a)\n"
"  b;\n"
"else if (c)\n"
Index: clang/lib/Format/UnwrappedLineParser.h
===
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -95,15 +95,16 @@
   bool parseLevel(const FormatToken *OpeningBrace = nullptr,
   bool CanContainBracedList = true,
   TokenType NextLBracesType = TT_Unknown,
-  IfStmtKind *IfKind = nullptr);
+  IfStmtKind *IfKind = nullptr,
+  FormatToken **IfLeftBrace = nullptr);
   bool mightFitOnOneLine(UnwrappedLine ,
  const FormatToken *OpeningBrace = nullptr) const;
-  void parseBlock(bool MustBeDeclaration = false, unsigned AddLevels = 1u,
-  bool MunchSemi = true, bool KeepBraces = true,
-  IfStmtKind *IfKind = nullptr,
-  bool UnindentWhitesmithsBraces = false,
-  bool CanContainBracedList = true,
-  TokenType NextLBracesType = TT_Unknown);
+  FormatToken *parseBlock(bool MustBeDeclaration = false,
+  unsigned AddLevels = 1u, bool MunchSemi = true,
+  bool KeepBraces = true, IfStmtKind *IfKind = nullptr,
+  bool UnindentWhitesmithsBraces = false,
+  

[clang] 5ead1f1 - [clang-format] Remove braces of else blocks that embody an if block

2022-06-08 Thread via cfe-commits

Author: owenca
Date: 2022-06-08T16:05:20-07:00
New Revision: 5ead1f13a2d8ca33e9e93c06acee941a857905c6

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

LOG: [clang-format] Remove braces of else blocks that embody an if block

Fixes #55663.

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

Added: 


Modified: 
clang/lib/Format/UnwrappedLineParser.cpp
clang/lib/Format/UnwrappedLineParser.h
clang/unittests/Format/FormatTest.cpp

Removed: 




diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp 
b/clang/lib/Format/UnwrappedLineParser.cpp
index aa1411d67799..d2d69aabfd80 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -469,18 +469,21 @@ bool 
UnwrappedLineParser::precededByCommentOrPPDirective() const {
 /// \param CanContainBracedList If the content can contain (at any level) a
 /// braced list.
 /// \param NextLBracesType The type for left brace found in this level.
-/// \param IfKind The if statement kind in the level.
+/// \param IfKind The \p if statement kind in the level.
+/// \param IfLeftBrace The left brace of the \p if block in the level.
 /// \returns true if a simple block of if/else/for/while, or false otherwise.
 /// (A simple block has a single statement.)
 bool UnwrappedLineParser::parseLevel(const FormatToken *OpeningBrace,
  bool CanContainBracedList,
  TokenType NextLBracesType,
- IfStmtKind *IfKind) {
+ IfStmtKind *IfKind,
+ FormatToken **IfLeftBrace) {
   auto NextLevelLBracesType = NextLBracesType == TT_CompoundRequirementLBrace
   ? TT_BracedListLBrace
   : TT_Unknown;
   const bool IsPrecededByCommentOrPPDirective =
   !Style.RemoveBracesLLVM || precededByCommentOrPPDirective();
+  FormatToken *IfLBrace = nullptr;
   bool HasDoWhile = false;
   bool HasLabel = false;
   unsigned StatementCount = 0;
@@ -498,9 +501,9 @@ bool UnwrappedLineParser::parseLevel(const FormatToken 
*OpeningBrace,
   kind = tok::r_brace;
 
 auto ParseDefault = [this, OpeningBrace, NextLevelLBracesType, IfKind,
- , , ] {
+ , , , ] {
   parseStructuralElement(!OpeningBrace, NextLevelLBracesType, IfKind,
- HasDoWhile ? nullptr : ,
+ , HasDoWhile ? nullptr : ,
  HasLabel ? nullptr : );
   ++StatementCount;
   assert(StatementCount > 0 && "StatementCount overflow!");
@@ -545,7 +548,11 @@ bool UnwrappedLineParser::parseLevel(const FormatToken 
*OpeningBrace,
   return false;
 }
 const FormatToken *Next = Tokens->peekNextToken();
-return Next->isNot(tok::comment) || Next->NewlinesBefore > 0;
+if (Next->is(tok::comment) && Next->NewlinesBefore == 0)
+  return false;
+if (IfLeftBrace)
+  *IfLeftBrace = IfLBrace;
+return true;
   }
   nextToken();
   addUnwrappedLine();
@@ -818,12 +825,10 @@ bool UnwrappedLineParser::mightFitOnOneLine(
   return Line.Level * Style.IndentWidth + Length <= ColumnLimit;
 }
 
-void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, unsigned 
AddLevels,
- bool MunchSemi, bool KeepBraces,
- IfStmtKind *IfKind,
- bool UnindentWhitesmithsBraces,
- bool CanContainBracedList,
- TokenType NextLBracesType) {
+FormatToken *UnwrappedLineParser::parseBlock(
+bool MustBeDeclaration, unsigned AddLevels, bool MunchSemi, bool 
KeepBraces,
+IfStmtKind *IfKind, bool UnindentWhitesmithsBraces,
+bool CanContainBracedList, TokenType NextLBracesType) {
   assert(FormatTok->isOneOf(tok::l_brace, TT_MacroBlockBegin) &&
  "'{' or macro block token expected");
   FormatToken *Tok = FormatTok;
@@ -844,7 +849,7 @@ void UnwrappedLineParser::parseBlock(bool 
MustBeDeclaration, unsigned AddLevels,
 
   // Bail out if there are too many levels. Otherwise, the stack might 
overflow.
   if (Line->Level > 300)
-return;
+return nullptr;
 
   if (MacroBlock && FormatTok->is(tok::l_paren))
 parseParens();
@@ -868,31 +873,37 @@ void UnwrappedLineParser::parseBlock(bool 
MustBeDeclaration, unsigned AddLevels,
   if (AddLevels > 0u && Style.BreakBeforeBraces != FormatStyle::BS_Whitesmiths)
 Line->Level += AddLevels;
 
+  FormatToken *IfLBrace = nullptr;
   const bool SimpleBlock =
-  parseLevel(Tok, CanContainBracedList, 

[PATCH] D126157: [clang-format][NFC] Insert/remove braces in clang/lib/Format/

2022-06-08 Thread Owen Pan via Phabricator via cfe-commits
owenpan added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2769
 
-  if (Style.isCSharp()) {
 do {

owenpan wrote:
> curdeius wrote:
> > owenpan wrote:
> > > From 
> > > https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements,
> > >  it's unclear whether the braces should be removed when a block has a 
> > > do-while loop as its single statement. Should I put the braces back and 
> > > add handling of do-while for `RemoveBracesLLVM`?
> > It's indeed not clear, I remember having paused over this code for a moment.
> > I believe that the best course of action is to grep for both cases in the 
> > codebase and see what's more frequent (and whether there's some sort of 
> > consensus).
> Good idea. I don't think there's any consensus specifically about this but 
> will ask on discourse.llvm.org. To err on the conservative side, I will 
> manually add the braces back for now.
See D126512.



Comment at: clang/lib/Format/WhitespaceManager.cpp:231-239
+  if (Change.Tok->is(TT_LineComment) || !Change.IsInsideToken) {
 LastBlockComment = 
-  else {
-if ((Change.StartOfBlockComment = LastBlockComment))
+  } else {
+if ((Change.StartOfBlockComment = LastBlockComment)) {
   Change.IndentationOffset =
   Change.StartOfTokenColumn -
   Change.StartOfBlockComment->StartOfTokenColumn;

owenpan wrote:
> curdeius wrote:
> > owenpan wrote:
> > > It would be nice to remove the braces of the `else` here. See 
> > > https://github.com/llvm/llvm-project/issues/55663.
> > +1. Would you fix it manually here then and fix it later?
> I will leave it as is and use it as a test case for later. Implementing this 
> would add more complexity to `RemoveBracesLLVM`, which is complex enough 
> already. I will look into it in a few months as I will be away for the summer.
See D127260.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126157

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


[PATCH] D127361: [NFC] clang: add test for PR55886

2022-06-08 Thread Matheus Izvekov via Phabricator via cfe-commits
mizvekov created this revision.
Herald added a project: All.
mizvekov requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Signed-off-by: Matheus Izvekov 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127361

Files:
  clang/test/AST/ast-dump-template-decls.cpp


Index: clang/test/AST/ast-dump-template-decls.cpp
===
--- clang/test/AST/ast-dump-template-decls.cpp
+++ clang/test/AST/ast-dump-template-decls.cpp
@@ -108,3 +108,22 @@
 // CHECK: FunctionTemplateDecl 0x{{[^ ]*}} parent 0x{{[^ ]*}} prev 0x{{[^ ]*}} 
 col:13 f
 // CHECK-NEXT: TemplateTypeParmDecl 0x{{[^ ]*}}  
col:20 typename depth 1 index 0 Uy
 void V::f() {}
+
+namespace PR55886 {
+template  struct C {
+  template  using type1 = U(T);
+};
+using type2 = typename C::type1;
+// CHECK:  TypeAliasDecl 0x{{[^ ]*}}  col:7 
type2 'typename C::type1':'void (int)'
+// CHECK-NEXT: ElaboratedType 0x{{[^ ]*}} 'typename C::type1' sugar
+// CHECK-NEXT: TemplateSpecializationType 0x{{[^ ]*}} 'type1' sugar 
alias type1
+// CHECK-NEXT: TemplateArgument type 'void'
+// CHECK-NEXT: BuiltinType 0x{{[^ ]*}} 'void'
+// CHECK-NEXT: FunctionProtoType 0x{{[^ ]*}} 'void (int)' cdecl
+// CHECK-NEXT: SubstTemplateTypeParmType 0x{{[^ ]*}} 'void' sugar
+// CHECK-NEXT: TemplateTypeParmType 0x{{[^ ]*}} 'U' dependent depth 0 index 0
+// CHECK-NEXT: TemplateTypeParm 0x{{[^ ]*}} 'U'
+// CHECK-NEXT: BuiltinType 0x{{[^ ]*}} 'void'
+// CHECK-NEXT: SubstTemplateTypeParmType 0x{{[^ ]*}} 'int' sugar
+// CHECK-NEXT: TemplateTypeParmType 0x{{[^ ]*}} 'T' dependent depth 0 index 0
+} // namespace PR55886


Index: clang/test/AST/ast-dump-template-decls.cpp
===
--- clang/test/AST/ast-dump-template-decls.cpp
+++ clang/test/AST/ast-dump-template-decls.cpp
@@ -108,3 +108,22 @@
 // CHECK: FunctionTemplateDecl 0x{{[^ ]*}} parent 0x{{[^ ]*}} prev 0x{{[^ ]*}}  col:13 f
 // CHECK-NEXT: TemplateTypeParmDecl 0x{{[^ ]*}}  col:20 typename depth 1 index 0 Uy
 void V::f() {}
+
+namespace PR55886 {
+template  struct C {
+  template  using type1 = U(T);
+};
+using type2 = typename C::type1;
+// CHECK:  TypeAliasDecl 0x{{[^ ]*}}  col:7 type2 'typename C::type1':'void (int)'
+// CHECK-NEXT: ElaboratedType 0x{{[^ ]*}} 'typename C::type1' sugar
+// CHECK-NEXT: TemplateSpecializationType 0x{{[^ ]*}} 'type1' sugar alias type1
+// CHECK-NEXT: TemplateArgument type 'void'
+// CHECK-NEXT: BuiltinType 0x{{[^ ]*}} 'void'
+// CHECK-NEXT: FunctionProtoType 0x{{[^ ]*}} 'void (int)' cdecl
+// CHECK-NEXT: SubstTemplateTypeParmType 0x{{[^ ]*}} 'void' sugar
+// CHECK-NEXT: TemplateTypeParmType 0x{{[^ ]*}} 'U' dependent depth 0 index 0
+// CHECK-NEXT: TemplateTypeParm 0x{{[^ ]*}} 'U'
+// CHECK-NEXT: BuiltinType 0x{{[^ ]*}} 'void'
+// CHECK-NEXT: SubstTemplateTypeParmType 0x{{[^ ]*}} 'int' sugar
+// CHECK-NEXT: TemplateTypeParmType 0x{{[^ ]*}} 'T' dependent depth 0 index 0
+} // namespace PR55886
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D127267: [NVPTX] Add setAuxTarget override rather than make a new TargetInfo

2022-06-08 Thread Ryan Prichard via Phabricator via cfe-commits
rprichard added a comment.

In D127267#3568388 , @rprichard wrote:

> Aside: There are only two calls to AllocateTarget: one in 
> TargetInfo::CreateTargetInfo and one in NVPTXTargetInfo::NVPTXTargetInfo. 
> This change removes NVPTXTargetInfo's call.

There are various places that call clang::TargetInfo::CreateTargetInfo, though, 
so if a call site had been providing a clang::TargetOptions with a set 
HostTriple field, but not calling clang::TargetInfo::setAuxTarget, then maybe 
the NVPTXTargetInfo behavior could change. I'll audit the calls of 
clang::TargetInfo::CreateTargetInfo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127267

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


[PATCH] D127357: [pseudo] wip/prototype: use LR0 instead of SLR1 table

2022-06-08 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision.
sammccall added a reviewer: hokein.
Herald added a project: All.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, alextsao1999.
Herald added a project: clang-tools-extra.

This ignores the lookahead token when deciding what to reduce, we always reduce
unconditionally. The main effects are:

- all potential partial parses are visible e.g. to error recovery. (In error 
scenarios, the intended reduction may not occur with SLR1 when the error occurs 
at the lookahead token).
- the size of the LR table is much smaller (In this patch we go from 340kB => 
120kB, and the encoding isn't efficient)
- we explore more dead-ends due to lack of lookahead, and parser runs slower
- with this patch, the LR0 table parses ~1/3 slower than the SLR1. (LR0 allows 
code simplifications so we may win some of this back)

This patch uses `eod` as a dummy lookahead token for reduce actions:
i.e. reduces are Actions[State, eod]. The structure of glrParse is preserved:
we still track heads as "pending actions". To allow either LR0 or SLR1 to be
used, we perform lookups for both the actual lookahead token and eod.

In a real implementation we'd probably want to:

- switch glrParse to track head nodes, instead of pending actions on them. This 
should simplify the code there somewhat.
- use a separate storage in LRTable for reduce actions, like a parallel 
vector (sorted by stateid) and vector (indexed by stateid)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127357

Files:
  clang-tools-extra/pseudo/benchmarks/Benchmark.cpp
  clang-tools-extra/pseudo/include/clang-pseudo/LRTable.h
  clang-tools-extra/pseudo/lib/GLR.cpp
  clang-tools-extra/pseudo/lib/grammar/LRTableBuild.cpp
  clang-tools-extra/pseudo/test/lr-build-basic.test
  clang-tools-extra/pseudo/test/lr-build-conflicts.test
  clang-tools-extra/pseudo/tool/ClangPseudo.cpp

Index: clang-tools-extra/pseudo/tool/ClangPseudo.cpp
===
--- clang-tools-extra/pseudo/tool/ClangPseudo.cpp
+++ clang-tools-extra/pseudo/tool/ClangPseudo.cpp
@@ -108,7 +108,7 @@
   llvm::outs() << G->dump();
 if (PrintGraph)
   llvm::outs() << clang::pseudo::LRGraph::buildLR0(*G).dumpForTests(*G);
-auto LRTable = clang::pseudo::LRTable::buildSLR(*G);
+auto LRTable = clang::pseudo::LRTable::buildLR0(*G);
 if (PrintTable)
   llvm::outs() << LRTable.dumpForTests(*G);
 if (PrintStatistics)
Index: clang-tools-extra/pseudo/test/lr-build-conflicts.test
===
--- clang-tools-extra/pseudo/test/lr-build-conflicts.test
+++ clang-tools-extra/pseudo/test/lr-build-conflicts.test
@@ -36,12 +36,10 @@
 # TABLE-NEXT: 'EOF': accept
 # TABLE-NEXT: '-': shift state 3
 # TABLE-NEXT: State 2
-# TABLE-NEXT: 'EOF': reduce by rule 1 'expr := IDENTIFIER'
-# TABLE-NEXT: '-': reduce by rule 1 'expr := IDENTIFIER'
+# TABLE-NEXT: 'EOD': reduce by rule 1 'expr := IDENTIFIER'
 # TABLE-NEXT: State 3
 # TABLE-NEXT: 'IDENTIFIER': shift state 2
 # TABLE-NEXT: 'expr': go to state 4
 # TABLE-NEXT: State 4
-# TABLE-NEXT: 'EOF': reduce by rule 0 'expr := expr - expr'
+# TABLE-NEXT: 'EOD': reduce by rule 0 'expr := expr - expr'
 # TABLE-NEXT: '-': shift state 3
-# TABLE-NEXT: '-': reduce by rule 0 'expr := expr - expr'
Index: clang-tools-extra/pseudo/test/lr-build-basic.test
===
--- clang-tools-extra/pseudo/test/lr-build-basic.test
+++ clang-tools-extra/pseudo/test/lr-build-basic.test
@@ -24,6 +24,6 @@
 # TABLE-NEXT: State 1
 # TABLE-NEXT: 'EOF': accept
 # TABLE-NEXT: State 2
-# TABLE-NEXT: 'EOF': reduce by rule 1 'expr := id'
+# TABLE-NEXT: 'EOD': reduce by rule 1 'expr := id'
 # TABLE-NEXT: State 3
-# TABLE-NEXT: 'EOF': reduce by rule 0 'id := IDENTIFIER'
+# TABLE-NEXT: 'EOD': reduce by rule 0 'id := IDENTIFIER'
Index: clang-tools-extra/pseudo/lib/grammar/LRTableBuild.cpp
===
--- clang-tools-extra/pseudo/lib/grammar/LRTableBuild.cpp
+++ clang-tools-extra/pseudo/lib/grammar/LRTableBuild.cpp
@@ -135,5 +135,31 @@
   return std::move(Build).build(G.table(), Graph.states().size());
 }
 
+LRTable LRTable::buildLR0(const Grammar ) {
+  auto Graph = LRGraph::buildLR0(G);
+  Builder Build(Graph.startStates());
+  for (const auto  : Graph.edges()) {
+Action Act = isToken(T.Label) ? Action::shift(T.Dst) : Action::goTo(T.Dst);
+Build.insert({T.Src, T.Label, Act});
+  }
+  assert(Graph.states().size() <= (1 << StateBits) &&
+ "Graph states execceds the maximum limit!");
+  auto FollowSets = followSets(G);
+  for (StateID SID = 0; SID < Graph.states().size(); ++SID) {
+for (const Item  : Graph.states()[SID].Items) {
+  // If we've just parsed the start symbol, we can accept the input.
+  if 

[PATCH] D127267: [NVPTX] Add setAuxTarget override rather than make a new TargetInfo

2022-06-08 Thread Ryan Prichard via Phabricator via cfe-commits
rprichard added a comment.

I don't expect this change to affect the compiler behavior by itself -- is 
there a particular test that should be written?

The code that I'm moving into NVPTXTargetInfo::setAuxTarget is already tested 
via clang/test/Preprocessor/cuda-types.cu.

Aside: There are only two calls to AllocateTarget: one in 
TargetInfo::CreateTargetInfo and one in NVPTXTargetInfo::NVPTXTargetInfo. This 
change removes NVPTXTargetInfo's call.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127267

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


[PATCH] D127271: [pseudo] Fix link time undefined reference to llvm::EnableABIBreakingChecks

2022-06-08 Thread John McIver via Phabricator via cfe-commits
jmciver added a comment.

Thanks @sammccall  for taking the time to review!

Can you please commit on my behalf?

Name: `John McIver`
Email: `john.mciver@gmail.com`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127271

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


[PATCH] D127260: [clang-format] Remove braces of else blocks that embody an if block

2022-06-08 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment.

In D127260#3567476 , @curdeius wrote:

> Still looks good. Was there a particular case where the previous version 
> didn't work?

The assertion I added on line 2626, which would ensure that the `RemoveBraces` 
lambda on line 898 didn't miss any potentially removable braces, would fail 
because `parseLevel()` was indiscriminately passing the `l_brace` of the last 
`if` in the level to its caller. This new version is also a tiny bit more 
efficient because `IfLBrace` on line 2624 will be null if the `else` block that 
encloses the `if` block is not a simple block.

Because neither version would remove non-optional braces, I decided not to add 
a test case just for the assertion, e.g.,

  if (a)
b;
  else {
c;
if (d) {
  e;
  f;
}
  }


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

https://reviews.llvm.org/D127260

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


[PATCH] D127271: [pseudo] Fix link time undefined reference to llvm::EnableABIBreakingChecks

2022-06-08 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127271

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


[PATCH] D127271: [pseudo] Fix link time undefined reference to llvm::EnableABIBreakingChecks

2022-06-08 Thread John McIver via Phabricator via cfe-commits
jmciver added a comment.

This patch is associated to: https://github.com/llvm/llvm-project/issues/55935


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127271

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


[PATCH] D126723: [pseudo] GC GSS nodes, reuse them with a freelist

2022-06-08 Thread Sam McCall via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG94b2ca18c10b: [pseudo] GC GSS nodes, reuse them with a 
freelist (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126723

Files:
  clang-tools-extra/pseudo/include/clang-pseudo/GLR.h
  clang-tools-extra/pseudo/lib/GLR.cpp
  clang-tools-extra/pseudo/tool/ClangPseudo.cpp
  clang-tools-extra/pseudo/unittests/GLRTest.cpp

Index: clang-tools-extra/pseudo/unittests/GLRTest.cpp
===
--- clang-tools-extra/pseudo/unittests/GLRTest.cpp
+++ clang-tools-extra/pseudo/unittests/GLRTest.cpp
@@ -393,6 +393,28 @@
 "[  0, end) └─IDENTIFIER := tok[0]\n");
 }
 
+TEST(GSSTest, GC) {
+  //  ┌-A-┬-AB
+  //  ├-B-┘
+  // Root-+-C
+  //  ├-D
+  //  └-E
+  GSS GSStack;
+  auto *Root = GSStack.addNode(0, nullptr, {});
+  auto *A = GSStack.addNode(0, nullptr, {Root});
+  auto *B = GSStack.addNode(0, nullptr, {Root});
+  auto *C = GSStack.addNode(0, nullptr, {Root});
+  auto *D = GSStack.addNode(0, nullptr, {Root});
+  auto *AB = GSStack.addNode(0, nullptr, {A, B});
+
+  EXPECT_EQ(1u, GSStack.gc({AB, C})) << "D is destroyed";
+  EXPECT_EQ(0u, GSStack.gc({AB, C})) << "D is already gone";
+  auto *E = GSStack.addNode(0, nullptr, {Root});
+  EXPECT_EQ(D, E) << "Storage of GCed node D is reused for E";
+  EXPECT_EQ(3u, GSStack.gc({A, E})) << "Destroys B, AB, C";
+  EXPECT_EQ(1u, GSStack.gc({E})) << "Destroys A";
+}
+
 } // namespace
 } // namespace pseudo
 } // namespace clang
Index: clang-tools-extra/pseudo/tool/ClangPseudo.cpp
===
--- clang-tools-extra/pseudo/tool/ClangPseudo.cpp
+++ clang-tools-extra/pseudo/tool/ClangPseudo.cpp
@@ -134,7 +134,7 @@
 llvm::outs() << "Forest bytes: " << Arena.bytes()
  << " nodes: " << Arena.nodeCount() << "\n";
 llvm::outs() << "GSS bytes: " << GSS.bytes()
- << " nodes: " << GSS.nodeCount() << "\n";
+ << " nodes: " << GSS.nodesCreated() << "\n";
   }
 }
   }
Index: clang-tools-extra/pseudo/lib/GLR.cpp
===
--- clang-tools-extra/pseudo/lib/GLR.cpp
+++ clang-tools-extra/pseudo/lib/GLR.cpp
@@ -12,6 +12,7 @@
 #include "clang/Basic/TokenKinds.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorHandling.h"
@@ -65,6 +66,18 @@
   std::vector NewHeads = {
   GSS.addNode(/*State=*/Params.Table.getStartState(StartSymbol),
   /*ForestNode=*/nullptr, {})};
+  auto MaybeGC = [&, Roots(std::vector{}), I(0u)]() mutable {
+assert(PendingShift.empty() && PendingReduce.empty() &&
+   PendingAccept.empty() && "Running GC at the wrong time!");
+
+if (++I != 20) // Run periodically to balance CPU and memory usage.
+  return;
+I = 0;
+
+// We need to copy the list: Roots is consumed by the GC.
+Roots = NewHeads;
+GSS.gc(std::move(Roots));
+  };
   for (const ForestNode  : Terminals) {
 LLVM_DEBUG(llvm::dbgs() << llvm::formatv("Next token {0} (id={1})\n",
  G.symbolName(Terminal.symbol()),
@@ -80,6 +93,7 @@
 
 glrShift(PendingShift, Terminal, Params,
  [&](const GSS::Node *NewHead) { NewHeads.push_back(NewHead); });
+MaybeGC();
   }
   LLVM_DEBUG(llvm::dbgs() << llvm::formatv("Next is eof\n"));
   for (const auto *Heads : NewHeads)
@@ -373,5 +387,72 @@
   assert(Sequences.empty());
 }
 
+const GSS::Node *GSS::addNode(LRTable::StateID State, const ForestNode *Symbol,
+  llvm::ArrayRef Parents) {
+  Node *Result = new (allocate(Parents.size()))
+  Node({State, GCParity, static_cast(Parents.size())});
+  Alive.push_back(Result);
+  ++NodesCreated;
+  Result->Payload = Symbol;
+  if (!Parents.empty())
+llvm::copy(Parents, reinterpret_cast(Result + 1));
+  return Result;
+}
+
+GSS::Node *GSS::allocate(unsigned Parents) {
+  if (FreeList.size() <= Parents)
+FreeList.resize(Parents + 1);
+  auto  = FreeList[Parents];
+  if (!SizedList.empty()) {
+auto *Result = SizedList.back();
+SizedList.pop_back();
+return Result;
+  }
+  return static_cast(
+  Arena.Allocate(sizeof(Node) + Parents * sizeof(Node *), alignof(Node)));
+}
+
+void GSS::destroy(Node *N) {
+  unsigned ParentCount = N->ParentCount;
+  N->~Node();
+  assert(FreeList.size() > ParentCount && "established on construction!");
+  FreeList[ParentCount].push_back(N);
+}
+
+unsigned GSS::gc(std::vector &) {
+#ifndef NDEBUG
+  auto ParityMatches = [&](const Node *N) { return N->GCParity == 

[clang-tools-extra] 94b2ca1 - [pseudo] GC GSS nodes, reuse them with a freelist

2022-06-08 Thread Sam McCall via cfe-commits

Author: Sam McCall
Date: 2022-06-08T23:39:59+02:00
New Revision: 94b2ca18c10bfb4107a850d63148fc51e65d9512

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

LOG: [pseudo] GC GSS nodes, reuse them with a freelist

Most GSS nodes have short effective lifetimes, keeping them around until the
end of the parse is wasteful. Mark and sweep them every 20 tokens.

When parsing clangd/AST.cpp, this reduces the GSS memory from 1MB to 20kB.
We pay ~5% performance for this according to the glrParse benchmark.
(Parsing more tokens between GCs doesn't seem to improve this further).

Compared to the refcounting approach in https://reviews.llvm.org/D126337, this
is simpler (at least the complexity is better isolated) and has >2x less
overhead. It doesn't provide death handlers (for error-handling) but we have
an alternative solution in mind.

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

Added: 


Modified: 
clang-tools-extra/pseudo/include/clang-pseudo/GLR.h
clang-tools-extra/pseudo/lib/GLR.cpp
clang-tools-extra/pseudo/tool/ClangPseudo.cpp
clang-tools-extra/pseudo/unittests/GLRTest.cpp

Removed: 




diff  --git a/clang-tools-extra/pseudo/include/clang-pseudo/GLR.h 
b/clang-tools-extra/pseudo/include/clang-pseudo/GLR.h
index 70d6e46e5aed3..fc5ebeda1f279 100644
--- a/clang-tools-extra/pseudo/include/clang-pseudo/GLR.h
+++ b/clang-tools-extra/pseudo/include/clang-pseudo/GLR.h
@@ -68,6 +68,8 @@ struct GSS {
   struct alignas(struct Node *) Node {
 // LR state describing how parsing should continue from this head.
 LRTable::StateID State;
+// Used internally to track reachability during garbage collection.
+bool GCParity;
 // Number of the parents of this node.
 // The parents hold previous parsed symbols, and may resume control after
 // this node is reduced.
@@ -77,10 +79,6 @@ struct GSS {
 // (In the literature, the node is attached to the *edge* to the parent).
 const ForestNode *Payload = nullptr;
 
-// FIXME: Most nodes live a fairly short time, and are simply discarded.
-// Is it worth refcounting them (we have empty padding) and returning to a
-// freelist, to keep the working set small?
-
 llvm::ArrayRef parents() const {
   return llvm::makeArrayRef(reinterpret_cast(this + 
1),
 ParentCount);
@@ -90,23 +88,26 @@ struct GSS {
 
   // Allocates a new node in the graph.
   const Node *addNode(LRTable::StateID State, const ForestNode *Symbol,
-  llvm::ArrayRef Parents) {
-++NodeCount;
-Node *Result = new (Arena.Allocate(
-sizeof(Node) + Parents.size() * sizeof(Node *), alignof(Node)))
-Node({State, static_cast(Parents.size())});
-Result->Payload = Symbol;
-if (!Parents.empty())
-  llvm::copy(Parents, reinterpret_cast(Result + 1));
-return Result;
-  }
+  llvm::ArrayRef Parents);
+  // Frees all nodes not reachable as ancestors of Roots, and returns the 
count.
+  // Calling this periodically prevents steady memory growth of the GSS.
+  unsigned gc(std::vector &);
 
   size_t bytes() const { return Arena.getTotalMemory() + sizeof(*this); }
-  size_t nodeCount() const { return NodeCount; }
+  size_t nodesCreated() const { return NodesCreated; }
 
 private:
+  // Nodes are recycled using freelists.
+  // They are variable size, so use one free-list per distinct #parents.
+  std::vector> FreeList;
+  Node *allocate(unsigned Parents);
+  void destroy(Node *N);
+  // The list of nodes created and not destroyed - our candidates for gc().
+  std::vector Alive;
+  bool GCParity = false; // All nodes should match this, except during GC.
+
   llvm::BumpPtrAllocator Arena;
-  unsigned NodeCount = 0;
+  unsigned NodesCreated = 0;
 };
 llvm::raw_ostream <<(llvm::raw_ostream &, const GSS::Node &);
 

diff  --git a/clang-tools-extra/pseudo/lib/GLR.cpp 
b/clang-tools-extra/pseudo/lib/GLR.cpp
index 10fa0d0568c02..1031e3dd2007c 100644
--- a/clang-tools-extra/pseudo/lib/GLR.cpp
+++ b/clang-tools-extra/pseudo/lib/GLR.cpp
@@ -12,6 +12,7 @@
 #include "clang/Basic/TokenKinds.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorHandling.h"
@@ -65,6 +66,18 @@ const ForestNode (const TokenStream , const 
ParseParams ,
   std::vector NewHeads = {
   GSS.addNode(/*State=*/Params.Table.getStartState(StartSymbol),
   /*ForestNode=*/nullptr, {})};
+  auto MaybeGC = [&, Roots(std::vector{}), I(0u)]() mutable 
{
+assert(PendingShift.empty() && PendingReduce.empty() &&
+   PendingAccept.empty() && "Running GC at the wrong time!");
+
+if (++I != 20) // Run 

[PATCH] D126723: [pseudo] GC GSS nodes, reuse them with a freelist

2022-06-08 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments.



Comment at: clang-tools-extra/pseudo/lib/GLR.cpp:78
+// We need to copy the list: Roots is consumed by the GC.
+Roots = NewHeads;
+GSS.gc(std::move(Roots));

hokein wrote:
> nit: I'd rather pass the NewHeads as a vector parameter, and get rid of the 
> `Roots` variable in the lambda, but up to you.
That would result in reallocating the array every time: we would allocate a new 
vector to copy  NewHeads into, copy it, gc() would own it and destroy 
(deallocate) it at the end.

As the code is now, Roots stays alive. Each time we GC we fill it up with 
elements, and then gc() clears them out again, but we never actually deallocate 
the buffer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126723

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


[clang-tools-extra] bbc58c5 - [pseudo] Restore accidentally removed debug print

2022-06-08 Thread Sam McCall via cfe-commits

Author: Sam McCall
Date: 2022-06-08T23:39:34+02:00
New Revision: bbc58c5e9ba39b234b6f526b313bbc35cbc2ba0f

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

LOG: [pseudo] Restore accidentally removed debug print

Added: 


Modified: 
clang-tools-extra/pseudo/lib/grammar/LRTable.cpp

Removed: 




diff  --git a/clang-tools-extra/pseudo/lib/grammar/LRTable.cpp 
b/clang-tools-extra/pseudo/lib/grammar/LRTable.cpp
index 74e4fb0fedb5..16c4029eec08 100644
--- a/clang-tools-extra/pseudo/lib/grammar/LRTable.cpp
+++ b/clang-tools-extra/pseudo/lib/grammar/LRTable.cpp
@@ -47,6 +47,7 @@ Statistics of the LR parsing table:
 std::string LRTable::dumpForTests(const Grammar ) const {
   std::string Result;
   llvm::raw_string_ostream OS(Result);
+  OS << "LRTable:\n";
   for (StateID S = 0; S < StateOffset.size() - 1; ++S) {
 OS << llvm::formatv("State {0}\n", S);
 for (uint16_t Terminal = 0; Terminal < NumTerminals; ++Terminal) {



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


[PATCH] D127006: [pseudo] Invert rows/columns of LRTable storage for speedup. NFC

2022-06-08 Thread Sam McCall via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG93bcff8aa855: [pseudo] Invert rows/columns of LRTable 
storage for speedup. NFC (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127006

Files:
  clang-tools-extra/pseudo/include/clang-pseudo/LRTable.h
  clang-tools-extra/pseudo/lib/grammar/LRTable.cpp
  clang-tools-extra/pseudo/lib/grammar/LRTableBuild.cpp
  clang-tools-extra/pseudo/tool/ClangPseudo.cpp

Index: clang-tools-extra/pseudo/tool/ClangPseudo.cpp
===
--- clang-tools-extra/pseudo/tool/ClangPseudo.cpp
+++ clang-tools-extra/pseudo/tool/ClangPseudo.cpp
@@ -111,6 +111,8 @@
 auto LRTable = clang::pseudo::LRTable::buildSLR(*G);
 if (PrintTable)
   llvm::outs() << LRTable.dumpForTests(*G);
+if (PrintStatistics)
+  llvm::outs() << LRTable.dumpStatistics();
 
 if (ParseableStream) {
   clang::pseudo::ForestArena Arena;
Index: clang-tools-extra/pseudo/lib/grammar/LRTableBuild.cpp
===
--- clang-tools-extra/pseudo/lib/grammar/LRTableBuild.cpp
+++ clang-tools-extra/pseudo/lib/grammar/LRTableBuild.cpp
@@ -44,7 +44,7 @@
   : StartStates(StartStates) {}
 
   bool insert(Entry E) { return Entries.insert(std::move(E)).second; }
-  LRTable build(const GrammarTable ) && {
+  LRTable build(const GrammarTable , unsigned NumStates) && {
 // E.g. given the following parsing table with 3 states and 3 terminals:
 //
 //ab c
@@ -55,44 +55,34 @@
 // +---++---+-+
 //
 // The final LRTable:
-//  - TerminalOffset: [a] = 0, [b] = 1, [c] = 4, [d] = 4 (d is a sentinel)
-//  -  States: [ 1,0,  0,  2]
-//Actions: [ acc, s0, r0, r1]
-//   ~~~ corresponding range for terminal a
-//~~ corresponding range for terminal b
-// First step, we sort all entries by (Symbol, State, Action).
+//  - StateOffset: [s0] = 0, [s1] = 2, [s2] = 3, [sentinel] = 4
+//  - Symbols: [ b,   b,   a,  b]
+//Actions: [ s0, r0, acc, r1]
+//   ~~ range for state 0
+//    range for state 1
+//~~ range for state 2
+// First step, we sort all entries by (State, Symbol, Action).
 std::vector Sorted(Entries.begin(), Entries.end());
 llvm::sort(Sorted, [](const Entry , const Entry ) {
-  return std::forward_as_tuple(L.Symbol, L.State, L.Act.opaque()) <
- std::forward_as_tuple(R.Symbol, R.State, R.Act.opaque());
+  return std::forward_as_tuple(L.State, L.Symbol, L.Act.opaque()) <
+ std::forward_as_tuple(R.State, R.Symbol, R.Act.opaque());
 });
 
 LRTable Table;
 Table.Actions.reserve(Sorted.size());
-Table.States.reserve(Sorted.size());
+Table.Symbols.reserve(Sorted.size());
 // We are good to finalize the States and Actions.
 for (const auto  : Sorted) {
   Table.Actions.push_back(E.Act);
-  Table.States.push_back(E.State);
+  Table.Symbols.push_back(E.Symbol);
 }
 // Initialize the terminal and nonterminal offset, all ranges are empty by
 // default.
-Table.TerminalOffset = std::vector(GT.Terminals.size() + 1, 0);
-Table.NontermOffset = std::vector(GT.Nonterminals.size() + 1, 0);
+Table.StateOffset = std::vector(NumStates + 1, 0);
 size_t SortedIndex = 0;
-for (SymbolID NonterminalID = 0; NonterminalID < Table.NontermOffset.size();
- ++NonterminalID) {
-  Table.NontermOffset[NonterminalID] = SortedIndex;
-  while (SortedIndex < Sorted.size() &&
- Sorted[SortedIndex].Symbol == NonterminalID)
-++SortedIndex;
-}
-for (size_t Terminal = 0; Terminal < Table.TerminalOffset.size();
- ++Terminal) {
-  Table.TerminalOffset[Terminal] = SortedIndex;
-  while (SortedIndex < Sorted.size() &&
- Sorted[SortedIndex].Symbol ==
- tokenSymbol(static_cast(Terminal)))
+for (StateID State = 0; State < Table.StateOffset.size(); ++State) {
+  Table.StateOffset[State] = SortedIndex;
+  while (SortedIndex < Sorted.size() && Sorted[SortedIndex].State == State)
 ++SortedIndex;
 }
 Table.StartStates = std::move(StartStates);
@@ -106,10 +96,13 @@
 
 LRTable LRTable::buildForTests(const GrammarTable ,
llvm::ArrayRef Entries) {
+  StateID MaxState = 0;
+  for (const auto  : Entries)
+MaxState = std::max(MaxState, Entry.State);
   Builder Build({});
   for (const Entry  : Entries)
 Build.insert(E);
-  return std::move(Build).build(GT);
+  return std::move(Build).build(GT, /*NumStates=*/MaxState + 1);
 }
 
 LRTable 

[clang-tools-extra] 93bcff8 - [pseudo] Invert rows/columns of LRTable storage for speedup. NFC

2022-06-08 Thread Sam McCall via cfe-commits

Author: Sam McCall
Date: 2022-06-08T23:35:14+02:00
New Revision: 93bcff8aa85512f2b24e822d857b426c31b6d051

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

LOG: [pseudo] Invert rows/columns of LRTable storage for speedup. NFC

There are more states than symbols.
This means first partioning the action list by state leaves us with a smaller
range to binary search over. This improves find() a lot and glrParse() by 7%.
The tradeoff is storing more smaller ranges increases the size of the offsets
array, overall grammar memory is +1% (337->340KB).

Before:
glrParse188795975 ns188778003 ns   77 
bytes_per_second=1.98068M/s
After:
glrParse175936203 ns175916873 ns   81 
bytes_per_second=2.12548M/s

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

Added: 


Modified: 
clang-tools-extra/pseudo/include/clang-pseudo/LRTable.h
clang-tools-extra/pseudo/lib/grammar/LRTable.cpp
clang-tools-extra/pseudo/lib/grammar/LRTableBuild.cpp
clang-tools-extra/pseudo/tool/ClangPseudo.cpp

Removed: 




diff  --git a/clang-tools-extra/pseudo/include/clang-pseudo/LRTable.h 
b/clang-tools-extra/pseudo/include/clang-pseudo/LRTable.h
index 735ef3b781ad4..3cff9aec8c5eb 100644
--- a/clang-tools-extra/pseudo/include/clang-pseudo/LRTable.h
+++ b/clang-tools-extra/pseudo/include/clang-pseudo/LRTable.h
@@ -145,9 +145,8 @@ class LRTable {
 
   size_t bytes() const {
 return sizeof(*this) + Actions.capacity() * sizeof(Action) +
-   States.capacity() * sizeof(StateID) +
-   NontermOffset.capacity() * sizeof(uint32_t) +
-   TerminalOffset.capacity() * sizeof(uint32_t);
+   Symbols.capacity() * sizeof(SymbolID) +
+   StateOffset.capacity() * sizeof(uint32_t);
   }
 
   std::string dumpStatistics() const;
@@ -170,17 +169,15 @@ class LRTable {
   // Conceptually the LR table is a multimap from (State, SymbolID) => Action.
   // Our physical representation is quite 
diff erent for compactness.
 
-  // Index is nonterminal SymbolID, value is the offset into States/Actions
-  // where the entries for this nonterminal begin.
-  // Give a nonterminal id, the corresponding half-open range of StateIdx is
-  // [NontermIdx[id], NontermIdx[id+1]).
-  std::vector NontermOffset;
-  // Similar to NontermOffset, but for terminals, index is tok::TokenKind.
-  std::vector TerminalOffset;
-  // Parallel to Actions, the value is State (rows of the matrix).
-  // Grouped by the SymbolID, and only subranges are sorted.
-  std::vector States;
-  // A flat list of available actions, sorted by (SymbolID, State).
+  // Index is StateID, value is the offset into Symbols/Actions
+  // where the entries for this state begin.
+  // Give a state id, the corresponding half-open range of Symbols/Actions is
+  // [StateOffset[id], StateOffset[id+1]).
+  std::vector StateOffset;
+  // Parallel to Actions, the value is SymbolID (columns of the matrix).
+  // Grouped by the StateID, and only subranges are sorted.
+  std::vector Symbols;
+  // A flat list of available actions, sorted by (State, SymbolID).
   std::vector Actions;
   // A sorted table, storing the start state for each target parsing symbol.
   std::vector> StartStates;

diff  --git a/clang-tools-extra/pseudo/lib/grammar/LRTable.cpp 
b/clang-tools-extra/pseudo/lib/grammar/LRTable.cpp
index 745ad44bafa6c..74e4fb0fedb53 100644
--- a/clang-tools-extra/pseudo/lib/grammar/LRTable.cpp
+++ b/clang-tools-extra/pseudo/lib/grammar/LRTable.cpp
@@ -34,27 +34,20 @@ llvm::raw_ostream <<(llvm::raw_ostream , const 
LRTable::Action ) {
 }
 
 std::string LRTable::dumpStatistics() const {
-  StateID NumOfStates = 0;
-  for (StateID It : States)
-NumOfStates = std::max(It, NumOfStates);
   return llvm::formatv(R"(
 Statistics of the LR parsing table:
 number of states: {0}
 number of actions: {1}
 size of the table (bytes): {2}
 )",
-   NumOfStates, Actions.size(), bytes())
+   StateOffset.size() - 1, Actions.size(), bytes())
   .str();
 }
 
 std::string LRTable::dumpForTests(const Grammar ) const {
   std::string Result;
   llvm::raw_string_ostream OS(Result);
-  StateID MaxState = 0;
-  for (StateID It : States)
-MaxState = std::max(MaxState, It);
-  OS << "LRTable:\n";
-  for (StateID S = 0; S <= MaxState; ++S) {
+  for (StateID S = 0; S < StateOffset.size() - 1; ++S) {
 OS << llvm::formatv("State {0}\n", S);
 for (uint16_t Terminal = 0; Terminal < NumTerminals; ++Terminal) {
   SymbolID TokID = tokenSymbol(static_cast(Terminal));
@@ -97,26 +90,22 @@ LRTable::StateID LRTable::getGoToState(StateID State,
 }
 
 llvm::ArrayRef LRTable::find(StateID Src, SymbolID ID) const {
-  size_t Idx = isToken(ID) ? static_cast(symbolToToken(ID)) : ID;
-  

[PATCH] D125677: [pseudo] Remove the explicit Accept actions.

2022-06-08 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision.
sammccall added inline comments.



Comment at: clang-tools-extra/pseudo/lib/GLR.cpp:86
+  const ForestNode *Result = nullptr;
+  StateID AcceptState = Params.Table.getGoToState(StartState, StartSymbol);
+  glrReduce(PendingReduce, Params, [&](const GSS::Node *NewHead) {

hokein wrote:
> sammccall wrote:
> > this line introduces a requirement that the start symbol be a nonterminal - 
> > if this is new, can we doc it?
> this is not a new requirement, it is rather implicit previously (we use 
> `G.findNonterminal` to pass the start-symbol argument of the `glrParse`). I 
> added an assertion in the glrParse.
> 
> It would be nice to diagnose it in the bnf grammar parsing time (will add a 
> followup patch).
FWIW I think an assertion (and possibly a comment) is the right tradeoff here, 
maintaining diagnostics for all this stuff seems like overkill


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125677

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


[PATCH] D126536: [pseudo] Add grammar annotations support.

2022-06-08 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.



Comment at: clang-tools-extra/pseudo/include/clang-pseudo/Grammar.h:91
+// attribute value (all attributes share a namespace).
+// An ExtensionID is the index into a table of attribute values.
+using ExtensionID = uint16_t;

nit: I'd drop this sentence, as using it to index into a table is only ever 
used to produce debug representations I think - usually we use ExtensionID as 
an index into a map



Comment at: clang-tools-extra/pseudo/include/clang-pseudo/Grammar.h:219
+  // ExtensionID is the index of the table.
+  std::vector AttributeValues;
 };

looking at this again, ExtensionNames seems clearer as ExtensionNames[ExtID] 
seems more obvious that the kinds agree. But up to you


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126536

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


[PATCH] D126984: [clang] Add initial support for gcc's optimize function attribute

2022-06-08 Thread Stephen Long via Phabricator via cfe-commits
steplong added a comment.

In D126984#3567822 , @aaron.ballman 
wrote:

> Two final (I hope) nits! One is in the code, the other is that this should 
> have a release note for the new attribute. Assuming precommit CI pipeline 
> passes, then I think this is all set.

Haha, no worries! I appreciate the review


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126984

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


[PATCH] D127269: Add llvm's Support lib to the psuedoCXX library

2022-06-08 Thread Nathan Lanza via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGdd2f2909188b: Add llvms Support lib to the psuedoCXX 
library (authored by lanza).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127269

Files:
  clang-tools-extra/pseudo/lib/cxx/CMakeLists.txt


Index: clang-tools-extra/pseudo/lib/cxx/CMakeLists.txt
===
--- clang-tools-extra/pseudo/lib/cxx/CMakeLists.txt
+++ clang-tools-extra/pseudo/lib/cxx/CMakeLists.txt
@@ -1,3 +1,7 @@
+set(LLVM_LINK_COMPONENTS
+  Support
+  )
+
 add_clang_library(clangPseudoCXX
   CXX.cpp
 


Index: clang-tools-extra/pseudo/lib/cxx/CMakeLists.txt
===
--- clang-tools-extra/pseudo/lib/cxx/CMakeLists.txt
+++ clang-tools-extra/pseudo/lib/cxx/CMakeLists.txt
@@ -1,3 +1,7 @@
+set(LLVM_LINK_COMPONENTS
+  Support
+  )
+
 add_clang_library(clangPseudoCXX
   CXX.cpp
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] dd2f290 - Add llvm's Support lib to the psuedoCXX library

2022-06-08 Thread Nathan Lanza via cfe-commits

Author: Nathan Lanza
Date: 2022-06-08T17:12:02-04:00
New Revision: dd2f2909188bf56716248675c58e4699c1c6b903

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

LOG: Add llvm's Support lib to the psuedoCXX library

This is failing to find `EnableABIBreakingCheck` at link time. Add
Support to provide it here.

Reviewed By: hokein

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

Added: 


Modified: 
clang-tools-extra/pseudo/lib/cxx/CMakeLists.txt

Removed: 




diff  --git a/clang-tools-extra/pseudo/lib/cxx/CMakeLists.txt 
b/clang-tools-extra/pseudo/lib/cxx/CMakeLists.txt
index 9e10f2ba5388..775b3b8c9538 100644
--- a/clang-tools-extra/pseudo/lib/cxx/CMakeLists.txt
+++ b/clang-tools-extra/pseudo/lib/cxx/CMakeLists.txt
@@ -1,3 +1,7 @@
+set(LLVM_LINK_COMPONENTS
+  Support
+  )
+
 add_clang_library(clangPseudoCXX
   CXX.cpp
 



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


[PATCH] D126061: [clang] Reject non-declaration C++11 attributes on declarations

2022-06-08 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: clang/lib/Parse/ParseStmt.cpp:242-246
+Decl = ParseDeclaration(DeclaratorContext::Block, DeclEnd, CXX11Attrs,
+GNUAttrs, );
   } else {
-Decl = ParseDeclaration(DeclaratorContext::Block, DeclEnd, Attrs);
+Decl = ParseDeclaration(DeclaratorContext::Block, DeclEnd, CXX11Attrs,
+GNUAttrs);

mboehme wrote:
> rsmith wrote:
> > mboehme wrote:
> > > aaron.ballman wrote:
> > > > rsmith wrote:
> > > > > I think this is the only place where we're passing decl-specifier-seq 
> > > > > attributes into `ParseDeclaration`. There are only two possible cases 
> > > > > here:
> > > > > 
> > > > > 1) We have a simple-declaration, and can `ParseSimpleDeclaration` 
> > > > > directly.
> > > > > 2) We have a static-assert, using, or namespace alias declaration, 
> > > > > which don't permit attributes at all.
> > > > > 
> > > > > So I think we *could* simplify this so that decl-spec attributes are 
> > > > > never passed into `ParseDeclaration`:
> > > > > 
> > > > > * If the next token is `kw_static_assert`, `kw_using`, or 
> > > > > `kw_namespace`, then prohibit attributes and call `ParseDeclaration`.
> > > > > * Otherwise, call `ParseSimpleDeclaration` and pass in the attributes.
> > > > > * Remove the `DeclSpecAttrs` list from `ParseDeclaration`.
> > > > > 
> > > > > I'm not requesting a change here -- I'm not sure whether that's a net 
> > > > > improvement or not -- but it seems worth considering.
> > > > I think this is a good avenue to explore -- passing in two different 
> > > > attribute lists means someone will at some point get it wrong by 
> > > > accident, so only having one attribute list reduces the chances for 
> > > > bugs later. I don't imagine static assertions or namespaces will get 
> > > > leading attributes. However...
> > > > 
> > > > I think asm-declaration and using-directive are also a bit special -- 
> > > > they're allowed to have leading attributes: 
> > > > http://eel.is/c++draft/dcl.dcl#nt:asm-declaration and 
> > > > http://eel.is/c++draft/dcl.dcl#nt:using-directive
> > > > 
> > > > Do we also have to handle opaque-enum-declaration here? 
> > > > http://eel.is/c++draft/dcl.dcl#nt:block-declaration
> > > I investigated this, but I'm not sure it's a net win.
> > > 
> > > > * If the next token is `kw_static_assert`, `kw_using`, or 
> > > > `kw_namespace`, then prohibit attributes and call `ParseDeclaration`.
> > > 
> > > Or if the next token is `kw_inline` and the token after that is 
> > > `kw_namespace`...
> > > 
> > > I think going down this path would mean duplicating all of the case 
> > > distinctions in `ParseDeclaration()` -- and if we add more cases in the 
> > > future, we'd have to make sure to replicate them here. I think overall it 
> > > keeps the code simpler to accept the additional `DeclSpecAttrs` parameter.
> > > 
> > > > Do we also have to handle opaque-enum-declaration here? 
> > > > http://eel.is/c++draft/dcl.dcl#nt:block-declaration
> > > 
> > > A moot point, probably, but I believe this is handled by 
> > > `ParseEnumSpecifier()`, which is called from 
> > > `ParseDeclarationSpecifiers()`, so there would be no need to handle it 
> > > separately.
> > > > * If the next token is `kw_static_assert`, `kw_using`, or 
> > > > `kw_namespace`, then prohibit attributes and call `ParseDeclaration`.
> > > 
> > > Or if the next token is `kw_inline` and the token after that is 
> > > `kw_namespace`...
> > 
> > This function is parsing block declarations, so we don't have to handle 
> > `inline namespace` here because that can't appear as a block declaration. 
> > The only reason we need to handle `namespace` is to support namespace 
> > aliases (`namespace A = B;`), which are (perhaps surprisingly) allowed at 
> > block scope.
> > 
> > > I think going down this path would mean duplicating all of the case 
> > > distinctions in `ParseDeclaration()` -- and if we add more cases in the 
> > > future, we'd have to make sure to replicate them here. I think overall it 
> > > keeps the code simpler to accept the additional `DeclSpecAttrs` parameter.
> > 
> > It's not all the cases, only the block-declaration cases. If we wanted to 
> > directly follow the grammar, we could split a `ParseBlockDeclaration` 
> > function out of `ParseDeclaration`, but I guess there'd be so little left 
> > in `ParseDeclaration` that it wouldn't be worth it, and we'd still have to 
> > pass the `DeclSpecAttrs` parameter to `ParseBlockDeclaration`, so that 
> > doesn't save us anything. I suppose it'd also regress our diagnostic 
> > quality for declarations that aren't allowed at block scope. OK, I'm 
> > convinced :-)
> Just confirming: You're not asking me to do anything here, correct?
Yes, no action requested. Thanks for talking through this option with me! :)



Comment at: clang/lib/Sema/ParsedAttr.cpp:228
+  switch 

[PATCH] D127338: [Clang][CoverageMapping] Fix compile time explosions by adjusting only appropriated skipped ranges

2022-06-08 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno added a comment.

> Just noticed that 3 test cases failed. Please fix them before landing.

Sure thing, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127338

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


[PATCH] D127338: [Clang][CoverageMapping] Fix compile time explosions by adjusting only appropriated skipped ranges

2022-06-08 Thread Zequan Wu via Phabricator via cfe-commits
zequanwu added a comment.

Just noticed that 3 test cases failed. Please fix them before landing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127338

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


[PATCH] D127338: [Clang][CoverageMapping] Fix compile time explosions by adjusting only appropriated skipped ranges

2022-06-08 Thread Zequan Wu via Phabricator via cfe-commits
zequanwu accepted this revision.
zequanwu added a comment.
This revision is now accepted and ready to land.

LGTM, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127338

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


[PATCH] D126929: Add sanitizer metadata attributes to clang IR gen.

2022-06-08 Thread Vitaly Buka via Phabricator via cfe-commits
vitalybuka added inline comments.



Comment at: clang/lib/CodeGen/SanitizerMetadata.cpp:55
+
+  bool IsExcluded = CGM.isInNoSanitizeList(GV, Loc, Ty);
+  IsExcluded |= (NoSanitizeMask == SanitizerKind::All);

vitalybuka wrote:
> it can be in some weird ubsan check ignore list, and this way it will 
> propagate on asan/hwasan
> I don't think you can avoid extending isInNoSanitizeList (in a separate patch)
you you can introduce:

```
bool CodeGenModule::isInNoSanitizeList(SanitizerMask Kind, llvm::GlobalVariable 
*GV,
   SourceLocation Loc) const {
```

similar to

```
bool CodeGenModule::isInNoSanitizeList(SanitizerMask Kind, llvm::Function *Fn,
   SourceLocation Loc) const {
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126929

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


[PATCH] D126984: [clang] Add initial support for gcc's optimize function attribute

2022-06-08 Thread Stephen Long via Phabricator via cfe-commits
steplong updated this revision to Diff 435313.
steplong added a comment.

- Add Arg when creating Attr


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126984

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/attr-optimize.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/attr-optimize.c

Index: clang/test/Sema/attr-optimize.c
===
--- /dev/null
+++ clang/test/Sema/attr-optimize.c
@@ -0,0 +1,52 @@
+// RUN: %clang_cc1 -verify -fsyntax-only %s
+
+__attribute__((optimize(a))) // expected-error {{use of undeclared identifier 'a'}}
+void
+f1() {}
+
+int b = 1;
+__attribute__((optimize(b))) // expected-error {{'optimize' attribute requires a string}}
+void
+f2() {}
+
+__attribute__((optimize("O0", "O1"))) // expected-error {{'optimize' attribute takes one argument}}
+void
+f3() {}
+
+__attribute__((optimize("Og"))) // expected-no-error
+void
+f4() {}
+
+__attribute__((optimize("O-1"))) // expected-warning {{invalid optimization level 'O-1' specified; the argument to '-O' should be a non-negative integer, '-Os', '-Oz', '-Og', or '-Ofast'; attribute ignored}}
+void
+f5() {}
+
+__attribute__((optimize("O+1"))) // expected-warning {{invalid optimization level 'O+1' specified; the argument to '-O' should be a non-negative integer, '-Os', '-Oz', '-Og', or '-Ofast'; attribute ignored}}
+void
+f6() {}
+
+__attribute__((optimize("O0"))) // expected-no-error
+void
+f7() {}
+
+__attribute__((optimize("Os"))) // expected-no-error
+void
+f8() {}
+
+__attribute__((optimize("O44"))) // expected-no-error
+void
+f9() {}
+
+__attribute__((optimize("Oz"))) // expected-no-error
+void
+f10() {}
+
+__attribute__((optimize("Ofast"))) // expected-no-error
+void
+f11() {}
+
+__attribute__((optimize("O"))) // expected-no-error
+void
+f12() {}
+
+__attribute__((optimize("O0"))) // expected-error {{expected identifier or '('}}
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -142,6 +142,7 @@
 // CHECK-NEXT: ObjCSubclassingRestricted (SubjectMatchRule_objc_interface)
 // CHECK-NEXT: OpenCLIntelReqdSubGroupSize (SubjectMatchRule_function)
 // CHECK-NEXT: OpenCLNoSVM (SubjectMatchRule_variable)
+// CHECK-NEXT: Optimize (SubjectMatchRule_function)
 // CHECK-NEXT: OptimizeNone (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: Overloadable (SubjectMatchRule_function)
 // CHECK-NEXT: Owner (SubjectMatchRule_record_not_is_union)
Index: clang/test/CodeGen/attr-optimize.c
===
--- /dev/null
+++ clang/test/CodeGen/attr-optimize.c
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -O2 -S -emit-llvm %s -o - | FileCheck %s --check-prefix=O2
+// RUN: %clang_cc1 -O0 -S -emit-llvm %s -o - | FileCheck %s --check-prefix=O0
+
+__attribute__((optimize("O0"))) void f1(void) {}
+// O2: @f1{{.*}}[[ATTR_OPTNONE:#[0-9]+]]
+// O0: @f1{{.*}}[[ATTR_OPTNONE:#[0-9]+]]
+
+__attribute__((optimize("Os"))) void f2(void) {}
+// O2: @f2{{.*}}[[ATTR_OPTSIZE:#[0-9]+]]
+// O0: @f2{{.*}}[[ATTR_OPTNONE]]
+
+__attribute__((optimize("Og"))) void f3(void) {}
+// O2: @f3{{.*}}[[ATTR_OPTSIZE]]
+// O0: @f3{{.*}}[[ATTR_OPTNONE]]
+
+__attribute__((optimize("Oz"))) void f4(void) {}
+// O2: @f4{{.*}}[[ATTR_OPTSIZE]]
+// O0: @f4{{.*}}[[ATTR_OPTNONE]]
+
+__attribute__((optimize("Ofast"))) void f5(void) {}
+// O2: @f5{{.*}}[[ATTR_OPTSIZE]]
+// O0: @f5{{.*}}[[ATTR_OPTNONE]]
+
+// O2: attributes [[ATTR_OPTNONE]] = { {{.*}}optnone{{.*}} }
+// O2: attributes [[ATTR_OPTSIZE]] = { {{.*}}optsize{{.*}} }
+
+// Check that O0 overrides the attribute
+// O0: attributes [[ATTR_OPTNONE]] = { {{.*}}optnone{{.*}} }
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -4833,6 +4833,46 @@
 D->addAttr(Optnone);
 }
 
+static void handleOptimizeAttr(Sema , Decl *D, const ParsedAttr ) {
+  StringRef Arg;
+  if (!S.checkStringLiteralArgumentAttr(AL, 0, Arg))
+return;
+
+  StringRef Level;
+  // Check if argument is prefixed with "-O" or "O"
+  if (Arg.str().rfind("-O", 0) == 0)
+Level = Arg.substr(2);
+  else if (Arg.str().rfind("O", 0) == 0)
+Level = Arg.substr(1);
+  else
+S.Diag(AL.getLoc(), diag::warn_invalid_optimize_attr_level) << Arg;
+
+  llvm::StringMap StrToKind = {
+  {"", OptimizeAttr::O0},  {"s", OptimizeAttr::Os},
+  {"g", 

[PATCH] D126929: Add sanitizer metadata attributes to clang IR gen.

2022-06-08 Thread Vitaly Buka via Phabricator via cfe-commits
vitalybuka added a comment.

No need to recover my snapshot, I'll just comment here.




Comment at: clang/lib/CodeGen/SanitizerMetadata.cpp:20
 
+using GlobalVariable = llvm::GlobalVariable;
+using GVSanitizerMetadata = GlobalVariable::SanitizerMetadata;

Seems inconsistent with the rest.
it no so frequently use here to justify indirection.



Comment at: clang/lib/CodeGen/SanitizerMetadata.cpp:55
+
+  bool IsExcluded = CGM.isInNoSanitizeList(GV, Loc, Ty);
+  IsExcluded |= (NoSanitizeMask == SanitizerKind::All);

it can be in some weird ubsan check ignore list, and this way it will propagate 
on asan/hwasan
I don't think you can avoid extending isInNoSanitizeList (in a separate patch)



Comment at: clang/lib/CodeGen/SanitizerMetadata.cpp:60
+ SanitizerKind::KernelAddress)) {
+IsExcluded |= CGM.isInNoSanitizeList(GV, Loc, Ty, "address");
+IsExcluded |= NoSanitizeSet.hasOneOf(SanitizerKind::Address |

the last arg is not sanitizer, just a particulate check of it
like "init" here llvm-project/compiler-rt/lib/asan/asan_ignorelist.txt



Comment at: clang/lib/CodeGen/SanitizerMetadata.cpp:64
  bool IsDynInit) {
-  if (!isAsanHwasanOrMemTag(CGM.getLangOpts().Sanitize))
-return;

May be early isAsanHwasanOrMemTag check here is useful to avoid string stuff 
below for compilation without sanitizers.



Comment at: clang/lib/CodeGen/SanitizerMetadata.cpp:70
 
-  auto getNoSanitizeMask = [](const VarDecl ) {
-if (D.hasAttr())

I don't insist but one it's cleaner with lambda and return
if you prefer your way please revert lambda in a separate patch



Comment at: clang/lib/CodeGen/SanitizerMetadata.cpp:91-93
+  llvm::LLVMContext  = CGM.getLLVMContext();
   llvm::Metadata *LocDescr = nullptr;
   llvm::Metadata *GlobalName = nullptr;

undo this line move



Comment at: clang/lib/CodeGen/SanitizerMetadata.cpp:118
+  SanitizerMask NoSanitizeMask;
+  for (auto *Attr : D.specific_attrs()) {
+NoSanitizeMask |= Attr->getMask();

don't use {} for one liners



Comment at: clang/lib/CodeGen/SanitizerMetadata.cpp:122
+
+  if (D.hasAttr()) {
+NoSanitizeMask = SanitizerKind::All;

this if probably should go before the loop



Comment at: clang/lib/CodeGen/SanitizerMetadata.cpp:126
+
   std::string QualName;
   llvm::raw_string_ostream OS(QualName);

moving QualName calculation is unnecessary, please move it back before 
NoSanitizeMask


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126929

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


[PATCH] D126100: Add sanitizer-specific GlobalValue attributes.

2022-06-08 Thread Vitaly Buka via Phabricator via cfe-commits
vitalybuka accepted this revision.
vitalybuka added inline comments.
This revision is now accepted and ready to land.



Comment at: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:1389
+
+  if (GV.hasSanitizerMetadata())
+Vals.push_back(serializeSanitizerMetadata(GV.getSanitizerMetadata()));

I guess VBR encoding does not like UINT_MAX, it will occupy more space then 
unconditional pushback.
Please use the same approach as:  Vals.push_back(GV.hasComdat() ? 
VE.getComdatID(GV.getComdat()) : 0);


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126100

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


[PATCH] D125677: [pseudo] Remove the explicit Accept actions.

2022-06-08 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments.



Comment at: clang-tools-extra/pseudo/lib/GLR.cpp:86
+  const ForestNode *Result = nullptr;
+  StateID AcceptState = Params.Table.getGoToState(StartState, StartSymbol);
+  glrReduce(PendingReduce, Params, [&](const GSS::Node *NewHead) {

sammccall wrote:
> this line introduces a requirement that the start symbol be a nonterminal - 
> if this is new, can we doc it?
this is not a new requirement, it is rather implicit previously (we use 
`G.findNonterminal` to pass the start-symbol argument of the `glrParse`). I 
added an assertion in the glrParse.

It would be nice to diagnose it in the bnf grammar parsing time (will add a 
followup patch).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125677

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


[PATCH] D125677: [pseudo] Remove the explicit Accept actions.

2022-06-08 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 435309.
hokein marked an inline comment as done.
hokein added a comment.

switch to the previous simple version


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125677

Files:
  clang-tools-extra/pseudo/include/clang-pseudo/LRTable.h
  clang-tools-extra/pseudo/lib/GLR.cpp
  clang-tools-extra/pseudo/lib/grammar/LRTable.cpp
  clang-tools-extra/pseudo/lib/grammar/LRTableBuild.cpp
  clang-tools-extra/pseudo/test/lr-build-basic.test
  clang-tools-extra/pseudo/test/lr-build-conflicts.test
  clang-tools-extra/pseudo/unittests/GLRTest.cpp
  clang-tools-extra/pseudo/unittests/LRTableTest.cpp

Index: clang-tools-extra/pseudo/unittests/LRTableTest.cpp
===
--- clang-tools-extra/pseudo/unittests/LRTableTest.cpp
+++ clang-tools-extra/pseudo/unittests/LRTableTest.cpp
@@ -33,7 +33,7 @@
   std::vector Entries = {
   {/* State */ 0, tokenSymbol(tok::semi), Action::shift(0)},
   {/* State */ 0, tokenSymbol(tok::semi), Action::reduce(0)},
-  {/* State */ 1, tokenSymbol(tok::eof), Action::accept(2)},
+  {/* State */ 1, tokenSymbol(tok::eof), Action::reduce(2)},
   {/* State */ 2, tokenSymbol(tok::semi), Action::reduce(1)}};
   GrammarTable GT;
   LRTable T = LRTable::buildForTests(GT, Entries);
@@ -41,7 +41,7 @@
   EXPECT_THAT(T.find(0, tokenSymbol(tok::semi)),
   UnorderedElementsAre(Action::shift(0), Action::reduce(0)));
   EXPECT_THAT(T.find(1, tokenSymbol(tok::eof)),
-  UnorderedElementsAre(Action::accept(2)));
+  UnorderedElementsAre(Action::reduce(2)));
   EXPECT_THAT(T.find(1, tokenSymbol(tok::semi)), IsEmpty());
   EXPECT_THAT(T.find(2, tokenSymbol(tok::semi)),
   UnorderedElementsAre(Action::reduce(1)));
Index: clang-tools-extra/pseudo/unittests/GLRTest.cpp
===
--- clang-tools-extra/pseudo/unittests/GLRTest.cpp
+++ clang-tools-extra/pseudo/unittests/GLRTest.cpp
@@ -393,6 +393,29 @@
 "[  0, end) └─IDENTIFIER := tok[0]\n");
 }
 
+TEST_F(GLRTest, NoExplicitAccept) {
+  build(R"bnf(
+_ := test
+
+test := IDENTIFIER test
+test := IDENTIFIER
+  )bnf");
+  clang::LangOptions LOptions;
+  // Given the following input, and the grammar above, we perform two reductions
+  // of the nonterminal `test` when the next token is `eof`, verify that the
+  // parser stops at the right state.
+  const TokenStream  = cook(lex("id id", LOptions), LOptions);
+  auto LRTable = LRTable::buildSLR(*G);
+
+  const ForestNode  =
+  glrParse(Tokens, {*G, LRTable, Arena, GSStack}, id("test"));
+  EXPECT_EQ(Parsed.dumpRecursive(*G),
+"[  0, end) test := IDENTIFIER test\n"
+"[  0,   1) ├─IDENTIFIER := tok[0]\n"
+"[  1, end) └─test := IDENTIFIER\n"
+"[  1, end)   └─IDENTIFIER := tok[1]\n");
+}
+
 } // namespace
 } // namespace pseudo
 } // namespace clang
Index: clang-tools-extra/pseudo/test/lr-build-conflicts.test
===
--- clang-tools-extra/pseudo/test/lr-build-conflicts.test
+++ clang-tools-extra/pseudo/test/lr-build-conflicts.test
@@ -33,7 +33,6 @@
 # TABLE-NEXT: 'IDENTIFIER': shift state 2
 # TABLE-NEXT: 'expr': go to state 1
 # TABLE-NEXT: State 1
-# TABLE-NEXT: 'EOF': accept
 # TABLE-NEXT: '-': shift state 3
 # TABLE-NEXT: State 2
 # TABLE-NEXT: 'EOF': reduce by rule 1 'expr := IDENTIFIER'
Index: clang-tools-extra/pseudo/test/lr-build-basic.test
===
--- clang-tools-extra/pseudo/test/lr-build-basic.test
+++ clang-tools-extra/pseudo/test/lr-build-basic.test
@@ -22,7 +22,6 @@
 # TABLE-NEXT: 'expr': go to state 1
 # TABLE-NEXT: 'id': go to state 2
 # TABLE-NEXT: State 1
-# TABLE-NEXT: 'EOF': accept
 # TABLE-NEXT: State 2
 # TABLE-NEXT: 'EOF': reduce by rule 1 'expr := id'
 # TABLE-NEXT: State 3
Index: clang-tools-extra/pseudo/lib/grammar/LRTableBuild.cpp
===
--- clang-tools-extra/pseudo/lib/grammar/LRTableBuild.cpp
+++ clang-tools-extra/pseudo/lib/grammar/LRTableBuild.cpp
@@ -124,11 +124,11 @@
   auto FollowSets = followSets(G);
   for (StateID SID = 0; SID < Graph.states().size(); ++SID) {
 for (const Item  : Graph.states()[SID].Items) {
-  // If we've just parsed the start symbol, we can accept the input.
-  if (G.lookupRule(I.rule()).Target == G.underscore() && !I.hasNext()) {
-Build.insert({SID, tokenSymbol(tok::eof), Action::accept(I.rule())});
+  // If we've just parsed the start symbol, this means we successfully parse
+  // the input. We don't add the reduce action of `_ := start_symbol` in the
+  // LRTable (the GLR parser handles it specifically).
+  if 

[PATCH] D126929: Add sanitizer metadata attributes to clang IR gen.

2022-06-08 Thread Vitaly Buka via Phabricator via cfe-commits
vitalybuka added a comment.

In D126929#3567491 , @hctim wrote:

> Update.

Sorry if it want clear that I some changes. You undone my snapshot.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126929

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


[clang] f0a6a57 - Switch links to use https consistently

2022-06-08 Thread Aaron Ballman via cfe-commits

Author: Aaron Ballman
Date: 2022-06-08T15:53:55-04:00
New Revision: f0a6a573309b1807de057f032a6db5e8d4a3c10d

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

LOG: Switch links to use https consistently

The WG14 website was recently updated to support SSL, and we might as
well make use of that.

Added: 


Modified: 
clang/www/c_status.html

Removed: 




diff  --git a/clang/www/c_status.html b/clang/www/c_status.html
index e561f5359a16..17d18d8724f8 100644
--- a/clang/www/c_status.html
+++ b/clang/www/c_status.html
@@ -85,7 +85,7 @@ C99 implementation status
 
 Clang implements a significant portion of the ISO 9899:1999 (C99) standard, 
but the status of individual proposals is still under investigation.
 Note, the list of C99 features comes from the C99 committee draft. Not all 
C99 documents are publicly available, so the documents referenced in this 
section may be inaccurate, unknown, or not linked.
-
   Yes
 
 
   remove implicit function declaration
-  http://www.open-std.org/jtc1/sc22/wg14/www/docs/n636.htm;>N636
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n636.htm;>N636
   Unknown
 
 
   preprocessor arithmetic done in intmax_t/uintmax_t
-  http://www.open-std.org/jtc1/sc22/wg14/www/docs/n736.htm;>N736
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n736.htm;>N736
   Unknown
 
 
   mixed declarations and code
-  http://www.open-std.org/jtc1/sc22/wg14/www/docs/n740.htm;>N740
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n740.htm;>N740
   Yes
 
 
@@ -264,17 +264,17 @@ C99 implementation status
 
 
   integer constant type rules
-  http://www.open-std.org/jtc1/sc22/wg14/www/docs/n629.htm;>N629
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n629.htm;>N629
   Unknown
 
 
   integer promotion rules
-  http://www.open-std.org/jtc1/sc22/wg14/www/docs/n725.htm;>N725
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n725.htm;>N725
   Unknown
 
 
   macros with a variable number of arguments
-  http://www.open-std.org/jtc1/sc22/wg14/www/docs/n707.htm;>N707
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n707.htm;>N707
   Yes
 
 
@@ -289,12 +289,12 @@ C99 implementation status
 
 
   inline functions
-  http://www.open-std.org/jtc1/sc22/wg14/www/docs/n741.htm;>N741
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n741.htm;>N741
   Yes
 
 
   boolean type in stdbool.h
-  http://www.open-std.org/jtc1/sc22/wg14/www/docs/n815.htm;>N815
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n815.htm;>N815
   Yes
 
 
@@ -319,7 +319,7 @@ C99 implementation status
 
 
   _Pragma preprocessing operator
-  http://www.open-std.org/jtc1/sc22/wg14/www/docs/n634.ps;>N634
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n634.ps;>N634
   
@@ -329,26 +329,26 @@ C99 implementation status
   standard pragmas
 
   
-http://www.open-std.org/jtc1/sc22/wg14/www/docs/n631.htm;>N631
+https://www.open-std.org/jtc1/sc22/wg14/www/docs/n631.htm;>N631
 Unknown
   
   
-http://www.open-std.org/jtc1/sc22/wg14/www/docs/n696.ps;>N696
+https://www.open-std.org/jtc1/sc22/wg14/www/docs/n696.ps;>N696
 Unknown
   
 
   __func__ predefined identifier
-  http://www.open-std.org/jtc1/sc22/wg14/www/docs/n611.ps;>N611
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n611.ps;>N611
   Yes
 
 
   va_copy macro
-  http://www.open-std.org/jtc1/sc22/wg14/www/docs/n671.htm;>N671
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n671.htm;>N671
   Yes
 
 
   LIA compatibility annex
-  http://www.open-std.org/jtc1/sc22/wg14/www/docs/n792.htm;>N792
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n792.htm;>N792
   No
 
 
@@ -368,7 +368,7 @@ C99 implementation status
 
 
   relaxed restrictions on portable header names
-  http://www.open-std.org/jtc1/sc22/wg14/www/docs/n772.htm;>N772
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n772.htm;>N772
   Unknown
 
 
@@ -395,218 +395,218 @@ C11 implementation status
  
 
   A finer-grained specification for sequencing
-  http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1252.htm;>N1252
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1252.htm;>N1252
   Unknown
 
 
   Clarification of expressions
-  http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1282.pdf;>N1282
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1282.pdf;>N1282
   Unknown
 
 
   Extending the lifetime of 

[clang] b685426 - Add missing entries for Annex F and Annex H to the C status page

2022-06-08 Thread Aaron Ballman via cfe-commits

Author: Aaron Ballman
Date: 2022-06-08T15:53:54-04:00
New Revision: b685426970df3b12ac3640110e43db2ee3f340c6

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

LOG: Add missing entries for Annex F and Annex H to the C status page

Added: 


Modified: 
clang/www/c_status.html

Removed: 




diff  --git a/clang/www/c_status.html b/clang/www/c_status.html
index 3ba814c757cb1..e561f5359a163 100644
--- a/clang/www/c_status.html
+++ b/clang/www/c_status.html
@@ -894,6 +894,11 @@ C2x implementation status
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2728.htm;>N2728
   Unknown
 
+
+  IEC 60559 binding
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2749.pdf;>N2749
+  Unknown
+
 
   Static initialization of DFP zeros
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2755.htm;>N2755
@@ -905,6 +910,11 @@ C2x implementation status
   Yes
 
 
+
+  Annex F overflow and underflow
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2747.pdf;>N2747
+  Yes
+
 
   Remove UB from Incomplete Types in Function Parameters
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2770.pdf;>N2770
@@ -950,6 +960,16 @@ C2x implementation status
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2843.pdf;>N2843
   Unknown
 
+
+  Clarification about expression transformations
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2846.pdf;>N2846
+  Unknown
+
+
+  Contradiction about INFINITY macro
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2848.pdf;>N2848
+  Unknown
+
 
   Require exact-width integer type interfaces
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2872.htm;>N2872
@@ -961,6 +981,11 @@ C2x implementation status
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2701.htm;>N2701
   Yes
 
+
+  Quantum exponent of NaN (version 2)
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2754.htm;>N2754
+  Unknown
+
 
   The noreturn attribute
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2764.pdf;>N2764
@@ -1011,6 +1036,11 @@ C2x implementation status
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2844.pdf;>N2844
   Unknown
 
+
+  Revised Suggestions of Change for Numerically Equal/Equivalent
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2847.pdf;>N2847
+  Unknown
+
 
   5.2.4.2.2 Cleanup, Again Again (N2806 update)
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2879.pdf;>N2879
@@ -1037,6 +1067,11 @@ C2x implementation status
 
   
 
+
+  Type annex tgmath narrowing macros with integer args v2
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2931.pdf;>N2931
+  Unknown
+
 
   Revise spelling of keywords v7
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2934.pdf;>N2934
@@ -1053,6 +1088,11 @@ C2x implementation status
   Yes
 
 
+
+  Annex X (replacing Annex H) for IEC 60559 interchange
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2601.pdf;>N2601
+  No
+
 
   Indeterminate Values and Trap Representations
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2861.pdf;>N2861



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


[PATCH] D127338: [Clang][CoverageMapping] Fix compile time explosions by adjusting only appropriated skipped ranges

2022-06-08 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno created this revision.
bruno added reviewers: vsk, zequanwu.
Herald added subscribers: hoy, modimo, wenlei.
Herald added a project: All.
bruno requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

D83592  added comments to be part of skipped 
regions, and as part of that, it
also shrinks a skipped range if it spans a line that contains a non-comment
token. This is done by `adjustSkippedRange`.

The `adjustSkippedRange` currently runs on skipped regions that are not
comments, causing a 5min regression while building a big C++ files without any
comments.

Fix the compile time introduced in D83592  by 
tagging SkippedRange with kind
information and use that to decide what needs additional processing. There are
no good way to add a testcase for this that I'm aware.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127338

Files:
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/lib/CodeGen/CoverageMappingGen.h

Index: clang/lib/CodeGen/CoverageMappingGen.h
===
--- clang/lib/CodeGen/CoverageMappingGen.h
+++ clang/lib/CodeGen/CoverageMappingGen.h
@@ -31,15 +31,29 @@
 class Stmt;
 
 struct SkippedRange {
+  enum Kind {
+PPIfElse, // Preprocessor #if/#else ...
+EmptyLine,
+Comment,
+  };
+
   SourceRange Range;
   // The location of token before the skipped source range.
   SourceLocation PrevTokLoc;
   // The location of token after the skipped source range.
   SourceLocation NextTokLoc;
+  // The nature of this skipped range
+  Kind RangeKind;
+
+  bool isComment() { return RangeKind == Comment; }
+  bool isEmptyLine() { return RangeKind == EmptyLine; }
+  bool isPPIfElse() { return RangeKind == PPIfElse; }
 
-  SkippedRange(SourceRange Range, SourceLocation PrevTokLoc = SourceLocation(),
+  SkippedRange(SourceRange Range, Kind K,
+   SourceLocation PrevTokLoc = SourceLocation(),
SourceLocation NextTokLoc = SourceLocation())
-  : Range(Range), PrevTokLoc(PrevTokLoc), NextTokLoc(NextTokLoc) {}
+  : Range(Range), PrevTokLoc(PrevTokLoc), NextTokLoc(NextTokLoc),
+RangeKind(K) {}
 };
 
 /// Stores additional source code information like skipped ranges which
@@ -62,7 +76,7 @@
 
   std::vector () { return SkippedRanges; }
 
-  void AddSkippedRange(SourceRange Range);
+  void AddSkippedRange(SourceRange Range, SkippedRange::Kind RangeKind);
 
   void SourceRangeSkipped(SourceRange Range, SourceLocation EndifLoc) override;
 
Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -60,26 +60,27 @@
   return CoverageInfo;
 }
 
-void CoverageSourceInfo::AddSkippedRange(SourceRange Range) {
+void CoverageSourceInfo::AddSkippedRange(SourceRange Range,
+ SkippedRange::Kind RangeKind) {
   if (EmptyLineCommentCoverage && !SkippedRanges.empty() &&
   PrevTokLoc == SkippedRanges.back().PrevTokLoc &&
   SourceMgr.isWrittenInSameFile(SkippedRanges.back().Range.getEnd(),
 Range.getBegin()))
 SkippedRanges.back().Range.setEnd(Range.getEnd());
   else
-SkippedRanges.push_back({Range, PrevTokLoc});
+SkippedRanges.push_back({Range, RangeKind, PrevTokLoc});
 }
 
 void CoverageSourceInfo::SourceRangeSkipped(SourceRange Range, SourceLocation) {
-  AddSkippedRange(Range);
+  AddSkippedRange(Range, SkippedRange::PPIfElse);
 }
 
 void CoverageSourceInfo::HandleEmptyline(SourceRange Range) {
-  AddSkippedRange(Range);
+  AddSkippedRange(Range, SkippedRange::EmptyLine);
 }
 
 bool CoverageSourceInfo::HandleComment(Preprocessor , SourceRange Range) {
-  AddSkippedRange(Range);
+  AddSkippedRange(Range, SkippedRange::Comment);
   return false;
 }
 
@@ -335,6 +336,8 @@
   /// This shrinks the skipped range if it spans a line that contains a
   /// non-comment token. If shrinking the skipped range would make it empty,
   /// this returns None.
+  /// Note this function can potentially be expensive because
+  /// getSpellingLineNumber uses getLineNumber, which is expensive.
   Optional adjustSkippedRange(SourceManager ,
   SourceLocation LocStart,
   SourceLocation LocEnd,
@@ -382,9 +385,14 @@
   auto CovFileID = getCoverageFileID(LocStart);
   if (!CovFileID)
 continue;
-  Optional SR =
-  adjustSkippedRange(SM, LocStart, LocEnd, I.PrevTokLoc, I.NextTokLoc);
-  if (!SR.hasValue())
+  Optional SR;
+  if (I.isComment())
+SR = adjustSkippedRange(SM, LocStart, LocEnd, I.PrevTokLoc,
+I.NextTokLoc);
+  else if (I.isPPIfElse())
+SR = {SM, LocStart, LocEnd};
+
+  if 

[PATCH] D112579: Allow non-variadic functions to be attributed with `__attribute__((format))`

2022-06-08 Thread Félix Cloutier via Phabricator via cfe-commits
fcloutier updated this revision to Diff 435302.
fcloutier added a comment.
Herald added a project: All.

Apologies the long delay: things happened and I was pulled away. I have some 
time to finish this change now. I recommend re-reading the discussion up to now 
since it's not _that_ long and it provides a lot of very useful context.

The new change addresses requests from the previous round. The most substantial 
changes are around how Clang detects that a format string is being forwarded to 
another format function. This is now expressed in terms of transitions from 
format argument passing styles, such that given the following 3 function 
archetypes:

  c
  void fixed(const char *, int) __attribute__((format(printf, 1, 2)));
  void variadic(const char *, ...) __attribute__((format(printf, 1, 2)));
  void valist(const char *, va_list) __attribute__((format(printf, 1, 0)));

there are no warnings for:

- a `variadic` function forwarding its format to a `valist` function
- a `valist` function forwarding its format to another `valist` function
- a `fixed` function forwarding its format to another `fixed` function (new)
- a `fixed` function forwarding its format to a `variadic` function (new)

In other words, for instance, `fixed` can call `variadic` in its implementation 
without a warning. Anything else, like forwarding the format of a `valist` 
function to a `fixed` function, is a diagnostic.

`fixed` to `fixed`/`variadic` transitions don't check that arguments have 
compatible types, but it conceivably could. This is a limitation of the current 
implementation. However, at this point, we don't think that this is a very 
worthwhile effort; this could change in the future if adoption of the `format` 
attribute on functions with a fixed signature ramps up.

I also added a number of tests to make sure that we still have reasonable 
warnings. One interesting edge case when using `__attribute__((format))` on 
functions with fixed parameters is that it's possible to come up with 
combinations that are impossible, for instance:

  c
  struct nontrivial { nontrivial(); ~nontrivial(); };
  void foo(const char *, nontrivial) __attribute__((format(printf, 1, 2)));

It's not a diagnostic to declare this function, however it is always a 
diagnostic to call it because no printf format specifier can format a 
`nontrivial` object. Ideally there would be a diagnostic on the declaration, 
but I think that it's sufficient as it is.


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

https://reviews.llvm.org/D112579

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/FormatString.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Sema/attr-format.c
  clang/test/Sema/format-strings.c
  clang/test/SemaCXX/attr-format.cpp

Index: clang/test/SemaCXX/attr-format.cpp
===
--- clang/test/SemaCXX/attr-format.cpp
+++ clang/test/SemaCXX/attr-format.cpp
@@ -1,7 +1,11 @@
 // RUN: %clang_cc1 -fsyntax-only -Wformat-nonliteral -verify %s
+#include 
+
+int printf(const char *fmt, ...) __attribute__((format(printf, 1, 2)));
+
 struct S {
-  static void f(const char*, ...) __attribute__((format(printf, 1, 2)));
-  static const char* f2(const char*) __attribute__((format_arg(1)));
+  static void f(const char *, ...) __attribute__((format(printf, 1, 2)));
+  static const char *f2(const char *) __attribute__((format_arg(1)));
 
   // GCC has a hidden 'this' argument in member functions which is why
   // the format argument is argument 2 here.
@@ -38,6 +42,47 @@
 
 // Make sure we interpret member operator calls as having an implicit
 // this argument.
-void test_operator_call(S s, const char* str) {
+void test_operator_call(S s, const char *str) {
   s("%s", str);
 }
+
+template 
+void format(const char *fmt, Args &&...args) // expected-warning{{GCC requires functions with the 'format' attribute to be variadic}}
+__attribute__((format(printf, 1, 2)));
+
+template 
+Arg (Arg ) { return a; }
+
+struct foo {
+  int big[10];
+  foo();
+  ~foo();
+
+  template 
+  void format(const char *const fmt, Args &&...args) // expected-warning{{GCC requires functions with the 'format' attribute to be variadic}}
+  __attribute__((format(printf, 2, 3))) {
+printf(fmt, expand(args)...);
+  }
+};
+
+void format_invalid_nonpod(const char *fmt, struct foo f) // expected-warning{{GCC requires functions with the 'format' attribute to be variadic}}
+__attribute__((format(printf, 1, 2)));
+
+void do_format() {
+  int x = 123;
+  int  = x;
+  const char *s = "world";
+  format("bare string");
+  format("%s", 123); // expected-warning{{format specifies type 'char *' but the argument has type 'int'}}
+  format("%s %s %u %d %i %p\n", "hello", s, 10u, x, y, _format);
+  format("%s %s %u %d %i %p\n", "hello", s, 10u, x, y, do_format);
+  format("bad format %s"); // expected-warning{{more 

[PATCH] D127217: [include-cleaner] Fix build error in unit test

2022-06-08 Thread Nathan Ridge via Phabricator via cfe-commits
nridge accepted this revision.
nridge added a comment.
This revision is now accepted and ready to land.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127217

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


[PATCH] D126984: [clang] Add initial support for gcc's optimize function attribute

2022-06-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

Two final (I hope) nits! One is in the code, the other is that this should have 
a release note for the new attribute. Assuming precommit CI pipeline passes, 
then I think this is all set.




Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4860
+  if (It != StrToKind.end()) {
+D->addAttr(::new (S.Context) OptimizeAttr(S.Context, AL, "", It->second));
+return;

Sorry for missing this before, but on both of the calls to `Create` here, you 
should be passing in `Arg` so that the AST retains full source fidelity (this 
matters for things like pretty printing).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126984

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


[PATCH] D125499: Enabling the detection of devtoolset-11 toolchain.

2022-06-08 Thread Kamau Bridgeman via Phabricator via cfe-commits
kamaub added a comment.

In D125499#3541689 , @tstellar wrote:

> I'm curious what is your system configuration where this patch actually 
> allows for detection of devtoolset?  I noticed that if clang and gcc are both 
> installed to /usr/, then driver will pick the gcc in /usr/ over the one in 
> /opt/rh/.../

The machine I needed the devtoolset-11 path for is not the same as the current 
bot failure trying to be addressed in https://reviews.llvm.org/D127310 but all 
there isn'y anything special to the configuration, I have 
`/opt/rh/devtoolset-11/root/usr` installed as normal from the yum repositories 
and I had a `check-all` build failure without this patch

  ~/Github/build $ ninja check-all
  [25/1032] Generating ScudoUnitTestsObjects.wrappers_cpp_test.cpp.powerpc64.o
  FAILED: 
projects/compiler-rt/lib/scudo/standalone/tests/ScudoUnitTestsObjects.wrappers_cpp_test.cpp.powerpc64.o
  cd /home/kamaub/Github/build/projects/compiler-rt/lib/scudo/standalone/tests 
&& /home/kamaub/Github/build/./bin/clang -g -Wno-suggest-override 
-DGTEST_NO_LLVM_SUPPORT=1 -DGTEST_HAS_RTTI=0 
-I/home/kamaub/Github/llvm-project/llvm/utils/unittest/googletest/include 
-I/home/kamaub/Github/llvm-project/llvm/utils/unittest/googletest 
-I/home/kamaub/Github/llvm-project/compiler-rt/include 
-I/home/kamaub/Github/llvm-project/compiler-rt/lib 
-I/home/kamaub/Github/llvm-project/compiler-rt/lib/scudo/standalone 
-I/home/kamaub/Github/llvm-project/compiler-rt/lib/scudo/standalone/include 
-DGTEST_HAS_RTTI=0 -g -Wno-mismatched-new-delete -m64 -c -o 
ScudoUnitTestsObjects.wrappers_cpp_test.cpp.powerpc64.o 
/home/kamaub/Github/llvm-project/compiler-rt/lib/scudo/standalone/tests/wrappers_cpp_test.cpp
  
/home/kamaub/Github/llvm-project/compiler-rt/lib/scudo/standalone/tests/wrappers_cpp_test.cpp:118:19:
 error: no member named 'make_unique' in namespace 'std'
  NoTags = std::make_unique();
   ~^
  
/home/kamaub/Github/llvm-project/compiler-rt/lib/scudo/standalone/tests/wrappers_cpp_test.cpp:118:66:
 error: expected '(' for function-style cast or type construction
  NoTags = std::make_unique();
~~~^
  
/home/kamaub/Github/llvm-project/compiler-rt/lib/scudo/standalone/tests/wrappers_cpp_test.cpp:118:68:
 error: expected expression
  NoTags = std::make_unique();
 ^
  3 errors generated.




Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:2156
 Prefixes.push_back("/opt/rh/devtoolset-10/root/usr");
 Prefixes.push_back("/opt/rh/devtoolset-9/root/usr");
 Prefixes.push_back("/opt/rh/devtoolset-8/root/usr");

tstellar wrote:
> MaskRay wrote:
> > The detection is wasted every Linux user (even if they don't use 
> > RedHat/Fedora). Some may need to be refactored to detect `/opt/rh` first. 
> > Some ancient devtoolset-* may be deleted now.
> @kamaub Are you planning to address these comments in a follow up change?
@tstellar Sorry I missed this, thank you for pointing it out, I should have 
addressed this in a follow-up chain but I email notifications are not working 
as only saw these comment while taking a look at 
https://reviews.llvm.org/D127310




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125499

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


[PATCH] D112661: [clangd] reuse preambles from other files when possible

2022-06-08 Thread Quentin Chateau via Phabricator via cfe-commits
qchateau added a comment.

@nridge after rebasing this change for several months, I find it less and less 
useful. It seems both clangd and the IDE I'm using are making significant 
improvements that make this changes less and less impactful.
I closed this revision as I feel that it now passed the threshold where the 
annoyance of maintaining it is greater than the return I'm getting from it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112661

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


[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays for stricter handling of flexible arrays

2022-06-08 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment.

In D126864#3567647 , @nickdesaulniers 
wrote:

> @kees maybe we should think about what would be needed for toolchains that 
> don't yet support `-fstrict-flex-arrays` in kernel sources? Does this become 
> a toolchain portability issue for older released toolchains we still intend 
> to support?

Gaining this for the kernel is just to make sure all the trailing arrays are 
actually getting found and handled. The kernel will handle this fine either 
way. (It already has to treat UAPI 1-element array conversions very 
carefully...)


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

https://reviews.llvm.org/D126864

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


[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays for stricter handling of flexible arrays

2022-06-08 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment.

In D126864#3564519 , @efriedma wrote:

>> As for SOCK_MAXADDRLEN, that's a horrid hack, and the definition of struct 
>> sockaddr needs to change. :)
>
> Do we want some builtin define so headers can check if we're in 
> -fstrict-flex-arrays mode?  It might be hard to mess with the definitions 
> otherwise.

Worst case, we don't need this at least for the kernel.

The kernel has machinery to test if a command line flag is available, and if 
so, set a preprocessor define via `-D` command line invocation which can be 
used as a guard in kernel sources.

Would this preprocessor define be handy for code outside the kernel? /me shrug

But for now we don't need it.

@kees maybe we should think about what would be needed for toolchains that 
don't yet support `-fstrict-flex-arrays` in kernel sources? Does this become a 
toolchain portability issue for older released toolchains we still intend to 
support?


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

https://reviews.llvm.org/D126864

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


[PATCH] D127312: [clang][dataflow] Remove IndirectionValue class, moving PointeeLoc field into PointerValue and ReferenceValue

2022-06-08 Thread weiyi via Phabricator via cfe-commits
wyt added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:63
-auto *IndVal2 = cast(Val2);
-assert(IndVal1->getKind() == IndVal2->getKind());
-if (>getPointeeLoc() == >getPointeeLoc())

gribozavr2 wrote:
> This assert was lost in the new implementation.
The assertion was to ensure that the values were either both Reference or both 
Pointer, as an IndirectionValue could be either. 
Since we've removed IndirectionVal and have separate casts for 
ReferenceVal/PointerVal, the assertion is not neccessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127312

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


[PATCH] D127312: [clang][dataflow] Remove IndirectionValue class, moving PointeeLoc field into PointerValue and ReferenceValue

2022-06-08 Thread weiyi via Phabricator via cfe-commits
wyt updated this revision to Diff 435271.
wyt marked 2 inline comments as done.
wyt added a comment.

Rename function as suggested


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127312

Files:
  clang/include/clang/Analysis/FlowSensitive/Value.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -122,24 +122,24 @@
   // [[p]]
 }
   )";
-  runDataflow(
-  Code, [](llvm::ArrayRef<
-   std::pair>>
-   Results,
-   ASTContext ) {
-ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
-const Environment  = Results[0].second.Env;
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext ) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment  = Results[0].second.Env;
 
-const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
-ASSERT_THAT(FooDecl, NotNull());
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
 
-const StorageLocation *FooLoc =
-Env.getStorageLocation(*FooDecl, SkipPast::None);
-ASSERT_TRUE(isa_and_nonnull(FooLoc));
+const StorageLocation *FooLoc =
+Env.getStorageLocation(*FooDecl, SkipPast::None);
+ASSERT_TRUE(isa_and_nonnull(FooLoc));
 
-const Value *FooVal = Env.getValue(*FooLoc);
-EXPECT_TRUE(isa_and_nonnull(FooVal));
-  });
+const Value *FooVal = Env.getValue(*FooLoc);
+EXPECT_TRUE(isa_and_nonnull(FooVal));
+  });
 }
 
 TEST_F(TransferTest, StructVarDecl) {
@@ -293,29 +293,29 @@
   // [[p]]
 }
   )";
-  runDataflow(
-  Code, [](llvm::ArrayRef<
-   std::pair>>
-   Results,
-   ASTContext ) {
-ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
-const Environment  = Results[0].second.Env;
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext ) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment  = Results[0].second.Env;
 
-const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
-ASSERT_THAT(FooDecl, NotNull());
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
 
-const StorageLocation *FooLoc =
-Env.getStorageLocation(*FooDecl, SkipPast::None);
-ASSERT_TRUE(isa_and_nonnull(FooLoc));
+const StorageLocation *FooLoc =
+Env.getStorageLocation(*FooDecl, SkipPast::None);
+ASSERT_TRUE(isa_and_nonnull(FooLoc));
 
-const ReferenceValue *FooVal =
-cast(Env.getValue(*FooLoc));
-const StorageLocation  = FooVal->getPointeeLoc();
-EXPECT_TRUE(isa());
+const ReferenceValue *FooVal =
+cast(Env.getValue(*FooLoc));
+const StorageLocation  = FooVal->getPointeeLoc();
+EXPECT_TRUE(isa());
 
-const Value *FooPointeeVal = Env.getValue(FooPointeeLoc);
-EXPECT_TRUE(isa_and_nonnull(FooPointeeVal));
-  });
+const Value *FooPointeeVal = Env.getValue(FooPointeeLoc);
+EXPECT_TRUE(isa_and_nonnull(FooPointeeVal));
+  });
 }
 
 TEST_F(TransferTest, SelfReferentialReferenceVarDecl) {
@@ -3327,23 +3327,23 @@
 ASSERT_THAT(LDecl, NotNull());
 
 // Inner.
-auto *LVal = dyn_cast(
-InnerEnv.getValue(*LDecl, SkipPast::None));
+auto *LVal =
+dyn_cast(InnerEnv.getValue(*LDecl, SkipPast::None));
 ASSERT_THAT(LVal, NotNull());
 
 EXPECT_EQ(>getPointeeLoc(),
   InnerEnv.getStorageLocation(*ValDecl, SkipPast::Reference));
 
 // Outer.
-LVal = dyn_cast(
-OuterEnv.getValue(*LDecl, SkipPast::None));
+LVal =
+dyn_cast(OuterEnv.getValue(*LDecl, SkipPast::None));
 ASSERT_THAT(LVal, NotNull());
 
 // The loop body may not have been executed, so we should not conclude
 // that `l` points to `val`.
 EXPECT_NE(>getPointeeLoc(),
   OuterEnv.getStorageLocation(*ValDecl, SkipPast::Reference));
-});
+  });
 }
 
 TEST_F(TransferTest, DoesNotCrashOnUnionThisExpr) {
Index: 

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays for stricter handling of flexible arrays

2022-06-08 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment.

In D126864#3564519 , @efriedma wrote:

> Do we want some builtin define so headers can check if we're in 
> -fstrict-flex-arrays mode?  It might be hard to mess with the definitions 
> otherwise.

Hm, maybe? It wonder if that's more confusing, as code would need to do really 
careful management of allocation sizes.

  struct foo {
  u32 len;
  #ifndef STRICT_FLEX_ARRAYS
  u32 array[14];
  #else
  u32 array[];
  #endif
  };

`sizeof(struct foo)` will change. Or, even more ugly:

  struct foo {
union {
  struct {
u32 len_old;
u32 array_old[14];
  };
  struct {
u32 len;
u32 array[];
  };
};
  };

But then `sizeof(struct foo)` stays the same.

(FWIW, the kernel is effectively doing the latter without any define.)


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

https://reviews.llvm.org/D126864

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


[PATCH] D127196: [clang][dataflow] Enable use of synthetic properties on all Value instances.

2022-06-08 Thread Dmitri Gribenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG49ed5bf51958: [clang][dataflow] Enable use of synthetic 
properties on all Value instances. (authored by wyt, committed by gribozavr).

Changed prior to commit:
  https://reviews.llvm.org/D127196?vs=435121=435265#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127196

Files:
  clang/include/clang/Analysis/FlowSensitive/Value.h
  clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
  clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -312,7 +312,7 @@
 if (const auto *E = selectFirst(
 "call", match(cxxConstructExpr(HasSpecialBoolType).bind("call"), *S,
   getASTContext( {
-  auto  = *cast(Env.createValue(E->getType()));
+  auto  = *Env.createValue(E->getType());
   ConstructorVal.setProperty("is_set", Env.getBoolLiteralValue(false));
   Env.setValue(*Env.getStorageLocation(*E, SkipPast::None), ConstructorVal);
 } else if (const auto *E = selectFirst(
@@ -327,8 +327,7 @@
   Env.getStorageLocation(*Object, SkipPast::ReferenceThenPointer);
   assert(ObjectLoc != nullptr);
 
-  auto  =
-  *cast(Env.createValue(Object->getType()));
+  auto  = *Env.createValue(Object->getType());
   ConstructorVal.setProperty("is_set", Env.getBoolLiteralValue(true));
   Env.setValue(*ObjectLoc, ConstructorVal);
 }
@@ -342,13 +341,11 @@
 Decl->getName() != "SpecialBool")
   return false;
 
-auto *IsSet1 = cast_or_null(
-cast()->getProperty("is_set"));
+auto *IsSet1 = cast_or_null(Val1.getProperty("is_set"));
 if (IsSet1 == nullptr)
   return true;
 
-auto *IsSet2 = cast_or_null(
-cast()->getProperty("is_set"));
+auto *IsSet2 = cast_or_null(Val2.getProperty("is_set"));
 if (IsSet2 == nullptr)
   return false;
 
@@ -365,18 +362,16 @@
 Decl->getName() != "SpecialBool")
   return true;
 
-auto *IsSet1 = cast_or_null(
-cast()->getProperty("is_set"));
+auto *IsSet1 = cast_or_null(Val1.getProperty("is_set"));
 if (IsSet1 == nullptr)
   return true;
 
-auto *IsSet2 = cast_or_null(
-cast()->getProperty("is_set"));
+auto *IsSet2 = cast_or_null(Val2.getProperty("is_set"));
 if (IsSet2 == nullptr)
   return true;
 
 auto  = MergedEnv.makeAtomicBoolValue();
-cast()->setProperty("is_set", IsSet);
+MergedVal.setProperty("is_set", IsSet);
 if (Env1.flowConditionImplies(*IsSet1) &&
 Env2.flowConditionImplies(*IsSet2))
   MergedEnv.addToFlowCondition(IsSet);
@@ -426,32 +421,31 @@
   /*[[p4]]*/
 }
   )";
-  runDataflow(Code,
-  [](llvm::ArrayRef<
- std::pair>>
- Results,
- ASTContext ) {
-ASSERT_THAT(Results, ElementsAre(Pair("p4", _), Pair("p3", _),
- Pair("p2", _), Pair("p1", _)));
-const Environment  = Results[3].second.Env;
-const Environment  = Results[2].second.Env;
-const Environment  = Results[1].second.Env;
-const Environment  = Results[0].second.Env;
+  runDataflow(
+  Code, [](llvm::ArrayRef<
+   std::pair>>
+   Results,
+   ASTContext ) {
+ASSERT_THAT(Results, ElementsAre(Pair("p4", _), Pair("p3", _),
+ Pair("p2", _), Pair("p1", _)));
+const Environment  = Results[3].second.Env;
+const Environment  = Results[2].second.Env;
+const Environment  = Results[1].second.Env;
+const Environment  = Results[0].second.Env;
 
-const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
-ASSERT_THAT(FooDecl, NotNull());
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
 
-auto GetFooValue = [FooDecl](const Environment ) {
-  return cast(
-  cast(Env.getValue(*FooDecl, SkipPast::None))
-  ->getProperty("is_set"));
-};
+auto GetFooValue = [FooDecl](const Environment ) {
+  return cast(
+  Env.getValue(*FooDecl, SkipPast::None)->getProperty("is_set"));
+};
 
-EXPECT_FALSE(Env1.flowConditionImplies(*GetFooValue(Env1)));
-EXPECT_TRUE(Env2.flowConditionImplies(*GetFooValue(Env2)));
-

[clang] 49ed5bf - [clang][dataflow] Enable use of synthetic properties on all Value instances.

2022-06-08 Thread Dmitri Gribenko via cfe-commits

Author: Wei Yi Tee
Date: 2022-06-08T20:20:26+02:00
New Revision: 49ed5bf51958aaeb209804da794c85d06207c3ed

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

LOG: [clang][dataflow] Enable use of synthetic properties on all Value 
instances.

This patch moves the implementation of synthetic properties from the 
StructValue class into the Value base class so that it can be used across all 
Value instances.

Reviewed By: gribozavr2, ymandel, sgatev, xazax.hun

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

Added: 


Modified: 
clang/include/clang/Analysis/FlowSensitive/Value.h
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

Removed: 




diff  --git a/clang/include/clang/Analysis/FlowSensitive/Value.h 
b/clang/include/clang/Analysis/FlowSensitive/Value.h
index 859cf7ff21b5b..1aedd8a300dd5 100644
--- a/clang/include/clang/Analysis/FlowSensitive/Value.h
+++ b/clang/include/clang/Analysis/FlowSensitive/Value.h
@@ -26,6 +26,9 @@ namespace clang {
 namespace dataflow {
 
 /// Base class for all values computed by abstract interpretation.
+///
+/// Don't use `Value` instances by value. All `Value` instances are allocated
+/// and owned by `DataflowAnalysisContext`.
 class Value {
 public:
   enum class Kind {
@@ -48,8 +51,22 @@ class Value {
 
   Kind getKind() const { return ValKind; }
 
+  /// Returns the value of the synthetic property with the given `Name` or null
+  /// if the property isn't assigned a value.
+  Value *getProperty(llvm::StringRef Name) const {
+auto It = Properties.find(Name);
+return It == Properties.end() ? nullptr : It->second;
+  }
+
+  /// Assigns `Val` as the value of the synthetic property with the given
+  /// `Name`.
+  void setProperty(llvm::StringRef Name, Value ) {
+Properties.insert_or_assign(Name, );
+  }
+
 private:
   Kind ValKind;
+  llvm::StringMap Properties;
 };
 
 /// Models a boolean.
@@ -215,22 +232,8 @@ class StructValue final : public Value {
   /// Assigns `Val` as the child value for `D`.
   void setChild(const ValueDecl , Value ) { Children[] =  }
 
-  /// Returns the value of the synthetic property with the given `Name` or null
-  /// if the property isn't assigned a value.
-  Value *getProperty(llvm::StringRef Name) const {
-auto It = Properties.find(Name);
-return It == Properties.end() ? nullptr : It->second;
-  }
-
-  /// Assigns `Val` as the value of the synthetic property with the given
-  /// `Name`.
-  void setProperty(llvm::StringRef Name, Value ) {
-Properties.insert_or_assign(Name, );
-  }
-
 private:
   llvm::DenseMap Children;
-  llvm::StringMap Properties;
 };
 
 } // namespace dataflow

diff  --git 
a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp 
b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index 02cf2c75579ff..cf93a07f832df 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -178,9 +178,9 @@ StructValue (Environment , 
BoolValue ) {
 }
 
 /// Returns the symbolic value that represents the "has_value" property of the
-/// optional value `Val`. Returns null if `Val` is null.
-BoolValue *getHasValue(Value *Val) {
-  if (auto *OptionalVal = cast_or_null(Val)) {
+/// optional value `OptionalVal`. Returns null if `OptionalVal` is null.
+BoolValue *getHasValue(Value *OptionalVal) {
+  if (OptionalVal) {
 return cast(OptionalVal->getProperty("has_value"));
   }
   return nullptr;
@@ -221,8 +221,8 @@ int countOptionalWrappers(const ASTContext , 
QualType Type) {
 void initializeOptionalReference(const Expr *OptionalExpr,
  const MatchFinder::MatchResult &,
  LatticeTransferState ) {
-  if (auto *OptionalVal = cast_or_null(
-  State.Env.getValue(*OptionalExpr, SkipPast::Reference))) {
+  if (auto *OptionalVal =
+  State.Env.getValue(*OptionalExpr, SkipPast::Reference)) {
 if (OptionalVal->getProperty("has_value") == nullptr) {
   OptionalVal->setProperty("has_value", State.Env.makeAtomicBoolValue());
 }
@@ -231,8 +231,8 @@ void initializeOptionalReference(const Expr *OptionalExpr,
 
 void transferUnwrapCall(const Expr *UnwrapExpr, const Expr *ObjectExpr,
 LatticeTransferState ) {
-  if (auto *OptionalVal = cast_or_null(
-  State.Env.getValue(*ObjectExpr, SkipPast::ReferenceThenPointer))) {
+  if (auto *OptionalVal =
+  State.Env.getValue(*ObjectExpr, SkipPast::ReferenceThenPointer)) {
 auto *HasValueVal = getHasValue(OptionalVal);
 assert(HasValueVal != nullptr);
 

diff  --git 

[PATCH] D126984: [clang] Add initial support for gcc's optimize function attribute

2022-06-08 Thread Stephen Long via Phabricator via cfe-commits
steplong updated this revision to Diff 435264.
steplong added a comment.
Herald added a subscriber: jdoerfert.

- Fix up docs and comments
- Fix failing pragma-attribute-supported-attributes-list.test
- Remove debug print
- Change to StringMap


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126984

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/attr-optimize.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/attr-optimize.c

Index: clang/test/Sema/attr-optimize.c
===
--- /dev/null
+++ clang/test/Sema/attr-optimize.c
@@ -0,0 +1,52 @@
+// RUN: %clang_cc1 -verify -fsyntax-only %s
+
+__attribute__((optimize(a))) // expected-error {{use of undeclared identifier 'a'}}
+void
+f1() {}
+
+int b = 1;
+__attribute__((optimize(b))) // expected-error {{'optimize' attribute requires a string}}
+void
+f2() {}
+
+__attribute__((optimize("O0", "O1"))) // expected-error {{'optimize' attribute takes one argument}}
+void
+f3() {}
+
+__attribute__((optimize("Og"))) // expected-no-error
+void
+f4() {}
+
+__attribute__((optimize("O-1"))) // expected-warning {{invalid optimization level 'O-1' specified; the argument to '-O' should be a non-negative integer, '-Os', '-Oz', '-Og', or '-Ofast'; attribute ignored}}
+void
+f5() {}
+
+__attribute__((optimize("O+1"))) // expected-warning {{invalid optimization level 'O+1' specified; the argument to '-O' should be a non-negative integer, '-Os', '-Oz', '-Og', or '-Ofast'; attribute ignored}}
+void
+f6() {}
+
+__attribute__((optimize("O0"))) // expected-no-error
+void
+f7() {}
+
+__attribute__((optimize("Os"))) // expected-no-error
+void
+f8() {}
+
+__attribute__((optimize("O44"))) // expected-no-error
+void
+f9() {}
+
+__attribute__((optimize("Oz"))) // expected-no-error
+void
+f10() {}
+
+__attribute__((optimize("Ofast"))) // expected-no-error
+void
+f11() {}
+
+__attribute__((optimize("O"))) // expected-no-error
+void
+f12() {}
+
+__attribute__((optimize("O0"))) // expected-error {{expected identifier or '('}}
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -142,6 +142,7 @@
 // CHECK-NEXT: ObjCSubclassingRestricted (SubjectMatchRule_objc_interface)
 // CHECK-NEXT: OpenCLIntelReqdSubGroupSize (SubjectMatchRule_function)
 // CHECK-NEXT: OpenCLNoSVM (SubjectMatchRule_variable)
+// CHECK-NEXT: Optimize (SubjectMatchRule_function)
 // CHECK-NEXT: OptimizeNone (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: Overloadable (SubjectMatchRule_function)
 // CHECK-NEXT: Owner (SubjectMatchRule_record_not_is_union)
Index: clang/test/CodeGen/attr-optimize.c
===
--- /dev/null
+++ clang/test/CodeGen/attr-optimize.c
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -O2 -S -emit-llvm %s -o - | FileCheck %s --check-prefix=O2
+// RUN: %clang_cc1 -O0 -S -emit-llvm %s -o - | FileCheck %s --check-prefix=O0
+
+__attribute__((optimize("O0"))) void f1(void) {}
+// O2: @f1{{.*}}[[ATTR_OPTNONE:#[0-9]+]]
+// O0: @f1{{.*}}[[ATTR_OPTNONE:#[0-9]+]]
+
+__attribute__((optimize("Os"))) void f2(void) {}
+// O2: @f2{{.*}}[[ATTR_OPTSIZE:#[0-9]+]]
+// O0: @f2{{.*}}[[ATTR_OPTNONE]]
+
+__attribute__((optimize("Og"))) void f3(void) {}
+// O2: @f3{{.*}}[[ATTR_OPTSIZE]]
+// O0: @f3{{.*}}[[ATTR_OPTNONE]]
+
+__attribute__((optimize("Oz"))) void f4(void) {}
+// O2: @f4{{.*}}[[ATTR_OPTSIZE]]
+// O0: @f4{{.*}}[[ATTR_OPTNONE]]
+
+__attribute__((optimize("Ofast"))) void f5(void) {}
+// O2: @f5{{.*}}[[ATTR_OPTSIZE]]
+// O0: @f5{{.*}}[[ATTR_OPTNONE]]
+
+// O2: attributes [[ATTR_OPTNONE]] = { {{.*}}optnone{{.*}} }
+// O2: attributes [[ATTR_OPTSIZE]] = { {{.*}}optsize{{.*}} }
+
+// Check that O0 overrides the attribute
+// O0: attributes [[ATTR_OPTNONE]] = { {{.*}}optnone{{.*}} }
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -4833,6 +4833,46 @@
 D->addAttr(Optnone);
 }
 
+static void handleOptimizeAttr(Sema , Decl *D, const ParsedAttr ) {
+  StringRef Arg;
+  if (!S.checkStringLiteralArgumentAttr(AL, 0, Arg))
+return;
+
+  StringRef Level;
+  // Check if argument is prefixed with "-O" or "O"
+  if (Arg.str().rfind("-O", 0) == 0)
+Level = Arg.substr(2);
+  else if (Arg.str().rfind("O", 0) == 0)
+Level = Arg.substr(1);
+  else
+S.Diag(AL.getLoc(), 

[PATCH] D127243: [clang][deps] Make order of module dependencies deterministic

2022-06-08 Thread Ben Langmuir 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 rG835fcf2aa512: [clang][deps] Make order of module 
dependencies deterministic (authored by benlangmuir).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127243

Files:
  clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp


Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -199,7 +199,7 @@
   MDC.Consumer.handleDependencyOutputOpts(*MDC.Opts);
 
   for (auto & : MDC.ModularDeps)
-MDC.Consumer.handleModuleDependency(I.second);
+MDC.Consumer.handleModuleDependency(*I.second);
 
   for (auto & : MDC.FileDeps)
 MDC.Consumer.handleFileDependency(I);
@@ -212,11 +212,12 @@
   assert(M == M->getTopLevelModule() && "Expected top level module!");
 
   // If this module has been handled already, just return its ID.
-  auto ModI = MDC.ModularDeps.insert({M, ModuleDeps{}});
+  auto ModI = MDC.ModularDeps.insert({M, nullptr});
   if (!ModI.second)
-return ModI.first->second.ID;
+return ModI.first->second->ID;
 
-  ModuleDeps  = ModI.first->second;
+  ModI.first->second = std::make_unique();
+  ModuleDeps  = *ModI.first->second;
 
   MD.ID.ModuleName = M->getFullModuleName();
   MD.ImportedByMainFile = DirectModularDeps.contains(M);
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
@@ -176,7 +176,7 @@
   private:
 std::vector Dependencies;
 std::vector PrebuiltModuleDeps;
-std::map ClangModuleDeps;
+llvm::MapVector> 
ClangModuleDeps;
 std::string ContextHash;
 std::vector OutputPaths;
 const llvm::StringSet<> 
Index: clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
===
--- clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -149,9 +149,9 @@
   /// The parent dependency collector.
   ModuleDepCollector 
   /// Working set of direct modular dependencies.
-  llvm::DenseSet DirectModularDeps;
+  llvm::SetVector DirectModularDeps;
   /// Working set of direct modular dependencies that have already been built.
-  llvm::DenseSet DirectPrebuiltModularDeps;
+  llvm::SetVector DirectPrebuiltModularDeps;
 
   void handleImport(const Module *Imported);
 
@@ -199,7 +199,7 @@
   /// textually included header files.
   std::vector FileDeps;
   /// Direct and transitive modular dependencies of the main source file.
-  std::unordered_map ModularDeps;
+  llvm::MapVector> ModularDeps;
   /// Options that control the dependency output generation.
   std::unique_ptr Opts;
   /// The original Clang invocation passed to dependency scanner.


Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -199,7 +199,7 @@
   MDC.Consumer.handleDependencyOutputOpts(*MDC.Opts);
 
   for (auto & : MDC.ModularDeps)
-MDC.Consumer.handleModuleDependency(I.second);
+MDC.Consumer.handleModuleDependency(*I.second);
 
   for (auto & : MDC.FileDeps)
 MDC.Consumer.handleFileDependency(I);
@@ -212,11 +212,12 @@
   assert(M == M->getTopLevelModule() && "Expected top level module!");
 
   // If this module has been handled already, just return its ID.
-  auto ModI = MDC.ModularDeps.insert({M, ModuleDeps{}});
+  auto ModI = MDC.ModularDeps.insert({M, nullptr});
   if (!ModI.second)
-return ModI.first->second.ID;
+return ModI.first->second->ID;
 
-  ModuleDeps  = ModI.first->second;
+  ModI.first->second = std::make_unique();
+  ModuleDeps  = *ModI.first->second;
 
   MD.ID.ModuleName = M->getFullModuleName();
   MD.ImportedByMainFile = DirectModularDeps.contains(M);
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
@@ -176,7 +176,7 @@
   private:
 std::vector Dependencies;
 std::vector PrebuiltModuleDeps;
-std::map ClangModuleDeps;
+llvm::MapVector> ClangModuleDeps;
 std::string 

[PATCH] D127229: [clang][deps] Set -disable-free for module compilations

2022-06-08 Thread Ben Langmuir 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 rG7a72dca74a27: [clang][deps] Set -disable-free for module 
compilations (authored by benlangmuir).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127229

Files:
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/test/ClangScanDeps/modules-disable-free.c


Index: clang/test/ClangScanDeps/modules-disable-free.c
===
--- /dev/null
+++ clang/test/ClangScanDeps/modules-disable-free.c
@@ -0,0 +1,34 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s|DIR|%/t|g" %t/compile-commands.json.in > 
%t/compile-commands.json
+
+// RUN: clang-scan-deps -compilation-database %t/compile-commands.json -j 1 
-format experimental-full \
+// RUN:   -mode preprocess-dependency-directives -generate-modules-path-args > 
%t/output
+// RUN: FileCheck %s < %t/output
+
+// CHECK: "-disable-free",
+
+//--- compile-commands.json.in
+
+[{
+  "directory": "DIR",
+  "command": "clang -c DIR/main.c -fmodules 
-fmodules-cache-path=DIR/module-cache -fimplicit-modules 
-fimplicit-module-maps",
+  "file": "DIR/main.c"
+}]
+
+//--- module.modulemap
+
+module A {
+  header "a.h"
+}
+
+//--- a.h
+
+void a(void);
+
+//--- main.c
+
+#include "a.h"
+void m() {
+  a();
+}
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -137,11 +137,11 @@
   DependencyScanningAction(
   StringRef WorkingDirectory, DependencyConsumer ,
   llvm::IntrusiveRefCntPtr DepFS,
-  ScanningOutputFormat Format, bool OptimizeArgs,
+  ScanningOutputFormat Format, bool OptimizeArgs, bool DisableFree,
   llvm::Optional ModuleName = None)
   : WorkingDirectory(WorkingDirectory), Consumer(Consumer),
 DepFS(std::move(DepFS)), Format(Format), OptimizeArgs(OptimizeArgs),
-ModuleName(ModuleName) {}
+DisableFree(DisableFree), ModuleName(ModuleName) {}
 
   bool runInvocation(std::shared_ptr Invocation,
  FileManager *FileMgr,
@@ -149,6 +149,8 @@
  DiagnosticConsumer *DiagConsumer) override {
 // Make a deep copy of the original Clang invocation.
 CompilerInvocation OriginalInvocation(*Invocation);
+// Restore the value of DisableFree, which may be modified by Tooling.
+OriginalInvocation.getFrontendOpts().DisableFree = DisableFree;
 
 // Create a compiler instance to handle the actual work.
 CompilerInstance ScanInstance(std::move(PCHContainerOps));
@@ -255,6 +257,7 @@
   llvm::IntrusiveRefCntPtr DepFS;
   ScanningOutputFormat Format;
   bool OptimizeArgs;
+  bool DisableFree;
   llvm::Optional ModuleName;
 };
 
@@ -329,9 +332,13 @@
 
   return runWithDiags(CreateAndPopulateDiagOpts(FinalCCommandLine).release(),
   [&](DiagnosticConsumer , DiagnosticOptions ) 
{
+// DisableFree is modified by Tooling for running
+// in-process; preserve the original value, which is
+// always true for a driver invocation.
+bool DisableFree = true;
 DependencyScanningAction Action(
 WorkingDirectory, Consumer, DepFS, Format,
-OptimizeArgs, ModuleName);
+OptimizeArgs, DisableFree, ModuleName);
 // Create an invocation that uses the underlying file
 // system to ensure that any file system requests that
 // are made by the driver do not go through the


Index: clang/test/ClangScanDeps/modules-disable-free.c
===
--- /dev/null
+++ clang/test/ClangScanDeps/modules-disable-free.c
@@ -0,0 +1,34 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s|DIR|%/t|g" %t/compile-commands.json.in > %t/compile-commands.json
+
+// RUN: clang-scan-deps -compilation-database %t/compile-commands.json -j 1 -format experimental-full \
+// RUN:   -mode preprocess-dependency-directives -generate-modules-path-args > %t/output
+// RUN: FileCheck %s < %t/output
+
+// CHECK: "-disable-free",
+
+//--- compile-commands.json.in
+
+[{
+  "directory": "DIR",
+  "command": "clang -c DIR/main.c -fmodules -fmodules-cache-path=DIR/module-cache -fimplicit-modules -fimplicit-module-maps",
+  "file": "DIR/main.c"
+}]
+
+//--- module.modulemap
+
+module A {
+  header "a.h"
+}
+
+//--- a.h
+
+void a(void);
+
+//--- main.c
+
+#include "a.h"
+void m() {
+  a();
+}
Index: 

[clang] 835fcf2 - [clang][deps] Make order of module dependencies deterministic

2022-06-08 Thread Ben Langmuir via cfe-commits

Author: Ben Langmuir
Date: 2022-06-08T11:09:17-07:00
New Revision: 835fcf2aa5127bf99a1e6397c72153f00a0497b2

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

LOG: [clang][deps] Make order of module dependencies deterministic

This fixes the underlying module dependencies, which had a
non-deterministic order, which was also visible in the order of calls to
DependencyConsumer methods. This was not directly observable in
the clang-scan-deps utility, because it was previously seeing a sorted
order from std::map in DependencyScanningTool. However, the underlying
API previously created a likely issue for any other clients. Note: if
you only apply the change from DependencyScanningTool, you can see the
issue in clang-scan-deps, and existing tests will fail
non-deterministicaly.

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

Added: 


Modified: 
clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp

Removed: 




diff  --git 
a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h 
b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
index 363e159dd990f..e0a4d6a554eb2 100644
--- a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -149,9 +149,9 @@ class ModuleDepCollectorPP final : public PPCallbacks {
   /// The parent dependency collector.
   ModuleDepCollector 
   /// Working set of direct modular dependencies.
-  llvm::DenseSet DirectModularDeps;
+  llvm::SetVector DirectModularDeps;
   /// Working set of direct modular dependencies that have already been built.
-  llvm::DenseSet DirectPrebuiltModularDeps;
+  llvm::SetVector DirectPrebuiltModularDeps;
 
   void handleImport(const Module *Imported);
 
@@ -199,7 +199,7 @@ class ModuleDepCollector final : public DependencyCollector 
{
   /// textually included header files.
   std::vector FileDeps;
   /// Direct and transitive modular dependencies of the main source file.
-  std::unordered_map ModularDeps;
+  llvm::MapVector> ModularDeps;
   /// Options that control the dependency output generation.
   std::unique_ptr Opts;
   /// The original Clang invocation passed to dependency scanner.

diff  --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp 
b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
index 6fd3a83fd3f7b..5041a8eb6e597 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
@@ -176,7 +176,7 @@ DependencyScanningTool::getFullDependencies(
   private:
 std::vector Dependencies;
 std::vector PrebuiltModuleDeps;
-std::map ClangModuleDeps;
+llvm::MapVector> 
ClangModuleDeps;
 std::string ContextHash;
 std::vector OutputPaths;
 const llvm::StringSet<> 

diff  --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp 
b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index 21a422f8dbbae..6c7fdbc3944a6 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -199,7 +199,7 @@ void ModuleDepCollectorPP::EndOfMainFile() {
   MDC.Consumer.handleDependencyOutputOpts(*MDC.Opts);
 
   for (auto & : MDC.ModularDeps)
-MDC.Consumer.handleModuleDependency(I.second);
+MDC.Consumer.handleModuleDependency(*I.second);
 
   for (auto & : MDC.FileDeps)
 MDC.Consumer.handleFileDependency(I);
@@ -212,11 +212,12 @@ ModuleID ModuleDepCollectorPP::handleTopLevelModule(const 
Module *M) {
   assert(M == M->getTopLevelModule() && "Expected top level module!");
 
   // If this module has been handled already, just return its ID.
-  auto ModI = MDC.ModularDeps.insert({M, ModuleDeps{}});
+  auto ModI = MDC.ModularDeps.insert({M, nullptr});
   if (!ModI.second)
-return ModI.first->second.ID;
+return ModI.first->second->ID;
 
-  ModuleDeps  = ModI.first->second;
+  ModI.first->second = std::make_unique();
+  ModuleDeps  = *ModI.first->second;
 
   MD.ID.ModuleName = M->getFullModuleName();
   MD.ImportedByMainFile = DirectModularDeps.contains(M);



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


[clang] 7a72dca - [clang][deps] Set -disable-free for module compilations

2022-06-08 Thread Ben Langmuir via cfe-commits

Author: Ben Langmuir
Date: 2022-06-08T11:09:17-07:00
New Revision: 7a72dca74a27f1f6198cfabb064dc43274ee005d

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

LOG: [clang][deps] Set -disable-free for module compilations

The command-line arguments for module builds are cc1 commands, so they
do not implicitly set -disable-free like a driver invocation, and
Tooling will disable it for the scanning instance itself. Set
-disable-free explicitly so that separate invocations for building
modules will not pay for freeing memory unnecessarily.

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

Added: 
clang/test/ClangScanDeps/modules-disable-free.c

Modified: 
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

Removed: 




diff  --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp 
b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
index 04f7044bc4236..16e6ac59e85fe 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -137,11 +137,11 @@ class DependencyScanningAction : public 
tooling::ToolAction {
   DependencyScanningAction(
   StringRef WorkingDirectory, DependencyConsumer ,
   llvm::IntrusiveRefCntPtr DepFS,
-  ScanningOutputFormat Format, bool OptimizeArgs,
+  ScanningOutputFormat Format, bool OptimizeArgs, bool DisableFree,
   llvm::Optional ModuleName = None)
   : WorkingDirectory(WorkingDirectory), Consumer(Consumer),
 DepFS(std::move(DepFS)), Format(Format), OptimizeArgs(OptimizeArgs),
-ModuleName(ModuleName) {}
+DisableFree(DisableFree), ModuleName(ModuleName) {}
 
   bool runInvocation(std::shared_ptr Invocation,
  FileManager *FileMgr,
@@ -149,6 +149,8 @@ class DependencyScanningAction : public tooling::ToolAction 
{
  DiagnosticConsumer *DiagConsumer) override {
 // Make a deep copy of the original Clang invocation.
 CompilerInvocation OriginalInvocation(*Invocation);
+// Restore the value of DisableFree, which may be modified by Tooling.
+OriginalInvocation.getFrontendOpts().DisableFree = DisableFree;
 
 // Create a compiler instance to handle the actual work.
 CompilerInstance ScanInstance(std::move(PCHContainerOps));
@@ -255,6 +257,7 @@ class DependencyScanningAction : public tooling::ToolAction 
{
   llvm::IntrusiveRefCntPtr DepFS;
   ScanningOutputFormat Format;
   bool OptimizeArgs;
+  bool DisableFree;
   llvm::Optional ModuleName;
 };
 
@@ -329,9 +332,13 @@ llvm::Error DependencyScanningWorker::computeDependencies(
 
   return runWithDiags(CreateAndPopulateDiagOpts(FinalCCommandLine).release(),
   [&](DiagnosticConsumer , DiagnosticOptions ) 
{
+// DisableFree is modified by Tooling for running
+// in-process; preserve the original value, which is
+// always true for a driver invocation.
+bool DisableFree = true;
 DependencyScanningAction Action(
 WorkingDirectory, Consumer, DepFS, Format,
-OptimizeArgs, ModuleName);
+OptimizeArgs, DisableFree, ModuleName);
 // Create an invocation that uses the underlying file
 // system to ensure that any file system requests that
 // are made by the driver do not go through the

diff  --git a/clang/test/ClangScanDeps/modules-disable-free.c 
b/clang/test/ClangScanDeps/modules-disable-free.c
new file mode 100644
index 0..16e5092128764
--- /dev/null
+++ b/clang/test/ClangScanDeps/modules-disable-free.c
@@ -0,0 +1,34 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s|DIR|%/t|g" %t/compile-commands.json.in > 
%t/compile-commands.json
+
+// RUN: clang-scan-deps -compilation-database %t/compile-commands.json -j 1 
-format experimental-full \
+// RUN:   -mode preprocess-dependency-directives -generate-modules-path-args > 
%t/output
+// RUN: FileCheck %s < %t/output
+
+// CHECK: "-disable-free",
+
+//--- compile-commands.json.in
+
+[{
+  "directory": "DIR",
+  "command": "clang -c DIR/main.c -fmodules 
-fmodules-cache-path=DIR/module-cache -fimplicit-modules 
-fimplicit-module-maps",
+  "file": "DIR/main.c"
+}]
+
+//--- module.modulemap
+
+module A {
+  header "a.h"
+}
+
+//--- a.h
+
+void a(void);
+
+//--- main.c
+
+#include "a.h"
+void m() {
+  a();
+}



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


[PATCH] D126832: [clang][tablegen] adds human documentation to `WarningOption`

2022-06-08 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment.

In D126832#3567035 , @aaron.ballman 
wrote:

> In D126832#3554816 , @cjdb wrote:
>
>> In D126832#3553569 , 
>> @aaron.ballman wrote:
>>
>>> Can you post some examples of the output from this option so we can see 
>>> what the end results look like more easily?
>>
>> Right now this doesn't do anything at all, except feed the documentation 
>> from tablegen to Clang. It's just taking the text (represented by `...`) 
>> from `code Documentation [{...}]`, trimming any surrounding spaces, and 
>> putting that into `DIAG_ENTRY`.
>>
>> e.g.
>>
>>   code Documentation [{
>> Hello, world!
>>   
>> Goodbye, world!
>>   }]
>>
>> would be forwarded as `"Hello, world!\n\n  Goodbye, world!"`
>
> Thanks, that helps me to visualize what's going on. Any thoughts on how we 
> could test this functionality, or are we going to assume that the testing 
> comes from its usage when we actually make use of the new information 
> threaded in?

Good question. Unless this functionality already has tests (and I'm gathering 
it doesn't), I think we can rely on Clang //being// the test. (Though we should 
consider ourselves on notice for adding robust unit tests at some point.)




Comment at: clang/utils/TableGen/ClangDiagnosticsEmitter.cpp:1550-1553
+if (!Documentation.empty())
+  OS << "R\"(" << StringRef(Documentation).trim() << ")\"";
+else
+  OS << R"("")";

aaron.ballman wrote:
> Isn't this functionally equivalent? (It means we're using a raw empty string 
> rather than a normal empty string, but that shouldn't matter.)
Wow, I can no longer say I've never done `if (condition) return true; else 
return false;`  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126832

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


[PATCH] D126929: Add sanitizer metadata attributes to clang IR gen.

2022-06-08 Thread Mitch Phillips via Phabricator via cfe-commits
hctim updated this revision to Diff 435257.
hctim marked 3 inline comments as done.
hctim added a comment.

Update.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126929

Files:
  clang/lib/CodeGen/SanitizerMetadata.cpp
  clang/lib/CodeGen/SanitizerMetadata.h
  clang/test/CodeGen/Inputs/sanitizer-special-case-list-globals.txt
  clang/test/CodeGen/asan-globals.cpp
  clang/test/CodeGen/sanitize-init-order.cpp
  clang/test/CodeGen/sanitizer-special-case-list-globals.c

Index: clang/test/CodeGen/sanitizer-special-case-list-globals.c
===
--- /dev/null
+++ clang/test/CodeGen/sanitizer-special-case-list-globals.c
@@ -0,0 +1,55 @@
+// Verify that ignorelist sections correctly select sanitizers to apply
+// ignorelist entries to.
+
+// RUN: %clang_cc1 -fsanitize=address -emit-llvm %s -o -\
+// RUN: -fsanitize-ignorelist=%S/Inputs/sanitizer-special-case-list-globals.txt \
+// RUN: | FileCheck %s --check-prefix=ASAN
+
+// Note: HWASan effectively reorders globals (it puts the unsanitized ones
+// first), which is hard to check for, as 'CHECK-DAG' doesn't play terribly
+// nicely with 'CHECK-NOT'. This is why the 'always_ignored' and
+// 'hwasan_ignored' comes first in this file.
+// RUN: %clang_cc1 -fsanitize=hwaddress -emit-llvm %s -o -\
+// RUN: -fsanitize-ignorelist=%S/Inputs/sanitizer-special-case-list-globals.txt \
+// RUN: | FileCheck %s --check-prefix=HWASAN
+
+// TODO(hctim): Move over to memtag-globals when it's implemented. For now
+// though, it's fine, the frontend still annotates based on any memtag sanitizer
+// being used.
+// RUN: %clang_cc1 -fsanitize=memtag-heap -triple=aarch64-linux-android31 -emit-llvm %s -o -\
+// RUN: -fsanitize-ignorelist=%S/Inputs/sanitizer-special-case-list-globals.txt \
+// RUN: | FileCheck %s --check-prefix=MEMTAG
+
+// ASAN:   @always_ignored = global {{.*}}, no_sanitize_address
+// HWASAN: @always_ignored = global {{.*}}, no_sanitize_hwaddress
+// MEMTAG: @always_ignored = global {{.*}}, no_sanitize_memtag
+unsigned always_ignored;
+
+// ASAN:   @hwasan_ignored = global
+// ASAN-NOT:   no_sanitize_address
+// HWASAN: @hwasan_ignored = global {{.*}}, no_sanitize_hwaddress
+// MEMTAG: @hwasan_ignored = global
+// MEMTAG-NOT: no_sanitize_memtag
+unsigned hwasan_ignored;
+
+// ASAN:   @asan_ignored = global {{.*}}, no_sanitize_address
+// HWASAN: @asan_ignored.hwasan = {{.*}} global
+// HWASAN-NOT: no_sanitize_hwaddress
+// MEMTAG: @asan_ignored = global
+// MEMTAG-NOT: no_sanitize_memtag
+unsigned asan_ignored;
+
+// ASAN:   @memtag_ignored = global
+// ASAN-NOT:   no_sanitize_address
+// HWASAN: @memtag_ignored.hwasan = {{.*}} global
+// HWASAN-NOT: no_sanitize_hwaddress
+// MEMTAG: @memtag_ignored = global {{.*}}, no_sanitize_memtag
+unsigned memtag_ignored;
+
+// ASAN:   @never_ignored = global
+// ASAN-NOT:   no_sanitize_address
+// HWASAN: @never_ignored.hwasan = {{.*}} global
+// HWASAN-NOT: no_sanitize_hwaddress
+// MEMTAG: @never_ignored = global
+// MEMTAG-NOT: no_sanitize_memtag
+unsigned never_ignored;
Index: clang/test/CodeGen/sanitize-init-order.cpp
===
--- clang/test/CodeGen/sanitize-init-order.cpp
+++ clang/test/CodeGen/sanitize-init-order.cpp
@@ -36,12 +36,29 @@
 
 // Check that ASan init-order checking ignores structs with trivial default
 // constructor.
+
+// CHECK: @s1 = global
+// CHECK-NOT: sanitize_address_dyninit
+// CHECK: @s2 = global
+// CHECK-NOT: sanitize_address_dyninit
+// CHECK: @s3 = global {{.*}}, sanitize_address_dyninit
+// CHECK: @{{.*}}array{{.*}} = global {{.*}}, sanitize_address_dyninit
+
 // CHECK: !llvm.asan.globals = !{![[GLOB_1:[0-9]+]], ![[GLOB_2:[0-9]+]], ![[GLOB_3:[0-9]+]], ![[GLOB_4:[0-9]+]]
 // CHECK: ![[GLOB_1]] = !{%struct.PODStruct* {{.*}}, i1 false, i1 false}
 // CHECK: ![[GLOB_2]] = !{%struct.PODWithDtor* {{.*}}, i1 false, i1 false}
 // CHECK: ![[GLOB_3]] = !{%struct.PODWithCtorAndDtor* {{.*}}, i1 true, i1 false}
 // CHECK: ![[GLOB_4]] = !{{{.*}}class.NS::PODWithCtor{{.*}}, i1 true, i1 false}
 
+// IGNORELIST: @s1 = global
+// IGNORELIST-NOT: sanitize_address_dyninit
+// IGNORELIST: @s2 = global
+// IGNORELIST-NOT: sanitize_address_dyninit
+// IGNORELIST: @s3 = global
+// IGNORELIST-NOT: sanitize_address_dyninit
+// IGNORELIST: @{{.*}}array{{.*}} = global
+// IGNORELIST-NOT: sanitize_address_dyninit
+
 // IGNORELIST: !llvm.asan.globals = !{![[GLOB_1:[0-9]+]], ![[GLOB_2:[0-9]+]], ![[GLOB_3:[0-9]+]], ![[GLOB_4:[0-9]+]]}
 // IGNORELIST: ![[GLOB_1]] = !{%struct.PODStruct* {{.*}}, i1 false, i1 false}
 // IGNORELIST: ![[GLOB_2]] = !{%struct.PODWithDtor* {{.*}}, i1 false, i1 false}
Index: clang/test/CodeGen/asan-globals.cpp
===
--- clang/test/CodeGen/asan-globals.cpp
+++ clang/test/CodeGen/asan-globals.cpp

[PATCH] D126929: Add sanitizer metadata attributes to clang IR gen.

2022-06-08 Thread Mitch Phillips via Phabricator via cfe-commits
hctim marked 3 inline comments as done.
hctim added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2767-2781
+  if (NoSanitizeL.containsGlobal(LangOpts.Sanitize.Mask, GV->getName(), 
Category))
 return true;
-  if (NoSanitizeL.containsLocation(EnabledAsanMask, Loc, Category))
+  if (NoSanitizeL.containsLocation(LangOpts.Sanitize.Mask, Loc, Category))
 return true;
   // Check global type.
   if (!Ty.isNull()) {
 // Drill down the array types: if global variable of a fixed type is

vitalybuka wrote:
> can this lines be landed separately?
sure



Comment at: clang/lib/CodeGen/SanitizerMetadata.cpp:44
 
+  if (CGM.isInNoSanitizeList(GV, Loc, Ty))
+Meta.NoSanitize = true;

vitalybuka wrote:
> it's incorrect as is
> isInNoSanitizeList is sanitizer specific
> we need to add isInNoSanitizeList version which return relevant SanitizerMask
sure, added the sanitizer-specific parsing as well, including a test 
(sanitizer-special-case-list-globals.txt)



Comment at: clang/lib/CodeGen/SanitizerMetadata.cpp:69
+  SanitizerKind::KernelAddress)) {
+Meta.IsDynInit = IsDynInit && !CGM.isInNoSanitizeList(GV, Loc, Ty, "init");
+  }

vitalybuka wrote:
> Why don't we care about IsExcluded here?
doesn't matter as the global is disabled anyway, and this preserves previous 
behaviour which the existing tests assert on.

i'll make it conditional in the follow up change, as all the 
`llvm.asan.globals` tests need to be deleted. added a note.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126929

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


[PATCH] D126832: [clang][tablegen] adds human documentation to `WarningOption`

2022-06-08 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb updated this revision to Diff 435255.
cjdb marked 2 inline comments as done.
cjdb added a comment.

applies feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126832

Files:
  clang/include/clang/Basic/DiagnosticCategories.h
  clang/include/clang/Basic/DiagnosticIDs.h
  clang/lib/Basic/DiagnosticIDs.cpp
  clang/tools/diagtool/DiagnosticNames.cpp
  clang/utils/TableGen/ClangDiagnosticsEmitter.cpp

Index: clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
===
--- clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
+++ clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
@@ -1534,14 +1534,22 @@
 const bool hasSubGroups =
 !SubGroups.empty() || (IsPedantic && !GroupsInPedantic.empty());
 if (hasSubGroups) {
-  OS << "/* DiagSubGroup" << I.second.IDNo << " */ " << SubGroupIndex;
+  OS << "/* DiagSubGroup" << I.second.IDNo << " */ " << SubGroupIndex
+ << ", ";
   if (IsPedantic)
 SubGroupIndex += GroupsInPedantic.size();
   SubGroupIndex += SubGroups.size() + 1;
 } else {
-  OS << "0";
+  OS << "0, ";
 }
 
+std::string Documentation = I.second.Defs.back()
+->getValue("Documentation")
+->getValue()
+->getAsUnquotedString();
+
+OS << "R\"(" << StringRef(Documentation).trim() << ")\"";
+
 OS << ")\n";
   }
   OS << "#endif // DIAG_ENTRY\n\n";
Index: clang/tools/diagtool/DiagnosticNames.cpp
===
--- clang/tools/diagtool/DiagnosticNames.cpp
+++ clang/tools/diagtool/DiagnosticNames.cpp
@@ -66,7 +66,7 @@
 
 // Second the table of options, sorted by name for fast binary lookup.
 static const GroupRecord OptionTable[] = {
-#define DIAG_ENTRY(GroupName, FlagNameOffset, Members, SubGroups)  \
+#define DIAG_ENTRY(GroupName, FlagNameOffset, Members, SubGroups, Docs)\
   {FlagNameOffset, Members, SubGroups},
 #include "clang/Basic/DiagnosticGroups.inc"
 #undef DIAG_ENTRY
Index: clang/lib/Basic/DiagnosticIDs.cpp
===
--- clang/lib/Basic/DiagnosticIDs.cpp
+++ clang/lib/Basic/DiagnosticIDs.cpp
@@ -607,6 +607,7 @@
 uint16_t NameOffset;
 uint16_t Members;
 uint16_t SubGroups;
+StringRef Documentation;
 
 // String is stored with a pascal-style length byte.
 StringRef getName() const {
@@ -618,12 +619,17 @@
 
 // Second the table of options, sorted by name for fast binary lookup.
 static const WarningOption OptionTable[] = {
-#define DIAG_ENTRY(GroupName, FlagNameOffset, Members, SubGroups)  \
-  {FlagNameOffset, Members, SubGroups},
+#define DIAG_ENTRY(GroupName, FlagNameOffset, Members, SubGroups, Docs)\
+  {FlagNameOffset, Members, SubGroups, Docs},
 #include "clang/Basic/DiagnosticGroups.inc"
 #undef DIAG_ENTRY
 };
 
+/// Given a diagnostic group ID, return its documentation.
+StringRef DiagnosticIDs::getWarningOptionDocumentation(diag::Group Group) {
+  return OptionTable[static_cast(Group)].Documentation;
+}
+
 StringRef DiagnosticIDs::getWarningOptionForGroup(diag::Group Group) {
   return OptionTable[static_cast(Group)].getName();
 }
Index: clang/include/clang/Basic/DiagnosticIDs.h
===
--- clang/include/clang/Basic/DiagnosticIDs.h
+++ clang/include/clang/Basic/DiagnosticIDs.h
@@ -231,6 +231,9 @@
   /// "deprecated-declarations".
   static StringRef getWarningOptionForGroup(diag::Group);
 
+  /// Given a diagnostic group ID, return its documentation.
+  static StringRef getWarningOptionDocumentation(diag::Group GroupID);
+
   /// Given a group ID, returns the flag that toggles the group.
   /// For example, for "deprecated-declarations", returns
   /// Group::DeprecatedDeclarations.
Index: clang/include/clang/Basic/DiagnosticCategories.h
===
--- clang/include/clang/Basic/DiagnosticCategories.h
+++ clang/include/clang/Basic/DiagnosticCategories.h
@@ -21,7 +21,8 @@
 };
 
 enum class Group {
-#define DIAG_ENTRY(GroupName, FlagNameOffset, Members, SubGroups) GroupName,
+#define DIAG_ENTRY(GroupName, FlagNameOffset, Members, SubGroups, Docs)\
+  GroupName,
 #include "clang/Basic/DiagnosticGroups.inc"
 #undef CATEGORY
 #undef DIAG_ENTRY
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D127260: [clang-format] Remove braces of else blocks that embody an if block

2022-06-08 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

Still looks good. Was there a particular case where the previous version didn't 
work?


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

https://reviews.llvm.org/D127260

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


[PATCH] D127310: [clang][driver] fix to correctly set devtoolset on RHEL

2022-06-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

Do you have more authoritative answer when /root/usr is used and when it isn't?
I'd wish that we have comments for them.

This change also needs a unit test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127310

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


[PATCH] D126796: [clang][driver] adds `-print-diagnostics`

2022-06-08 Thread Christopher Di Bella 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 rG288c1bff96fc: [clang][driver] adds `-print-diagnostics` 
(authored by cjdb).

Changed prior to commit:
  https://reviews.llvm.org/D126796?vs=435223=435253#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126796

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/DiagnosticIDs.cpp
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/print-diagnostic-options.c


Index: clang/test/Driver/print-diagnostic-options.c
===
--- /dev/null
+++ clang/test/Driver/print-diagnostic-options.c
@@ -0,0 +1,13 @@
+// Test that -print-diagnostic-options prints warning groups and disablers
+
+// RUN: %clang -print-diagnostic-options | FileCheck %s
+
+// CHECK:  -W
+// CHECK:  -Wno-
+// CHECK:  -W#pragma-messages
+// CHECK:  -Wno-#pragma-messages
+// CHECK:  -W#warnings
+// CHECK:  -Wabi
+// CHECK:  -Wno-abi
+// CHECK:  -Wall
+// CHECK:  -Wno-all
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2010,6 +2010,13 @@
 return false;
   }
 
+  if (C.getArgs().hasArg(options::OPT_print_diagnostic_options)) {
+std::vector Flags = DiagnosticIDs::getDiagnosticFlags();
+for (std::size_t I = 0; I != Flags.size(); I += 2)
+  llvm::outs() << "  " << Flags[I] << "\n  " << Flags[I + 1] << "\n\n";
+return false;
+  }
+
   // FIXME: The following handlers should use a callback mechanism, we don't
   // know what the client would like to do.
   if (Arg *A = C.getArgs().getLastArg(options::OPT_print_file_name_EQ)) {
Index: clang/lib/Basic/DiagnosticIDs.cpp
===
--- clang/lib/Basic/DiagnosticIDs.cpp
+++ clang/lib/Basic/DiagnosticIDs.cpp
@@ -653,7 +653,7 @@
 }
 
 std::vector DiagnosticIDs::getDiagnosticFlags() {
-  std::vector Res;
+  std::vector Res{"-W", "-Wno-"};
   for (size_t I = 1; DiagGroupNames[I] != '\0';) {
 std::string Diag(DiagGroupNames + I + 1, DiagGroupNames[I]);
 I += DiagGroupNames[I] + 1;
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -4005,6 +4005,8 @@
   HelpText<"Print the paths used for finding ROCm installation">;
 def print_runtime_dir : Flag<["-", "--"], "print-runtime-dir">,
   HelpText<"Print the directory pathname containing clangs runtime libraries">;
+def print_diagnostic_options : Flag<["-", "--"], "print-diagnostic-options">,
+  HelpText<"Print all of Clang's warning options">;
 def private__bundle : Flag<["-"], "private_bundle">;
 def pthreads : Flag<["-"], "pthreads">;
 defm pthread : BoolOption<"", "pthread",


Index: clang/test/Driver/print-diagnostic-options.c
===
--- /dev/null
+++ clang/test/Driver/print-diagnostic-options.c
@@ -0,0 +1,13 @@
+// Test that -print-diagnostic-options prints warning groups and disablers
+
+// RUN: %clang -print-diagnostic-options | FileCheck %s
+
+// CHECK:  -W
+// CHECK:  -Wno-
+// CHECK:  -W#pragma-messages
+// CHECK:  -Wno-#pragma-messages
+// CHECK:  -W#warnings
+// CHECK:  -Wabi
+// CHECK:  -Wno-abi
+// CHECK:  -Wall
+// CHECK:  -Wno-all
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2010,6 +2010,13 @@
 return false;
   }
 
+  if (C.getArgs().hasArg(options::OPT_print_diagnostic_options)) {
+std::vector Flags = DiagnosticIDs::getDiagnosticFlags();
+for (std::size_t I = 0; I != Flags.size(); I += 2)
+  llvm::outs() << "  " << Flags[I] << "\n  " << Flags[I + 1] << "\n\n";
+return false;
+  }
+
   // FIXME: The following handlers should use a callback mechanism, we don't
   // know what the client would like to do.
   if (Arg *A = C.getArgs().getLastArg(options::OPT_print_file_name_EQ)) {
Index: clang/lib/Basic/DiagnosticIDs.cpp
===
--- clang/lib/Basic/DiagnosticIDs.cpp
+++ clang/lib/Basic/DiagnosticIDs.cpp
@@ -653,7 +653,7 @@
 }
 
 std::vector DiagnosticIDs::getDiagnosticFlags() {
-  std::vector Res;
+  std::vector Res{"-W", "-Wno-"};
   for (size_t I = 1; DiagGroupNames[I] != '\0';) {
 std::string Diag(DiagGroupNames + I + 1, DiagGroupNames[I]);
 I += DiagGroupNames[I] + 1;
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -4005,6 +4005,8 @@
   HelpText<"Print the paths used for finding ROCm 

[clang] 288c1bf - [clang][driver] adds `-print-diagnostics`

2022-06-08 Thread Christopher Di Bella via cfe-commits

Author: Christopher Di Bella
Date: 2022-06-08T17:55:31Z
New Revision: 288c1bff96fc9042d01b7055dc6e1049d8b54b26

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

LOG: [clang][driver] adds `-print-diagnostics`

Prints a list of all the warnings that Clang offers.

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

Added: 
clang/test/Driver/print-diagnostic-options.c

Modified: 
clang/include/clang/Driver/Options.td
clang/lib/Basic/DiagnosticIDs.cpp
clang/lib/Driver/Driver.cpp

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index b2feed0e36329..de3cd2ded8bef 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4005,6 +4005,8 @@ def print_rocm_search_dirs : Flag<["-", "--"], 
"print-rocm-search-dirs">,
   HelpText<"Print the paths used for finding ROCm installation">;
 def print_runtime_dir : Flag<["-", "--"], "print-runtime-dir">,
   HelpText<"Print the directory pathname containing clangs runtime libraries">;
+def print_diagnostic_options : Flag<["-", "--"], "print-diagnostic-options">,
+  HelpText<"Print all of Clang's warning options">;
 def private__bundle : Flag<["-"], "private_bundle">;
 def pthreads : Flag<["-"], "pthreads">;
 defm pthread : BoolOption<"", "pthread",

diff  --git a/clang/lib/Basic/DiagnosticIDs.cpp 
b/clang/lib/Basic/DiagnosticIDs.cpp
index a8a17994f7fc8..cc6b19646d1d6 100644
--- a/clang/lib/Basic/DiagnosticIDs.cpp
+++ b/clang/lib/Basic/DiagnosticIDs.cpp
@@ -653,7 +653,7 @@ StringRef DiagnosticIDs::getWarningOptionForDiag(unsigned 
DiagID) {
 }
 
 std::vector DiagnosticIDs::getDiagnosticFlags() {
-  std::vector Res;
+  std::vector Res{"-W", "-Wno-"};
   for (size_t I = 1; DiagGroupNames[I] != '\0';) {
 std::string Diag(DiagGroupNames + I + 1, DiagGroupNames[I]);
 I += DiagGroupNames[I] + 1;

diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 8e698a2a7cbef..8f7e411696e2c 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -2010,6 +2010,13 @@ bool Driver::HandleImmediateArgs(const Compilation ) {
 return false;
   }
 
+  if (C.getArgs().hasArg(options::OPT_print_diagnostic_options)) {
+std::vector Flags = DiagnosticIDs::getDiagnosticFlags();
+for (std::size_t I = 0; I != Flags.size(); I += 2)
+  llvm::outs() << "  " << Flags[I] << "\n  " << Flags[I + 1] << "\n\n";
+return false;
+  }
+
   // FIXME: The following handlers should use a callback mechanism, we don't
   // know what the client would like to do.
   if (Arg *A = C.getArgs().getLastArg(options::OPT_print_file_name_EQ)) {

diff  --git a/clang/test/Driver/print-diagnostic-options.c 
b/clang/test/Driver/print-diagnostic-options.c
new file mode 100644
index 0..fc6d1bf2eff53
--- /dev/null
+++ b/clang/test/Driver/print-diagnostic-options.c
@@ -0,0 +1,13 @@
+// Test that -print-diagnostic-options prints warning groups and disablers
+
+// RUN: %clang -print-diagnostic-options | FileCheck %s
+
+// CHECK:  -W
+// CHECK:  -Wno-
+// CHECK:  -W#pragma-messages
+// CHECK:  -Wno-#pragma-messages
+// CHECK:  -W#warnings
+// CHECK:  -Wabi
+// CHECK:  -Wno-abi
+// CHECK:  -Wall
+// CHECK:  -Wno-all



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


[PATCH] D127196: [clang][dataflow] Enable use of synthetic properties on all Value instances.

2022-06-08 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision.
gribozavr2 added inline comments.



Comment at: clang/include/clang/Analysis/FlowSensitive/Value.h:28
 
 /// Base class for all values computed by abstract interpretation.
+/// Don't use `Value` instances by value. All `Value` instances are allocated

Please add a blank line between the short description and the rest.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127196

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


[PATCH] D127312: [clang][dataflow] Remove IndirectionValue class, moving PointeeLoc field into PointerValue and ReferenceValue

2022-06-08 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:53
 
+static bool indirectionEquivalentValues(Value *Val1, Value *Val2) {
+  if (auto *IndVal1 = dyn_cast(Val1)) {

WDYT about "areEquivalentIndirectionValues"?



Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:63
-auto *IndVal2 = cast(Val2);
-assert(IndVal1->getKind() == IndVal2->getKind());
-if (>getPointeeLoc() == >getPointeeLoc())

This assert was lost in the new implementation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127312

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


[clang] 28aa7d1 - [docs][clang] Minor typo fix

2022-06-08 Thread Jose Manuel Monsalve Diaz via cfe-commits

Author: Jose Manuel Monsalve Diaz
Date: 2022-06-08T17:41:04Z
New Revision: 28aa7d188492223eb600317239f97b3f76dff1ee

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

LOG: [docs][clang] Minor typo fix

Changing "iamge" to "image"

Added: 


Modified: 
clang/docs/ClangOffloadPackager.rst

Removed: 




diff  --git a/clang/docs/ClangOffloadPackager.rst 
b/clang/docs/ClangOffloadPackager.rst
index 707462b77eb8..1664e415fe77 100644
--- a/clang/docs/ClangOffloadPackager.rst
+++ b/clang/docs/ClangOffloadPackager.rst
@@ -95,7 +95,7 @@ the following values for the :ref:`offload 
kind` and the
 +---+---+---+
 | IMG_Fatbinary | 0x04  | The image is a CUDA fatbinary file|
 +---+---+---+
-| IMG_PTX   | 0x05  | The iamge is a CUDA PTX file  |
+| IMG_PTX   | 0x05  | The image is a CUDA PTX file  |
 +---+---+---+
 
 .. table:: Offload Kind



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


[PATCH] D127229: [clang][deps] Set -disable-free for module compilations

2022-06-08 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 accepted this revision.
jansvoboda11 added a comment.
This revision is now accepted and ready to land.

LGTM!




Comment at: 
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp:338
+// always true for a driver invocation.
+bool DisableFree = true;
 DependencyScanningAction Action(

benlangmuir wrote:
> jansvoboda11 wrote:
> > jansvoboda11 wrote:
> > > I see the driver is adding `-disable-free` conditionally:
> > > 
> > > ```
> > >   if (!C.isForDiagnostics())
> > > CmdArgs.push_back("-disable-free");
> > > ```
> > > 
> > > Does that change anything for this patch?
> > If this is always `true` for our purposes, is there a reason for passing 
> > this argument into `DependencyScanningAction` instead of just hard-coding 
> > it there?
> `C.isForDiagnostics()` is only true in 
> `Driver::generateCompilationDiagnostics`, which didn't seem relevant to this 
> tool.
> 
> > If this is always true for our purposes, is there a reason for passing this 
> > argument into DependencyScanningAction instead of just hard-coding it there?
> 
> My thinking here was that making this an explicit option forces us to think 
> about it if we add another way to trigger dependency scanning that doesn't 
> use a driver invocation directly - for example, in our downstream branch 
> `experimental/cas/main`, we have another path through this code that starts 
> from a cc1 command-line, in which case I would think we should not add 
> -disable-free unless the original command-line set it, since it's explicit in 
> cc1.  Obviously we don't need to bend over backwards to accommodate 
> out-of-tree code here, but it seemed like a good indication this should be 
> explicit, since it's easy to do that.  WDYT?
That makes sense, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127229

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


[PATCH] D125349: [Sema] Fix crash for C11 atomic in gnu++ mode

2022-06-08 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

This seems like the right idea to me, yeah.  It doesn't look like the patch 
handles `volatile _Atomic` correctly, though.

I know we do a lot of candidate prefiltering, and that that can be difficult 
because of uesr-defined conversions.  How do those things interact with 
qualifiers?  Like, I notice the existing code is adding both `(T &, T)` and 
`(volatile T &, T)` when the LHS is `volatile`; is there a reason that's 
necessary?  Because we can't actually convert the LHS to remove those 
qualifiers, and adding combinatoric candidates could get very expensive if we 
start doing it for arbitrary extended qualifiers, which it feels like we ought 
to.  (You could certainly have e.g. a `volatile _Atomic __myaddrspace` 
l-value.)  If this is only necessary when the LHS has a user-defined 
conversion, then maybe we could at least not do it in the normal case just 
because the RHS is an overloaded type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125349

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


[PATCH] D127170: [WebAssembly] Implement remaining relaxed SIMD instructions

2022-06-08 Thread Thomas Lively via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGaff679a48c43: [WebAssembly] Implement remaining relaxed SIMD 
instructions (authored by tlively).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127170

Files:
  clang/include/clang/Basic/BuiltinsWebAssembly.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins-wasm.c
  llvm/include/llvm/IR/IntrinsicsWebAssembly.td
  llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
  llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
  llvm/test/MC/WebAssembly/simd-encodings.s

Index: llvm/test/MC/WebAssembly/simd-encodings.s
===
--- llvm/test/MC/WebAssembly/simd-encodings.s
+++ llvm/test/MC/WebAssembly/simd-encodings.s
@@ -830,8 +830,13 @@
 # CHECK: f64x2.relaxed_max # encoding: [0xfd,0x90,0x02]
 f64x2.relaxed_max
 
-# TODO: i16x8.relaxed_q15mulr_s # encoding: [0xfd,0x91,0x02]
-# TODO: i16x8.dot_i8x16_i7x16_s # encoding: [0xfd,0x92,0x02]
-# TODO: i32x4.dot_i8x16_i7x16_add_s # encoding: [0xfd,0x93,0x02]
+# CHECK: i16x8.relaxed_q15mulr_s # encoding: [0xfd,0x91,0x02]
+i16x8.relaxed_q15mulr_s
+
+# CHECK: i16x8.dot_i8x16_i7x16_s # encoding: [0xfd,0x92,0x02]
+i16x8.dot_i8x16_i7x16_s
+
+# CHECK: i32x4.dot_i8x16_i7x16_add_s # encoding: [0xfd,0x93,0x02]
+i32x4.dot_i8x16_i7x16_add_s
 
 end_function
Index: llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
===
--- llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
+++ llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
@@ -368,6 +368,30 @@
   ret <8 x i16> %v
 }
 
+; CHECK-LABEL: relaxed_q15mulr_s_i16x8:
+; CHECK-NEXT: .functype relaxed_q15mulr_s_i16x8 (v128, v128) -> (v128){{$}}
+; CHECK-NEXT: i16x8.relaxed_q15mulr_s $push[[R:[0-9]+]]=, $0, $1{{$}}
+; CHECK-NEXT: return $pop[[R]]{{$}}
+declare <8 x i16> @llvm.wasm.relaxed.q15mulr.signed(<8 x i16>, <8 x i16>)
+define <8 x i16> @relaxed_q15mulr_s_i16x8(<8 x i16> %a, <8 x i16> %b) {
+  %v = call <8 x i16> @llvm.wasm.relaxed.q15mulr.signed(
+<8 x i16> %a, <8 x i16> %b
+  )
+  ret <8 x i16> %v
+}
+
+; CHECK-LABEL: dot_i8x16_i7x16_s_i16x8:
+; CHECK-NEXT: .functype dot_i8x16_i7x16_s_i16x8 (v128, v128) -> (v128){{$}}
+; CHECK-NEXT: i16x8.dot_i8x16_i7x16_s $push[[R:[0-9]+]]=, $0, $1{{$}}
+; CHECK-NEXT: return $pop[[R]]{{$}}
+declare <8 x i16> @llvm.wasm.dot.i8x16.i7x16.signed(<16 x i8>, <16 x i8>)
+define <8 x i16> @dot_i8x16_i7x16_s_i16x8(<16 x i8> %a, <16 x i8> %b) {
+  %v = call <8 x i16> @llvm.wasm.dot.i8x16.i7x16.signed(
+<16 x i8> %a, <16 x i8> %b
+  )
+  ret <8 x i16> %v
+}
+
 ; ==
 ; 4 x i32
 ; ==
@@ -568,6 +592,20 @@
   ret <4 x i32> %a
 }
 
+; CHECK-LABEL: dot_i8x16_i7x16_add_s_i32x4:
+; CHECK-NEXT: .functype dot_i8x16_i7x16_add_s_i32x4 (v128, v128, v128) -> (v128){{$}}
+; CHECK-NEXT: i32x4.dot_i8x16_i7x16_add_s $push[[R:[0-9]+]]=, $0, $1, $2{{$}}
+; CHECK-NEXT: return $pop[[R]]{{$}}
+declare <4 x i32> @llvm.wasm.dot.i8x16.i7x16.add.signed(<16 x i8>, <16 x i8>,
+<4 x i32>)
+define <4 x i32> @dot_i8x16_i7x16_add_s_i32x4(<16 x i8> %a, <16 x i8> %b,
+  <4 x i32> %c) {
+  %v = call <4 x i32> @llvm.wasm.dot.i8x16.i7x16.add.signed(
+<16 x i8> %a, <16 x i8> %b, <4 x i32> %c
+  )
+  ret <4 x i32> %v
+}
+
 ; ==
 ; 2 x i64
 ; ==
Index: llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
===
--- llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
+++ llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
@@ -1403,7 +1403,6 @@
 defm "" : SIMDLANESELECT;
 defm "" : SIMDLANESELECT;
 
-
 //===--===//
 // Relaxed floating-point min and max.
 //===--===//
@@ -1426,3 +1425,30 @@
RelaxedBinary;
 defm SIMD_RELAXED_FMAX :
RelaxedBinary;
+
+//===--===//
+// Relaxed rounding q15 multiplication
+//===--===//
+
+defm RELAXED_Q15MULR_S :
+  RelaxedBinary;
+
+//===--===//
+// Relaxed integer dot product
+//===--===//
+
+defm RELAXED_DOT :
+  RELAXED_I<(outs V128:$dst), (ins V128:$lhs, V128:$rhs), (outs), (ins),
+

[clang] aff679a - [WebAssembly] Implement remaining relaxed SIMD instructions

2022-06-08 Thread Thomas Lively via cfe-commits

Author: Thomas Lively
Date: 2022-06-08T10:32:10-07:00
New Revision: aff679a48c438924c4fca4e9eaa38f91c022ffe3

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

LOG: [WebAssembly] Implement remaining relaxed SIMD instructions

Add codegen, intrinsics, and builtins for the i16x8.relaxed_q15mulr_s,
i16x8.dot_i8x16_i7x16_s, and i32x4.dot_i8x16_i7x16_add_s instructions. These are
the last instructions from the relaxed SIMD proposal[1] that had not been
implemented.

[1]:
https://github.com/WebAssembly/relaxed-simd/blob/main/proposals/relaxed-simd/Overview.md.

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

Added: 


Modified: 
clang/include/clang/Basic/BuiltinsWebAssembly.def
clang/lib/CodeGen/CGBuiltin.cpp
clang/test/CodeGen/builtins-wasm.c
llvm/include/llvm/IR/IntrinsicsWebAssembly.td
llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
llvm/test/MC/WebAssembly/simd-encodings.s

Removed: 




diff  --git a/clang/include/clang/Basic/BuiltinsWebAssembly.def 
b/clang/include/clang/Basic/BuiltinsWebAssembly.def
index 24fb24f81fc14..03c6162f62e3f 100644
--- a/clang/include/clang/Basic/BuiltinsWebAssembly.def
+++ b/clang/include/clang/Basic/BuiltinsWebAssembly.def
@@ -184,5 +184,10 @@ TARGET_BUILTIN(__builtin_wasm_relaxed_trunc_u_i32x4_f32x4, 
"V4UiV4f", "nc", "rel
 TARGET_BUILTIN(__builtin_wasm_relaxed_trunc_s_zero_i32x4_f64x2, "V4iV2d", 
"nc", "relaxed-simd")
 TARGET_BUILTIN(__builtin_wasm_relaxed_trunc_u_zero_i32x4_f64x2, "V4UiV2d", 
"nc", "relaxed-simd")
 
+TARGET_BUILTIN(__builtin_wasm_relaxed_q15mulr_s_i16x8, "V8sV8sV8s", "nc", 
"relaxed-simd")
+
+TARGET_BUILTIN(__builtin_wasm_dot_i8x16_i7x16_s_i16x8, "V8sV16ScV16Sc", "nc", 
"relaxed-simd")
+TARGET_BUILTIN(__builtin_wasm_dot_i8x16_i7x16_add_s_i32x4, "V4iV16ScV16ScV4i", 
"nc", "relaxed-simd")
+
 #undef BUILTIN
 #undef TARGET_BUILTIN

diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 1c10d32de5fe9..9d2e6dfb320d5 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -18684,6 +18684,26 @@ Value 
*CodeGenFunction::EmitWebAssemblyBuiltinExpr(unsigned BuiltinID,
 Function *Callee = CGM.getIntrinsic(IntNo);
 return Builder.CreateCall(Callee, {Vec});
   }
+  case WebAssembly::BI__builtin_wasm_relaxed_q15mulr_s_i16x8: {
+Value *LHS = EmitScalarExpr(E->getArg(0));
+Value *RHS = EmitScalarExpr(E->getArg(1));
+Function *Callee = 
CGM.getIntrinsic(Intrinsic::wasm_relaxed_q15mulr_signed);
+return Builder.CreateCall(Callee, {LHS, RHS});
+  }
+  case WebAssembly::BI__builtin_wasm_dot_i8x16_i7x16_s_i16x8: {
+Value *LHS = EmitScalarExpr(E->getArg(0));
+Value *RHS = EmitScalarExpr(E->getArg(1));
+Function *Callee = 
CGM.getIntrinsic(Intrinsic::wasm_dot_i8x16_i7x16_signed);
+return Builder.CreateCall(Callee, {LHS, RHS});
+  }
+  case WebAssembly::BI__builtin_wasm_dot_i8x16_i7x16_add_s_i32x4: {
+Value *LHS = EmitScalarExpr(E->getArg(0));
+Value *RHS = EmitScalarExpr(E->getArg(1));
+Value *Acc = EmitScalarExpr(E->getArg(2));
+Function *Callee =
+CGM.getIntrinsic(Intrinsic::wasm_dot_i8x16_i7x16_add_signed);
+return Builder.CreateCall(Callee, {LHS, RHS, Acc});
+  }
   default:
 return nullptr;
   }

diff  --git a/clang/test/CodeGen/builtins-wasm.c 
b/clang/test/CodeGen/builtins-wasm.c
index 6ea97e293077a..d9ea753ee86a2 100644
--- a/clang/test/CodeGen/builtins-wasm.c
+++ b/clang/test/CodeGen/builtins-wasm.c
@@ -777,3 +777,24 @@ u32x4 relaxed_trunc_u_zero_i32x4_f64x2(f64x2 x) {
   // WEBASSEMBLY: call <4 x i32> @llvm.wasm.relaxed.trunc.unsigned.zero(<2 x 
double> %x)
   // WEBASSEMBLY-NEXT: ret
 }
+
+i16x8 relaxed_q15mulr_s_i16x8(i16x8 a, i16x8 b) {
+  return __builtin_wasm_relaxed_q15mulr_s_i16x8(a, b);
+  // WEBASSEMBLY: call <8 x i16> @llvm.wasm.relaxed.q15mulr.signed(
+  // WEBASSEMBLY-SAME: <8 x i16> %a, <8 x i16> %b)
+  // WEBASSEMBLY-NEXT: ret
+}
+
+i16x8 dot_i8x16_i7x16_s_i16x8(i8x16 a, i8x16 b) {
+  return __builtin_wasm_dot_i8x16_i7x16_s_i16x8(a, b);
+  // WEBASSEMBLY: call <8 x i16> @llvm.wasm.dot.i8x16.i7x16.signed(
+  // WEBASSEMBLY-SAME: <16 x i8> %a, <16 x i8> %b)
+  // WEBASSEMBLY-NEXT: ret
+}
+
+i32x4 dot_i8x16_i7x16_add_s_i32x4(i8x16 a, i8x16 b, i32x4 c) {
+  return __builtin_wasm_dot_i8x16_i7x16_add_s_i32x4(a, b, c);
+  // WEBASSEMBLY: call <4 x i32> @llvm.wasm.dot.i8x16.i7x16.add.signed(
+  // WEBASSEMBLY-SAME: <16 x i8> %a, <16 x i8> %b, <4 x i32> %c)
+  // WEBASSEMBLY-NEXT: ret
+}

diff  --git a/llvm/include/llvm/IR/IntrinsicsWebAssembly.td 
b/llvm/include/llvm/IR/IntrinsicsWebAssembly.td
index 2f3edfe806579..f313be1b2235d 100644
--- a/llvm/include/llvm/IR/IntrinsicsWebAssembly.td
+++ 

[PATCH] D127229: [clang][deps] Set -disable-free for module compilations

2022-06-08 Thread Ben Langmuir via Phabricator via cfe-commits
benlangmuir added inline comments.



Comment at: 
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp:338
+// always true for a driver invocation.
+bool DisableFree = true;
 DependencyScanningAction Action(

jansvoboda11 wrote:
> jansvoboda11 wrote:
> > I see the driver is adding `-disable-free` conditionally:
> > 
> > ```
> >   if (!C.isForDiagnostics())
> > CmdArgs.push_back("-disable-free");
> > ```
> > 
> > Does that change anything for this patch?
> If this is always `true` for our purposes, is there a reason for passing this 
> argument into `DependencyScanningAction` instead of just hard-coding it there?
`C.isForDiagnostics()` is only true in 
`Driver::generateCompilationDiagnostics`, which didn't seem relevant to this 
tool.

> If this is always true for our purposes, is there a reason for passing this 
> argument into DependencyScanningAction instead of just hard-coding it there?

My thinking here was that making this an explicit option forces us to think 
about it if we add another way to trigger dependency scanning that doesn't use 
a driver invocation directly - for example, in our downstream branch 
`experimental/cas/main`, we have another path through this code that starts 
from a cc1 command-line, in which case I would think we should not add 
-disable-free unless the original command-line set it, since it's explicit in 
cc1.  Obviously we don't need to bend over backwards to accommodate out-of-tree 
code here, but it seemed like a good indication this should be explicit, since 
it's easy to do that.  WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127229

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


[PATCH] D127293: [clang-tidy] Ignore other members in a union if any member of it is initialized in cppcoreguidelines-pro-type-member-init

2022-06-08 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp:558
+  S1() {}
+  // CHECK-MESSAGES-NOT: warning:
+  union {

This line is redundant and likely to cause unintended test failures, same goes 
below.
If there was a warning on this line and no directive for it, the check script 
would fail due to that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127293

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


[PATCH] D54943: [clang-tidy] implement new check 'misc-const-correctness' to add 'const' to unmodified variables

2022-06-08 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments.



Comment at: clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.h:35
+TransformPointersAsValues(
+Options.get("TransformPointersAsValues", false)) {}
+

JonasToth wrote:
> njames93 wrote:
> > It may be worth adding some validation to these. If AnalyzeValues, 
> > AnalyzeReferences and WarnPointersAsValues are all false, this whole check 
> > is basically a no-op.
> Whats the proper way to react? Short-circuit the `registerMatchers`, similar 
> to language support?
> I think thats the way I would go about this.
`ClangTidyCheck::configurationDiag` is the way to warn about issues with config.


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

https://reviews.llvm.org/D54943

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


[PATCH] D127310: [clang][driver] fix to correctly set devtoolset on RHEL

2022-06-08 Thread Quinn Pham via Phabricator via cfe-commits
quinnp marked 2 inline comments as done.
quinnp added a comment.

Thank you @nemanjai! I've updated the patch based on your suggestion and tested 
it for both the existing testcase and the RHEL buildbot failure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127310

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


[PATCH] D127243: [clang][deps] Make order of module dependencies deterministic

2022-06-08 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 accepted this revision.
jansvoboda11 added a comment.
This revision is now accepted and ready to land.

Thanks for explaining this, LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127243

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


[PATCH] D127310: [clang][driver] fix to correctly set devtoolset on RHEL

2022-06-08 Thread Quinn Pham via Phabricator via cfe-commits
quinnp updated this revision to Diff 435239.
quinnp added a comment.

Addressing review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127310

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp


Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2153,8 +2153,10 @@
   }
 }
 
-if (ChosenToolsetVersion > 0)
+if (ChosenToolsetVersion > 0) {
   Prefixes.push_back(ChosenToolsetDir);
+  Prefixes.push_back(ChosenToolsetDir + "/root/usr");
+}
   }
 
   // Fall back to /usr which is used by most non-Solaris systems.


Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2153,8 +2153,10 @@
   }
 }
 
-if (ChosenToolsetVersion > 0)
+if (ChosenToolsetVersion > 0) {
   Prefixes.push_back(ChosenToolsetDir);
+  Prefixes.push_back(ChosenToolsetDir + "/root/usr");
+}
   }
 
   // Fall back to /usr which is used by most non-Solaris systems.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D127243: [clang][deps] Make order of module dependencies deterministic

2022-06-08 Thread Ben Langmuir via Phabricator via cfe-commits
benlangmuir updated this revision to Diff 435237.
benlangmuir added a comment.

Removed use of std::unique_ptr in DependencyScanningTool.cpp, per review 
feedback.

@jansvoboda11  Note: there is another map containing 
`std::unique_ptr` in `ModuleDepCollector`, but that one is required 
for correctness.  The problem is that there is code that forms a `ModuleDeps &` 
lvalue and assumes it will be correct after mutating the containing 
`ModularDeps` map.  For example, the signature of `addAllSubmoduleDeps` 
requires this.  If we want to switch to using this as a value in the map, we 
would need to do the lookup in the map again after each mutation. This code 
worked previously with `unordered_map`, because it guarantees that keys and 
values are not mutated by unrelated mutations of the containing map, unlike 
`MapVector`.  In practice, this means there is no additional overhead from the 
unique_ptr, because it was already doing a separate allocation for each value 
in the unordered_map.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127243

Files:
  clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp


Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -199,7 +199,7 @@
   MDC.Consumer.handleDependencyOutputOpts(*MDC.Opts);
 
   for (auto & : MDC.ModularDeps)
-MDC.Consumer.handleModuleDependency(I.second);
+MDC.Consumer.handleModuleDependency(*I.second);
 
   for (auto & : MDC.FileDeps)
 MDC.Consumer.handleFileDependency(I);
@@ -212,11 +212,12 @@
   assert(M == M->getTopLevelModule() && "Expected top level module!");
 
   // If this module has been handled already, just return its ID.
-  auto ModI = MDC.ModularDeps.insert({M, ModuleDeps{}});
+  auto ModI = MDC.ModularDeps.insert({M, nullptr});
   if (!ModI.second)
-return ModI.first->second.ID;
+return ModI.first->second->ID;
 
-  ModuleDeps  = ModI.first->second;
+  ModI.first->second = std::make_unique();
+  ModuleDeps  = *ModI.first->second;
 
   MD.ID.ModuleName = M->getFullModuleName();
   MD.ImportedByMainFile = DirectModularDeps.contains(M);
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
@@ -176,7 +176,7 @@
   private:
 std::vector Dependencies;
 std::vector PrebuiltModuleDeps;
-std::map ClangModuleDeps;
+llvm::MapVector> 
ClangModuleDeps;
 std::string ContextHash;
 std::vector OutputPaths;
 const llvm::StringSet<> 
Index: clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
===
--- clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -149,9 +149,9 @@
   /// The parent dependency collector.
   ModuleDepCollector 
   /// Working set of direct modular dependencies.
-  llvm::DenseSet DirectModularDeps;
+  llvm::SetVector DirectModularDeps;
   /// Working set of direct modular dependencies that have already been built.
-  llvm::DenseSet DirectPrebuiltModularDeps;
+  llvm::SetVector DirectPrebuiltModularDeps;
 
   void handleImport(const Module *Imported);
 
@@ -199,7 +199,7 @@
   /// textually included header files.
   std::vector FileDeps;
   /// Direct and transitive modular dependencies of the main source file.
-  std::unordered_map ModularDeps;
+  llvm::MapVector> ModularDeps;
   /// Options that control the dependency output generation.
   std::unique_ptr Opts;
   /// The original Clang invocation passed to dependency scanner.


Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -199,7 +199,7 @@
   MDC.Consumer.handleDependencyOutputOpts(*MDC.Opts);
 
   for (auto & : MDC.ModularDeps)
-MDC.Consumer.handleModuleDependency(I.second);
+MDC.Consumer.handleModuleDependency(*I.second);
 
   for (auto & : MDC.FileDeps)
 MDC.Consumer.handleFileDependency(I);
@@ -212,11 +212,12 @@
   assert(M == M->getTopLevelModule() && "Expected top level module!");
 
   // If this module has been handled already, just return its ID.
-  auto ModI = MDC.ModularDeps.insert({M, ModuleDeps{}});
+  auto ModI = MDC.ModularDeps.insert({M, nullptr});
   if (!ModI.second)
-return 

  1   2   3   >