[PATCH] D116786: [clangd] Add designator inlay hints for initializer lists.

2022-01-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment.

> Agree. I guess then we ignore the issue for this patch? As you say that's 
> affects other hints too, and a mitigation (hopefully) won't be specific to 
> the kind of hint.

Yup.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116786

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


[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

2022-01-24 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision.
curdeius added a comment.

LGTM.




Comment at: clang/lib/Format/DefinitionBlockSeparator.cpp:141-145
+if (NextLine->MightBeFunctionDecl &&
+NextLine->mightBeFunctionDefinition())
+  if (NextLine->First->NewlinesBefore == 1 &&
+  OperateLine->First->is(TT_FunctionLikeOrFreestandingMacro))
+return true;

Please merge these two `if`s into one.


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

https://reviews.llvm.org/D117520

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


[PATCH] D118016: [clang-tidy] Change code of SignalHandlerCheck (NFC).

2022-01-24 Thread Balázs Kéri via Phabricator via cfe-commits
balazske added a comment.

The plan is to take over functionality from D91164 
, D97960  and 
D33825  after this change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118016

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


[clang] 81793bd - [clang-format] Assert Line->First and State.NextToken->Previous. NFC.

2022-01-24 Thread Marek Kurdej via cfe-commits

Author: Marek Kurdej
Date: 2022-01-24T09:36:46+01:00
New Revision: 81793bd276afefea0e525307676181478fc614c9

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

LOG: [clang-format] Assert Line->First and State.NextToken->Previous. NFC.

Cf. scan-build reports:
* 
https://llvm.org/reports/scan-build/report-FormatToken.cpp-precomputeFormattingInfos-35-93e1e1.html#EndPath
* 
https://llvm.org/reports/scan-build/report-ContinuationIndenter.cpp-addTokenOnCurrentLine-15-dfdc6d.html#EndPath

Added: 


Modified: 
clang/lib/Format/ContinuationIndenter.cpp
clang/lib/Format/FormatToken.cpp

Removed: 




diff  --git a/clang/lib/Format/ContinuationIndenter.cpp 
b/clang/lib/Format/ContinuationIndenter.cpp
index 28f13c06e308..b66584652bc8 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -543,13 +543,15 @@ unsigned ContinuationIndenter::addTokenToState(LineState 
&State, bool Newline,
bool DryRun,
unsigned ExtraSpaces) {
   const FormatToken &Current = *State.NextToken;
+  assert(State.NextToken->Previous);
+  const FormatToken &Previous = *State.NextToken->Previous;
 
   assert(!State.Stack.empty());
   State.NoContinuation = false;
 
   if ((Current.is(TT_ImplicitStringLiteral) &&
-   (Current.Previous->Tok.getIdentifierInfo() == nullptr ||
-Current.Previous->Tok.getIdentifierInfo()->getPPKeywordID() ==
+   (Previous.Tok.getIdentifierInfo() == nullptr ||
+Previous.Tok.getIdentifierInfo()->getPPKeywordID() ==
 tok::pp_not_keyword))) {
 unsigned EndColumn =
 SourceMgr.getSpellingColumnNumber(Current.WhitespaceRange.getEnd());
@@ -579,7 +581,9 @@ unsigned ContinuationIndenter::addTokenToState(LineState 
&State, bool Newline,
 void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
  unsigned ExtraSpaces) {
   FormatToken &Current = *State.NextToken;
+  assert(State.NextToken->Previous);
   const FormatToken &Previous = *State.NextToken->Previous;
+
   if (Current.is(tok::equal) &&
   (State.Line->First->is(tok::kw_for) || Current.NestingLevel == 0) &&
   State.Stack.back().VariablePos == 0) {
@@ -775,6 +779,7 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState 
&State, bool DryRun,
 unsigned ContinuationIndenter::addTokenOnNewLine(LineState &State,
  bool DryRun) {
   FormatToken &Current = *State.NextToken;
+  assert(State.NextToken->Previous);
   const FormatToken &Previous = *State.NextToken->Previous;
 
   // Extra penalty that needs to be added because of the way certain line

diff  --git a/clang/lib/Format/FormatToken.cpp 
b/clang/lib/Format/FormatToken.cpp
index def5663d0449..59d6f29bb54d 100644
--- a/clang/lib/Format/FormatToken.cpp
+++ b/clang/lib/Format/FormatToken.cpp
@@ -189,6 +189,7 @@ void CommaSeparatedList::precomputeFormattingInfos(const 
FormatToken *Token) {
 
   bool HasSeparatingComment = false;
   for (unsigned i = 0, e = Commas.size() + 1; i != e; ++i) {
+assert(ItemBegin);
 // Skip comments on their own line.
 while (ItemBegin->HasUnescapedNewline && ItemBegin->isTrailingComment()) {
   ItemBegin = ItemBegin->Next;



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


[PATCH] D117522: [clang-tidy] Add modernize-macro-to-enum check

2022-01-24 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 402415.
LegalizeAdulthood added a comment.

Testing on iterated dynamics revealed a number of shortcomings;
update the code to address those:

- Improve handling of include guards
- Handle conditional compilation blocks better
- Reject macros that expand to floating point literals
- Start a new enum when macros have intervening text


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

https://reviews.llvm.org/D117522

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.h
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-macro-to-enum.rst
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum2.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum3.h
  clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp
@@ -0,0 +1,137 @@
+// RUN: %check_clang_tidy %s modernize-macro-to-enum %t -- -- -I%S/Inputs/modernize-macro-to-enum
+
+#if 1
+#include "modernize-macro-to-enum.h"
+
+// These macros are skipped due to being inside a conditional compilation block.
+#define GOO_RED 1
+#define GOO_GREEN 2
+#define GOO_BLUE 3
+
+#endif
+
+#define RED 0xFF
+#define GREEN 0x00FF00
+#define BLUE 0xFF
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: Replace macro with enum
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'RED' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'GREEN' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'BLUE' defines an integral constant; prefer an enum instead
+// CHECK-FIXES: enum {
+// CHECK-FIXES-NEXT: RED = 0xFF,
+// CHECK-FIXES-NEXT: GREEN = 0x00FF00,
+// CHECK-FIXES-NEXT: BLUE = 0xFF
+// CHECK-FIXES-NEXT: };
+
+// Verify that comments are preserved.
+#define CoordModeOrigin 0   /* relative to the origin */
+#define CoordModePrevious   1   /* relative to previous point */
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: Replace macro with enum
+// CHECK-MESSAGES: :[[@LINE-3]]:9: warning: Macro 'CoordModeOrigin' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-3]]:9: warning: Macro 'CoordModePrevious' defines an integral constant; prefer an enum instead
+// CHECK-FIXES: enum {
+// CHECK-FIXES-NEXT: CoordModeOrigin = 0,   /* relative to the origin */
+// CHECK-FIXES-NEXT: CoordModePrevious =   1   /* relative to previous point */
+// CHECK-FIXES-NEXT: };
+
+// Verify that multiline comments are preserved.
+#define BadDrawable 9   /* parameter not a Pixmap or Window */
+#define BadAccess   10  /* depending on context:
+- key/button already grabbed
+- attempt to free an illegal 
+  cmap entry 
+- attempt to store into a read-only 
+  color map entry. */
+// - attempt to modify the access control
+//   list from other than the local host.
+//
+#define BadAlloc11  /* insufficient resources */
+// CHECK-MESSAGES: :[[@LINE-11]]:1: warning: Replace macro with enum
+// CHECK-MESSAGES: :[[@LINE-12]]:9: warning: Macro 'BadDrawable' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-12]]:9: warning: Macro 'BadAccess' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'BadAlloc' defines an integral constant; prefer an enum instead
+// CHECK-FIXES: enum {
+// CHECK-FIXES-NEXT: BadDrawable = 9,   /* parameter not a Pixmap or Window */
+// CHECK-FIXES-NEXT: BadAccess =   10,  /* depending on context:
+// CHECK-FIXES-NEXT: - key/button already grabbed
+// CHECK-FIXES-NEXT: - attempt to free an illegal 
+// CHECK-FIXES-NEXT:   cmap entry 
+// CHECK-FIXES-NEXT: - attempt to store into a read-only 
+// CHECK-FIXES-NEXT:   color map entry. */
+// CHECK-FIXES-NEXT: // - attempt to modify 

[PATCH] D117522: [clang-tidy] Add modernize-macro-to-enum check

2022-01-24 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 402416.
LegalizeAdulthood added a comment.

Switch back to SmallVector after debugging with std::vector
Drop unnecessary includes


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

https://reviews.llvm.org/D117522

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.h
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-macro-to-enum.rst
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum2.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum3.h
  clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp
@@ -0,0 +1,137 @@
+// RUN: %check_clang_tidy %s modernize-macro-to-enum %t -- -- -I%S/Inputs/modernize-macro-to-enum
+
+#if 1
+#include "modernize-macro-to-enum.h"
+
+// These macros are skipped due to being inside a conditional compilation block.
+#define GOO_RED 1
+#define GOO_GREEN 2
+#define GOO_BLUE 3
+
+#endif
+
+#define RED 0xFF
+#define GREEN 0x00FF00
+#define BLUE 0xFF
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: Replace macro with enum
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'RED' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'GREEN' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'BLUE' defines an integral constant; prefer an enum instead
+// CHECK-FIXES: enum {
+// CHECK-FIXES-NEXT: RED = 0xFF,
+// CHECK-FIXES-NEXT: GREEN = 0x00FF00,
+// CHECK-FIXES-NEXT: BLUE = 0xFF
+// CHECK-FIXES-NEXT: };
+
+// Verify that comments are preserved.
+#define CoordModeOrigin 0   /* relative to the origin */
+#define CoordModePrevious   1   /* relative to previous point */
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: Replace macro with enum
+// CHECK-MESSAGES: :[[@LINE-3]]:9: warning: Macro 'CoordModeOrigin' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-3]]:9: warning: Macro 'CoordModePrevious' defines an integral constant; prefer an enum instead
+// CHECK-FIXES: enum {
+// CHECK-FIXES-NEXT: CoordModeOrigin = 0,   /* relative to the origin */
+// CHECK-FIXES-NEXT: CoordModePrevious =   1   /* relative to previous point */
+// CHECK-FIXES-NEXT: };
+
+// Verify that multiline comments are preserved.
+#define BadDrawable 9   /* parameter not a Pixmap or Window */
+#define BadAccess   10  /* depending on context:
+- key/button already grabbed
+- attempt to free an illegal 
+  cmap entry 
+- attempt to store into a read-only 
+  color map entry. */
+// - attempt to modify the access control
+//   list from other than the local host.
+//
+#define BadAlloc11  /* insufficient resources */
+// CHECK-MESSAGES: :[[@LINE-11]]:1: warning: Replace macro with enum
+// CHECK-MESSAGES: :[[@LINE-12]]:9: warning: Macro 'BadDrawable' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-12]]:9: warning: Macro 'BadAccess' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'BadAlloc' defines an integral constant; prefer an enum instead
+// CHECK-FIXES: enum {
+// CHECK-FIXES-NEXT: BadDrawable = 9,   /* parameter not a Pixmap or Window */
+// CHECK-FIXES-NEXT: BadAccess =   10,  /* depending on context:
+// CHECK-FIXES-NEXT: - key/button already grabbed
+// CHECK-FIXES-NEXT: - attempt to free an illegal 
+// CHECK-FIXES-NEXT:   cmap entry 
+// CHECK-FIXES-NEXT: - attempt to store into a read-only 
+// CHECK-FIXES-NEXT:   color map entry. */
+// CHECK-FIXES-NEXT: // - attempt to modify the access control
+// CHECK-FIXES-NEXT: //   list from other than the local host.
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: BadAlloc = 

[PATCH] D117600: [CGCall] Annotate operator new with inaccessiblememonly if AssumeSaneOperatorNew is on

2022-01-24 Thread Nikita Popov via Phabricator via cfe-commits
nikic added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:2071
   RetAttrs.addAttribute(llvm::Attribute::NoAlias);
+  FuncAttrs.addAttribute("inaccessiblememonly");
+}

This should be `Attribute::InacccessibleMemOnly`. It's not a string attribute.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117600

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


[PATCH] D118011: [RISCV] Adjust predicates and update intrinsic for clmul and clmulh in Zbkc extension

2022-01-24 Thread WangLian via Phabricator via cfe-commits
Jimerlife updated this revision to Diff 402414.
Jimerlife retitled this revision from "[RISCV][NFC] Add "zbkc" predicate for 
clmul and clmulh pattern" to "[RISCV] Adjust predicates and update intrinsic 
for clmul and clmulh in Zbkc extension ".
Jimerlife edited the summary of this revision.
Jimerlife added a reviewer: kito-cheng.
Jimerlife added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

1. Add IR tests for clmul and clmulh in zbkc extension.
2. Updating BuiltinsRISCV.def, I rename "__builtin__riscv__clmul" to 
"__builtin_riscv_clmul_kc" in zbkc extension, because cannot share same name 
"__builtin__riscv_clmul" in zbc and zbkc extension.
3. Add C tests for clmul and clmulh in zbkc extension


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118011

Files:
  clang/include/clang/Basic/BuiltinsRISCV.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/RISCV/rvb-intrinsics/riscv32-zbkc.c
  clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbkc.c
  llvm/include/llvm/IR/IntrinsicsRISCV.td
  llvm/lib/Target/RISCV/RISCVInstrInfoZb.td
  llvm/test/CodeGen/RISCV/rv32zbc-zbkc-intrinsic.ll
  llvm/test/CodeGen/RISCV/rv64zbc-zbkc-intrinsic.ll

Index: llvm/test/CodeGen/RISCV/rv64zbc-zbkc-intrinsic.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/rv64zbc-zbkc-intrinsic.ll
@@ -0,0 +1,37 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv64 -mattr=+zbc -verify-machineinstrs < %s \
+; RUN:   | FileCheck %s -check-prefix=RV64ZBC
+; RUN: llc -mtriple=riscv64 -mattr=+zbkc -verify-machineinstrs < %s \
+; RUN:   | FileCheck %s -check-prefix=RV64ZBKC
+
+declare i64 @llvm.riscv.clmul.i64(i64 %a, i64 %b)
+
+define i64 @clmul64(i64 %a, i64 %b) nounwind {
+; RV64ZBC-LABEL: clmul64:
+; RV64ZBC:   # %bb.0:
+; RV64ZBC-NEXT:clmul a0, a0, a1
+; RV64ZBC-NEXT:ret
+;
+; RV64ZBKC-LABEL: clmul64:
+; RV64ZBKC:   # %bb.0:
+; RV64ZBKC-NEXT:clmul a0, a0, a1
+; RV64ZBKC-NEXT:ret
+  %tmp = call i64 @llvm.riscv.clmul.i64(i64 %a, i64 %b)
+ ret i64 %tmp
+}
+
+declare i64 @llvm.riscv.clmulh.i64(i64 %a, i64 %b)
+
+define i64 @clmul64h(i64 %a, i64 %b) nounwind {
+; RV64ZBC-LABEL: clmul64h:
+; RV64ZBC:   # %bb.0:
+; RV64ZBC-NEXT:clmulh a0, a0, a1
+; RV64ZBC-NEXT:ret
+;
+; RV64ZBKC-LABEL: clmul64h:
+; RV64ZBKC:   # %bb.0:
+; RV64ZBKC-NEXT:clmulh a0, a0, a1
+; RV64ZBKC-NEXT:ret
+  %tmp = call i64 @llvm.riscv.clmulh.i64(i64 %a, i64 %b)
+ ret i64 %tmp
+}
Index: llvm/test/CodeGen/RISCV/rv32zbc-zbkc-intrinsic.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/rv32zbc-zbkc-intrinsic.ll
@@ -0,0 +1,37 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv32 -mattr=+zbc -verify-machineinstrs < %s \
+; RUN:   | FileCheck %s -check-prefix=RV32ZBC
+; RUN: llc -mtriple=riscv32 -mattr=+zbkc -verify-machineinstrs < %s \
+; RUN:   | FileCheck %s -check-prefix=RV32ZBKC
+
+declare i32 @llvm.riscv.clmul.i32(i32 %a, i32 %b)
+
+define i32 @clmul32(i32 %a, i32 %b) nounwind {
+; RV32ZBC-LABEL: clmul32:
+; RV32ZBC:   # %bb.0:
+; RV32ZBC-NEXT:clmul a0, a0, a1
+; RV32ZBC-NEXT:ret
+;
+; RV32ZBKC-LABEL: clmul32:
+; RV32ZBKC:   # %bb.0:
+; RV32ZBKC-NEXT:clmul a0, a0, a1
+; RV32ZBKC-NEXT:ret
+  %tmp = call i32 @llvm.riscv.clmul.i32(i32 %a, i32 %b)
+ ret i32 %tmp
+}
+
+declare i32 @llvm.riscv.clmulh.i32(i32 %a, i32 %b)
+
+define i32 @clmul32h(i32 %a, i32 %b) nounwind {
+; RV32ZBC-LABEL: clmul32h:
+; RV32ZBC:   # %bb.0:
+; RV32ZBC-NEXT:clmulh a0, a0, a1
+; RV32ZBC-NEXT:ret
+;
+; RV32ZBKC-LABEL: clmul32h:
+; RV32ZBKC:   # %bb.0:
+; RV32ZBKC-NEXT:clmulh a0, a0, a1
+; RV32ZBKC-NEXT:ret
+  %tmp = call i32 @llvm.riscv.clmulh.i32(i32 %a, i32 %b)
+ ret i32 %tmp
+}
Index: llvm/lib/Target/RISCV/RISCVInstrInfoZb.td
===
--- llvm/lib/Target/RISCV/RISCVInstrInfoZb.td
+++ llvm/lib/Target/RISCV/RISCVInstrInfoZb.td
@@ -1131,11 +1131,13 @@
   (PACKUW GPR:$rs1, GPR:$rs2)>;
 } // Predicates = [HasStdExtZbp, IsRV64]
 
-let Predicates = [HasStdExtZbc] in {
+let Predicates = [HasStdExtZbcOrZbkc] in {
 def : PatGprGpr;
 def : PatGprGpr;
+} // Predicates = [HasStdExtZbcOrZbkc]
+
+let Predicates = [HasStdExtZbc] in
 def : PatGprGpr;
-} // Predicates = [HasStdExtZbc]
 
 let Predicates = [HasStdExtZbe] in {
 def : PatGprGpr;
Index: llvm/include/llvm/IR/IntrinsicsRISCV.td
===
--- llvm/include/llvm/IR/IntrinsicsRISCV.td
+++ llvm/include/llvm/IR/IntrinsicsRISCV.td
@@ -88,9 +88,11 @@
   // Zbb
   def int_riscv_orc_b : BitManipGPRIntrinsics;
 
-  // Zbc
+  // ZbcorZbkc
   def int_riscv_clmul  : BitManipGPRGPRIntrinsics;
   def int_risc

[PATCH] D118011: [RISCV] Adjust predicates and update intrinsic for clmul and clmulh in Zbkc extension

2022-01-24 Thread WangLian via Phabricator via cfe-commits
Jimerlife added a comment.

In D118011#3265071 , @craig.topper 
wrote:

> Will you also be updating clang's RISCVBuiltins.def?
>
> Can you add tests?

I update BuiltinsRISCV.def and add "__builtin_riscv_clmul_kc" intrinsic for 
zbkc extension, but I am not sure if the name is  appropriate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118011

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


[PATCH] D118011: [RISCV] Adjust predicates and update intrinsic for clmul and clmulh in Zbkc extension

2022-01-24 Thread WangLian via Phabricator via cfe-commits
Jimerlife added a comment.

In D118011#3265069 , @kito-cheng 
wrote:

> Actually this patch is NOT NFC, you fixed an issue that is: `zbkc` isn't 
> include `cmul` and `clmulh`.
>
> So I would suggest you:
>
> 1. Remove NFC from the title.
> 2. Add test case to test `cmul` and `clmulh` is available for `zbkc` after 
> this patch.

Thank you. I adjust patch according your suggestions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118011

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


[clang] ba84578 - [clang][sema] Add missing diagnostic parameter

2022-01-24 Thread Timm Bäder via cfe-commits

Author: Timm Bäder
Date: 2022-01-24T09:59:20+01:00
New Revision: ba845787b3fdd03380b8651d6ce11afeac9d6bba

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

LOG: [clang][sema] Add missing diagnostic parameter

The test case otherwise fails an assertion in Diagnostic::getArgKind().

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

Added: 
clang/test/Modules/cxx20-export-import.cpp

Modified: 
clang/lib/Sema/SemaModule.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaModule.cpp b/clang/lib/Sema/SemaModule.cpp
index 996063f83e946..747734f2d0ff0 100644
--- a/clang/lib/Sema/SemaModule.cpp
+++ b/clang/lib/Sema/SemaModule.cpp
@@ -395,7 +395,7 @@ DeclResult Sema::ActOnModuleImport(SourceLocation StartLoc,
 // [module.interface]p1:
 // An export-declaration shall inhabit a namespace scope and appear in the
 // purview of a module interface unit.
-Diag(ExportLoc, diag::err_export_not_in_module_interface);
+Diag(ExportLoc, diag::err_export_not_in_module_interface) << 0;
   }
 
   return Import;

diff  --git a/clang/test/Modules/cxx20-export-import.cpp 
b/clang/test/Modules/cxx20-export-import.cpp
new file mode 100644
index 0..a2620bd600649
--- /dev/null
+++ b/clang/test/Modules/cxx20-export-import.cpp
@@ -0,0 +1,3 @@
+
+// RUN: %clang_cc1 -std=c++20 -fmodules -fmodules-cache-path=%t 
-fimplicit-module-maps -I%S/Inputs -verify %s
+export import dummy; // expected-error {{export declaration can only be used 
within a module interface unit after the module declaration}}



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


[PATCH] D118011: [RISCV] Adjust predicates and update intrinsic for clmul and clmulh in Zbkc extension

2022-01-24 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added inline comments.



Comment at: llvm/include/llvm/IR/IntrinsicsRISCV.td:91
 
-  // Zbc
+  // ZbcorZbkc
   def int_riscv_clmul  : BitManipGPRGPRIntrinsics;

Add spaces around “or”


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118011

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


[clang] 3ad6de3 - [clang][tests] Fix a c++/libc++ -stdlib value typo

2022-01-24 Thread Timm Bäder via cfe-commits

Author: Timm Bäder
Date: 2022-01-24T09:59:20+01:00
New Revision: 3ad6de31c0cf5064867e6f9bf99e27e0b5c4128d

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

LOG: [clang][tests] Fix a c++/libc++ -stdlib value typo

"c++" is not usually a valid value for -stdlib.

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

Added: 


Modified: 
clang/test/Driver/wasm-toolchain.cpp

Removed: 




diff  --git a/clang/test/Driver/wasm-toolchain.cpp 
b/clang/test/Driver/wasm-toolchain.cpp
index 18ebddc2093b..df11324f2024 100644
--- a/clang/test/Driver/wasm-toolchain.cpp
+++ b/clang/test/Driver/wasm-toolchain.cpp
@@ -14,35 +14,35 @@
 
 // A basic C++ link command-line with unknown OS.
 
-// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-unknown-unknown 
--sysroot=/foo --stdlib=c++ %s 2>&1 \
+// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-unknown-unknown 
--sysroot=/foo --stdlib=libc++ %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=LINK %s
 // LINK: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK: wasm-ld{{.*}}" "-L/foo/lib" "crt1.o" "[[temp]]" "-lc++" "-lc++abi" 
"-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C++ link command-line with optimization with unknown OS.
 
-// RUN: %clangxx -### -O2 -no-canonical-prefixes -target 
wasm32-unknown-unknown --sysroot=/foo %s --stdlib=c++ 2>&1 \
+// RUN: %clangxx -### -O2 -no-canonical-prefixes -target 
wasm32-unknown-unknown --sysroot=/foo %s --stdlib=libc++ 2>&1 \
 // RUN:   | FileCheck -check-prefix=LINK_OPT %s
 // LINK_OPT: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK_OPT: wasm-ld{{.*}}" "-L/foo/lib" "crt1.o" "[[temp]]" "-lc++" 
"-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C++ link command-line with known OS.
 
-// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo --stdlib=c++ %s 2>&1 \
+// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo --stdlib=libc++ %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=LINK_KNOWN %s
 // LINK_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" 
"-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C++ link command-line with optimization with known OS.
 
-// RUN: %clangxx -### -O2 -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo %s --stdlib=c++ 2>&1 \
+// RUN: %clangxx -### -O2 -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo %s --stdlib=libc++ 2>&1 \
 // RUN:   | FileCheck -check-prefix=LINK_OPT_KNOWN %s
 // LINK_OPT_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" 
"-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C++ compile command-line with known OS.
 
-// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo --stdlib=c++ %s 2>&1 \
+// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo --stdlib=libc++ %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=COMPILE %s
 // COMPILE: clang{{.*}}" "-cc1"
 // COMPILE: "-resource-dir" "[[RESOURCE_DIR:[^"]*]]"



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


[PATCH] D116595: [clang][sema] Add missing diagnostic parameter

2022-01-24 Thread Timm Bäder via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGba845787b3fd: [clang][sema] Add missing diagnostic parameter 
(authored by tbaeder).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116595

Files:
  clang/lib/Sema/SemaModule.cpp
  clang/test/Modules/cxx20-export-import.cpp


Index: clang/test/Modules/cxx20-export-import.cpp
===
--- /dev/null
+++ clang/test/Modules/cxx20-export-import.cpp
@@ -0,0 +1,3 @@
+
+// RUN: %clang_cc1 -std=c++20 -fmodules -fmodules-cache-path=%t 
-fimplicit-module-maps -I%S/Inputs -verify %s
+export import dummy; // expected-error {{export declaration can only be used 
within a module interface unit after the module declaration}}
Index: clang/lib/Sema/SemaModule.cpp
===
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -395,7 +395,7 @@
 // [module.interface]p1:
 // An export-declaration shall inhabit a namespace scope and appear in the
 // purview of a module interface unit.
-Diag(ExportLoc, diag::err_export_not_in_module_interface);
+Diag(ExportLoc, diag::err_export_not_in_module_interface) << 0;
   }
 
   return Import;


Index: clang/test/Modules/cxx20-export-import.cpp
===
--- /dev/null
+++ clang/test/Modules/cxx20-export-import.cpp
@@ -0,0 +1,3 @@
+
+// RUN: %clang_cc1 -std=c++20 -fmodules -fmodules-cache-path=%t -fimplicit-module-maps -I%S/Inputs -verify %s
+export import dummy; // expected-error {{export declaration can only be used within a module interface unit after the module declaration}}
Index: clang/lib/Sema/SemaModule.cpp
===
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -395,7 +395,7 @@
 // [module.interface]p1:
 // An export-declaration shall inhabit a namespace scope and appear in the
 // purview of a module interface unit.
-Diag(ExportLoc, diag::err_export_not_in_module_interface);
+Diag(ExportLoc, diag::err_export_not_in_module_interface) << 0;
   }
 
   return Import;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D117862: [clang][tests] Fix a typo in wasm-toolchain.cpp tests

2022-01-24 Thread Timm Bäder via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3ad6de31c0cf: [clang][tests] Fix a c++/libc++ -stdlib value 
typo (authored by tbaeder).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117862

Files:
  clang/test/Driver/wasm-toolchain.cpp


Index: clang/test/Driver/wasm-toolchain.cpp
===
--- clang/test/Driver/wasm-toolchain.cpp
+++ clang/test/Driver/wasm-toolchain.cpp
@@ -14,35 +14,35 @@
 
 // A basic C++ link command-line with unknown OS.
 
-// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-unknown-unknown 
--sysroot=/foo --stdlib=c++ %s 2>&1 \
+// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-unknown-unknown 
--sysroot=/foo --stdlib=libc++ %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=LINK %s
 // LINK: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK: wasm-ld{{.*}}" "-L/foo/lib" "crt1.o" "[[temp]]" "-lc++" "-lc++abi" 
"-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C++ link command-line with optimization with unknown OS.
 
-// RUN: %clangxx -### -O2 -no-canonical-prefixes -target 
wasm32-unknown-unknown --sysroot=/foo %s --stdlib=c++ 2>&1 \
+// RUN: %clangxx -### -O2 -no-canonical-prefixes -target 
wasm32-unknown-unknown --sysroot=/foo %s --stdlib=libc++ 2>&1 \
 // RUN:   | FileCheck -check-prefix=LINK_OPT %s
 // LINK_OPT: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK_OPT: wasm-ld{{.*}}" "-L/foo/lib" "crt1.o" "[[temp]]" "-lc++" 
"-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C++ link command-line with known OS.
 
-// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo --stdlib=c++ %s 2>&1 \
+// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo --stdlib=libc++ %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=LINK_KNOWN %s
 // LINK_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" 
"-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C++ link command-line with optimization with known OS.
 
-// RUN: %clangxx -### -O2 -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo %s --stdlib=c++ 2>&1 \
+// RUN: %clangxx -### -O2 -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo %s --stdlib=libc++ 2>&1 \
 // RUN:   | FileCheck -check-prefix=LINK_OPT_KNOWN %s
 // LINK_OPT_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" 
"-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C++ compile command-line with known OS.
 
-// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo --stdlib=c++ %s 2>&1 \
+// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo --stdlib=libc++ %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=COMPILE %s
 // COMPILE: clang{{.*}}" "-cc1"
 // COMPILE: "-resource-dir" "[[RESOURCE_DIR:[^"]*]]"


Index: clang/test/Driver/wasm-toolchain.cpp
===
--- clang/test/Driver/wasm-toolchain.cpp
+++ clang/test/Driver/wasm-toolchain.cpp
@@ -14,35 +14,35 @@
 
 // A basic C++ link command-line with unknown OS.
 
-// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo --stdlib=c++ %s 2>&1 \
+// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo --stdlib=libc++ %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=LINK %s
 // LINK: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK: wasm-ld{{.*}}" "-L/foo/lib" "crt1.o" "[[temp]]" "-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C++ link command-line with optimization with unknown OS.
 
-// RUN: %clangxx -### -O2 -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s --stdlib=c++ 2>&1 \
+// RUN: %clangxx -### -O2 -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s --stdlib=libc++ 2>&1 \
 // RUN:   | FileCheck -check-prefix=LINK_OPT %s
 // LINK_OPT: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK_OPT: wasm-ld{{.*}}" "-L/foo/lib" "crt1.o" "[[temp]]" "-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C++ link command-line with known OS.
 
-// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo --stdlib=c++ %s 2>&1 \
+// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo --stdlib=libc++ %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=LINK_KNOWN %s
 // LINK_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" "-lc++" "-lc++

[PATCH] D118018: [RFC] [C+++20] [Modules] Mangling lambda in modules

2022-01-24 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu created this revision.
ChuanqiXu added reviewers: urnathan, aaron.ballman, dblaikie, rsmith, 
erichkeane, efriedma, gbenyei.
ChuanqiXu added a project: clang.
ChuanqiXu requested review of this revision.
Herald added a subscriber: cfe-commits.

This tries to solve https://github.com/llvm/llvm-project/issues/52857.

Simply, the key reason is the unnamed class might be externally visible too in 
c++ modules! It is a little bit surprising to me at the first sight. Since how 
can an entity without a name to be visible. However, I recognize that it should 
be so otherwise the optimizer would lose a chance to do IPO (interprocedural 
optimization, like inlining). The corresponding code would be: 
https://github.com/llvm/llvm-project/blob/d29e319263de17516f50cd46edbf1e62c1289dd4/clang/lib/AST/Decl.cpp#L1258-L1259.

So I think we didn't met the problem before we introduces C++ modules since we 
didn't met externally visible unnamed class before.

To my understanding, changing the behavior of mangler is not trivial. It 
matters to ABI compatibility and other many tools like demangler and debugger. 
So I marked this one as RFC although it doesn't contain many codes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118018

Files:
  clang/lib/AST/ItaniumMangle.cpp
  clang/test/CodeGenCXX/cxx20-lambda-modules.cpp


Index: clang/test/CodeGenCXX/cxx20-lambda-modules.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/cxx20-lambda-modules.cpp
@@ -0,0 +1,18 @@
+// Tests that the lambda in modules would be mangled properly.
+
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo "export module test;" >> %t/test.cppm
+// RUN: echo "export namespace test {" >> %t/test.cppm
+// RUN: echo "auto foo = [] {return 43;};" >> %t/test.cppm
+// RUN: echo "auto bar = [] {return 'a';};" >> %t/test.cppm
+// RUN: echo "}" >> %t/test.cppm
+// RUN: %clang_cc1  -std=c++20 %t/test.cppm -emit-module-interface -o 
%t/test.pcm
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fprebuilt-module-path=%t 
-std=c++20 %s -emit-llvm -o - | FileCheck %s
+import test;
+void use() {
+  test::foo();
+  test::bar();
+}
+// CHECK: call noundef i32 @"_ZW4testENK4test3$_0Ut_clEv"
+// CHECK: call noundef signext i8 @"_ZW4testENK4test3$_1Ut_clEv"
Index: clang/lib/AST/ItaniumMangle.cpp
===
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -1537,6 +1537,25 @@
   }
 }
 
+// Handle modules specially to avoid breaking ABI.
+// Otherwise it is possible to mangle two different lambda to the same 
name,
+// See issue 52857 for example.
+if (!TD->isExternallyVisible() || TD->getOwningModuleForLinkage()) {
+  // Get a unique id for the anonymous struct. If it is not a real output
+  // ID doesn't matter so use fake one.
+  unsigned AnonStructId = NullOut ? 0 : Context.getAnonymousStructId(TD);
+
+  // Mangle it as a source name in the form
+  // [n] $_
+  // where n is the length of the string.
+  SmallString<8> Str;
+  Str += "$_";
+  Str += llvm::utostr(AnonStructId);
+
+  Out << Str.size();
+  Out << Str;
+}
+
 if (TD->isExternallyVisible()) {
   unsigned UnnamedMangle = getASTContext().getManglingNumber(TD);
   Out << "Ut";
@@ -1544,22 +1563,8 @@
 Out << UnnamedMangle - 2;
   Out << '_';
   writeAbiTags(TD, AdditionalAbiTags);
-  break;
 }
 
-// Get a unique id for the anonymous struct. If it is not a real output
-// ID doesn't matter so use fake one.
-unsigned AnonStructId = NullOut ? 0 : Context.getAnonymousStructId(TD);
-
-// Mangle it as a source name in the form
-// [n] $_
-// where n is the length of the string.
-SmallString<8> Str;
-Str += "$_";
-Str += llvm::utostr(AnonStructId);
-
-Out << Str.size();
-Out << Str;
 break;
   }
 


Index: clang/test/CodeGenCXX/cxx20-lambda-modules.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/cxx20-lambda-modules.cpp
@@ -0,0 +1,18 @@
+// Tests that the lambda in modules would be mangled properly.
+
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo "export module test;" >> %t/test.cppm
+// RUN: echo "export namespace test {" >> %t/test.cppm
+// RUN: echo "auto foo = [] {return 43;};" >> %t/test.cppm
+// RUN: echo "auto bar = [] {return 'a';};" >> %t/test.cppm
+// RUN: echo "}" >> %t/test.cppm
+// RUN: %clang_cc1  -std=c++20 %t/test.cppm -emit-module-interface -o %t/test.pcm
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fprebuilt-module-path=%t -std=c++20 %s -emit-llvm -o - | FileCheck %s
+import test;
+void use() {
+  test::foo();
+  test::bar();
+}
+// CHECK: call noundef i32 @"_ZW4testENK4test3$_0Ut_clEv"
+// CHECK: call noundef signext i8 @"_ZW4testENK4test3$_1Ut_clEv"
Index: clang/lib/AST/ItaniumMangle.cpp
=

[PATCH] D118011: [RISCV] Adjust predicates and update intrinsic for clmul and clmulh in Zbkc extension

2022-01-24 Thread WangLian via Phabricator via cfe-commits
Jimerlife updated this revision to Diff 402426.
Jimerlife added a comment.

add spaces around "or"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118011

Files:
  clang/include/clang/Basic/BuiltinsRISCV.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/RISCV/rvb-intrinsics/riscv32-zbkc.c
  clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbkc.c
  llvm/include/llvm/IR/IntrinsicsRISCV.td
  llvm/lib/Target/RISCV/RISCVInstrInfoZb.td
  llvm/test/CodeGen/RISCV/rv32zbc-zbkc-intrinsic.ll
  llvm/test/CodeGen/RISCV/rv64zbc-zbkc-intrinsic.ll

Index: llvm/test/CodeGen/RISCV/rv64zbc-zbkc-intrinsic.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/rv64zbc-zbkc-intrinsic.ll
@@ -0,0 +1,37 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv64 -mattr=+zbc -verify-machineinstrs < %s \
+; RUN:   | FileCheck %s -check-prefix=RV64ZBC
+; RUN: llc -mtriple=riscv64 -mattr=+zbkc -verify-machineinstrs < %s \
+; RUN:   | FileCheck %s -check-prefix=RV64ZBKC
+
+declare i64 @llvm.riscv.clmul.i64(i64 %a, i64 %b)
+
+define i64 @clmul64(i64 %a, i64 %b) nounwind {
+; RV64ZBC-LABEL: clmul64:
+; RV64ZBC:   # %bb.0:
+; RV64ZBC-NEXT:clmul a0, a0, a1
+; RV64ZBC-NEXT:ret
+;
+; RV64ZBKC-LABEL: clmul64:
+; RV64ZBKC:   # %bb.0:
+; RV64ZBKC-NEXT:clmul a0, a0, a1
+; RV64ZBKC-NEXT:ret
+  %tmp = call i64 @llvm.riscv.clmul.i64(i64 %a, i64 %b)
+ ret i64 %tmp
+}
+
+declare i64 @llvm.riscv.clmulh.i64(i64 %a, i64 %b)
+
+define i64 @clmul64h(i64 %a, i64 %b) nounwind {
+; RV64ZBC-LABEL: clmul64h:
+; RV64ZBC:   # %bb.0:
+; RV64ZBC-NEXT:clmulh a0, a0, a1
+; RV64ZBC-NEXT:ret
+;
+; RV64ZBKC-LABEL: clmul64h:
+; RV64ZBKC:   # %bb.0:
+; RV64ZBKC-NEXT:clmulh a0, a0, a1
+; RV64ZBKC-NEXT:ret
+  %tmp = call i64 @llvm.riscv.clmulh.i64(i64 %a, i64 %b)
+ ret i64 %tmp
+}
Index: llvm/test/CodeGen/RISCV/rv32zbc-zbkc-intrinsic.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/rv32zbc-zbkc-intrinsic.ll
@@ -0,0 +1,37 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv32 -mattr=+zbc -verify-machineinstrs < %s \
+; RUN:   | FileCheck %s -check-prefix=RV32ZBC
+; RUN: llc -mtriple=riscv32 -mattr=+zbkc -verify-machineinstrs < %s \
+; RUN:   | FileCheck %s -check-prefix=RV32ZBKC
+
+declare i32 @llvm.riscv.clmul.i32(i32 %a, i32 %b)
+
+define i32 @clmul32(i32 %a, i32 %b) nounwind {
+; RV32ZBC-LABEL: clmul32:
+; RV32ZBC:   # %bb.0:
+; RV32ZBC-NEXT:clmul a0, a0, a1
+; RV32ZBC-NEXT:ret
+;
+; RV32ZBKC-LABEL: clmul32:
+; RV32ZBKC:   # %bb.0:
+; RV32ZBKC-NEXT:clmul a0, a0, a1
+; RV32ZBKC-NEXT:ret
+  %tmp = call i32 @llvm.riscv.clmul.i32(i32 %a, i32 %b)
+ ret i32 %tmp
+}
+
+declare i32 @llvm.riscv.clmulh.i32(i32 %a, i32 %b)
+
+define i32 @clmul32h(i32 %a, i32 %b) nounwind {
+; RV32ZBC-LABEL: clmul32h:
+; RV32ZBC:   # %bb.0:
+; RV32ZBC-NEXT:clmulh a0, a0, a1
+; RV32ZBC-NEXT:ret
+;
+; RV32ZBKC-LABEL: clmul32h:
+; RV32ZBKC:   # %bb.0:
+; RV32ZBKC-NEXT:clmulh a0, a0, a1
+; RV32ZBKC-NEXT:ret
+  %tmp = call i32 @llvm.riscv.clmulh.i32(i32 %a, i32 %b)
+ ret i32 %tmp
+}
Index: llvm/lib/Target/RISCV/RISCVInstrInfoZb.td
===
--- llvm/lib/Target/RISCV/RISCVInstrInfoZb.td
+++ llvm/lib/Target/RISCV/RISCVInstrInfoZb.td
@@ -1131,11 +1131,13 @@
   (PACKUW GPR:$rs1, GPR:$rs2)>;
 } // Predicates = [HasStdExtZbp, IsRV64]
 
-let Predicates = [HasStdExtZbc] in {
+let Predicates = [HasStdExtZbcOrZbkc] in {
 def : PatGprGpr;
 def : PatGprGpr;
+} // Predicates = [HasStdExtZbcOrZbkc]
+
+let Predicates = [HasStdExtZbc] in
 def : PatGprGpr;
-} // Predicates = [HasStdExtZbc]
 
 let Predicates = [HasStdExtZbe] in {
 def : PatGprGpr;
Index: llvm/include/llvm/IR/IntrinsicsRISCV.td
===
--- llvm/include/llvm/IR/IntrinsicsRISCV.td
+++ llvm/include/llvm/IR/IntrinsicsRISCV.td
@@ -88,9 +88,11 @@
   // Zbb
   def int_riscv_orc_b : BitManipGPRIntrinsics;
 
-  // Zbc
+  // Zbc or Zbkc
   def int_riscv_clmul  : BitManipGPRGPRIntrinsics;
   def int_riscv_clmulh : BitManipGPRGPRIntrinsics;
+
+  // Zbc
   def int_riscv_clmulr : BitManipGPRGPRIntrinsics;
 
   // Zbe
Index: clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbkc.c
===
--- /dev/null
+++ clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbkc.c
@@ -0,0 +1,33 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -triple riscv64 -target-feature +zbkc -emit-llvm %s -o - \
+// RUN: | FileCheck %s  -check-prefix=RV64ZBKC
+
+// RV64ZBKC-LABEL: @clmul(
+// RV64ZBKC-NEXT:  entry:
+// RV64ZBKC-NEXT:[[A_ADDR:%.*]] = al

[PATCH] D118021: [Driver] Use libatomic for 32-bit SPARC atomics support

2022-01-24 Thread Rainer Orth via Phabricator via cfe-commits
ro created this revision.
ro added reviewers: jyknight, venkatra, efriedma, jrtc27.
ro added a project: clang.
Herald added a subscriber: fedor.sergeev.
ro requested review of this revision.
Herald added a subscriber: cfe-commits.

Even after D86621 , `clang -m32` on 
Solaris/sparcv9 doesn't inline atomics with 8-byte
operands, unlike `gcc`.  This leads to many link failures in the testsuite 
(undefined references
to `__atomic_load_8` and `__sync_val_compare_and_swap_8`.  Until a proper 
codegen fix can be implemented, this patch works around the first of those by 
linking with `-latomic`.

Tested on `sparcv9-sun-solaris2.11`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118021

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/lib/Driver/ToolChains/Solaris.cpp


Index: clang/lib/Driver/ToolChains/Solaris.cpp
===
--- clang/lib/Driver/ToolChains/Solaris.cpp
+++ clang/lib/Driver/ToolChains/Solaris.cpp
@@ -132,6 +132,13 @@
   CmdArgs.push_back("-lssp_nonshared");
   CmdArgs.push_back("-lssp");
 }
+// LLVM lacks atomics support on 32-bit SPARC, so forcibly link with
+// libatomic as a workaround.
+if (getToolChain().getTriple().getArch() == llvm::Triple::sparc) {
+  CmdArgs.push_back(getAsNeededOption(getToolChain(), true));
+  CmdArgs.push_back("-latomic");
+  CmdArgs.push_back(getAsNeededOption(getToolChain(), false));
+}
 CmdArgs.push_back("-lgcc_s");
 CmdArgs.push_back("-lc");
 if (!Args.hasArg(options::OPT_shared)) {
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -114,6 +114,8 @@
   bool ForceStaticHostRuntime = false,
   bool IsOffloadingHost = false, bool GompNeedsRT = false);
 
+const char *getAsNeededOption(const ToolChain &TC, bool as_needed);
+
 llvm::opt::Arg *getLastProfileUseArg(const llvm::opt::ArgList &Args);
 llvm::opt::Arg *getLastProfileSampleUseArg(const llvm::opt::ArgList &Args);
 
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -737,7 +737,7 @@
   return false;
 }
 
-static const char *getAsNeededOption(const ToolChain &TC, bool as_needed) {
+const char *tools::getAsNeededOption(const ToolChain &TC, bool as_needed) {
   assert(!TC.getTriple().isOSAIX() &&
  "AIX linker does not support any form of --as-needed option yet.");
 


Index: clang/lib/Driver/ToolChains/Solaris.cpp
===
--- clang/lib/Driver/ToolChains/Solaris.cpp
+++ clang/lib/Driver/ToolChains/Solaris.cpp
@@ -132,6 +132,13 @@
   CmdArgs.push_back("-lssp_nonshared");
   CmdArgs.push_back("-lssp");
 }
+// LLVM lacks atomics support on 32-bit SPARC, so forcibly link with
+// libatomic as a workaround.
+if (getToolChain().getTriple().getArch() == llvm::Triple::sparc) {
+  CmdArgs.push_back(getAsNeededOption(getToolChain(), true));
+  CmdArgs.push_back("-latomic");
+  CmdArgs.push_back(getAsNeededOption(getToolChain(), false));
+}
 CmdArgs.push_back("-lgcc_s");
 CmdArgs.push_back("-lc");
 if (!Args.hasArg(options::OPT_shared)) {
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -114,6 +114,8 @@
   bool ForceStaticHostRuntime = false,
   bool IsOffloadingHost = false, bool GompNeedsRT = false);
 
+const char *getAsNeededOption(const ToolChain &TC, bool as_needed);
+
 llvm::opt::Arg *getLastProfileUseArg(const llvm::opt::ArgList &Args);
 llvm::opt::Arg *getLastProfileSampleUseArg(const llvm::opt::ArgList &Args);
 
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -737,7 +737,7 @@
   return false;
 }
 
-static const char *getAsNeededOption(const ToolChain &TC, bool as_needed) {
+const char *tools::getAsNeededOption(const ToolChain &TC, bool as_needed) {
   assert(!TC.getTriple().isOSAIX() &&
  "AIX linker does not support any form of --as-needed option yet.");
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D118021: [Driver] Use libatomic for 32-bit SPARC atomics support

2022-01-24 Thread John Paul Adrian Glaubitz via Phabricator via cfe-commits
glaubitz added a comment.

Ah, I actually ran into this very problem! Thanks for fixing this.

Will test this today on Linux and report back!

Could you maybe have a look at my updated revision here? 
https://reviews.llvm.org/D98575

I think this particular change should be save on Solaris as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118021

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


[PATCH] D117306: [clang-tidy] Add new check 'shared-ptr-array-mismatch'.

2022-01-24 Thread Balázs Kéri via Phabricator via cfe-commits
balazske updated this revision to Diff 402439.
balazske added a comment.

Fixed small issues.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117306

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/SharedPtrArrayMismatchCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SharedPtrArrayMismatchCheck.h
  clang-tools-extra/clang-tidy/bugprone/SmartPtrArrayMismatchCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SmartPtrArrayMismatchCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-shared-ptr-array-mismatch.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-shared-ptr-array-mismatch.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-shared-ptr-array-mismatch.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-shared-ptr-array-mismatch.cpp
@@ -0,0 +1,93 @@
+// RUN: %check_clang_tidy %s bugprone-shared-ptr-array-mismatch %t
+
+namespace std {
+
+template 
+struct shared_ptr {
+  template 
+  explicit shared_ptr(Y *) {}
+  template 
+  shared_ptr(Y *, Deleter) {}
+};
+
+} // namespace std
+
+struct A {};
+
+void f1() {
+  std::shared_ptr P1{new int};
+  std::shared_ptr P2{new int[10]};
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch]
+  // CHECK-FIXES: std::shared_ptr P2{new int[10]};
+  std::shared_ptr<  int  > P3{new int[10]};
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch]
+  // CHECK-FIXES: std::shared_ptr<  int[]  > P3{new int[10]};
+  std::shared_ptr P4(new int[10]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch]
+  // CHECK-FIXES: std::shared_ptr P4(new int[10]);
+  new std::shared_ptr(new int[10]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch]
+  std::shared_ptr P5(new int[10]);
+  std::shared_ptr P6(new int[10], [](const int *Ptr) {});
+}
+
+void f2() {
+  std::shared_ptr P1(new A);
+  std::shared_ptr P2(new A[10]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch]
+  // CHECK-FIXES: std::shared_ptr P2(new A[10]);
+  std::shared_ptr P3(new A[10]);
+}
+
+void f3() {
+  std::shared_ptr P1{new int}, P2{new int[10]}, P3{new int[10]};
+  // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch]
+  // CHECK-MESSAGES: :[[@LINE-2]]:57: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch]
+}
+
+struct S {
+  std::shared_ptr P1;
+  std::shared_ptr P2{new int[10]};
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch]
+  std::shared_ptr P3{new int}, P4{new int[10]};
+  // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch]
+  S() : P1{new int[10]} {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch]
+};
+
+void f_parm(std::shared_ptr);
+
+void f4() {
+  f_parm(std::shared_ptr{new int[10]});
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch]
+}
+
+std::shared_ptr f_ret() {
+  return std::shared_ptr(new int[10]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch]
+}
+
+template 
+void f_tmpl() {
+  std::shared_ptr P1{new T[10]};
+}
+
+void f5() {
+  f_tmpl();
+}
+
+#define CHAR_PTR_TYPE std::shared_ptr
+#define CHAR_PTR_VAR(X) \
+  X { new char[10] }
+#define CHAR_PTR_INIT(X, Y) \
+  std::shared_ptr X { Y }
+
+void f6() {
+  CHAR_PTR_TYPE P1{new char[10]};
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch]
+  std::shared_ptr CHAR_PTR_VAR(P2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch]
+  // CHECK-FIXES: std::shared_ptr CHAR_PTR_VAR(P2);
+  CHAR_PTR_INIT(P3, new char[10]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch]
+}
Index: clang-tools-extra

[PATCH] D118021: [Driver] Use libatomic for 32-bit SPARC atomics support

2022-01-24 Thread John Paul Adrian Glaubitz via Phabricator via cfe-commits
glaubitz added inline comments.



Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:138-139
+if (getToolChain().getTriple().getArch() == llvm::Triple::sparc) {
+  CmdArgs.push_back(getAsNeededOption(getToolChain(), true));
+  CmdArgs.push_back("-latomic");
+  CmdArgs.push_back(getAsNeededOption(getToolChain(), false));

Note, this will only work when `__atomic_compare_exchange()` is being used as 
``__sync_val_compare_and_swap_8` is not implemented by `libatomic` in gcc, see: 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63368 (Unless this has changed 
recently).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118021

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


[PATCH] D118023: Corrected fragment size for tf32 LD B matrix.

2022-01-24 Thread Jack Kirk via Phabricator via cfe-commits
JackAKirk created this revision.
JackAKirk added a reviewer: tra.
JackAKirk requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Signed-off-by: JackAKirk 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118023

Files:
  clang/lib/CodeGen/CGBuiltin.cpp


Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -17293,7 +17293,7 @@
   case NVPTX::BI__mma_tf32_m16n16k8_ld_a:
 return MMA_LDST(4, m16n16k8_load_a_tf32);
   case NVPTX::BI__mma_tf32_m16n16k8_ld_b:
-return MMA_LDST(2, m16n16k8_load_b_tf32);
+return MMA_LDST(4, m16n16k8_load_b_tf32);
   case NVPTX::BI__mma_tf32_m16n16k8_ld_c:
 return MMA_LDST(8, m16n16k8_load_c_f32);
 


Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -17293,7 +17293,7 @@
   case NVPTX::BI__mma_tf32_m16n16k8_ld_a:
 return MMA_LDST(4, m16n16k8_load_a_tf32);
   case NVPTX::BI__mma_tf32_m16n16k8_ld_b:
-return MMA_LDST(2, m16n16k8_load_b_tf32);
+return MMA_LDST(4, m16n16k8_load_b_tf32);
   case NVPTX::BI__mma_tf32_m16n16k8_ld_c:
 return MMA_LDST(8, m16n16k8_load_c_f32);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D117181: [PowerPC] Use IEEE long double in proper toolchain

2022-01-24 Thread Qiu Chaofan via Phabricator via cfe-commits
qiucf added a comment.

In D117181#3262288 , @jsji wrote:

>> Won't that end up producing a warning on ALL code built on any Linux distro 
>> with a GCC toolchain older than 12.1? That would be terrible.
>
> Good point. Yes, so should be something like:
>
>   IsDistroWithNewToolchain = ( Distro.IsRedhat() && Distro >= Distro::RHEL9  
> ||  (Distro.IsUbuntu() && Distro >= Distro::xxx)
>   bool IEEELongDouble = T.isOSLinux() && IsDistroWithNewToolchain;

Thanks for reminding about checking Linux distro version! Besides, would it be 
acceptable that we add a variable to cmake to determine default long double 
semantics (like current GCC)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117181

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


[PATCH] D118023: Corrected fragment size for tf32 LD B matrix.

2022-01-24 Thread Jack Kirk via Phabricator via cfe-commits
JackAKirk added a comment.

Note that the test, llvm/test/CodeGen/NVPTX/wmma.py line 210, had the correct 
value already but didn't seem to cover the mistake.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118023

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


[PATCH] D118021: [Driver] Use libatomic for 32-bit SPARC atomics support

2022-01-24 Thread Joerg Sonnenberger via Phabricator via cfe-commits
joerg added inline comments.



Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:135
 }
+// LLVM lacks atomics support on 32-bit SPARC, so forcibly link with
+// libatomic as a workaround.

This comment is misleading. It's not so much that LLVM doesn't support them, 
but that SPARCv8 doesn't have the necessary hardware support. The v8+ support 
is incomplete, which is a related problem though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118021

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


[PATCH] D118021: [Driver] Use libatomic for 32-bit SPARC atomics support

2022-01-24 Thread John Paul Adrian Glaubitz via Phabricator via cfe-commits
glaubitz added inline comments.



Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:135
 }
+// LLVM lacks atomics support on 32-bit SPARC, so forcibly link with
+// libatomic as a workaround.

joerg wrote:
> This comment is misleading. It's not so much that LLVM doesn't support them, 
> but that SPARCv8 doesn't have the necessary hardware support. The v8+ support 
> is incomplete, which is a related problem though.
As far as I know, 64-bit atomics are supported if you enable V8+ in GCC - 
without linking against `libatomic`:

```
glaubitz@gcc202:~$ cat atomic.c
#include 

int main()
{
  int64_t x = 0, y = 1;
  y = __sync_val_compare_and_swap(&x, x, y);
  return 0;
}
glaubitz@gcc202:~$ gcc -m32 -mv8plus atomic.c -o atomic
glaubitz@gcc202:~$
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118021

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


[PATCH] D118021: [Driver] Use libatomic for 32-bit SPARC atomics support

2022-01-24 Thread Rainer Orth via Phabricator via cfe-commits
ro added inline comments.



Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:135
 }
+// LLVM lacks atomics support on 32-bit SPARC, so forcibly link with
+// libatomic as a workaround.

glaubitz wrote:
> joerg wrote:
> > This comment is misleading. It's not so much that LLVM doesn't support 
> > them, but that SPARCv8 doesn't have the necessary hardware support. The v8+ 
> > support is incomplete, which is a related problem though.
> As far as I know, 64-bit atomics are supported if you enable V8+ in GCC - 
> without linking against `libatomic`:
> 
> ```
> glaubitz@gcc202:~$ cat atomic.c
> #include 
> 
> int main()
> {
>   int64_t x = 0, y = 1;
>   y = __sync_val_compare_and_swap(&x, x, y);
>   return 0;
> }
> glaubitz@gcc202:~$ gcc -m32 -mv8plus atomic.c -o atomic
> glaubitz@gcc202:~$
> ```
I know, that's why I referred to my patch to default `clang` on
Solaris/sparc to V8+.  I'll update the comment.

I'd tried to actually fix the underlying issue (`clang` not emitting
`casx` with `-m32 -mcpu=v9`), but ran into internal errors and
areas of LLVM I know nothing about.  I might post a WIP patch
for reference since there are several issues there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118021

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


[PATCH] D118021: [Driver] Use libatomic for 32-bit SPARC atomics support

2022-01-24 Thread John Paul Adrian Glaubitz via Phabricator via cfe-commits
glaubitz added inline comments.



Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:135
 }
+// LLVM lacks atomics support on 32-bit SPARC, so forcibly link with
+// libatomic as a workaround.

ro wrote:
> glaubitz wrote:
> > joerg wrote:
> > > This comment is misleading. It's not so much that LLVM doesn't support 
> > > them, but that SPARCv8 doesn't have the necessary hardware support. The 
> > > v8+ support is incomplete, which is a related problem though.
> > As far as I know, 64-bit atomics are supported if you enable V8+ in GCC - 
> > without linking against `libatomic`:
> > 
> > ```
> > glaubitz@gcc202:~$ cat atomic.c
> > #include 
> > 
> > int main()
> > {
> >   int64_t x = 0, y = 1;
> >   y = __sync_val_compare_and_swap(&x, x, y);
> >   return 0;
> > }
> > glaubitz@gcc202:~$ gcc -m32 -mv8plus atomic.c -o atomic
> > glaubitz@gcc202:~$
> > ```
> I know, that's why I referred to my patch to default `clang` on
> Solaris/sparc to V8+.  I'll update the comment.
> 
> I'd tried to actually fix the underlying issue (`clang` not emitting
> `casx` with `-m32 -mcpu=v9`), but ran into internal errors and
> areas of LLVM I know nothing about.  I might post a WIP patch
> for reference since there are several issues there.
I think @jrtc27 might be able to give advise here having the knowledge on the 
Tablegen stuff (if my mind serves me right).

The disassembly shows in any case that `casx` is being emitted as you say:

```
69c:   c7 f0 50 02 casx  [ %g1 ], %g2, %g3
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118021

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


[PATCH] D118021: [Driver] Use libatomic for 32-bit SPARC atomics support

2022-01-24 Thread Rainer Orth via Phabricator via cfe-commits
ro added inline comments.



Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:135
 }
+// LLVM lacks atomics support on 32-bit SPARC, so forcibly link with
+// libatomic as a workaround.

glaubitz wrote:
> ro wrote:
> > glaubitz wrote:
> > > joerg wrote:
> > > > This comment is misleading. It's not so much that LLVM doesn't support 
> > > > them, but that SPARCv8 doesn't have the necessary hardware support. The 
> > > > v8+ support is incomplete, which is a related problem though.
> > > As far as I know, 64-bit atomics are supported if you enable V8+ in GCC - 
> > > without linking against `libatomic`:
> > > 
> > > ```
> > > glaubitz@gcc202:~$ cat atomic.c
> > > #include 
> > > 
> > > int main()
> > > {
> > >   int64_t x = 0, y = 1;
> > >   y = __sync_val_compare_and_swap(&x, x, y);
> > >   return 0;
> > > }
> > > glaubitz@gcc202:~$ gcc -m32 -mv8plus atomic.c -o atomic
> > > glaubitz@gcc202:~$
> > > ```
> > I know, that's why I referred to my patch to default `clang` on
> > Solaris/sparc to V8+.  I'll update the comment.
> > 
> > I'd tried to actually fix the underlying issue (`clang` not emitting
> > `casx` with `-m32 -mcpu=v9`), but ran into internal errors and
> > areas of LLVM I know nothing about.  I might post a WIP patch
> > for reference since there are several issues there.
> I think @jrtc27 might be able to give advise here having the knowledge on the 
> Tablegen stuff (if my mind serves me right).
> 
> The disassembly shows in any case that `casx` is being emitted as you say:
> 
> ```
> 69c:   c7 f0 50 02 casx  [ %g1 ], %g2, %g3
> ```
Of course it does, thus my reference to `unlike gcc` in the
summary.  What Joerg meant, I believe, is that V8+ support
**in LLVM** is incomplete.



Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:135
 }
+// LLVM lacks atomics support on 32-bit SPARC, so forcibly link with
+// libatomic as a workaround.

ro wrote:
> glaubitz wrote:
> > ro wrote:
> > > glaubitz wrote:
> > > > joerg wrote:
> > > > > This comment is misleading. It's not so much that LLVM doesn't 
> > > > > support them, but that SPARCv8 doesn't have the necessary hardware 
> > > > > support. The v8+ support is incomplete, which is a related problem 
> > > > > though.
> > > > As far as I know, 64-bit atomics are supported if you enable V8+ in GCC 
> > > > - without linking against `libatomic`:
> > > > 
> > > > ```
> > > > glaubitz@gcc202:~$ cat atomic.c
> > > > #include 
> > > > 
> > > > int main()
> > > > {
> > > >   int64_t x = 0, y = 1;
> > > >   y = __sync_val_compare_and_swap(&x, x, y);
> > > >   return 0;
> > > > }
> > > > glaubitz@gcc202:~$ gcc -m32 -mv8plus atomic.c -o atomic
> > > > glaubitz@gcc202:~$
> > > > ```
> > > I know, that's why I referred to my patch to default `clang` on
> > > Solaris/sparc to V8+.  I'll update the comment.
> > > 
> > > I'd tried to actually fix the underlying issue (`clang` not emitting
> > > `casx` with `-m32 -mcpu=v9`), but ran into internal errors and
> > > areas of LLVM I know nothing about.  I might post a WIP patch
> > > for reference since there are several issues there.
> > I think @jrtc27 might be able to give advise here having the knowledge on 
> > the Tablegen stuff (if my mind serves me right).
> > 
> > The disassembly shows in any case that `casx` is being emitted as you say:
> > 
> > ```
> > 69c:   c7 f0 50 02 casx  [ %g1 ], %g2, %g3
> > ```
> Of course it does, thus my reference to `unlike gcc` in the
> summary.  What Joerg meant, I believe, is that V8+ support
> **in LLVM** is incomplete.
Right, `gcc` does all of this correctly.  One LLVM issue, e.g., is
that it handles `casx` as 64-bit only (cf. `SparcInstr64Bit.td`)
while it should be guarded by `HasV9` instead.



Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:138-139
+if (getToolChain().getTriple().getArch() == llvm::Triple::sparc) {
+  CmdArgs.push_back(getAsNeededOption(getToolChain(), true));
+  CmdArgs.push_back("-latomic");
+  CmdArgs.push_back(getAsNeededOption(getToolChain(), false));

glaubitz wrote:
> Note, this will only work when `__atomic_compare_exchange()` is being used as 
> ``__sync_val_compare_and_swap_8` is not implemented by `libatomic` in gcc, 
> see: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63368 (Unless this has 
> changed recently).
True, that the summary said `this patch works around the first of those`.  The 
other part is now D118024.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118021

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


[PATCH] D118021: [Driver] Use libatomic for 32-bit SPARC atomics support

2022-01-24 Thread John Paul Adrian Glaubitz via Phabricator via cfe-commits
glaubitz added inline comments.



Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:138-139
+if (getToolChain().getTriple().getArch() == llvm::Triple::sparc) {
+  CmdArgs.push_back(getAsNeededOption(getToolChain(), true));
+  CmdArgs.push_back("-latomic");
+  CmdArgs.push_back(getAsNeededOption(getToolChain(), false));

ro wrote:
> glaubitz wrote:
> > Note, this will only work when `__atomic_compare_exchange()` is being used 
> > as ``__sync_val_compare_and_swap_8` is not implemented by `libatomic` in 
> > gcc, see: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63368 (Unless this 
> > has changed recently).
> True, that the summary said `this patch works around the first of those`.  
> The other part is now D118024.
Ah, sorry. I missed that he was specifically talking about SPARCv8.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118021

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


[PATCH] D107769: [OpenCL] Make generic addrspace optional for -fdeclare-opencl-builtins

2022-01-24 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added a comment.
Herald added a subscriber: Naghasan.

In D107769#3064171 , @svenvh wrote:

> In D107769#2967565 , @Anastasia 
> wrote:
>
>> In D107769#2960441 , @svenvh wrote:
>>
>>> I have done an alternative spin of this patch: instead of introducing 
>>> `RequireDisabledExtension`, simply always make the `private`, `global`, and 
>>> `local` overloads available.  This makes tablegen diverge from `opencl-c.h` 
>>> though.  Perhaps we should also always add make the `private`, `global`, 
>>> and `local` overloads available in `opencl-c.h`?
>>
>> Yes, we could do this indeed as a clang extension. I don't think this will 
>> impact the user code. However, this means vendors will have to add 
>> definitions for extra overloads in OpenCL 2.0 mode?
>
> I wonder if making the `private`, `global`, and `local` overloads always 
> available would even be considered an extension.  Unless I overlooked 
> something, I cannot find anything in the spec saying that the `private`, 
> `global`, and `local` overloads should **not** be available when a `generic` 
> overload is available (even though this is what Clang has always done)?
>
> Making all overloads always available in e.g. CL2.0 mode will indeed affect 
> the generated calls, which means the definitions should be available when 
> consuming the generated IR.

The way I understand the spec for OpenCL C 2.0 is that whenever the address 
space of the pointer is not listed it means generic has to be used, here is one 
example:
https://www.khronos.org/registry/OpenCL/specs/2.2/html/OpenCL_C.html#vector-data-load-and-store-functions

  gentypen vloadn(size_t offset, const gentype *p)
  gentypen vloadn(size_t offset, const __constant gentype *p)

that has no address space (i.e. `generic`) and `constant` explicitly. So I 
think if address spaces are not listed explicitly they are not supposed to be 
available.

One implication of adding all address space overloads is that it makes library 
size larger, but my feeling is that we don't have that many functions with 
pointers to significantly impace the library size?


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

https://reviews.llvm.org/D107769

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


[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

2022-01-24 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment.

In D112646#3264010 , @avogelsgesang 
wrote:

> @xazax.hun ping re merging this to `main`. Would be amazing to get this in 
> still in time for clang 14.

Sorry for the delay. I'm on vacation at the moment and will not be able to 
commit it in the next couple of weeks. But I asked @whisperity to take a final 
look and commit it if there is nothing else that needs to be changed and he 
kindly agreed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112646

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


[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

2022-01-24 Thread Whisperity via Phabricator via cfe-commits
whisperity added a comment.

In D112646#3251025 , @avogelsgesang 
wrote:

> @xazax.hun Can you please merge this to `main` on my behalf? (I don't have 
> commit rights)

Hey! Sorry for my absence, I'm terribly busy with doing stuffs 😦 I'll check the 
patch now, generally looks good. Please specify what name and e-mail address 
you'd like to have the Git commit author attributed with!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112646

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


[clang] e5147f8 - [X86] Remove __builtin_ia32_pabs intrinsics and use generic __builtin_elementwise_abs

2022-01-24 Thread Simon Pilgrim via cfe-commits

Author: Simon Pilgrim
Date: 2022-01-24T11:25:21Z
New Revision: e5147f82e1cba6791252d8f44c1a014cd9ea7927

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

LOG: [X86] Remove __builtin_ia32_pabs intrinsics and use generic 
__builtin_elementwise_abs

D111986 added the generic `__builtin_elementwise_abs()` intrinsic with the same 
integer absolute behaviour as the SSE/AVX instructions (abs(INT_MIN) == INT_MIN)

This patch removes the `__builtin_ia32_pabs*` intrinsics and just uses 
`__builtin_elementwise_abs` - the existing tests see no changes:
```
__m256i test_mm256_abs_epi8(__m256i a) {
  // CHECK-LABEL: test_mm256_abs_epi8
  // CHECK: [[ABS:%.*]] = call <32 x i8> @llvm.abs.v32i8(<32 x i8> %{{.*}}, i1 
false)
  return _mm256_abs_epi8(a);
}
```
This requires us to add a `__v64qs` explicitly signed char vector type (we 
already have `__v16qs` and `__v32qs`).

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

Added: 


Modified: 
clang/include/clang/Basic/BuiltinsX86.def
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/Headers/avx2intrin.h
clang/lib/Headers/avx512bwintrin.h
clang/lib/Headers/avx512fintrin.h
clang/lib/Headers/avx512vlintrin.h
clang/lib/Headers/tmmintrin.h
clang/test/CodeGen/builtins-x86.c

Removed: 




diff  --git a/clang/include/clang/Basic/BuiltinsX86.def 
b/clang/include/clang/Basic/BuiltinsX86.def
index bc6208be45606..9b7c763b0c6c7 100644
--- a/clang/include/clang/Basic/BuiltinsX86.def
+++ b/clang/include/clang/Basic/BuiltinsX86.def
@@ -296,9 +296,6 @@ TARGET_BUILTIN(__builtin_ia32_pshufb128, "V16cV16cV16c", 
"ncV:128:", "ssse3")
 TARGET_BUILTIN(__builtin_ia32_psignb128, "V16cV16cV16c", "ncV:128:", "ssse3")
 TARGET_BUILTIN(__builtin_ia32_psignw128, "V8sV8sV8s", "ncV:128:", "ssse3")
 TARGET_BUILTIN(__builtin_ia32_psignd128, "V4iV4iV4i", "ncV:128:", "ssse3")
-TARGET_BUILTIN(__builtin_ia32_pabsb128, "V16cV16c", "ncV:128:", "ssse3")
-TARGET_BUILTIN(__builtin_ia32_pabsw128, "V8sV8s", "ncV:128:", "ssse3")
-TARGET_BUILTIN(__builtin_ia32_pabsd128, "V4iV4i", "ncV:128:", "ssse3")
 
 TARGET_BUILTIN(__builtin_ia32_ldmxcsr, "vUi", "n", "sse")
 TARGET_HEADER_BUILTIN(_mm_setcsr, "vUi", "nh","xmmintrin.h", ALL_LANGUAGES, 
"sse")
@@ -558,9 +555,6 @@ TARGET_BUILTIN(__builtin_ia32_vec_set_v8si, "V8iV8iiIi", 
"ncV:256:", "avx")
 
 // AVX2
 TARGET_BUILTIN(__builtin_ia32_mpsadbw256, "V32cV32cV32cIc", "ncV:256:", "avx2")
-TARGET_BUILTIN(__builtin_ia32_pabsb256, "V32cV32c", "ncV:256:", "avx2")
-TARGET_BUILTIN(__builtin_ia32_pabsw256, "V16sV16s", "ncV:256:", "avx2")
-TARGET_BUILTIN(__builtin_ia32_pabsd256, "V8iV8i", "ncV:256:", "avx2")
 TARGET_BUILTIN(__builtin_ia32_packsswb256, "V32cV16sV16s", "ncV:256:", "avx2")
 TARGET_BUILTIN(__builtin_ia32_packssdw256, "V16sV8iV8i", "ncV:256:", "avx2")
 TARGET_BUILTIN(__builtin_ia32_packuswb256, "V32cV16sV16s", "ncV:256:", "avx2")
@@ -927,8 +921,6 @@ TARGET_BUILTIN(__builtin_ia32_cvtudq2ps512_mask, 
"V16fV16iV16fUsIi", "ncV:512:",
 TARGET_BUILTIN(__builtin_ia32_cvtpd2ps512_mask, "V8fV8dV8fUcIi", "ncV:512:", 
"avx512f")
 TARGET_BUILTIN(__builtin_ia32_vcvtps2ph512_mask, "V16sV16fIiV16sUs", 
"ncV:512:", "avx512f")
 TARGET_BUILTIN(__builtin_ia32_vcvtph2ps512_mask, "V16fV16sV16fUsIi", 
"ncV:512:", "avx512f")
-TARGET_BUILTIN(__builtin_ia32_pabsd512, "V16iV16i", "ncV:512:", "avx512f")
-TARGET_BUILTIN(__builtin_ia32_pabsq512, "V8OiV8Oi", "ncV:512:", "avx512f")
 TARGET_BUILTIN(__builtin_ia32_pmaxsd512, "V16iV16iV16i", "ncV:512:", "avx512f")
 TARGET_BUILTIN(__builtin_ia32_pmaxsq512, "V8OiV8OiV8Oi", "ncV:512:", "avx512f")
 TARGET_BUILTIN(__builtin_ia32_pmaxud512, "V16iV16iV16i", "ncV:512:", "avx512f")
@@ -1045,8 +1037,6 @@ TARGET_BUILTIN(__builtin_ia32_ucmpd512_mask, 
"UsV16iV16iIiUs", "ncV:512:", "avx5
 TARGET_BUILTIN(__builtin_ia32_ucmpq512_mask, "UcV8OiV8OiIiUc", "ncV:512:", 
"avx512f")
 TARGET_BUILTIN(__builtin_ia32_ucmpw512_mask, "UiV32sV32sIiUi", "ncV:512:", 
"avx512bw")
 
-TARGET_BUILTIN(__builtin_ia32_pabsb512, "V64cV64c", "ncV:512:", "avx512bw")
-TARGET_BUILTIN(__builtin_ia32_pabsw512, "V32sV32s", "ncV:512:", "avx512bw")
 TARGET_BUILTIN(__builtin_ia32_packssdw512, "V32sV16iV16i", "ncV:512:", 
"avx512bw")
 TARGET_BUILTIN(__builtin_ia32_packsswb512, "V64cV32sV32s", "ncV:512:", 
"avx512bw")
 TARGET_BUILTIN(__builtin_ia32_packusdw512, "V32sV16iV16i", "ncV:512:", 
"avx512bw")
@@ -1198,8 +1188,6 @@ TARGET_BUILTIN(__builtin_ia32_getexppd128_mask, 
"V2dV2dV2dUc", "ncV:128:", "avx5
 TARGET_BUILTIN(__builtin_ia32_getexppd256_mask, "V4dV4dV4dUc", "ncV:256:", 
"avx512vl")
 TARGET_BUILTIN(__builtin_ia32_getexpps128_mask, "V4fV4fV4fUc", "ncV:128:", 
"avx512vl")
 TARGET_BUILTIN(__builtin_ia32_getexpps256_mask, "V8fV8fV8fUc", "ncV:256:", 
"avx512vl")
-TARGET_BUILTIN(__builtin_ia32_pabsq128, "V2OiV2Oi", "ncV:128:", "avx512vl")
-TARGET_BUIL

[PATCH] D118015: [RISCV][NFC] Rename RequiredExtensions to RequiredFeatures.

2022-01-24 Thread Alex Bradbury via Phabricator via cfe-commits
asb added a comment.

Makes sense to me, but I'll defer to someone else working more actively with 
this part of the codebase for a final LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118015

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


[clang] 3e50593 - [X86] Remove `__builtin_ia32_pmax/min` intrinsics and use generic `__builtin_elementwise_max/min`

2022-01-24 Thread Simon Pilgrim via cfe-commits

Author: Simon Pilgrim
Date: 2022-01-24T11:40:29Z
New Revision: 3e50593b18840ab4508a25d0f761afb65535a38d

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

LOG: [X86] Remove `__builtin_ia32_pmax/min` intrinsics and use generic 
`__builtin_elementwise_max/min`

D111985 added the generic `__builtin_elementwise_max` and 
`__builtin_elementwise_min` intrinsics with the same integer behaviour as the 
SSE/AVX instructions

This patch removes the `__builtin_ia32_pmax/min` intrinsics and just uses 
`__builtin_elementwise_max/min` - the existing tests see no changes:
```
__m256i test_mm256_max_epu32(__m256i a, __m256i b) {
  // CHECK-LABEL: test_mm256_max_epu32
  // CHECK: call <8 x i32> @llvm.umax.v8i32(<8 x i32> %{{.*}}, <8 x i32> 
%{{.*}})
  return _mm256_max_epu32(a, b);
}
```
This requires us to add a `__v64qs` explicitly signed char vector type (we 
already have `__v16qs` and `__v32qs`).

Sibling patch to D117791

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

Added: 


Modified: 
clang/include/clang/Basic/BuiltinsX86.def
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/Headers/avx2intrin.h
clang/lib/Headers/avx512bwintrin.h
clang/lib/Headers/avx512fintrin.h
clang/lib/Headers/avx512vlintrin.h
clang/lib/Headers/emmintrin.h
clang/lib/Headers/smmintrin.h
clang/test/CodeGen/builtins-x86.c

Removed: 




diff  --git a/clang/include/clang/Basic/BuiltinsX86.def 
b/clang/include/clang/Basic/BuiltinsX86.def
index 9b7c763b0c6c7..a8f5567248624 100644
--- a/clang/include/clang/Basic/BuiltinsX86.def
+++ b/clang/include/clang/Basic/BuiltinsX86.def
@@ -265,10 +265,6 @@ TARGET_BUILTIN(__builtin_ia32_psubusw128, "V8sV8sV8s", 
"ncV:128:", "sse2")
 TARGET_BUILTIN(__builtin_ia32_pmulhw128, "V8sV8sV8s", "ncV:128:", "sse2")
 TARGET_BUILTIN(__builtin_ia32_pavgb128, "V16cV16cV16c", "ncV:128:", "sse2")
 TARGET_BUILTIN(__builtin_ia32_pavgw128, "V8sV8sV8s", "ncV:128:", "sse2")
-TARGET_BUILTIN(__builtin_ia32_pmaxub128, "V16cV16cV16c", "ncV:128:", "sse2")
-TARGET_BUILTIN(__builtin_ia32_pmaxsw128, "V8sV8sV8s", "ncV:128:", "sse2")
-TARGET_BUILTIN(__builtin_ia32_pminub128, "V16cV16cV16c", "ncV:128:", "sse2")
-TARGET_BUILTIN(__builtin_ia32_pminsw128, "V8sV8sV8s", "ncV:128:", "sse2")
 TARGET_BUILTIN(__builtin_ia32_packsswb128, "V16cV8sV8s", "ncV:128:", "sse2")
 TARGET_BUILTIN(__builtin_ia32_packssdw128, "V8sV4iV4i", "ncV:128:", "sse2")
 TARGET_BUILTIN(__builtin_ia32_packuswb128, "V16cV8sV8s", "ncV:128:", "sse2")
@@ -377,14 +373,6 @@ TARGET_BUILTIN(__builtin_ia32_blendvpd, "V2dV2dV2dV2d", 
"ncV:128:", "sse4.1")
 TARGET_BUILTIN(__builtin_ia32_blendvps, "V4fV4fV4fV4f", "ncV:128:", "sse4.1")
 TARGET_BUILTIN(__builtin_ia32_packusdw128, "V8sV4iV4i", "ncV:128:", "sse4.1")
 
-TARGET_BUILTIN(__builtin_ia32_pmaxsb128, "V16cV16cV16c", "ncV:128:", "sse4.1")
-TARGET_BUILTIN(__builtin_ia32_pmaxsd128, "V4iV4iV4i", "ncV:128:", "sse4.1")
-TARGET_BUILTIN(__builtin_ia32_pmaxud128, "V4iV4iV4i", "ncV:128:", "sse4.1")
-TARGET_BUILTIN(__builtin_ia32_pmaxuw128, "V8sV8sV8s", "ncV:128:", "sse4.1")
-TARGET_BUILTIN(__builtin_ia32_pminsb128, "V16cV16cV16c", "ncV:128:", "sse4.1")
-TARGET_BUILTIN(__builtin_ia32_pminsd128, "V4iV4iV4i", "ncV:128:", "sse4.1")
-TARGET_BUILTIN(__builtin_ia32_pminud128, "V4iV4iV4i", "ncV:128:", "sse4.1")
-TARGET_BUILTIN(__builtin_ia32_pminuw128, "V8sV8sV8s", "ncV:128:", "sse4.1")
 TARGET_BUILTIN(__builtin_ia32_pmuldq128, "V2OiV4iV4i", "ncV:128:", "sse4.1")
 TARGET_BUILTIN(__builtin_ia32_roundps, "V4fV4fIi", "ncV:128:", "sse4.1")
 TARGET_BUILTIN(__builtin_ia32_roundss, "V4fV4fV4fIi", "ncV:128:", "sse4.1")
@@ -580,18 +568,6 @@ TARGET_BUILTIN(__builtin_ia32_phsubd256, "V8iV8iV8i", 
"ncV:256:", "avx2")
 TARGET_BUILTIN(__builtin_ia32_phsubsw256, "V16sV16sV16s", "ncV:256:", "avx2")
 TARGET_BUILTIN(__builtin_ia32_pmaddubsw256, "V16sV32cV32c", "ncV:256:", "avx2")
 TARGET_BUILTIN(__builtin_ia32_pmaddwd256, "V8iV16sV16s", "ncV:256:", "avx2")
-TARGET_BUILTIN(__builtin_ia32_pmaxub256, "V32cV32cV32c", "ncV:256:", "avx2")
-TARGET_BUILTIN(__builtin_ia32_pmaxuw256, "V16sV16sV16s", "ncV:256:", "avx2")
-TARGET_BUILTIN(__builtin_ia32_pmaxud256, "V8iV8iV8i", "ncV:256:", "avx2")
-TARGET_BUILTIN(__builtin_ia32_pmaxsb256, "V32cV32cV32c", "ncV:256:", "avx2")
-TARGET_BUILTIN(__builtin_ia32_pmaxsw256, "V16sV16sV16s", "ncV:256:", "avx2")
-TARGET_BUILTIN(__builtin_ia32_pmaxsd256, "V8iV8iV8i", "ncV:256:", "avx2")
-TARGET_BUILTIN(__builtin_ia32_pminub256, "V32cV32cV32c", "ncV:256:", "avx2")
-TARGET_BUILTIN(__builtin_ia32_pminuw256, "V16sV16sV16s", "ncV:256:", "avx2")
-TARGET_BUILTIN(__builtin_ia32_pminud256, "V8iV8iV8i", "ncV:256:", "avx2")
-TARGET_BUILTIN(__builtin_ia32_pminsb256, "V32cV32cV32c", "ncV:256:", "avx2")
-TARGET_BUILTIN(__builtin_ia32_pminsw256, "V16sV16sV16s", "ncV:256:", "avx2")
-TARGET_BUILTIN(__bu

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

2022-01-24 Thread Whisperity via Phabricator via cfe-commits
whisperity accepted this revision.
whisperity added a comment.

In D112646#3265670 , @whisperity 
wrote:

> Please specify what name and e-mail address you'd like to have the Git commit 
> author attributed with!

Nevermind, I forgot that you commented this:

In D112646#3264010 , @avogelsgesang 
wrote:

> there is a copy of this commit on 
> https://github.com/vogelsgesang/llvm-project/commits/avogelsgesang-tidy-readability-container-contains





LGTM. Committing in a moment. 😉 (I applied a fix to a typo in a comment in the 
test, `time begin` -> `time being`.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112646

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


[clang-tools-extra] 3696c70 - [clang-tidy] Add `readability-container-contains` check

2022-01-24 Thread via cfe-commits

Author: Adrian Vogelsgesang
Date: 2022-01-24T12:57:18+01:00
New Revision: 3696c70e67d9b9e54307ef25077bae7a6f76636e

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

LOG: [clang-tidy] Add `readability-container-contains` check

This commit introduces a new check `readability-container-contains` which finds
usages of `container.count()` and `container.find() != container.end()` and
instead recommends the `container.contains()` method introduced in C++20.

For containers which permit multiple entries per key (`multimap`, `multiset`,
...), `contains` is more efficient than `count` because `count` has to do
unnecessary additional work.

While this this performance difference does not exist for containers with only
a single entry per key (`map`, `unordered_map`, ...), `contains` still conveys
the intent better.

Reviewed By: xazax.hun, whisperity

Differential Revision: http://reviews.llvm.org/D112646

Added: 
clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp
clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.h
clang-tools-extra/docs/clang-tidy/checks/readability-container-contains.rst

clang-tools-extra/test/clang-tidy/checkers/readability-container-contains.cpp

Modified: 
clang-tools-extra/clang-tidy/readability/CMakeLists.txt
clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
index 22ce8f62751ec..ea09b2193eb7c 100644
--- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
@@ -7,6 +7,7 @@ add_clang_library(clangTidyReadabilityModule
   AvoidConstParamsInDecls.cpp
   BracesAroundStatementsCheck.cpp
   ConstReturnTypeCheck.cpp
+  ContainerContainsCheck.cpp
   ContainerDataPointerCheck.cpp
   ContainerSizeEmptyCheck.cpp
   ConvertMemberFunctionsToStatic.cpp

diff  --git 
a/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp
new file mode 100644
index 0..7a20480fb501c
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp
@@ -0,0 +1,144 @@
+//===--- ContainerContainsCheck.cpp - clang-tidy 
--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ContainerContainsCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+void ContainerContainsCheck::registerMatchers(MatchFinder *Finder) {
+  const auto SupportedContainers = hasType(
+  hasUnqualifiedDesugaredType(recordType(hasDeclaration(cxxRecordDecl(
+  hasAnyName("::std::set", "::std::unordered_set", "::std::map",
+ "::std::unordered_map", "::std::multiset",
+ "::std::unordered_multiset", "::std::multimap",
+ "::std::unordered_multimap"));
+
+  const auto CountCall =
+  cxxMemberCallExpr(on(SupportedContainers),
+callee(cxxMethodDecl(hasName("count"))),
+argumentCountIs(1))
+  .bind("call");
+
+  const auto FindCall =
+  cxxMemberCallExpr(on(SupportedContainers),
+callee(cxxMethodDecl(hasName("find"))),
+argumentCountIs(1))
+  .bind("call");
+
+  const auto EndCall = cxxMemberCallExpr(on(SupportedContainers),
+ callee(cxxMethodDecl(hasName("end"))),
+ argumentCountIs(0));
+
+  const auto Literal0 = integerLiteral(equals(0));
+  const auto Literal1 = integerLiteral(equals(1));
+
+  auto AddSimpleMatcher = [&](auto Matcher) {
+Finder->addMatcher(
+traverse(TK_IgnoreUnlessSpelledInSource, std::move(Matcher)), this);
+  };
+
+  // Find membership tests which use `count()`.
+  
Finder->addMatcher(implicitCastExpr(hasImplicitDestinationType(booleanType()),
+  hasSourceExpression(CountCall))
+ .bind("positiveComparison"),
+ this);
+  AddSimpleMatcher(
+  binaryOperator(hasLHS(CountCall), hasOperatorName("!="), 
hasRHS(Literal0))
+  .bind("positiveComparison

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

2022-01-24 Thread Whisperity via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3696c70e67d9: [clang-tidy] Add 
`readability-container-contains` check (authored by avogelsgesang, committed by 
whisperity).

Changed prior to commit:
  https://reviews.llvm.org/D112646?vs=400820&id=402464#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112646

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp
  clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.h
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-container-contains.rst
  clang-tools-extra/test/clang-tidy/checkers/readability-container-contains.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-container-contains.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-container-contains.cpp
@@ -0,0 +1,230 @@
+// RUN: %check_clang_tidy -std=c++20 %s readability-container-contains %t
+
+// Some *very* simplified versions of `map` etc.
+namespace std {
+
+template 
+struct map {
+  unsigned count(const Key &K) const;
+  bool contains(const Key &K) const;
+  void *find(const Key &K);
+  void *end();
+};
+
+template 
+struct set {
+  unsigned count(const Key &K) const;
+  bool contains(const Key &K) const;
+};
+
+template 
+struct unordered_set {
+  unsigned count(const Key &K) const;
+  bool contains(const Key &K) const;
+};
+
+template 
+struct multimap {
+  unsigned count(const Key &K) const;
+  bool contains(const Key &K) const;
+};
+
+} // namespace std
+
+// Check that we detect various common ways to check for membership
+int testDifferentCheckTypes(std::map &MyMap) {
+  if (MyMap.count(0))
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use 'contains' to check for membership [readability-container-contains]
+// CHECK-FIXES: if (MyMap.contains(0))
+return 1;
+  bool C1 = MyMap.count(1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: bool C1 = MyMap.contains(1);
+  auto C2 = static_cast(MyMap.count(1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:37: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C2 = static_cast(MyMap.contains(1));
+  auto C3 = MyMap.count(2) != 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C3 = MyMap.contains(2);
+  auto C4 = MyMap.count(3) > 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C4 = MyMap.contains(3);
+  auto C5 = MyMap.count(4) >= 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C5 = MyMap.contains(4);
+  auto C6 = MyMap.find(5) != MyMap.end();
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C6 = MyMap.contains(5);
+  return C1 + C2 + C3 + C4 + C5 + C6;
+}
+
+// Check that we detect various common ways to check for non-membership
+int testNegativeChecks(std::map &MyMap) {
+  bool C1 = !MyMap.count(-1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: bool C1 = !MyMap.contains(-1);
+  auto C2 = MyMap.count(-2) == 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C2 = !MyMap.contains(-2);
+  auto C3 = MyMap.count(-3) <= 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C3 = !MyMap.contains(-3);
+  auto C4 = MyMap.count(-4) < 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C4 = !MyMap.contains(-4);
+  auto C5 = MyMap.find(-5) == MyMap.end();
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C5 = !MyMap.contains(-5);
+  return C1 + C2 + C3 + C4 + C5;
+}
+
+// Check for various types
+int testDifferentTypes(std::map &M, std::unordered_set &US, std::set &S, std::multimap &MM) {
+  bool C1 = M.count(1001);
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: bool C1 = M.contains(1001);

[clang] e407443 - [X86] Remove avx512f integer and/or/xor/min/max reduction intrinsics and use generic equivalents

2022-01-24 Thread Simon Pilgrim via cfe-commits

Author: Simon Pilgrim
Date: 2022-01-24T11:57:53Z
New Revision: e4074432d5bf5c295f96eeed27c5b693f5b3bf16

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

LOG: [X86] Remove avx512f integer and/or/xor/min/max reduction intrinsics and 
use generic equivalents

None of these have any reordering issues, and they still emit the same 
reduction intrinsics without any change in the existing test coverage:

llvm-project\clang\test\CodeGen\X86\avx512-reduceIntrin.c
llvm-project\clang\test\CodeGen\X86\avx512-reduceMinMaxIntrin.c

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

Added: 


Modified: 
clang/include/clang/Basic/BuiltinsX86.def
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/Headers/avx512fintrin.h

Removed: 




diff  --git a/clang/include/clang/Basic/BuiltinsX86.def 
b/clang/include/clang/Basic/BuiltinsX86.def
index a8f5567248624..0669a96b942b3 100644
--- a/clang/include/clang/Basic/BuiltinsX86.def
+++ b/clang/include/clang/Basic/BuiltinsX86.def
@@ -2015,8 +2015,6 @@ TARGET_BUILTIN(__builtin_ia32_selectsd_128, 
"V2dUcV2dV2d", "ncV:128:", "avx512f"
 // generic reduction intrinsics
 TARGET_BUILTIN(__builtin_ia32_reduce_add_d512, "iV16i", "ncV:512:", "avx512f")
 TARGET_BUILTIN(__builtin_ia32_reduce_add_q512, "OiV8Oi", "ncV:512:", "avx512f")
-TARGET_BUILTIN(__builtin_ia32_reduce_and_d512, "iV16i", "ncV:512:", "avx512f")
-TARGET_BUILTIN(__builtin_ia32_reduce_and_q512, "OiV8Oi", "ncV:512:", "avx512f")
 TARGET_BUILTIN(__builtin_ia32_reduce_fadd_pd512, "ddV8d", "ncV:512:", 
"avx512f")
 TARGET_BUILTIN(__builtin_ia32_reduce_fadd_ps512, "ffV16f", "ncV:512:", 
"avx512f")
 TARGET_BUILTIN(__builtin_ia32_reduce_fadd_ph512, "xxV32x", "ncV:512:", 
"avx512fp16")
@@ -2039,16 +2037,6 @@ TARGET_BUILTIN(__builtin_ia32_reduce_fmul_ph256, 
"xxV16x", "ncV:256:", "avx512fp
 TARGET_BUILTIN(__builtin_ia32_reduce_fmul_ph128, "xxV8x", "ncV:128:", 
"avx512fp16,avx512vl")
 TARGET_BUILTIN(__builtin_ia32_reduce_mul_d512, "iV16i", "ncV:512:", "avx512f")
 TARGET_BUILTIN(__builtin_ia32_reduce_mul_q512, "OiV8Oi", "ncV:512:", "avx512f")
-TARGET_BUILTIN(__builtin_ia32_reduce_or_d512, "iV16i", "ncV:512:", "avx512f")
-TARGET_BUILTIN(__builtin_ia32_reduce_or_q512, "OiV8Oi", "ncV:512:", "avx512f")
-TARGET_BUILTIN(__builtin_ia32_reduce_smax_d512, "iV16i", "ncV:512:", "avx512f")
-TARGET_BUILTIN(__builtin_ia32_reduce_smax_q512, "OiV8Oi", "ncV:512:", 
"avx512f")
-TARGET_BUILTIN(__builtin_ia32_reduce_smin_d512, "iV16i", "ncV:512:", "avx512f")
-TARGET_BUILTIN(__builtin_ia32_reduce_smin_q512, "OiV8Oi", "ncV:512:", 
"avx512f")
-TARGET_BUILTIN(__builtin_ia32_reduce_umax_d512, "iV16i", "ncV:512:", "avx512f")
-TARGET_BUILTIN(__builtin_ia32_reduce_umax_q512, "OiV8Oi", "ncV:512:", 
"avx512f")
-TARGET_BUILTIN(__builtin_ia32_reduce_umin_d512, "iV16i", "ncV:512:", "avx512f")
-TARGET_BUILTIN(__builtin_ia32_reduce_umin_q512, "OiV8Oi", "ncV:512:", 
"avx512f")
 
 // MONITORX/MWAITX
 TARGET_BUILTIN(__builtin_ia32_monitorx, "vvC*UiUi", "n", "mwaitx")

diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 4c68b20067b99..cd35e7cbe76f7 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -14365,12 +14365,6 @@ Value *CodeGenFunction::EmitX86BuiltinExpr(unsigned 
BuiltinID,
 CGM.getIntrinsic(Intrinsic::vector_reduce_add, Ops[0]->getType());
 return Builder.CreateCall(F, {Ops[0]});
   }
-  case X86::BI__builtin_ia32_reduce_and_d512:
-  case X86::BI__builtin_ia32_reduce_and_q512: {
-Function *F =
-CGM.getIntrinsic(Intrinsic::vector_reduce_and, Ops[0]->getType());
-return Builder.CreateCall(F, {Ops[0]});
-  }
   case X86::BI__builtin_ia32_reduce_fadd_pd512:
   case X86::BI__builtin_ia32_reduce_fadd_ps512:
   case X86::BI__builtin_ia32_reduce_fadd_ph512:
@@ -14417,36 +14411,6 @@ Value *CodeGenFunction::EmitX86BuiltinExpr(unsigned 
BuiltinID,
 CGM.getIntrinsic(Intrinsic::vector_reduce_mul, Ops[0]->getType());
 return Builder.CreateCall(F, {Ops[0]});
   }
-  case X86::BI__builtin_ia32_reduce_or_d512:
-  case X86::BI__builtin_ia32_reduce_or_q512: {
-Function *F =
-CGM.getIntrinsic(Intrinsic::vector_reduce_or, Ops[0]->getType());
-return Builder.CreateCall(F, {Ops[0]});
-  }
-  case X86::BI__builtin_ia32_reduce_smax_d512:
-  case X86::BI__builtin_ia32_reduce_smax_q512: {
-Function *F =
-CGM.getIntrinsic(Intrinsic::vector_reduce_smax, Ops[0]->getType());
-return Builder.CreateCall(F, {Ops[0]});
-  }
-  case X86::BI__builtin_ia32_reduce_smin_d512:
-  case X86::BI__builtin_ia32_reduce_smin_q512: {
-Function *F =
-CGM.getIntrinsic(Intrinsic::vector_reduce_smin, Ops[0]->getType());
-return Builder.CreateCall(F, {Ops[0]});
-  }
-  case X86::BI__builtin_ia32_reduce_umax_d512:
-  case X86::BI__builti

[PATCH] D117881: [X86] Remove avx512f integer and/or/xor/min/max reduction intrinsics and use generic equivalents

2022-01-24 Thread Simon Pilgrim 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 rGe4074432d5bf: [X86] Remove avx512f integer 
and/or/xor/min/max reduction intrinsics and use… (authored by RKSimon).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117881

Files:
  clang/include/clang/Basic/BuiltinsX86.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Headers/avx512fintrin.h

Index: clang/lib/Headers/avx512fintrin.h
===
--- clang/lib/Headers/avx512fintrin.h
+++ clang/lib/Headers/avx512fintrin.h
@@ -9324,11 +9324,11 @@
 }
 
 static __inline__ long long __DEFAULT_FN_ATTRS512 _mm512_reduce_and_epi64(__m512i __W) {
-  return __builtin_ia32_reduce_and_q512(__W);
+  return __builtin_reduce_and((__v8di)__W);
 }
 
 static __inline__ long long __DEFAULT_FN_ATTRS512 _mm512_reduce_or_epi64(__m512i __W) {
-  return __builtin_ia32_reduce_or_q512(__W);
+  return __builtin_reduce_or((__v8di)__W);
 }
 
 static __inline__ long long __DEFAULT_FN_ATTRS512
@@ -9346,13 +9346,13 @@
 static __inline__ long long __DEFAULT_FN_ATTRS512
 _mm512_mask_reduce_and_epi64(__mmask8 __M, __m512i __W) {
   __W = _mm512_mask_mov_epi64(_mm512_set1_epi64(~0ULL), __M, __W);
-  return __builtin_ia32_reduce_and_q512(__W);
+  return __builtin_reduce_and((__v8di)__W);
 }
 
 static __inline__ long long __DEFAULT_FN_ATTRS512
 _mm512_mask_reduce_or_epi64(__mmask8 __M, __m512i __W) {
   __W = _mm512_maskz_mov_epi64(__M, __W);
-  return __builtin_ia32_reduce_or_q512(__W);
+  return __builtin_reduce_or((__v8di)__W);
 }
 
 // -0.0 is used to ignore the start value since it is the neutral value of
@@ -9390,12 +9390,12 @@
 
 static __inline__ int __DEFAULT_FN_ATTRS512
 _mm512_reduce_and_epi32(__m512i __W) {
-  return __builtin_ia32_reduce_and_d512((__v16si)__W);
+  return __builtin_reduce_and((__v16si)__W);
 }
 
 static __inline__ int __DEFAULT_FN_ATTRS512
 _mm512_reduce_or_epi32(__m512i __W) {
-  return __builtin_ia32_reduce_or_d512((__v16si)__W);
+  return __builtin_reduce_or((__v16si)__W);
 }
 
 static __inline__ int __DEFAULT_FN_ATTRS512
@@ -9413,13 +9413,13 @@
 static __inline__ int __DEFAULT_FN_ATTRS512
 _mm512_mask_reduce_and_epi32( __mmask16 __M, __m512i __W) {
   __W = _mm512_mask_mov_epi32(_mm512_set1_epi32(~0U), __M, __W);
-  return __builtin_ia32_reduce_and_d512((__v16si)__W);
+  return __builtin_reduce_and((__v16si)__W);
 }
 
 static __inline__ int __DEFAULT_FN_ATTRS512
 _mm512_mask_reduce_or_epi32(__mmask16 __M, __m512i __W) {
   __W = _mm512_maskz_mov_epi32(__M, __W);
-  return __builtin_ia32_reduce_or_d512((__v16si)__W);
+  return __builtin_reduce_or((__v16si)__W);
 }
 
 static __inline__ float __DEFAULT_FN_ATTRS512
@@ -9446,89 +9446,89 @@
 
 static __inline__ long long __DEFAULT_FN_ATTRS512
 _mm512_reduce_max_epi64(__m512i __V) {
-  return __builtin_ia32_reduce_smax_q512(__V);
+  return __builtin_reduce_max((__v8di)__V);
 }
 
 static __inline__ unsigned long long __DEFAULT_FN_ATTRS512
 _mm512_reduce_max_epu64(__m512i __V) {
-  return __builtin_ia32_reduce_umax_q512(__V);
+  return __builtin_reduce_max((__v8du)__V);
 }
 
 static __inline__ long long __DEFAULT_FN_ATTRS512
 _mm512_reduce_min_epi64(__m512i __V) {
-  return __builtin_ia32_reduce_smin_q512(__V);
+  return __builtin_reduce_min((__v8di)__V);
 }
 
 static __inline__ unsigned long long __DEFAULT_FN_ATTRS512
 _mm512_reduce_min_epu64(__m512i __V) {
-  return __builtin_ia32_reduce_umin_q512(__V);
+  return __builtin_reduce_min((__v8du)__V);
 }
 
 static __inline__ long long __DEFAULT_FN_ATTRS512
 _mm512_mask_reduce_max_epi64(__mmask8 __M, __m512i __V) {
   __V = _mm512_mask_mov_epi64(_mm512_set1_epi64(-__LONG_LONG_MAX__ - 1LL), __M, __V);
-  return __builtin_ia32_reduce_smax_q512(__V);
+  return __builtin_reduce_max((__v8di)__V);
 }
 
 static __inline__ unsigned long long __DEFAULT_FN_ATTRS512
 _mm512_mask_reduce_max_epu64(__mmask8 __M, __m512i __V) {
   __V = _mm512_maskz_mov_epi64(__M, __V);
-  return __builtin_ia32_reduce_umax_q512(__V);
+  return __builtin_reduce_max((__v8du)__V);
 }
 
 static __inline__ long long __DEFAULT_FN_ATTRS512
 _mm512_mask_reduce_min_epi64(__mmask8 __M, __m512i __V) {
   __V = _mm512_mask_mov_epi64(_mm512_set1_epi64(__LONG_LONG_MAX__), __M, __V);
-  return __builtin_ia32_reduce_smin_q512(__V);
+  return __builtin_reduce_min((__v8di)__V);
 }
 
 static __inline__ unsigned long long __DEFAULT_FN_ATTRS512
 _mm512_mask_reduce_min_epu64(__mmask8 __M, __m512i __V) {
   __V = _mm512_mask_mov_epi64(_mm512_set1_epi64(~0ULL), __M, __V);
-  return __builtin_ia32_reduce_umin_q512(__V);
+  return __builtin_reduce_min((__v8du)__V);
 }
 static __inline__ int __DEFAULT_FN_ATTRS512
 _mm512_reduce_max_epi32(__m512i __V) {
-  return __builtin_ia32_reduce_smax_d512((__v16si)__V);
+  return __builtin_reduce_max((__v16si)__V);
 }
 
 static __inline__ unsigned int __DEFAUL

[PATCH] D117301: [clang][NFC] Wrap TYPE_SWITCH in "do while (0)" in the interpreter

2022-01-24 Thread Simon Pilgrim via Phabricator via cfe-commits
RKSimon accepted this revision.
RKSimon added a comment.
This revision is now accepted and ready to land.

LGTM - cheers


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

https://reviews.llvm.org/D117301

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


[PATCH] D118018: [RFC] [C+++20] [Modules] Mangling lambda in modules

2022-01-24 Thread Nathan Sidwell via Phabricator via cfe-commits
urnathan added a comment.

I don't think this is wanted.  there are two cases:
a) lambda is attached to some ODR-visible entitity.   Something like (in 
module-purview at namespace-scope) '[maybe-export] auto var = []{};'.  Here the 
lambda acquires 'var' as its context.
b) lambda is not attached like that.  p1815 makes it ill-formed for the lambda 
to be exposed externally in that case.

see https://github.com/itanium-cxx-abi/cxx-abi/issues/84

FWIW I'm redoing the module mangling scheme, the current one is undemangleable


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118018

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


[PATCH] D118034: [C++20] [Modules] Don't complain about duplicated default template argument across modules

2022-01-24 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu created this revision.
ChuanqiXu added reviewers: rsmith, urnathan, aaron.ballman, erichkeane.
ChuanqiXu added a project: clang.
ChuanqiXu requested review of this revision.
Herald added a subscriber: cfe-commits.

See https://github.com/cplusplus/draft/pull/5204 for a detailed background.

Simply, the test attached to this patch should be accepted instead of being 
complained about the redefinition.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118034

Files:
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/Modules/Inputs/redundant-template-default-arg/foo.cppm
  clang/test/Modules/Inputs/redundant-template-default-arg/foo.h
  clang/test/Modules/redundant-template-default-arg.cpp


Index: clang/test/Modules/redundant-template-default-arg.cpp
===
--- /dev/null
+++ clang/test/Modules/redundant-template-default-arg.cpp
@@ -0,0 +1,7 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: %clang_cc1  -std=c++20 
%S/Inputs/redundant-template-default-arg/foo.cppm 
-I%S/Inputs/redundant-template-default-arg/. -emit-module-interface -o 
%t/foo.pcm
+// RUN: %clang_cc1  -fprebuilt-module-path=%t -std=c++20 %s 
-I%S/Inputs/redundant-template-default-arg/. -fsyntax-only -verify
+// expected-no-diagnostics
+import foo;
+#include "foo.h"
Index: clang/test/Modules/Inputs/redundant-template-default-arg/foo.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/redundant-template-default-arg/foo.h
@@ -0,0 +1,11 @@
+template 
+T v;
+
+template 
+int v2;
+
+template 
+class my_array {};
+
+template  typename C = my_array>
+int v3;
Index: clang/test/Modules/Inputs/redundant-template-default-arg/foo.cppm
===
--- /dev/null
+++ clang/test/Modules/Inputs/redundant-template-default-arg/foo.cppm
@@ -0,0 +1,3 @@
+module;
+#include "foo.h"
+export module foo;
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -2678,6 +2678,23 @@
NewParam != NewParamEnd; ++NewParam) {
 // Variables used to diagnose redundant default arguments
 bool RedundantDefaultArg = false;
+// Variable used to not diagnose redundant default arguments
+// from modules.
+//
+// [basic.def.odr]/13:
+// There can be more than one definition of a
+// ...
+// default template argument
+// ...
+// in a program provided that each definition appears in a different
+// translation unit and the definitions satisfy the [same-meaning
+// criteria of the ODR].
+//
+// Simply, the design of modules allows the definition of template default
+// argument to be repeated across translation unit. Note that the ASTReader
+// would check the ODR criteria mentioned above so we could skip it here.
+bool IsOldParamFromModule = false;
+
 SourceLocation OldDefaultLoc;
 SourceLocation NewDefaultLoc;
 
@@ -2710,6 +2727,8 @@
 OldDefaultLoc = OldTypeParm->getDefaultArgumentLoc();
 NewDefaultLoc = NewTypeParm->getDefaultArgumentLoc();
 SawDefaultArgument = true;
+if (OldTypeParm->isFromASTFile())
+  IsOldParamFromModule = true;
 RedundantDefaultArg = true;
 PreviousDefaultArgLoc = NewDefaultLoc;
   } else if (OldTypeParm && OldTypeParm->hasDefaultArgument()) {
@@ -2755,6 +2774,8 @@
 OldDefaultLoc = OldNonTypeParm->getDefaultArgumentLoc();
 NewDefaultLoc = NewNonTypeParm->getDefaultArgumentLoc();
 SawDefaultArgument = true;
+if (OldNonTypeParm->isFromASTFile())
+  IsOldParamFromModule = true;
 RedundantDefaultArg = true;
 PreviousDefaultArgLoc = NewDefaultLoc;
   } else if (OldNonTypeParm && OldNonTypeParm->hasDefaultArgument()) {
@@ -2799,6 +2820,8 @@
 OldDefaultLoc = OldTemplateParm->getDefaultArgument().getLocation();
 NewDefaultLoc = NewTemplateParm->getDefaultArgument().getLocation();
 SawDefaultArgument = true;
+if (OldTemplateParm->isFromASTFile())
+  IsOldParamFromModule = true;
 RedundantDefaultArg = true;
 PreviousDefaultArgLoc = NewDefaultLoc;
   } else if (OldTemplateParm && OldTemplateParm->hasDefaultArgument()) {
@@ -2826,7 +2849,7 @@
   Invalid = true;
 }
 
-if (RedundantDefaultArg) {
+if (RedundantDefaultArg && !IsOldParamFromModule) {
   // C++ [temp.param]p12:
   //   A template-parameter shall not be given default arguments
   //   by two different declarations in the same scope.


Index: clang/test/Modules/redundant-template-default-arg.cpp
===
--- /dev/null
+++ clang/test/Modules/redundant-template-default-arg.cpp
@@ -0,0 +1,7 @@
+// RUN: rm -rf %t
+// RUN: mkdi

[PATCH] D107769: [OpenCL] Make generic addrspace optional for -fdeclare-opencl-builtins

2022-01-24 Thread Sven van Haastregt via Phabricator via cfe-commits
svenvh added a comment.

In D107769#3265665 , @Anastasia wrote:

> The way I understand the spec for OpenCL C 2.0 is that whenever the address 
> space of the pointer is not listed it means generic has to be used, here is 
> one example:
> https://www.khronos.org/registry/OpenCL/specs/2.2/html/OpenCL_C.html#vector-data-load-and-store-functions
>
>   gentypen vloadn(size_t offset, const gentype *p)
>   gentypen vloadn(size_t offset, const __constant gentype *p)
>
> that has no address space (i.e. `generic`) and `constant` explicitly. So I 
> think if address spaces are not listed explicitly they are not supposed to be 
> available.

The unified specification (which "specifies all versions of OpenCL C") seems to 
be making all overloads available as I understand; it is perhaps subtly 
different from the previous specification?

https://www.khronos.org/registry/OpenCL/specs/3.0-unified/html/OpenCL_C.html#vector-data-load-and-store-functions

  gentypen vloadn(size_t offset, const __global gentype *p)
  gentypen vloadn(size_t offset, const __local gentype *p)
  gentypen vloadn(size_t offset, const __constant gentype *p)
  gentypen vloadn(size_t offset, const __private gentype *p)
  
  For OpenCL C 2.0, or OpenCL C 3.0 or newer with the 
__opencl_c_generic_address_space feature:
  
  gentypen vloadn(size_t offset, const gentype *p)

Since the `__constant` overload should always be available, I think a reader 
can assume that the overloads directly above and below `__constant` are also 
always available?  So that the generic overload is an optional addition to the 
list of overloads.  If not, I'd expect the spec to specify a condition before 
listing the specific overloads.

> One implication of adding all address space overloads is that it makes 
> library size larger, but my feeling is that we don't have that many functions 
> with pointers to significantly impace the library size?

This patch should be touching all of them.  Not that many indeed, but it might 
still have a non-negligible impact on OpenCL libraries due to the combination 
of #vector-sizes * #types * #addrspaces.


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

https://reviews.llvm.org/D107769

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


[PATCH] D118018: [RFC] [C+++20] [Modules] Mangling lambda in modules

2022-01-24 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment.

In D118018#3265810 , @urnathan wrote:

> I don't think this is wanted.  there are two cases:
> a) lambda is attached to some ODR-visible entitity.   Something like (in 
> module-purview at namespace-scope) '[maybe-export] auto var = []{};'.  Here 
> the lambda acquires 'var' as its context.
> b) lambda is not attached like that.  p1815 makes it ill-formed for the 
> lambda to be exposed externally in that case.
>
> see https://github.com/itanium-cxx-abi/cxx-abi/issues/84
>
> FWIW I'm redoing the module mangling scheme, the current one is undemangleable

Oh, great to hear you are willing to take this. I know you must know much more 
about mangling than me. If you are going to refactor the mangling system, this 
one would be meaning less. I would abandon it later.

BTW, do you mind to be assigned for issue 
https://github.com/llvm/llvm-project/issues/52857 and 
https://github.com/llvm/llvm-project/issues/53232?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118018

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


[PATCH] D116387: [CodeCompletion][clangd] Clean __uglified parameter names in completion & hover

2022-01-24 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments.



Comment at: clang-tools-extra/clangd/Hover.cpp:65
   Base.SuppressTemplateArgsInCXXConstructors = true;
+  Base.CleanUglifiedParameters = true;
   return Base;

IIUC, the code looks like we change the hover behavior as well, but I think our 
plan is to not change hover. 



Comment at: clang/lib/AST/DeclPrinter.cpp:889
+  printDeclType(T, (isa(D) && Policy.CleanUglifiedParameters &&
+D->getIdentifier())
+   ? D->getIdentifier()->deuglifiedName()

nit: `D->getIdentifier()` seems redundant -- `D->getName()` has an 
`isIdentifier()` assertion, we should expect that we can always get identifier 
from D here. 



Comment at: clang/lib/AST/DeclPrinter.cpp:1138
+if (TTP->getDeclName()) {
+  if (Policy.CleanUglifiedParameters && isa(D) &&
+  TTP->getIdentifier())

`isa(D)` is redundant as line 1128 has checked it.



Comment at: clang/lib/Basic/IdentifierTable.cpp:312
 
+StringRef IdentifierInfo::deuglifiedName() const {
+  StringRef Name = getName();

Not objecting the current approach -- an alternative is to incline this into 
`getName` by adding a parameter `Deuglify` to control the behavior.

Not sure it is much better, but it seem to save some boilerplate code like 
`Policy.CleanUglifiedParameters ? II->deuglifiedName() : II->getName()`. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116387

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


[PATCH] D118018: [RFC] [C+++20] [Modules] Mangling lambda in modules

2022-01-24 Thread Nathan Sidwell via Phabricator via cfe-commits
urnathan added a comment.

sure, please reassign them


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118018

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


[PATCH] D91000: [clang-tidy] Add cert-msc24-c checker.

2022-01-24 Thread Whisperity via Phabricator via cfe-commits
whisperity added a reviewer: whisperity.
whisperity added a comment.
Herald added a subscriber: rnkovacs.

Bump! Thanks @futogergely for picking up @ktomi996's work, and @steakhal for 
the help given during the implementation process!

In D91000#3231417 , @martong wrote:

> In D91000#3230055 , @futogergely 
> wrote:
>
>> I think for now it is enough to issue a warning of using these functions, 
>> and not suggest a replacement. Should we add an option to the checker to 
>> also check for these functions?
>
> IMHO, it is okay to start with just simply issuing the warning. Later we 
> might add that option (in a subsequent patch).

Yes, please. And I suggest indeed hiding it behind an option, e.g. 
//`ReportMoreUnsafeFunctions`// which I believe should default to true, but 
setting it to false would allow people who want their code to be "only" strict 
CERT-clean (that is, not care about these additional matches), can request 
doing so. Not offering a replacement right now is okay too, we can definitely 
work them in later.

In D91000#3197851 , @aaron.ballman 
wrote:

> In terms of whether we should expose the check in C++: I think we shouldn't. 
> Annex K *could* be supported by a C++ library implementation 
> (http://eel.is/c++draft/library#headers-10), but none of the identifiers are 
> added to namespace `std` and there's not a lot of Annex K support in C so I 
> imagine there's even less support in C++. We can always revisit the decision 
> later and expand support for C++ once we know what that support should look 
> like.

(Minor suggestion, but we could pull a trick with perhaps checking whether the 
macro is defined in the TU by the user and the library, and trying to match 
against the non-`std::` Annex K functions, and if they are available, consider 
the implementation the user is running under as 
"AnnexK-capable-even-in-C++-mode" and start issuing warnings? This might sound 
very technical, and definitely for later consideration!)

> I think we should probably also not enable the check when the user compiles 
> in C99 or earlier mode, because there is no Annex K available to provide 
> replacement functions.

Except for warning about `gets()` and suggesting `fgets()`?



Some nits: when it comes to inline comments, please mark the comments of the 
reviewers as "Done" when replying if you consider that comment was handled. 
Only you, the patch author, can do this. When there are too many open threads 
of comments, it gets messy to see what has been handled and what is still under 
discussion.




Comment at: 
clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp:41-42
+
+  // Matching the `gets` deprecated function without replacement.
+  auto DeprecatedFunctionNamesMatcher = hasAnyName("::gets");
+

futogergely wrote:
> aaron.ballman wrote:
> > This comment is not accurate. `gets_s()` is a secure replacement for 
> > `gets()`.
> If gets is removed from C11, and gets_s is introduced in C11, then gets_s 
> cannot be a replacement or? Maybe fgets?
> 
> Also I was wondering if we would like to disable this check for C99, maybe we 
> should remove the check for gets all together.
Yes, it's strange territory. If I make my code safer but //stay// pre-C11, I 
actually can't, because the new function isn't there yet. If I also //upgrade// 
then I'll **have to** make my code "safer", because the old function is now 
missing...

Given how brutally dangerous `gets` is (you very rarely see a documentation 
page just going **`Never use gets()!`**), I would suggest keeping at least the 
warning.

Although even the CERT rule mentions `gets_s()`. We could have some wiggle room 
here: do the warning for `gets()`, and suggest two alternatives: `fgets()`, or 
`gets_s()` + Annex K? `fgets(stdin, ...)` is also safe, if the buffer's size is 
given appropriately.



Comment at: 
clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp:41-42
+
+  // Matching the `gets` deprecated function without replacement.
+  auto DeprecatedFunctionNamesMatcher = hasAnyName("::gets");
+

whisperity wrote:
> futogergely wrote:
> > aaron.ballman wrote:
> > > This comment is not accurate. `gets_s()` is a secure replacement for 
> > > `gets()`.
> > If gets is removed from C11, and gets_s is introduced in C11, then gets_s 
> > cannot be a replacement or? Maybe fgets?
> > 
> > Also I was wondering if we would like to disable this check for C99, maybe 
> > we should remove the check for gets all together.
> Yes, it's strange territory. If I make my code safer but //stay// pre-C11, I 
> actually can't, because the new function isn't there yet. If I also 
> //upgrade// then I'll **have to** make my code "safer", because the old 
> function is now missing...
> 
> Given how brutally dangerous `gets` is (you very rarely see a documentation 

[PATCH] D117091: [Clang] Add attributes alloc_size and alloc_align to mm_malloc

2022-01-24 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment.

Ping @reames


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

https://reviews.llvm.org/D117091

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


[PATCH] D118011: [RISCV] Adjust predicates and update intrinsic for clmul and clmulh in Zbkc extension

2022-01-24 Thread Liao Chunyu via Phabricator via cfe-commits
liaolucy added a comment.

clang Zbkc patch: https://reviews.llvm.org/D112774, If there are any mistakes, 
you can help to point them out


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118011

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


[PATCH] D116085: [clang-tidy] Performance improvements for NOLINTBEGIN/END blocks

2022-01-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments.



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:214
+  std::string CheckName = getCheckName(Info.getID());
+  return NoLintHandler.shouldSuppress(DiagLevel, Info, CheckName, 
NoLintErrors);
+}

we don't seem to be passing allowio/enablenolintblocks ?



Comment at: clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp:89
+// as parsed from the file's character contents.
+class NoLintToken {
+public:

nit: Any reason for not making this class move-only ? everytime we copy, we are 
creating globs from scratch and losing the cache. i think it should be fine if 
we just did emplace_back in all places and moved tokens around rather than 
taking copies/pointers.



Comment at: clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp:133
+static SmallVector getNoLints(StringRef Buffer) {
+  static const StringRef NOLINT = "NOLINT";
+  SmallVector NoLints;

nit: `static constexpr llvm::StringLiteral`



Comment at: clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp:141
+if (NoLintPos == StringRef::npos)
+  return NoLints; // Buffer exhausted
+

nit: `break` instead?



Comment at: clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp:156
+// Get checks, if specified.
+const auto Checks = [&]() -> Optional {
+  // Opening bracket:

nit: maybe drop the lambda? direct version has similar/less indentation.
```
llvm::Optional Checks;
if(Pos < Buffer.size() && Buffer[Pos] == '(') {
  auto ClosingBracket = Buffer.find_first_of("\n)", ++Pos);
  if (ClosingBracket != Buffer.npos && Buffer[ClosingBracket] == ')')
   Checks = Buffer.slice(Pos, ClosingBracket).str();
}
```



Comment at: clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp:170
+
+NoLints.emplace_back(NoLintToken(*NoLintType, NoLintPos, Checks));
+  }

nit: you can drop the `NoLintToken`



Comment at: clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp:427
+NoLintDirectiveHandler::NoLintDirectiveHandler()
+: Impl(std::make_unique()) {}
+

no need for `class` here.



Comment at: clang-tools-extra/clang-tidy/NoLintDirectiveHandler.h:41
+  llvm::SmallVectorImpl &NoLintErrors,
+  bool AllowIO = true, bool EnableNoLintBlocks = true);
+

i feel like this is still an implementation detail of the ClangTidyContext, we 
have it publicly available just to reduce amount of extra code inside a single 
source file. so i think it makes sense for these parameters to not have 
defaults (as the one in ClangTidyContext already has).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116085

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


[PATCH] D118011: [RISCV] Adjust predicates and update intrinsic for clmul and clmulh in Zbkc extension

2022-01-24 Thread Alex via Phabricator via cfe-commits
alextsao1999 added a comment.

In D118011#3265374 , @Jimerlife wrote:

> In D118011#3265071 , @craig.topper 
> wrote:
>
>> Will you also be updating clang's RISCVBuiltins.def?
>>
>> Can you add tests?
>
> I update BuiltinsRISCV.def and add "__builtin_riscv_clmul_kc" intrinsic for 
> zbkc extension, but I am not sure if the name is  appropriate.

https://github.com/rvkrypto/rvkrypto-fips/blob/main/rvkintrin.md
Is the name as `__builtin_riscv_clmul_32` or `__builtin_riscv_clmul_64` better 
for conflict resolution?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118011

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


[PATCH] D98110: [NFC][clangd] Use table to collect option aliases

2022-01-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.

thanks, lgtm!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98110

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


[PATCH] D117846: [ASTMatchers] Add `isConstinit` matcher

2022-01-24 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117846

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


[clang] 589a939 - Add `isConstinit` matcher

2022-01-24 Thread Aaron Ballman via cfe-commits

Author: Evgeny Shulgin
Date: 2022-01-24T08:35:42-05:00
New Revision: 589a93907222cf2380198ca2172ff6697dd43d87

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

LOG: Add `isConstinit` matcher

Support C++20 constinit variables for AST Matchers.

Added: 


Modified: 
clang/docs/LibASTMatchersReference.html
clang/docs/ReleaseNotes.rst
clang/include/clang/ASTMatchers/ASTMatchers.h
clang/lib/ASTMatchers/Dynamic/Registry.cpp
clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Removed: 




diff  --git a/clang/docs/LibASTMatchersReference.html 
b/clang/docs/LibASTMatchersReference.html
index fb856de0936dc..4c3916c0325c9 100644
--- a/clang/docs/LibASTMatchersReference.html
+++ b/clang/docs/LibASTMatchersReference.html
@@ -5628,6 +5628,19 @@ Narrowing Matchers
 
 
 
+MatcherVarDecl>isConstinit
+Matches constinit 
variable declarations.
+
+Given:
+  constinit int foo = 42;
+  constinit const char* bar = "baz";
+  int baz = 42;
+  [[clang::require_constant_initialization]] int xyz = 42;
+varDecl(isConstinit())
+  matches the declaration of `foo` and `bar`, but not `baz` and `xyz`.
+
+
+
 MatcherVarDecl>isDefinition
 Matches if a 
declaration has a body attached.
 

diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 4fe037741256f..5cd2896de54d5 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -331,6 +331,8 @@ AST Matchers
   underlying type.
 - Added the ``isConsteval`` matcher to match ``consteval`` function
   declarations as well as `if consteval` and `if ! consteval` statements.
+- Added the ``isConstinit`` matcher to match ``constinit`` variable
+  declarations.
 
 clang-format
 

diff  --git a/clang/include/clang/ASTMatchers/ASTMatchers.h 
b/clang/include/clang/ASTMatchers/ASTMatchers.h
index c934b708cb96c..86bd44091b593 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchers.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -5211,6 +5211,23 @@ AST_POLYMORPHIC_MATCHER(isConstexpr,
   return Node.isConstexpr();
 }
 
+/// Matches constinit variable declarations.
+///
+/// Given:
+/// \code
+///   constinit int foo = 42;
+///   constinit const char* bar = "bar";
+///   int baz = 42;
+///   [[clang::require_constant_initialization]] int xyz = 42;
+/// \endcode
+/// varDecl(isConstinit())
+///   matches the declaration of `foo` and `bar`, but not `baz` and `xyz`.
+AST_MATCHER(VarDecl, isConstinit) {
+  if (const auto *CIA = Node.getAttr())
+return CIA->isConstinit();
+  return false;
+}
+
 /// Matches selection statements with initializer.
 ///
 /// Given:

diff  --git a/clang/lib/ASTMatchers/Dynamic/Registry.cpp 
b/clang/lib/ASTMatchers/Dynamic/Registry.cpp
index 2210c5413cc5a..47db6b51966ac 100644
--- a/clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ b/clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -406,6 +406,7 @@ RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(isConstQualified);
   REGISTER_MATCHER(isConsteval);
   REGISTER_MATCHER(isConstexpr);
+  REGISTER_MATCHER(isConstinit);
   REGISTER_MATCHER(isCopyAssignmentOperator);
   REGISTER_MATCHER(isCopyConstructor);
   REGISTER_MATCHER(isDefaultConstructor);

diff  --git a/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp 
b/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
index 51946e1430cf6..d1c9790401f02 100644
--- a/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ b/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -1841,6 +1841,25 @@ TEST_P(ASTMatchersTest, IsConstexpr_MatchesIfConstexpr) {
   notMatches("void baz() { if (1 > 0) {} }", ifStmt(isConstexpr(;
 }
 
+TEST_P(ASTMatchersTest, IsConstinit) {
+  if (!GetParam().isCXX20OrLater())
+return;
+
+  EXPECT_TRUE(matches("constinit int foo = 1;",
+  varDecl(hasName("foo"), isConstinit(;
+  EXPECT_TRUE(matches("extern constinit int foo;",
+  varDecl(hasName("foo"), isConstinit(;
+  EXPECT_TRUE(matches("constinit const char* foo = \"bar\";",
+  varDecl(hasName("foo"), isConstinit(;
+  EXPECT_TRUE(
+  notMatches("[[clang::require_constant_initialization]] int foo = 1;",
+ varDecl(hasName("foo"), isConstinit(;
+  EXPECT_TRUE(notMatches("constexpr int foo = 1;",
+ varDecl(hasName("foo"), isConstinit(;
+  EXPECT_TRUE(notMatches("static inline int foo = 1;",
+ varDecl(hasName("foo"), isConstinit(;
+}
+
 TEST_P(ASTMatchersTest, HasInitStatement_MatchesSelectionInitializers) {
   EXPECT_TRUE(notMatches("void baz() { if (1 > 0) {} }",
  ifStmt(hasInit

[PATCH] D118038: [clang][dataflow] Enable merging distinct values in Environment::join

2022-01-24 Thread Stanislav Gatev via Phabricator via cfe-commits
sgatev created this revision.
sgatev added reviewers: ymandel, xazax.hun, gribozavr2.
Herald added subscribers: steakhal, rnkovacs.
sgatev requested review of this revision.
Herald added a project: clang.

Make specializations of `DataflowAnalysis` extendable with domain-specific
logic for merging distinct values when joining environments. This could be
a strict lattice join or a more general widening operation.

This is part of the implementation of the dataflow analysis framework.
See "[RFC] A dataflow analysis framework for Clang AST" on cfe-dev.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118038

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/Value.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/TransferTest.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
@@ -9,6 +9,7 @@
 #include "NoopAnalysis.h"
 #include "TestingSupport.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Analysis/CFG.h"
@@ -16,6 +17,7 @@
 #include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/Value.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallSet.h"
@@ -35,8 +37,14 @@
 
 using namespace clang;
 using namespace dataflow;
+using namespace test;
+using namespace ast_matchers;
+using ::testing::_;
+using ::testing::ElementsAre;
 using ::testing::IsEmpty;
+using ::testing::NotNull;
 using ::testing::Pair;
+using ::testing::Test;
 using ::testing::UnorderedElementsAre;
 
 template 
@@ -174,7 +182,7 @@
   }
 };
 
-class NoreturnDestructorTest : public ::testing::Test {
+class NoreturnDestructorTest : public Test {
 protected:
   template 
   void runDataflow(llvm::StringRef Code, Matcher Expectations) {
@@ -300,4 +308,194 @@
   // FIXME: Called functions at point `p` should contain only "foo".
 }
 
+class OptionalIntAnalysis
+: public DataflowAnalysis {
+public:
+  explicit OptionalIntAnalysis(ASTContext &Context, BoolValue &HasValueTop)
+  : DataflowAnalysis(Context),
+HasValueTop(HasValueTop) {}
+
+  static NoopLattice initialElement() { return {}; }
+
+  void transfer(const Stmt *S, NoopLattice &, Environment &Env) {
+auto OptionalIntRecordDecl = recordDecl(hasName("OptionalInt"));
+auto HasOptionalIntType = hasType(OptionalIntRecordDecl);
+
+if (const auto *E = selectFirst(
+"call", match(cxxConstructExpr(HasOptionalIntType).bind("call"), *S,
+  getASTContext( {
+  auto &ConstructorVal = *cast(Env.createValue(E->getType()));
+  ConstructorVal.setProperty("has_value", Env.getBoolLiteralValue(false));
+  Env.setValue(*Env.getStorageLocation(*E, SkipPast::None), ConstructorVal);
+} else if (const auto *E = selectFirst(
+   "call",
+   match(cxxOperatorCallExpr(callee(cxxMethodDecl(ofClass(
+ OptionalIntRecordDecl
+ .bind("call"),
+ *S, getASTContext( {
+  assert(E->getNumArgs() > 0);
+  auto *Object = E->getArg(0);
+  assert(Object != nullptr);
+
+  auto *ObjectLoc =
+  Env.getStorageLocation(*Object, SkipPast::ReferenceThenPointer);
+  assert(ObjectLoc != nullptr);
+
+  auto &ConstructorVal =
+  *cast(Env.createValue(Object->getType()));
+  ConstructorVal.setProperty("has_value", Env.getBoolLiteralValue(true));
+  Env.setValue(*ObjectLoc, ConstructorVal);
+}
+  }
+
+  void merge(QualType Type, const Value &Val1, const Value &Val2,
+ Value &MergedVal, Environment &Env) final {
+if (!Type->isRecordType() ||
+Type->getAsCXXRecordDecl()->getQualifiedNameAsString() != "OptionalInt")
+  return;
+
+auto *HasValue1 =
+cast(cast(&Val1)->getProperty("has_value"));
+auto *HasValue2 =
+cast(cast(&Val2)->getProperty("has_value"

[PATCH] D117846: [ASTMatchers] Add `isConstinit` matcher

2022-01-24 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman closed this revision.
aaron.ballman added a comment.

I've commit on your behalf in 589a93907222cf2380198ca2172ff6697dd43d87 
, thanks 
for the new matcher!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117846

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


[clang-tools-extra] a0d5e93 - Add missing include llvm/ADT/STLExtras

2022-01-24 Thread via cfe-commits

Author: serge-sans-paille
Date: 2022-01-24T14:41:24+01:00
New Revision: a0d5e938fe9c1fd6ca492a91cdc3d841aa03fc0d

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

LOG: Add missing include llvm/ADT/STLExtras

Added: 


Modified: 
clang-tools-extra/clang-tidy/GlobList.cpp
llvm/include/llvm/ADT/CoalescingBitVector.h

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/GlobList.cpp 
b/clang-tools-extra/clang-tidy/GlobList.cpp
index b594d943cc075..fe41feef38abf 100644
--- a/clang-tools-extra/clang-tidy/GlobList.cpp
+++ b/clang-tools-extra/clang-tidy/GlobList.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "GlobList.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 
 namespace clang {

diff  --git a/llvm/include/llvm/ADT/CoalescingBitVector.h 
b/llvm/include/llvm/ADT/CoalescingBitVector.h
index 18803ecf209f4..82e2e1a9f9e25 100644
--- a/llvm/include/llvm/ADT/CoalescingBitVector.h
+++ b/llvm/include/llvm/ADT/CoalescingBitVector.h
@@ -15,6 +15,7 @@
 #define LLVM_ADT_COALESCINGBITVECTOR_H
 
 #include "llvm/ADT/IntervalMap.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/Debug.h"



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


[PATCH] D118038: [clang][dataflow] Enable merging distinct values in Environment::join

2022-01-24 Thread Stanislav Gatev via Phabricator via cfe-commits
sgatev updated this revision to Diff 402490.
sgatev added a comment.

Add missing include.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118038

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/Value.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/TransferTest.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
@@ -9,6 +9,7 @@
 #include "NoopAnalysis.h"
 #include "TestingSupport.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Analysis/CFG.h"
@@ -16,6 +17,7 @@
 #include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/Value.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallSet.h"
@@ -35,8 +37,14 @@
 
 using namespace clang;
 using namespace dataflow;
+using namespace test;
+using namespace ast_matchers;
+using ::testing::_;
+using ::testing::ElementsAre;
 using ::testing::IsEmpty;
+using ::testing::NotNull;
 using ::testing::Pair;
+using ::testing::Test;
 using ::testing::UnorderedElementsAre;
 
 template 
@@ -174,7 +182,7 @@
   }
 };
 
-class NoreturnDestructorTest : public ::testing::Test {
+class NoreturnDestructorTest : public Test {
 protected:
   template 
   void runDataflow(llvm::StringRef Code, Matcher Expectations) {
@@ -300,4 +308,194 @@
   // FIXME: Called functions at point `p` should contain only "foo".
 }
 
+class OptionalIntAnalysis
+: public DataflowAnalysis {
+public:
+  explicit OptionalIntAnalysis(ASTContext &Context, BoolValue &HasValueTop)
+  : DataflowAnalysis(Context),
+HasValueTop(HasValueTop) {}
+
+  static NoopLattice initialElement() { return {}; }
+
+  void transfer(const Stmt *S, NoopLattice &, Environment &Env) {
+auto OptionalIntRecordDecl = recordDecl(hasName("OptionalInt"));
+auto HasOptionalIntType = hasType(OptionalIntRecordDecl);
+
+if (const auto *E = selectFirst(
+"call", match(cxxConstructExpr(HasOptionalIntType).bind("call"), *S,
+  getASTContext( {
+  auto &ConstructorVal = *cast(Env.createValue(E->getType()));
+  ConstructorVal.setProperty("has_value", Env.getBoolLiteralValue(false));
+  Env.setValue(*Env.getStorageLocation(*E, SkipPast::None), ConstructorVal);
+} else if (const auto *E = selectFirst(
+   "call",
+   match(cxxOperatorCallExpr(callee(cxxMethodDecl(ofClass(
+ OptionalIntRecordDecl
+ .bind("call"),
+ *S, getASTContext( {
+  assert(E->getNumArgs() > 0);
+  auto *Object = E->getArg(0);
+  assert(Object != nullptr);
+
+  auto *ObjectLoc =
+  Env.getStorageLocation(*Object, SkipPast::ReferenceThenPointer);
+  assert(ObjectLoc != nullptr);
+
+  auto &ConstructorVal =
+  *cast(Env.createValue(Object->getType()));
+  ConstructorVal.setProperty("has_value", Env.getBoolLiteralValue(true));
+  Env.setValue(*ObjectLoc, ConstructorVal);
+}
+  }
+
+  void merge(QualType Type, const Value &Val1, const Value &Val2,
+ Value &MergedVal, Environment &Env) final {
+if (!Type->isRecordType() ||
+Type->getAsCXXRecordDecl()->getQualifiedNameAsString() != "OptionalInt")
+  return;
+
+auto *HasValue1 =
+cast(cast(&Val1)->getProperty("has_value"));
+auto *HasValue2 =
+cast(cast(&Val2)->getProperty("has_value"));
+if (HasValue1 == HasValue2) {
+  cast(&MergedVal)->setProperty("has_value", *HasValue1);
+} else {
+  cast(&MergedVal)->setProperty("has_value", HasValueTop);
+}
+  }
+
+private:
+  BoolValue &HasValueTop;
+};
+
+class WideningTest : public Test {
+protected:
+  template 
+  void runDataflow(llvm::StringRef Code, Matcher Match) {
+tooling::FileContentMappings FilesCo

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

2022-01-24 Thread Adrian Vogelsgesang via Phabricator via cfe-commits
avogelsgesang added a comment.

Thanks, @xazax.hun and @whisperity for helping me to get this over the finish 
line! :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112646

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


[PATCH] D118015: [RISCV][NFC] Rename RequiredExtensions to RequiredFeatures.

2022-01-24 Thread Zakk Chen via Phabricator via cfe-commits
khchen accepted this revision.
khchen added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118015

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


[PATCH] D116266: [SPIR-V] Add linking of separate translation units using spirv-link

2022-01-24 Thread Alexey Bader via Phabricator via cfe-commits
bader added inline comments.



Comment at: clang/docs/UsersManual.rst:3602
 
+Linking is done using ``spirv-link`` from `the SPIRV-Tools project
+`_. Similar to other 
external

@Anastasia, sorry for late feedback.
I think being able to link SPIR-V modules is a great feature, but I have a 
concerns regarding `spirv-link` tool.
The documentation says that the linker tool is still under development and from 
our experience this tool had issues blocking us from using it for SYCL mode. 
The last time new features were added to this tool is almost 4 year ago.
Do you know if there are any plans for to finish the development and if ? Are 
you aware of any "real-world usages" of this tool? Have you tried to use it for 
SPIR-V module produced from C++ (e.g. C++ for OpenCL)?
I think supporting SPIR-V extensions like [[ 
https://github.com/KhronosGroup/SPIRV-Registry/blob/main/extensions/KHR/SPV_KHR_linkonce_odr.asciidoc
 | SPV_KHR_linkonce_odr ]] is quite important for code size and JIT compilation 
time reduction. As this extension was ratified recently, I suppose `spirv-link` 
doesn't support it yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116266

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


[PATCH] D117939: [clang-tidy] Add more documentation about check development

2022-01-24 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

Thank you so much for working on this documentation, I really like the 
direction it's going!




Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:78
+build :program:`clang-tidy`.
+Since your new check will have associated documentation, you will also want to 
install
+`Sphinx `_ and enable it in the CMake 
configuration.





Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:228
 
+If you need to interact macros and preprocessor directives, you will want to
+override the method ``registerPPCallbacks``.  The ``add_new_check.py`` script





Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:229
+If you need to interact macros and preprocessor directives, you will want to
+override the method ``registerPPCallbacks``.  The ``add_new_check.py`` script
+does not generate an override for this method in the starting point for your

As usual, we're not super consistent, but most of our documentation is 
single-spaced (can you correct this throughout your changes?).



Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:233
+
+If your check applies only to specific dialect of C or C++, you will
+want to override the method ``isLanguageVersionSupported`` to reflect that.

I made it more generic, you can use this for more than just checking languages 
(you can check for other language features like whether `char` is signed or 
unsigned, etc).



Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:262
+Clang is implemented on top of LLVM and introduces its own set of classes that 
you
+will interact with while writing your check.  The most important of these are 
the
+classes relating the source code locations, source files, ranges of source 
locations

I'd argue the most important thing you interact with from Clang are the AST 
nodes. Maybe instead of "most important", we can be vague and just say "Some 
commonly used features are" or something like that?



Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:301-317
+The Transformer library allows you to write a check that transforms source 
code by
+expressing the transformation as a ``RewriteRule``.  The Transformer library 
provides
+functions for composing edits to source code to create rewrite rules.  Unless 
you need
+to perform low-level source location manipulation, you may want to consider 
writing your
+check with the Transformer library.  The `Clang Transformer Tutorial
+`_ describes the 
Transformer
+library in detail.

I think this documentation is really good, but at the same time, I don't think 
we have any clang-tidy checks that make use of the transformer library 
currently. I don't see a reason we shouldn't document this more prominently, 
but I'd like to hear from @ymandel and/or @alexfh whether they think the 
library is ready for officially supported use within clang-tidy and whether we 
need any sort of broader community discussion. (I don't see any issues here, 
just crossing t's and dotting i's in terms of process.)



Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:339-340
+matching expressions to simplify your matcher.  Just like breaking up a huge 
function
+into smaller chunks with intention revealing names can help you understand a 
complex
+algorithm, breaking up a matcher into smaller matchers with intention 
revealing names
+can help you understand a complicated matcher.  Once you have a working 
matcher, the





Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:343
+C++ API will be virtually identical to your interactively constructed matcher. 
 You can
+use local variables to preserve your intention revealing names that you 
applied to
+nested matchers.





Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:360-361
+
+Private custom matchers are a good example of auxiliary support code for your 
check
+that is best tested with a unit test.  It will be easier to test your matchers 
or
+other support classes by writing a unit test than by writing a ``FileCheck`` 
integration

Do we have any private matchers that are unit tested? My understanding was that 
the public matchers were all unit tested, but that the matchers which are local 
to a check are not exposed via a header file, and so they're not really unit 
testable.



Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:370
+Once you've covered your check with the basic "happy path" scenarios, you'll 
want to
+torture your check with some crazy C++ in order to ensure your check is 
robust.  Running
+your check on a large code base

[clang] 5e5efd8 - [clang-format] Fix SeparateDefinitionBlocks issues

2022-01-24 Thread via cfe-commits

Author: ksyx
Date: 2022-01-24T14:23:20Z
New Revision: 5e5efd8a91f2e340e79a73bedbc6ab66ad4a4281

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

LOG: [clang-format] Fix SeparateDefinitionBlocks issues

- Fixes https://github.com/llvm/llvm-project/issues/53227 that wrongly
  indents multiline comments
- Fixes wrong detection of single-line opening braces when used along
  with those only opening scopes, causing crashes due to duplicated
  replacements on the same token:
void foo()
{
  {
int x;
  }
}
- Fixes wrong recognition of first line of definition when the line
  starts with block comment, causing crashes due to duplicated
  replacements on the same token for this leads toward skipping the line
  starting with inline block comment:
/*
  Some descriptions about function
*/
/*inline*/ void bar() {
}
- Fixes wrong recognition of enum when used as a type name rather than
  starting definition block, causing crashes due to duplicated
  replacements on the same token since both actions for enum and for
  definition blocks were taken place:
void foobar(const enum EnumType e) {
}
- Change to use function keyword for JavaScript instead of comparing
  strings
- Resolves formatting conflict with options EmptyLineAfterAccessModifier
  and EmptyLineBeforeAccessModifier (prompts with --dry-run (-n) or
  --output-replacement-xml but no observable change)
- Recognize long (len>=5) uppercased name taking a single line as return
  type and fix the problem of adding newline below it, with adding new
  token type FunctionLikeOrFreestandingMacro and marking tokens in
  UnwrappedLineParser:
void
afunc(int x) {
  return;
}
TYPENAME
func(int x, int y) {
  // ...
}
- Remove redundant and repeated initialization
- Do no change to newlines before EOF

Reviewed By: MyDeveloperDay, curdeius, HazardyKnusperkeks
Differential Revision: https://reviews.llvm.org/D117520

Added: 


Modified: 
clang/lib/Format/DefinitionBlockSeparator.cpp
clang/lib/Format/DefinitionBlockSeparator.h
clang/lib/Format/FormatToken.h
clang/lib/Format/TokenAnnotator.cpp
clang/lib/Format/UnwrappedLineParser.cpp
clang/unittests/Format/DefinitionBlockSeparatorTest.cpp

Removed: 




diff  --git a/clang/lib/Format/DefinitionBlockSeparator.cpp 
b/clang/lib/Format/DefinitionBlockSeparator.cpp
index d09cd0bd33fbe..827564357f788 100644
--- a/clang/lib/Format/DefinitionBlockSeparator.cpp
+++ b/clang/lib/Format/DefinitionBlockSeparator.cpp
@@ -25,22 +25,27 @@ std::pair 
DefinitionBlockSeparator::analyze(
   assert(Style.SeparateDefinitionBlocks != FormatStyle::SDS_Leave);
   AffectedRangeMgr.computeAffectedLines(AnnotatedLines);
   tooling::Replacements Result;
-  separateBlocks(AnnotatedLines, Result);
+  separateBlocks(AnnotatedLines, Result, Tokens);
   return {Result, 0};
 }
 
 void DefinitionBlockSeparator::separateBlocks(
-SmallVectorImpl &Lines, tooling::Replacements &Result) {
+SmallVectorImpl &Lines, tooling::Replacements &Result,
+FormatTokenLexer &Tokens) {
   const bool IsNeverStyle =
   Style.SeparateDefinitionBlocks == FormatStyle::SDS_Never;
-  auto LikelyDefinition = [this](const AnnotatedLine *Line) {
+  const AdditionalKeywords &ExtraKeywords = Tokens.getKeywords();
+  auto LikelyDefinition = [this, ExtraKeywords](const AnnotatedLine *Line,
+bool ExcludeEnum = false) {
 if ((Line->MightBeFunctionDecl && Line->mightBeFunctionDefinition()) ||
 Line->startsWithNamespace())
   return true;
 FormatToken *CurrentToken = Line->First;
 while (CurrentToken) {
-  if (CurrentToken->isOneOf(tok::kw_class, tok::kw_struct, tok::kw_enum) ||
-  (Style.isJavaScript() && CurrentToken->TokenText == "function"))
+  if (CurrentToken->isOneOf(tok::kw_class, tok::kw_struct) ||
+  (Style.isJavaScript() && 
CurrentToken->is(ExtraKeywords.kw_function)))
+return true;
+  if (!ExcludeEnum && CurrentToken->is(tok::kw_enum))
 return true;
   CurrentToken = CurrentToken->Next;
 }
@@ -63,18 +68,25 @@ void DefinitionBlockSeparator::separateBlocks(
 AnnotatedLine *TargetLine;
 auto OpeningLineIndex = CurrentLine->MatchingOpeningBlockLineIndex;
 AnnotatedLine *OpeningLine = nullptr;
+const auto IsAccessSpecifierToken = [](const FormatToken *Token) {
+  return Token->isAccessSpecifier() || Token->isObjCAccessSpecifier();
+};
 const auto InsertReplacement = [&](const int NewlineToInsert) {
   assert(TargetLine);
   assert(TargetToken);
 
   // Do not handle EOF newlines.
-  if (TargetToken->is(tok::eof) && NewlineToInsert > 0)
+  if (TargetToken->is(tok::eof))
+

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

2022-01-24 Thread ksyx via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
ksyx marked 2 inline comments as done.
Closed by commit rG5e5efd8a91f2: [clang-format] Fix SeparateDefinitionBlocks 
issues (authored by ksyx).

Changed prior to commit:
  https://reviews.llvm.org/D117520?vs=402078&id=402498#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117520

Files:
  clang/lib/Format/DefinitionBlockSeparator.cpp
  clang/lib/Format/DefinitionBlockSeparator.h
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/DefinitionBlockSeparatorTest.cpp

Index: clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
===
--- clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
+++ clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
@@ -131,6 +131,73 @@
"\n"
"enum Bar { FOOBAR, BARFOO };\n",
Style);
+
+  FormatStyle BreakAfterReturnTypeStyle = Style;
+  BreakAfterReturnTypeStyle.AlwaysBreakAfterReturnType = FormatStyle::RTBS_All;
+  // Test uppercased long typename
+  verifyFormat("class Foo {\n"
+   "  void\n"
+   "  Bar(int t, int p) {\n"
+   "int r = t + p;\n"
+   "return r;\n"
+   "  }\n"
+   "\n"
+   "  HRESULT\n"
+   "  Foobar(int t, int p) {\n"
+   "int r = t * p;\n"
+   "return r;\n"
+   "  }\n"
+   "}\n",
+   BreakAfterReturnTypeStyle);
+}
+
+TEST_F(DefinitionBlockSeparatorTest, FormatConflict) {
+  FormatStyle Style = getLLVMStyle();
+  Style.SeparateDefinitionBlocks = FormatStyle::SDS_Always;
+  llvm::StringRef Code = "class Test {\n"
+ "public:\n"
+ "  static void foo() {\n"
+ "int t;\n"
+ "return 1;\n"
+ "  }\n"
+ "};";
+  std::vector Ranges = {1, tooling::Range(0, Code.size())};
+  EXPECT_EQ(reformat(Style, Code, Ranges, "").size(), 0u);
+}
+
+TEST_F(DefinitionBlockSeparatorTest, CommentBlock) {
+  FormatStyle Style = getLLVMStyle();
+  Style.SeparateDefinitionBlocks = FormatStyle::SDS_Always;
+  std::string Prefix = "enum Foo { FOO, BAR };\n"
+   "\n"
+   "/*\n"
+   "test1\n"
+   "test2\n"
+   "*/\n"
+   "int foo(int i, int j) {\n"
+   "  int r = i + j;\n"
+   "  return r;\n"
+   "}\n";
+  std::string Suffix = "enum Bar { FOOBAR, BARFOO };\n"
+   "\n"
+   "/* Comment block in one line*/\n"
+   "int bar3(int j, int k) {\n"
+   "  // A comment\n"
+   "  int r = j % k;\n"
+   "  return r;\n"
+   "}\n";
+  std::string CommentedCode = "/*\n"
+  "int bar2(int j, int k) {\n"
+  "  int r = j / k;\n"
+  "  return r;\n"
+  "}\n"
+  "*/\n";
+  verifyFormat(removeEmptyLines(Prefix) + "\n" + CommentedCode + "\n" +
+   removeEmptyLines(Suffix),
+   Style, Prefix + "\n" + CommentedCode + "\n" + Suffix);
+  verifyFormat(removeEmptyLines(Prefix) + "\n" + CommentedCode +
+   removeEmptyLines(Suffix),
+   Style, Prefix + "\n" + CommentedCode + Suffix);
 }
 
 TEST_F(DefinitionBlockSeparatorTest, UntouchBlockStartStyle) {
@@ -175,13 +242,15 @@
   FormatStyle NeverStyle = getLLVMStyle();
   NeverStyle.SeparateDefinitionBlocks = FormatStyle::SDS_Never;
 
-  auto TestKit = MakeUntouchTest("#ifdef FOO\n\n", "\n#elifndef BAR\n\n",
- "\n#endif\n\n", false);
+  auto TestKit = MakeUntouchTest("/* FOOBAR */\n"
+ "#ifdef FOO\n\n",
+ "\n#elifndef BAR\n\n", "\n#endif\n\n", false);
   verifyFormat(TestKit.first, AlwaysStyle, TestKit.second);
   verifyFormat(TestKit.second, NeverStyle, removeEmptyLines(TestKit.second));
 
-  TestKit =
-  MakeUntouchTest("#ifdef FOO\n", "#elifndef BAR\n", "#endif\n", false);
+  TestKit = MakeUntouchTest("/* FOOBAR */\n"
+"#ifdef FOO\n",
+"#elifndef BAR\n", "#endif\n", false);
   verifyFormat(TestKit.first, AlwaysStyle, TestKit.second);
   verifyFormat(TestKit.second, NeverStyle, removeEmptyLines(TestKit.second));
 
@@ -213,7 +282,7 @@
   "test1\n"
   "test2\n"
   "*/\n"
-  

[PATCH] D118038: [clang][dataflow] Enable merging distinct values in Environment::join

2022-01-24 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment.

Interesting. So clients can affect how the environment is being merged. As a 
result, we potentially cannot run multiple clients in the same fixed-point 
iteration, each of them will require separate passes.

I think this is OK, just wanted to to be explicit.




Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:67
+///  `Val1` and `Val2` must be distinct.
+virtual void merge(QualType Type, const Value &Val1, const Value &Val2,
+   Value &MergedVal, Environment &Env) {}

Previously, when the values were distinct, we did not include anything in the 
merged environment. With the new model, we will end up creating "default" 
values for every one of them. I wonder if this is wasteful. We could 
potentially also defer this until we have some real world data and can 
benchmark this. But I think we could consider changing the return type to bool 
to specify if the merged value should be included in the resulting environment 
at all and this could return false by default.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118038

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


[PATCH] D93298: [RISCV] add the MC layer support of Zfinx extension

2022-01-24 Thread Alex Bradbury via Phabricator via cfe-commits
asb added a comment.
Herald added a subscriber: pcwang-thead.

Thanks for your work on this. The way you've managed to use multiclasses to 
handle this with the 'ExtInfo' definitions takes a bit of unpicking to follow, 
but it does a really good job of keeping the instruction definitions largely 
unmodified. Nice!

I've got a few requests/questions on the test coverage:

- I'm not sure the inclusion of the load/store instructions in the z[d,f,h]inx 
test cases has much value, given those instructions are unmodified?
- Given that the set of F/D/Zfh instructions that _aren't_ supported under 
z[d,f,h]inx is relatively short, it's probably worth being exhaustive in the 
*-invalid.s test cases. It's only 8 instructions for Zfinx for instance.
- The *-invalid.s test cases should have coverage for using a floating point 
register with an operation that is supported by Z[d,f,h]inx
- The zdinx test cases don't include any coverage for when odd registers are 
used.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93298

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


[PATCH] D117181: [PowerPC] Use IEEE long double in proper toolchain

2022-01-24 Thread Jinsong Ji via Phabricator via cfe-commits
jsji added a comment.

> Besides, would it be acceptable that we add a variable to cmake to determine 
> default long double semantics (like current GCC)?

Yes, we can. But I think that can be in another patch, and we shouldn't rely on 
it -- as we shouldn't assume user will config/build clang/llvm on the target 
machine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117181

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


[PATCH] D117939: [clang-tidy] Add more documentation about check development

2022-01-24 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment.

@aaron.ballman I think this has exposed some limitations with the add-new-check 
script. Maybe there is merit for the script to be updated to support 
preprocessor callbacks and options, WDYT?


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

https://reviews.llvm.org/D117939

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


[PATCH] D117939: [clang-tidy] Add more documentation about check development

2022-01-24 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added inline comments.



Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:229
+If you need to interact macros and preprocessor directives, you will want to
+override the method ``registerPPCallbacks``.  The ``add_new_check.py`` script
+does not generate an override for this method in the starting point for your

aaron.ballman wrote:
> As usual, we're not super consistent, but most of our documentation is 
> single-spaced (can you correct this throughout your changes?).
People keep asking for this, but it doesn't matter for the final output and it 
isn't
specified anywhere in the style guide.



Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:233
+
+If your check applies only to specific dialect of C or C++, you will
+want to override the method ``isLanguageVersionSupported`` to reflect that.

aaron.ballman wrote:
> I made it more generic, you can use this for more than just checking 
> languages (you can check for other language features like whether `char` is 
> signed or unsigned, etc).
This came up in another review, but if you have a check that applies
to C++11 or later, do you have to check all the versions or can you
assume that the C++11 flag will be set when C++14 is requested
via command-line options?



Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:301-317
+The Transformer library allows you to write a check that transforms source 
code by
+expressing the transformation as a ``RewriteRule``.  The Transformer library 
provides
+functions for composing edits to source code to create rewrite rules.  Unless 
you need
+to perform low-level source location manipulation, you may want to consider 
writing your
+check with the Transformer library.  The `Clang Transformer Tutorial
+`_ describes the 
Transformer
+library in detail.

aaron.ballman wrote:
> I think this documentation is really good, but at the same time, I don't 
> think we have any clang-tidy checks that make use of the transformer library 
> currently. I don't see a reason we shouldn't document this more prominently, 
> but I'd like to hear from @ymandel and/or @alexfh whether they think the 
> library is ready for officially supported use within clang-tidy and whether 
> we need any sort of broader community discussion. (I don't see any issues 
> here, just crossing t's and dotting i's in terms of process.)
There are at least two checks that use the Transformer library currently.



Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:360-361
+
+Private custom matchers are a good example of auxiliary support code for your 
check
+that is best tested with a unit test.  It will be easier to test your matchers 
or
+other support classes by writing a unit test than by writing a ``FileCheck`` 
integration

aaron.ballman wrote:
> Do we have any private matchers that are unit tested? My understanding was 
> that the public matchers were all unit tested, but that the matchers which 
> are local to a check are not exposed via a header file, and so they're not 
> really unit testable.
I just did this for my switch/case/label update to simplify boolean expressions.
You do have to expose them via a header file, which isn't a big deal.


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

https://reviews.llvm.org/D117939

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


[PATCH] D117939: [clang-tidy] Add more documentation about check development

2022-01-24 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment.

In D117939#3266228 , @njames93 wrote:

> @aaron.ballman I think this has exposed some limitations with the 
> add-new-check script. Maybe there is merit for the script to be updated to 
> support preprocessor callbacks and options, WDYT?

It could certainly use an option to specify that your check is Transformer
based.


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

https://reviews.llvm.org/D117939

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


[PATCH] D93298: [RISCV] add the MC layer support of Zfinx extension

2022-01-24 Thread Alex Bradbury via Phabricator via cfe-commits
asb added a comment.

Not an issue for this MC-layer patch, but I've created 
https://github.com/riscv/riscv-zfinx/issues/14 to point out what seems to be an 
incorrect statement about the status quo on the ABI for 32-bit floating point 
types on RV64 in the Zfinx spec.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93298

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


[PATCH] D56303: [clang-tidy] Recognize labelled statements when simplifying boolean exprs

2022-01-24 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment.

A large portion of the changes, especially in the checks implementation file, 
appear to be NFC stylistic or formatting only fixes. While these changes are 
generally good, they shouldn't be a part of this patch. Instead they should be 
committed in their own NFC patch.
This makes it much easier to review the relevant changes needed to implement 
this new behaviour.


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

https://reviews.llvm.org/D56303

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


[PATCH] D117795: [AArch64] Fixes for strict FP vector instructions

2022-01-24 Thread Cameron McInally via Phabricator via cfe-commits
cameron.mcinally added a comment.

Is it possible to break the 4 subtasks into separate reviews?

  -In clang generate fcmps when appropriate for neon intrinsics
  -Fix legalization of scalarized strict FP vector operations
  -Add some missing strict FP handling to AArch64TargetLowering
  -Adjust the aarch64-neon-intrinsics-constrained.c clang test to expect the 
right output and un-XFAIL it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117795

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


[PATCH] D113237: [RISCV] Support I extension version 2.1

2022-01-24 Thread Zakk Chen via Phabricator via cfe-commits
khchen added a comment.
Herald added subscribers: pcwang-thead, eopXD.

https://groups.google.com/a/groups.riscv.org/g/sw-dev/c/aE1ZeHHCYf4
RISC-V GNU toolchain are going to bump the default ISA spec to 20191213, which 
means will default with I 2.1, A 2.1, F 2.2 and D 2.2. 
I think it will be good if LLVM could supports I 2.1 and have the same default 
ISA version with gnu...

@achieveartificialintelligence Do you know what is the relation between your 
patch and D115921 ?




Comment at: clang/lib/Basic/Targets/RISCV.cpp:236
   unsigned XLen = getTriple().isArch64Bit() ? 64 : 32;
-  auto ParseResult = llvm::RISCVISAInfo::parseFeatures(XLen, Features);
+  auto ParseResult = llvm::RISCVISAInfo::parseFeatures(XLen, false, Features);
   if (!ParseResult) {

```
auto ParseResult = llvm::RISCVISAInfo::parseFeatures(XLen, /*IsI2p1=*/ false, 
Features);
```



Comment at: clang/test/Driver/riscv-arch.c:3
 // RUN: -fsyntax-only 2>&1 | FileCheck %s
 // RUN: %clang -target riscv32-unknown-elf -march=rv32i2p0 -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck %s

could we have a check for `-march=rv32i2p1`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113237

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


[clang] 3ad35ba - [Templight] Don't display empty strings for names of unnamed template parameters

2022-01-24 Thread Kristóf Umann via cfe-commits

Author: Kristóf Umann
Date: 2022-01-24T16:37:11+01:00
New Revision: 3ad35ba4dea5240dd58476f0c85f0fe096d6c7ce

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

LOG: [Templight] Don't display empty strings for names of unnamed template 
parameters

Patch originally by oktal3000: 
https://github.com/mikael-s-persson/templight/pull/40

When a template parameter is unnamed, the name of -templight-dump might return
an empty string. This is fine, they are unnamed after all, but it might be more
user friendly to at least describe what entity is unnamed.

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

Added: 
clang/test/Templight/templight-empty-entries-fix.cpp

Modified: 
clang/lib/Frontend/FrontendActions.cpp

Removed: 




diff  --git a/clang/lib/Frontend/FrontendActions.cpp 
b/clang/lib/Frontend/FrontendActions.cpp
index 5b77c3e01aace..ad2e6039477f8 100644
--- a/clang/lib/Frontend/FrontendActions.cpp
+++ b/clang/lib/Frontend/FrontendActions.cpp
@@ -8,9 +8,10 @@
 
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/AST/ASTConsumer.h"
+#include "clang/AST/Decl.h"
 #include "clang/Basic/FileManager.h"
-#include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/LangStandard.h"
+#include "clang/Basic/TargetInfo.h"
 #include "clang/Frontend/ASTConsumers.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
@@ -23,6 +24,7 @@
 #include "clang/Sema/TemplateInstCallback.h"
 #include "clang/Serialization/ASTReader.h"
 #include "clang/Serialization/ASTWriter.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
@@ -480,25 +482,92 @@ class DefaultTemplateInstCallback : public 
TemplateInstantiationCallback {
 Out << "---" << YAML << "\n";
   }
 
+  static void printEntryName(const Sema &TheSema, const Decl *Entity,
+ llvm::raw_string_ostream &OS) {
+auto *NamedTemplate = cast(Entity);
+
+PrintingPolicy Policy = TheSema.Context.getPrintingPolicy();
+// FIXME: Also ask for FullyQualifiedNames?
+Policy.SuppressDefaultTemplateArgs = false;
+NamedTemplate->getNameForDiagnostic(OS, Policy, true);
+
+if (!OS.str().empty())
+  return;
+
+Decl *Ctx = Decl::castFromDeclContext(NamedTemplate->getDeclContext());
+NamedDecl *NamedCtx = dyn_cast_or_null(Ctx);
+
+if (const auto *Decl = dyn_cast(NamedTemplate)) {
+  if (const auto *R = dyn_cast(Decl)) {
+if (R->isLambda()) {
+  OS << "lambda at ";
+  Decl->getLocation().print(OS, TheSema.getSourceManager());
+  return;
+}
+  }
+  OS << "unnamed " << Decl->getKindName();
+  return;
+}
+
+if (const auto *Decl = dyn_cast(NamedTemplate)) {
+  OS << "unnamed function parameter " << Decl->getFunctionScopeIndex()
+ << " ";
+  if (Decl->getFunctionScopeDepth() > 0)
+OS << "(at depth " << Decl->getFunctionScopeDepth() << ") ";
+  OS << "of ";
+  NamedCtx->getNameForDiagnostic(OS, TheSema.getLangOpts(), true);
+  return;
+}
+
+if (const auto *Decl = dyn_cast(NamedTemplate)) {
+  if (const Type *Ty = Decl->getTypeForDecl()) {
+if (const auto *TTPT = dyn_cast_or_null(Ty)) {
+  OS << "unnamed template type parameter " << TTPT->getIndex() << " ";
+  if (TTPT->getDepth() > 0)
+OS << "(at depth " << TTPT->getDepth() << ") ";
+  OS << "of ";
+  NamedCtx->getNameForDiagnostic(OS, TheSema.getLangOpts(), true);
+  return;
+}
+  }
+}
+
+if (const auto *Decl = dyn_cast(NamedTemplate)) {
+  OS << "unnamed template non-type parameter " << Decl->getIndex() << " ";
+  if (Decl->getDepth() > 0)
+OS << "(at depth " << Decl->getDepth() << ") ";
+  OS << "of ";
+  NamedCtx->getNameForDiagnostic(OS, TheSema.getLangOpts(), true);
+  return;
+}
+
+if (const auto *Decl = dyn_cast(NamedTemplate)) {
+  OS << "unnamed template template parameter " << Decl->getIndex() << " ";
+  if (Decl->getDepth() > 0)
+OS << "(at depth " << Decl->getDepth() << ") ";
+  OS << "of ";
+  NamedCtx->getNameForDiagnostic(OS, TheSema.getLangOpts(), true);
+  return;
+}
+
+llvm_unreachable("Failed to retrieve a name for this entry!");
+OS << "unnamed identifier";
+  }
+
   template 
   static TemplightEntry getTemplightEntry(const Sema &TheSema,
   const CodeSynthesisContext &Inst) {
 TemplightEntry Entry;
 Entry.Kind = toString(Inst.Kind);
 Entry.Event = BeginInstantiation ? "Begin" : "End";
-if (auto *NamedTemplate = dyn_cast_or_null(Inst.Entity)) {
-  llvm::raw_str

[PATCH] D115521: [Templight] Don't display empty strings for names of unnamed template parameters

2022-01-24 Thread Kristóf Umann 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 rG3ad35ba4dea5: [Templight] Don't display empty strings 
for names of unnamed template parameters (authored by Szelethus).

Changed prior to commit:
  https://reviews.llvm.org/D115521?vs=397054&id=402527#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115521

Files:
  clang/lib/Frontend/FrontendActions.cpp
  clang/test/Templight/templight-empty-entries-fix.cpp

Index: clang/test/Templight/templight-empty-entries-fix.cpp
===
--- /dev/null
+++ clang/test/Templight/templight-empty-entries-fix.cpp
@@ -0,0 +1,333 @@
+// RUN: %clang_cc1 -templight-dump -Wno-unused-value %s 2>&1 | FileCheck %s
+
+void a() {
+  [] {};
+}
+
+// CHECK-LABEL: {{^---$}}
+// CHECK: {{^name:[ ]+'lambda at .*templight-empty-entries-fix.cpp:4:3'$}}
+// CHECK: {{^kind:[ ]+Memoization$}}
+// CHECK: {{^event:[ ]+Begin$}}
+// CHECK: {{^orig:[ ]+'.*templight-empty-entries-fix.cpp:4:3'$}}
+// CHECK: {{^poi:[ ]+'.*templight-empty-entries-fix.cpp:4:3'$}}
+// CHECK-LABEL: {{^---$}}
+// CHECK: {{^name:[ ]+'lambda at .*templight-empty-entries-fix.cpp:4:3'$}}
+// CHECK: {{^kind:[ ]+Memoization$}}
+// CHECK: {{^event:[ ]+End$}}
+// CHECK: {{^orig:[ ]+'.*templight-empty-entries-fix.cpp:4:3'$}}
+// CHECK: {{^poi:[ ]+'.*templight-empty-entries-fix.cpp:4:3'$}}
+
+template  void a() { a(); }
+
+// CHECK-LABEL: {{^---$}}
+// CHECK: {{^name:[ ]+a$}}
+// CHECK: {{^kind:[ ]+DeducedTemplateArgumentSubstitution$}}
+// CHECK: {{^event:[ ]+Begin$}}
+// CHECK: {{^orig:[ ]+'.*templight-empty-entries-fix.cpp:20:25'$}}
+// CHECK: {{^poi:[ ]+'.*templight-empty-entries-fix.cpp:20:31'$}}
+// CHECK-LABEL: {{^---$}}
+// CHECK: {{^name:[ ]+unnamed template non-type parameter 0 of a$}}
+// CHECK: {{^kind:[ ]+DefaultTemplateArgumentInstantiation$}}
+// CHECK: {{^event:[ ]+Begin$}}
+// CHECK: {{^orig:[ ]+'.*templight-empty-entries-fix.cpp:20:15'$}}
+// CHECK: {{^poi:[ ]+'.*templight-empty-entries-fix.cpp:20:25'$}}
+// CHECK-LABEL: {{^---$}}
+// CHECK: {{^name:[ ]+unnamed template non-type parameter 0 of a$}}
+// CHECK: {{^kind:[ ]+DefaultTemplateArgumentInstantiation$}}
+// CHECK: {{^event:[ ]+End$}}
+// CHECK: {{^orig:[ ]+'.*templight-empty-entries-fix.cpp:20:15'$}}
+// CHECK: {{^poi:[ ]+'.*templight-empty-entries-fix.cpp:20:25'$}}
+// CHECK-LABEL: {{^---$}}
+// CHECK: {{^name:[ ]+a$}}
+// CHECK: {{^kind:[ ]+DeducedTemplateArgumentSubstitution$}}
+// CHECK: {{^event:[ ]+End$}}
+// CHECK: {{^orig:[ ]+'.*templight-empty-entries-fix.cpp:20:25'$}}
+// CHECK: {{^poi:[ ]+'.*templight-empty-entries-fix.cpp:20:31'$}}
+// CHECK-LABEL: {{^---$}}
+// CHECK: {{^name:[ ]+'a<0>'$}}
+// CHECK: {{^kind:[ ]+TemplateInstantiation$}}
+// CHECK: {{^event:[ ]+Begin$}}
+// CHECK: {{^orig:[ ]+'.*templight-empty-entries-fix.cpp:20:25'$}}
+// CHECK: {{^poi:[ ]+'.*templight-empty-entries-fix.cpp:20:31'$}}
+// CHECK-LABEL: {{^---$}}
+// CHECK: {{^name:[ ]+'a<0>'$}}
+// CHECK: {{^kind:[ ]+TemplateInstantiation$}}
+// CHECK: {{^event:[ ]+End$}}
+// CHECK: {{^orig:[ ]+'.*templight-empty-entries-fix.cpp:20:25'$}}
+// CHECK: {{^poi:[ ]+'.*templight-empty-entries-fix.cpp:20:31'$}}
+
+template  struct b { typedef int c; };
+template ::c> void a() { a(); }
+
+// CHECK-LABEL: {{^---$}}
+// CHECK: {{^name:[ ]+a$}}
+// CHECK: {{^kind:[ ]+DeducedTemplateArgumentSubstitution$}}
+// CHECK: {{^event:[ ]+Begin$}}
+// CHECK: {{^orig:[ ]+'.*templight-empty-entries-fix.cpp:60:57'$}}
+// CHECK: {{^poi:[ ]+'.*templight-empty-entries-fix.cpp:60:63'$}}
+// CHECK-LABEL: {{^---$}}
+// CHECK: {{^name:[ ]+d$}}
+// CHECK: {{^kind:[ ]+DefaultTemplateArgumentInstantiation$}}
+// CHECK: {{^event:[ ]+Begin$}}
+// CHECK: {{^orig:[ ]+'.*templight-empty-entries-fix.cpp:60:16'$}}
+// CHECK: {{^poi:[ ]+'.*templight-empty-entries-fix.cpp:60:57'$}}
+// CHECK-LABEL: {{^---$}}
+// CHECK: {{^name:[ ]+d$}}
+// CHECK: {{^kind:[ ]+DefaultTemplateArgumentInstantiation$}}
+// CHECK: {{^event:[ ]+End$}}
+// CHECK: {{^orig:[ ]+'.*templight-empty-entries-fix.cpp:60:16'$}}
+// CHECK: {{^poi:[ ]+'.*templight-empty-entries-fix.cpp:60:57'$}}
+// CHECK-LABEL: {{^---$}}
+// CHECK: {{^name:[ ]+unnamed template type parameter 1 of a$}}
+// CHECK: {{^kind:[ ]+DefaultTemplateArgumentInstantiation$}}
+// CHECK: {{^event:[ ]+Begin$}}
+// CHECK: {{^orig:[ ]+'.*templight-empty-entries-fix.cpp:60:32'$}}
+// CHECK: {{^poi:[ ]+'.*templight-empty-entries-fix.cpp:60:57'$}}
+// CHECK-LABEL: {{^---$}}
+// CHECK: {{^name:[ ]+'b<1>'$}}
+// CHECK: {{^kind:[ ]+TemplateInstantiation$}}
+// CHECK: {{^event:[ ]+Begin$}}
+// CHECK: {{^orig:[ ]+'.*templight-empty-entries-fix.cpp:59:23'$}}
+// CHECK: {{^poi:[ ]+'.*templight-empty-entries-fix.cpp:60:43'$}}
+// CHECK-LABEL: {{^---$}}
+// CHECK: {{^name:[ ]+'b<1>'$}}
+// CHECK: {{^kind:[ ]+TemplateInstantiation$}}
+// CHECK: {{^event:[ ]+En

[PATCH] D118038: [clang][dataflow] Enable merging distinct values in Environment::join

2022-01-24 Thread Stanislav Gatev via Phabricator via cfe-commits
sgatev updated this revision to Diff 402529.
sgatev marked an inline comment as done.
sgatev added a comment.

Address reviewers' comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118038

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/Value.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/TransferTest.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
@@ -9,6 +9,7 @@
 #include "NoopAnalysis.h"
 #include "TestingSupport.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Analysis/CFG.h"
@@ -16,6 +17,7 @@
 #include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/Value.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallSet.h"
@@ -35,8 +37,14 @@
 
 using namespace clang;
 using namespace dataflow;
+using namespace test;
+using namespace ast_matchers;
+using ::testing::_;
+using ::testing::ElementsAre;
 using ::testing::IsEmpty;
+using ::testing::NotNull;
 using ::testing::Pair;
+using ::testing::Test;
 using ::testing::UnorderedElementsAre;
 
 template 
@@ -174,7 +182,7 @@
   }
 };
 
-class NoreturnDestructorTest : public ::testing::Test {
+class NoreturnDestructorTest : public Test {
 protected:
   template 
   void runDataflow(llvm::StringRef Code, Matcher Expectations) {
@@ -300,4 +308,195 @@
   // FIXME: Called functions at point `p` should contain only "foo".
 }
 
+class OptionalIntAnalysis
+: public DataflowAnalysis {
+public:
+  explicit OptionalIntAnalysis(ASTContext &Context, BoolValue &HasValueTop)
+  : DataflowAnalysis(Context),
+HasValueTop(HasValueTop) {}
+
+  static NoopLattice initialElement() { return {}; }
+
+  void transfer(const Stmt *S, NoopLattice &, Environment &Env) {
+auto OptionalIntRecordDecl = recordDecl(hasName("OptionalInt"));
+auto HasOptionalIntType = hasType(OptionalIntRecordDecl);
+
+if (const auto *E = selectFirst(
+"call", match(cxxConstructExpr(HasOptionalIntType).bind("call"), *S,
+  getASTContext( {
+  auto &ConstructorVal = *cast(Env.createValue(E->getType()));
+  ConstructorVal.setProperty("has_value", Env.getBoolLiteralValue(false));
+  Env.setValue(*Env.getStorageLocation(*E, SkipPast::None), ConstructorVal);
+} else if (const auto *E = selectFirst(
+   "call",
+   match(cxxOperatorCallExpr(callee(cxxMethodDecl(ofClass(
+ OptionalIntRecordDecl
+ .bind("call"),
+ *S, getASTContext( {
+  assert(E->getNumArgs() > 0);
+  auto *Object = E->getArg(0);
+  assert(Object != nullptr);
+
+  auto *ObjectLoc =
+  Env.getStorageLocation(*Object, SkipPast::ReferenceThenPointer);
+  assert(ObjectLoc != nullptr);
+
+  auto &ConstructorVal =
+  *cast(Env.createValue(Object->getType()));
+  ConstructorVal.setProperty("has_value", Env.getBoolLiteralValue(true));
+  Env.setValue(*ObjectLoc, ConstructorVal);
+}
+  }
+
+  bool merge(QualType Type, const Value &Val1, const Value &Val2,
+ Value &MergedVal, Environment &Env) final {
+if (!Type->isRecordType() ||
+Type->getAsCXXRecordDecl()->getQualifiedNameAsString() != "OptionalInt")
+  return false;
+
+auto *HasValue1 =
+cast(cast(&Val1)->getProperty("has_value"));
+auto *HasValue2 =
+cast(cast(&Val2)->getProperty("has_value"));
+if (HasValue1 == HasValue2) {
+  cast(&MergedVal)->setProperty("has_value", *HasValue1);
+} else {
+  cast(&MergedVal)->setProperty("has_value", HasValueTop);
+}
+return true;
+  }
+
+private:
+  BoolValue &HasValueTop;
+};
+
+class WideningTest : public Test {
+protected:
+  template 
+  void runDataflow(llvm::S

[PATCH] D118038: [clang][dataflow] Enable merging distinct values in Environment::join

2022-01-24 Thread Stanislav Gatev via Phabricator via cfe-commits
sgatev added a comment.

Right. To model the behavior of a language feature (e.g. a simple model that 
tracks whether a `std::optional` value is engaged) clients can attach 
properties to symbolic values and decide how distinct values are joined. This 
doesn't completely rule out the possibility of running multiple clients in the 
same fixed-point iteration. In fact, we think this could be beneficial as 
clients can use each others' results by enriching the environment (e.g. dead 
code analysis can take advantage of understanding of whether a `std::optional` 
value is always (not) engaged). This is a direction that we're currently 
exploring. It does require some care to ensure that the clients are not 
stepping on each others' toes. For example, we should probably replace the 
current `Properties` mechanism with something more robust to ensure that 
properties are stored in isolated namespaces.




Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:67
+///  `Val1` and `Val2` must be distinct.
+virtual void merge(QualType Type, const Value &Val1, const Value &Val2,
+   Value &MergedVal, Environment &Env) {}

xazax.hun wrote:
> Previously, when the values were distinct, we did not include anything in the 
> merged environment. With the new model, we will end up creating "default" 
> values for every one of them. I wonder if this is wasteful. We could 
> potentially also defer this until we have some real world data and can 
> benchmark this. But I think we could consider changing the return type to 
> bool to specify if the merged value should be included in the resulting 
> environment at all and this could return false by default.
Good idea. Let's only include the merge value if necessary for now. I changed 
the interface to return a bool indicating whether the merged value should be 
included or not. I added a FIXME to consider not storing the constructed value 
in the context if it isn't necessary. I'd prefer to address that separately, to 
keep the scope of this patch smaller.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118038

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


[PATCH] D118011: [RISCV] Adjust predicates and update intrinsic for clmul and clmulh in Zbkc extension

2022-01-24 Thread Xinlong Wu via Phabricator via cfe-commits
VincentWu added a comment.

Hi, Jimerlife

it is a great work to implement the intrinsic of `zbkb`.
however, I don't know whether you have noticed that there is already to patch 
https://reviews.llvm.org/D112774 and https://reviews.llvm.org/D102310.
these two patches have included almost all the work you did in your patch.

I think we should better avoid overlap as much as possible.

therefore, could you please comment on the difference between these two patches 
as you see it?
let's focus on only one version.

You are also welcome to join the bi-weekly LLVM RISC-V meetings to discuss with 
us if you want. You can subscribe to the calendar via this link 
https://calendar.google.com/calendar/embed?src=lowrisc.org_0n5pkesfjcnp0bh5hps1p0bd80%40group.calendar.google.com


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118011

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


[PATCH] D115640: [OpenCL] Add support of __opencl_c_device_enqueue feature macro.

2022-01-24 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added inline comments.



Comment at: clang/test/Misc/opencl-c-3.0.incorrect_options.cl:21
+
 // CHECK-FP64: error: options cl_khr_fp64 and __opencl_c_fp64 are set to 
different values
 

azabaznov wrote:
> Anastasia wrote:
> > Anastasia wrote:
> > > I can't remember if we have discussed this already, but could we use 
> > > `-verify` for these errors?
> > We should be able to remove `FileCheck` and replace `CHECK` directives with 
> > something like:
> > `//expected-error@*{{options cl_khr_fp64 and __opencl_c_fp64 are set to 
> > different values}}`
> I don't think we can test it like this because there is different output for 
> each invalid combination, so we need to check them with label.
We normally use macros to guard the expected errors but however, this is a 
better fit for a refactoring patch... so I am ok if it's done separately...  

However, don't you need to add

```
//expected-no-diagnostics
```
directive?



Comment at: clang/test/SemaOpenCL/storageclass.cl:2
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL1.2
-// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL3.0 
-cl-ext=-__opencl_c_program_scope_global_variables,-__opencl_c_generic_address_space,-__opencl_c_pipes
-// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL3.0 
-cl-ext=+__opencl_c_program_scope_global_variables,-__opencl_c_generic_address_space,-__opencl_c_pipes
-// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL3.0 
-cl-ext=-__opencl_c_program_scope_global_variables,+__opencl_c_generic_address_space
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL3.0 
-cl-ext=-__opencl_c_program_scope_global_variables,-__opencl_c_generic_address_space,-__opencl_c_pipes,-__opencl_c_device_enqueue
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL3.0 
-cl-ext=+__opencl_c_program_scope_global_variables,-__opencl_c_generic_address_space,-__opencl_c_pipes,-__opencl_c_device_enqueue

azabaznov wrote:
> Anastasia wrote:
> > Anastasia wrote:
> > > These lines are getting a bit longer. Do we actually need to set 
> > > `-__opencl_c_device_enqueue` for this test? Same for some other tests...
> > ping
> Yeah, we need that here because we are turning off generic AS and PSV in this 
> test. Note that `__opencl_c_device_enqueue` is turned off with `-cl-ext=-all`.
since this test doesn't check anything for `__opencl_c_device_enqueue` why do 
we need to switch this off explicitly?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115640

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


[clang] c133516 - Don't run test/ClangScanDeps/modules-symlink.c on Windows

2022-01-24 Thread Hans Wennborg via cfe-commits

Author: Hans Wennborg
Date: 2022-01-24T16:52:01+01:00
New Revision: c1335166b2659b02784b9dfb562c6b8b1c746407

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

LOG: Don't run test/ClangScanDeps/modules-symlink.c on Windows

'ln -s' isn't Windows friendly.

Added: 


Modified: 
clang/test/ClangScanDeps/modules-symlink.c

Removed: 




diff  --git a/clang/test/ClangScanDeps/modules-symlink.c 
b/clang/test/ClangScanDeps/modules-symlink.c
index 1a2fe2d9f512..46831b0a3fc0 100644
--- a/clang/test/ClangScanDeps/modules-symlink.c
+++ b/clang/test/ClangScanDeps/modules-symlink.c
@@ -1,5 +1,6 @@
 // RUN: rm -rf %t
 // RUN: split-file %s %t
+// UNSUPPORTED: system-windows
 
 //--- cdb_pch.json
 [



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


[PATCH] D118038: [clang][dataflow] Enable merging distinct values in Environment::join

2022-01-24 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.

I think the canonical approach most of the time is to have a map lattice that 
maps from values to your lattice elements. Storing properties directly at the 
object sounds like an interesting approach. I'll be curious to see how it works 
out. Do we expect the framework to propagate these properties automatically 
across copy ctors/copy assignments, move operations etc?




Comment at: 
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp:364
+} else {
+  cast(&MergedVal)->setProperty("has_value", HasValueTop);
+}

An alternative approach would be to just return false here and whenever the 
property lookup fails just assume the top value. Since this is just a simple 
test, feel free to leave this as is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118038

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


[PATCH] D117569: Constexpr not supported with __declspec(dllimport).

2022-01-24 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment.

In D117569#3262595 , @zahiraam wrote:

> @majnemer
>
> I did take an example completely different with no use of the constexpr in 
> order to understand (at least for me) when the _imp prefix is generated with 
> CL.
> This is sample.cpp:
> #include
>
> __declspec(dllexport) void PrintHello()
> {
>
>   std::cout << "hello" << std::endl;
>
> }
>
> $ cl  sample.cpp /LD /EHsc
> Microsoft (R) C/C++ Optimizing Compiler Version 19.16.27045 for x64
> Copyright (C) Microsoft Corporation.  All rights reserved.
>
> sample.cpp
> Microsoft (R) Incremental Linker Version 14.16.27045.0
> Copyright (C) Microsoft Corporation.  All rights reserved.
>
> /out:sample.dll
> /dll
> /implib:sample.lib
> sample.obj
>
>   Creating library sample.lib and object sample.exp
>
> $
> This is testapp.cpp:
> extern void __declspec(dllimport)PrintHello();
>
> void main()
> {
>
>   PrintHello();
>
> }
>
> $ cl  testapp.cpp /EHsc /link sample.lib
> Microsoft (R) C/C++ Optimizing Compiler Version 19.16.27045 for x64
> Copyright (C) Microsoft Corporation.  All rights reserved.
>
> testapp.cpp
> Microsoft (R) Incremental Linker Version 14.16.27045.0
> Copyright (C) Microsoft Corporation.  All rights reserved.
>
> /out:testapp.exe
> sample.lib
> testapp.obj
> $
> $ dumpbin /symbols testapp.obj | grep imp
> 008  UNDEF  notype   External | __imp_?PrintHello@@YAXXZ 
> (__declspec(dllimport) void __cdecl PrintHello(void))
> $
>
> My understanding that the _imp prefix is generated because of the dllimport 
> in testapp.cpp.
> So if we go back to the test case above:
>
> extern int __declspec(dllimport) dll_import_int;
> constexpr int& dll_import_constexpr_ref = dll_import_int;
> int& get() {
>
>   return dll_import_constexpr_ref;
>
> }
> int main () {
>
>   get();
>   return 0;
>
> }
>
> CLANG (with this patch and ICL for that matter) generates the symbol:
>
> 011  UNDEF  notype   External | __imp_?dll_import_int@@3HA 
> (__declspec(dllimport) int dll_import_int)
>
> but not MSVC. This seems incorrect to me since the _imp prefix is always 
> generated for dllimport symbols?

OTOH it does seem odd that clang is generating for the test case 2 UNDEF 
symbols. One with the _imp mangling and one without?


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

https://reviews.llvm.org/D117569

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


[PATCH] D107769: [OpenCL] Make generic addrspace optional for -fdeclare-opencl-builtins

2022-01-24 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added a comment.

In D107769#3265867 , @svenvh wrote:

> In D107769#3265665 , @Anastasia 
> wrote:
>
>> The way I understand the spec for OpenCL C 2.0 is that whenever the address 
>> space of the pointer is not listed it means generic has to be used, here is 
>> one example:
>> https://www.khronos.org/registry/OpenCL/specs/2.2/html/OpenCL_C.html#vector-data-load-and-store-functions
>>
>>   gentypen vloadn(size_t offset, const gentype *p)
>>   gentypen vloadn(size_t offset, const __constant gentype *p)
>>
>> that has no address space (i.e. `generic`) and `constant` explicitly. So I 
>> think if address spaces are not listed explicitly they are not supposed to 
>> be available.
>
> The unified specification (which "specifies all versions of OpenCL C") seems 
> to be making all overloads available as I understand; it is perhaps subtly 
> different from the previous specification?
>
> https://www.khronos.org/registry/OpenCL/specs/3.0-unified/html/OpenCL_C.html#vector-data-load-and-store-functions
>
>   gentypen vloadn(size_t offset, const __global gentype *p)
>   gentypen vloadn(size_t offset, const __local gentype *p)
>   gentypen vloadn(size_t offset, const __constant gentype *p)
>   gentypen vloadn(size_t offset, const __private gentype *p)
>   
>   For OpenCL C 2.0, or OpenCL C 3.0 or newer with the 
> __opencl_c_generic_address_space feature:
>   
>   gentypen vloadn(size_t offset, const gentype *p)
>
> Since the `__constant` overload should always be available, I think a reader 
> can assume that the overloads directly above and below `__constant` are also 
> always available?  So that the generic overload is an optional addition to 
> the list of overloads.  If not, I'd expect the spec to specify a condition 
> before listing the specific overloads.

I agree that OpenCL 3.0 spec might have broken the backward compatibility for 
OpenCL 2.0, however, I am not convinced it has been done intentionally. 
Perhaps, it's worth to clarify that?


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

https://reviews.llvm.org/D107769

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


[PATCH] D117929: [XRay] Add support for RISCV

2022-01-24 Thread Ashwin Poduval via Phabricator via cfe-commits
ashwin98 updated this revision to Diff 402533.
ashwin98 added a comment.
Herald added a subscriber: pcwang-thead.

Updated the diff, made the following changes:

1. Merged the riscv files into xray_riscv.cpp and removed the if-else code for 
%hi()
2. Cleaned up the issues related to indenting and comments in 
RISCVAsmPrinter.cpp
3. Updated the test file to pass -verify-machineinstrs and remove unnecessary 
attributes as well as {{.*}}s
4. Fixed riscv32 comments - it is now only commented out in 
cmake/Modules/AllSupportedArchDefs.cmake

I have been testing this patch on qemu using ubuntu for riscv64, the comment 
that Phabricator detects in the supported architecture definitions cmake file 
is probably an issue with syntax highlighting. Nevertheless, we could instead 
comment out riscv32 in clang/lib/Driver/XRayArgs, which would also throw up an 
error during compilation stating that the target is not supported.


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

https://reviews.llvm.org/D117929

Files:
  clang/lib/Driver/XRayArgs.cpp
  compiler-rt/cmake/Modules/AllSupportedArchDefs.cmake
  compiler-rt/lib/xray/CMakeLists.txt
  compiler-rt/lib/xray/xray_interface.cpp
  compiler-rt/lib/xray/xray_riscv.cpp
  compiler-rt/lib/xray/xray_trampoline_riscv32.S
  compiler-rt/lib/xray/xray_trampoline_riscv64.S
  compiler-rt/lib/xray/xray_tsc.h
  llvm/lib/CodeGen/XRayInstrumentation.cpp
  llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
  llvm/lib/Target/RISCV/RISCVSubtarget.h
  llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll

Index: llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll
@@ -0,0 +1,70 @@
+; RUN: llc -mtriple=riscv32-unknown-elf -mattr=+d -verify-machineinstrs < %s | FileCheck --check-prefix=CHECK %s
+; RUN: llc -mtriple=riscv32-unknown-linux-gnu -mattr=+d -verify-machineinstrs < %s | FileCheck --check-prefix=CHECK %s
+; RUN: llc -mtriple=riscv64-unknown-elf -mattr=+d -verify-machineinstrs < %s | FileCheck --check-prefix=CHECK --check-prefix=CHECK-RISCV64 %s
+; RUN: llc -mtriple=riscv64-unknown-linux-gnu -mattr=+d -verify-machineinstrs < %s | FileCheck --check-prefix=CHECK --check-prefix=CHECK-RISCV64 %s
+
+define i32 @foo() nounwind "function-instrument"="xray-always" {
+; CHECK:.p2align 2
+; CHECK-LABEL:  .Lxray_sled_0:
+; CHECK-NEXT:   j .Ltmp0
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-LABEL:  .Ltmp0:
+  ret i32 0
+; CHECK:.p2align 2
+; CHECK-LABEL:  .Lxray_sled_1:
+; CHECK-NEXT:   j .Ltmp1
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-LABEL:  .Ltmp1:
+; CHECK-NEXT:   ret
+}
+; CHECK:.section xray_instr_map,"ao",@progbits,foo
+; CHECK-LABEL:  .Lxray_sleds_start0:
+; CHECK:.Lxray_sled_0-.Ltmp2
+; CHECK:.Lxray_sled_1-.Ltmp3
+; CHECK-LABEL:  .Lxray_sleds_end0:
Index: llvm/lib/Target/RISCV/RISCVSubtarget.h
===
--- llvm/lib/Target/RISCV/RISCVSubtarget.h
+++ llvm/lib/Target/RISCV/RISCVSubtarget.h
@@ -187,6 +187,8 @@
   unsigned getMaxInterleaveFactor() const {
 return hasVInstructions() ? MaxInterleaveFactor : 1;
   }
+  // Add XRay support - needs double precision floats at present
+  bool isXRaySupported() const override { return hasStdExtD(); }
 
 protected:
   // GlobalISel related APIs.
Index: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
===
--- llvm/lib/Target/RI

[PATCH] D117899: [OpenCL] Make read_write images optional for -fdeclare-opencl-builtins

2022-01-24 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia accepted this revision.
Anastasia 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/D117899/new/

https://reviews.llvm.org/D117899

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


[PATCH] D115921: [RISCV] Refactor the RISCV ISA extension info and target features to support multiple extension version

2022-01-24 Thread Zakk Chen via Phabricator via cfe-commits
khchen added a comment.
Herald added subscribers: pcwang-thead, eopXD.

In D115921#3206434 , @luismarques 
wrote:

> I think this would benefit from increased test coverage, namely to show that 
> the mattr command-line options are properly handled. Some possible ideas:
>
> - Tests with the correct extension versions (maybe add a test file that 
> exercises the version for all extensions).
> - Tests that show an error message with unsupported versions.
> - A test that shows that something like mattr=+m,+m2p1 is allowed (or not).
>
> Nit: fix the lint / no new line warnings.

I agree with @luismarques, we need increased test coverage.

BTW, I think this patch need to work with D113237 
 to show the ability on supporting multiple 
extension version.

> Anybody else has more comments about support multi-version extension? Or it 
> has been decided already by RISC-V foundation?

IMPO, supporting multi version for ratified extensions in compiler does make 
sense to me. 
GCC already did it, but I'm not sure is there any related discussion before 
when gcc decided to support multi version extension (or misa-spec).




Comment at: clang/test/Driver/riscv-arch-version.c:7
+// RUN:   FileCheck -check-prefix=RV32-M2P0 %s
+// RV32-M2P0: "-target-feature" "+m"
+

I'm thinking do we also need to have `-target-feature +m2p0` here since we are 
going to support multiple extension version?



Comment at: llvm/lib/Target/RISCV/RISCV.td:14
 
//===--===//
 
 def FeatureStdExtM

nit: It will be good to have a comment to talk about what's default version 
come from?
I guess the default is -misa-spec=2.2, right?




Comment at: llvm/test/CodeGen/RISCV/attributes-version.ll:3
+
+; RUN: llc -mtriple=riscv32 -mattr=+m %s -o - | FileCheck --check-prefix=RV32M 
%s
+; RUN: llc -mtriple=riscv32 -mattr=+a %s -o - | FileCheck --check-prefix=RV32A 
%s

For example, we need to have test for `llc -mattr=+m,+m2p0`, and invalid test 
for `mattr=+m,+m1p9`


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

https://reviews.llvm.org/D115921

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


[PATCH] D107769: [OpenCL] Make generic addrspace optional for -fdeclare-opencl-builtins

2022-01-24 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added a comment.

Looping in @yaxunl for further feedback regarding OpenCL 2.0 behavior 
specifically.


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

https://reviews.llvm.org/D107769

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


[PATCH] D116597: [analyzer] Don't track function calls as control dependencies

2022-01-24 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 402549.
Szelethus added a comment.

Fix tests, mention that this is purely a heuristic.


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

https://reviews.llvm.org/D116597

Files:
  clang/include/clang/Analysis/CFG.h
  clang/lib/Analysis/CFG.cpp
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/test/Analysis/return-value-guaranteed.cpp
  clang/test/Analysis/track-control-dependency-conditions.cpp

Index: clang/test/Analysis/track-control-dependency-conditions.cpp
===
--- clang/test/Analysis/track-control-dependency-conditions.cpp
+++ clang/test/Analysis/track-control-dependency-conditions.cpp
@@ -149,14 +149,13 @@
   int *x = 0; // expected-note-re^}}'x' initialized to a null pointer value{{$
 
   if (ConvertsToBool())
-// debug-note-re@-1^}}Tracking condition 'ConvertsToBool()'{{$
-// expected-note-re@-2^}}Assuming the condition is true{{$
-// expected-note-re@-3^}}Taking true branch{{$
+// expected-note-re@-1^}}Assuming the condition is true{{$
+// expected-note-re@-2^}}Taking true branch{{$
 *x = 5; // expected-warning{{Dereference of null pointer}}
 // expected-note@-1{{Dereference of null pointer}}
 }
 
-} // end of namespace variable_declaration_in_condition
+} // namespace conversion_to_bool
 
 namespace note_from_different_but_not_nested_stackframe {
 
@@ -186,14 +185,13 @@
 int *getIntPtr();
 
 void storeValue(int **i) {
-  *i = getIntPtr(); // tracking-note-re^}}Value assigned to 'i', which participates in a condition later{{$
+  *i = getIntPtr();
 }
 
 int *conjurePointer() {
   int *i;
-  storeValue(&i); // tracking-note-re^}}Calling 'storeValue'{{$
-  // tracking-note-re@-1^}}Returning from 'storeValue'{{$
-  return i; // tracking-note-re^}}Returning pointer (loaded from 'i'), which participates in a condition later{{$
+  storeValue(&i);
+  return i;
 }
 
 void f(int *ptr) {
@@ -201,11 +199,8 @@
// expected-note-re@-1^}}Taking false branch{{$
 ;
   if (!conjurePointer())
-// tracking-note-re@-1^}}Calling 'conjurePointer'{{$
-// tracking-note-re@-2^}}Returning from 'conjurePointer'{{$
-// debug-note-re@-3^}}Tracking condition '!conjurePointer()'{{$
-// expected-note-re@-4^}}Assuming the condition is true{{$
-// expected-note-re@-5^}}Taking true branch{{$
+// expected-note-re@-1^}}Assuming the condition is true{{$
+// expected-note-re@-2^}}Taking true branch{{$
 *ptr = 5; // expected-warning{{Dereference of null pointer}}
   // expected-note@-1{{Dereference of null pointer}}
 }
@@ -226,9 +221,8 @@
// expected-note-re@-1^}}Taking false branch{{$
 ;
   if (!conjurePointer())
-// debug-note-re@-1^}}Tracking condition '!conjurePointer()'{{$
-// expected-note-re@-2^}}Assuming the condition is true{{$
-// expected-note-re@-3^}}Taking true branch{{$
+// expected-note-re@-1^}}Assuming the condition is true{{$
+// expected-note-re@-2^}}Taking true branch{{$
 *ptr = 5; // expected-warning{{Dereference of null pointer}}
   // expected-note@-1{{Dereference of null pointer}}
 }
@@ -246,9 +240,8 @@
   int *x = 0; // expected-note-re^}}'x' initialized to a null pointer value{{$
 
   if (cast(conjure()))
-// debug-note-re@-1^}}Tracking condition 'cast(conjure())'{{$
-// expected-note-re@-2^}}Assuming the condition is false{{$
-// expected-note-re@-3^}}Taking false branch{{$
+// expected-note-re@-1^}}Assuming the condition is false{{$
+// expected-note-re@-2^}}Taking false branch{{$
 return;
   *x = 5; // expected-warning{{Dereference of null pointer}}
   // expected-note@-1{{Dereference of null pointer}}
@@ -266,9 +259,8 @@
// expected-note-re@-1^}}Taking false branch{{$
 ;
   if (!flipCoin())
-// debug-note-re@-1^}}Tracking condition '!flipCoin()'{{$
-// expected-note-re@-2^}}Assuming the condition is true{{$
-// expected-note-re@-3^}}Taking true branch{{$
+// expected-note-re@-1^}}Assuming the condition is true{{$
+// expected-note-re@-2^}}Taking true branch{{$
 *ptr = 5; // expected-warning{{Dereference of null pointer}}
   // expected-note@-1{{Dereference of null pointer}}
 }
@@ -278,11 +270,9 @@
 bool coin();
 
 bool flipCoin() {
-  if (coin()) // tracking-note-re^}}Assuming the condition is false{{$
-  // tracking-note-re@-1^}}Taking false branch{{$
-  // debug-note-re@-2^}}Tracking condition 'coin()'{{$
+  if (coin())
 return true;
-  return coin(); // tracking-note-re^}}Returning value, wh

[PATCH] D118038: [clang][dataflow] Enable merging distinct values in Environment::join

2022-01-24 Thread Stanislav Gatev via Phabricator via cfe-commits
sgatev updated this revision to Diff 402550.
sgatev added a comment.

Address reviewers' comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118038

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/Value.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/TransferTest.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
@@ -9,6 +9,7 @@
 #include "NoopAnalysis.h"
 #include "TestingSupport.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Analysis/CFG.h"
@@ -16,6 +17,7 @@
 #include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/Value.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallSet.h"
@@ -35,8 +37,15 @@
 
 using namespace clang;
 using namespace dataflow;
+using namespace test;
+using namespace ast_matchers;
+using ::testing::_;
+using ::testing::ElementsAre;
 using ::testing::IsEmpty;
+using ::testing::IsNull;
+using ::testing::NotNull;
 using ::testing::Pair;
+using ::testing::Test;
 using ::testing::UnorderedElementsAre;
 
 template 
@@ -174,7 +183,7 @@
   }
 };
 
-class NoreturnDestructorTest : public ::testing::Test {
+class NoreturnDestructorTest : public Test {
 protected:
   template 
   void runDataflow(llvm::StringRef Code, Matcher Expectations) {
@@ -300,4 +309,184 @@
   // FIXME: Called functions at point `p` should contain only "foo".
 }
 
+class OptionalIntAnalysis
+: public DataflowAnalysis {
+public:
+  explicit OptionalIntAnalysis(ASTContext &Context)
+  : DataflowAnalysis(Context) {}
+
+  static NoopLattice initialElement() { return {}; }
+
+  void transfer(const Stmt *S, NoopLattice &, Environment &Env) {
+auto OptionalIntRecordDecl = recordDecl(hasName("OptionalInt"));
+auto HasOptionalIntType = hasType(OptionalIntRecordDecl);
+
+if (const auto *E = selectFirst(
+"call", match(cxxConstructExpr(HasOptionalIntType).bind("call"), *S,
+  getASTContext( {
+  auto &ConstructorVal = *cast(Env.createValue(E->getType()));
+  ConstructorVal.setProperty("has_value", Env.getBoolLiteralValue(false));
+  Env.setValue(*Env.getStorageLocation(*E, SkipPast::None), ConstructorVal);
+} else if (const auto *E = selectFirst(
+   "call",
+   match(cxxOperatorCallExpr(callee(cxxMethodDecl(ofClass(
+ OptionalIntRecordDecl
+ .bind("call"),
+ *S, getASTContext( {
+  assert(E->getNumArgs() > 0);
+  auto *Object = E->getArg(0);
+  assert(Object != nullptr);
+
+  auto *ObjectLoc =
+  Env.getStorageLocation(*Object, SkipPast::ReferenceThenPointer);
+  assert(ObjectLoc != nullptr);
+
+  auto &ConstructorVal =
+  *cast(Env.createValue(Object->getType()));
+  ConstructorVal.setProperty("has_value", Env.getBoolLiteralValue(true));
+  Env.setValue(*ObjectLoc, ConstructorVal);
+}
+  }
+
+  bool merge(QualType Type, const Value &Val1, const Value &Val2,
+ Value &MergedVal, Environment &Env) final {
+if (!Type->isRecordType() ||
+Type->getAsCXXRecordDecl()->getQualifiedNameAsString() != "OptionalInt")
+  return false;
+
+auto *HasValue1 = cast_or_null(
+cast(&Val1)->getProperty("has_value"));
+if (HasValue1 == nullptr)
+  return false;
+
+auto *HasValue2 = cast_or_null(
+cast(&Val2)->getProperty("has_value"));
+if (HasValue2 == nullptr)
+  return false;
+
+if (HasValue1 != HasValue2)
+  return false;
+
+cast(&MergedVal)->setProperty("has_value", *HasValue1);
+return true;
+  }
+};
+
+class WideningTest : public Test {
+protected:
+  template 
+  void runDataflow(llvm::StringRef Code, Matcher Match) {
+tooling::FileCon

[PATCH] D118044: [ARM] Undeprecate complex IT blocks

2022-01-24 Thread David Spickett via Phabricator via cfe-commits
DavidSpickett added a comment.

Please define what "complex" is, in the commit message and (if it doesn't make 
the description a whole paragraph) the command line help.

Given that we're flipping a default for v8 thumb targets does this warrant a 
release note?




Comment at: llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp:10977
-Warning(IDLoc, "deprecated instruction in IT block");
-  }
 }

Is this warning something you'd still want if you opted into restricting IT 
blocks? Or can we say that because there has/will not be any core produced that 
follows the deprecation, you would never need the warning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118044

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


[PATCH] D118038: [clang][dataflow] Enable merging distinct values in Environment::join

2022-01-24 Thread Stanislav Gatev via Phabricator via cfe-commits
sgatev marked an inline comment as done.
sgatev added a comment.

Yes, I think we should be able to propagate properties across copy and move 
operations in the framework. Clients can override this behavior by modeling the 
copy and move operations and storing different values in the environment.




Comment at: 
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp:364
+} else {
+  cast(&MergedVal)->setProperty("has_value", HasValueTop);
+}

xazax.hun wrote:
> An alternative approach would be to just return false here and whenever the 
> property lookup fails just assume the top value. Since this is just a simple 
> test, feel free to leave this as is.
Right. This seems simpler so let's go with it for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118038

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


[PATCH] D118011: [RISCV] Adjust predicates and update intrinsic for clmul and clmulh in Zbkc extension

2022-01-24 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added a comment.

In D118011#3266014 , @alextsao1999 
wrote:

> In D118011#3265374 , @Jimerlife 
> wrote:
>
>> In D118011#3265071 , @craig.topper 
>> wrote:
>>
>>> Will you also be updating clang's RISCVBuiltins.def?
>>>
>>> Can you add tests?
>>
>> I update BuiltinsRISCV.def and add "__builtin_riscv_clmul_kc" intrinsic for 
>> zbkc extension, but I am not sure if the name is  appropriate.
>
> https://github.com/rvkrypto/rvkrypto-fips/blob/main/rvkintrin.md
> Is the name as `__builtin_riscv_clmul_32` or `__builtin_riscv_clmul_64` 
> better for conflict resolution?

The builtins for clmul have been in for months without _32 or _64 since they 
operate on the "long" data type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118011

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


[PATCH] D118050: [analyzer] Different address spaces cannot overlap

2022-01-24 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers created this revision.
vabridgers added reviewers: steakhal, martong.
Herald added subscribers: manas, ASDenysPetrov, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun.
vabridgers requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This change fixes an assert that occurs in the SMT layer when refuting a
finding that uses pointers of two different sizes. This was found in a
downstream build that supports two different pointer sizes, The CString
Checker was attempting to compute an overlap for the 'to' and 'from'
pointers, where the pointers were of different sizes.

The problem was reproduced using a amdgcn target, and an upstreamable
test produced for the fix.

The assert seen is:

`*Solver->getSort(LHS) == *Solver->getSort(RHS) && "AST's must have the same 
sort!"'

Ack to steakhal for reviewing the fix, and creating the test case.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118050

Files:
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/test/Analysis/cstring-checker-addressspace.c


Index: clang/test/Analysis/cstring-checker-addressspace.c
===
--- /dev/null
+++ clang/test/Analysis/cstring-checker-addressspace.c
@@ -0,0 +1,21 @@
+// RUN: %clang_analyze_cc1 -triple amdgcn-unknown-unknown \
+// RUN: -analyze -analyzer-checker=core,alpha.unix.cstring \
+// RUN: -analyze -analyzer-checker=debug.ExprInspection \
+// RUN: -analyzer-config crosscheck-with-z3=true -verify %s
+// REQUIRES: z3
+
+void clang_analyzer_warnIfReached();
+
+#define DEVICE __attribute__((address_space(256)))
+_Static_assert(sizeof(int *) == 4, "");
+_Static_assert(sizeof(DEVICE int *) == 8, "");
+
+// Copy from host to device memory.
+DEVICE void *memcpy(DEVICE void *dst, const void *src, unsigned long len);
+
+void top(DEVICE int *dst, int *src, int len) {
+  memcpy(dst, src, len);
+
+  // Create a bugreport for triggering Z3 refutation.
+  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -449,6 +449,11 @@
 
   ProgramStateRef stateTrue, stateFalse;
 
+  // Assume different address spaces cannot overlap.
+  if (First.Expression->getType()->getPointeeType().getAddressSpace() !=
+  Second.Expression->getType()->getPointeeType().getAddressSpace())
+return state;
+
   // Get the buffer values and make sure they're known locations.
   const LocationContext *LCtx = C.getLocationContext();
   SVal firstVal = state->getSVal(First.Expression, LCtx);


Index: clang/test/Analysis/cstring-checker-addressspace.c
===
--- /dev/null
+++ clang/test/Analysis/cstring-checker-addressspace.c
@@ -0,0 +1,21 @@
+// RUN: %clang_analyze_cc1 -triple amdgcn-unknown-unknown \
+// RUN: -analyze -analyzer-checker=core,alpha.unix.cstring \
+// RUN: -analyze -analyzer-checker=debug.ExprInspection \
+// RUN: -analyzer-config crosscheck-with-z3=true -verify %s
+// REQUIRES: z3
+
+void clang_analyzer_warnIfReached();
+
+#define DEVICE __attribute__((address_space(256)))
+_Static_assert(sizeof(int *) == 4, "");
+_Static_assert(sizeof(DEVICE int *) == 8, "");
+
+// Copy from host to device memory.
+DEVICE void *memcpy(DEVICE void *dst, const void *src, unsigned long len);
+
+void top(DEVICE int *dst, int *src, int len) {
+  memcpy(dst, src, len);
+
+  // Create a bugreport for triggering Z3 refutation.
+  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -449,6 +449,11 @@
 
   ProgramStateRef stateTrue, stateFalse;
 
+  // Assume different address spaces cannot overlap.
+  if (First.Expression->getType()->getPointeeType().getAddressSpace() !=
+  Second.Expression->getType()->getPointeeType().getAddressSpace())
+return state;
+
   // Get the buffer values and make sure they're known locations.
   const LocationContext *LCtx = C.getLocationContext();
   SVal firstVal = state->getSVal(First.Expression, LCtx);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 7cd441f - [clang][NFC] Wrap TYPE_SWITCH in "do while (0)" in the interpreter

2022-01-24 Thread Owen Pan via cfe-commits

Author: Owen Pan
Date: 2022-01-24T09:05:27-08:00
New Revision: 7cd441ff537e00c743236658bfbcfc16c30ce031

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

LOG: [clang][NFC] Wrap TYPE_SWITCH in "do while (0)" in the interpreter

Wraps the expansions of TYPE_SWITCH and COMPOSITE_TYPE_SWITCH in
the constexpr interpreter with "do { ... } while (0)" so that these
macros can be used like this:

if (llvm::Optional T = Ctx.classify(FieldTy))
  TYPE_SWITCH(*T, Ok &= ReturnValue(FP.deref(), Value));
else
  Ok &= Composite(FieldTy, FP, Value);

This bug was found while testing D116316. See also review comment:
https://reviews.llvm.org/D64146?id=208520#inline-584131

Also cleaned up the macro definitions by removing the superfluous
do-while statements and removed the unused INT_TPYE_SWITCH macro.

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

Added: 


Modified: 
clang/lib/AST/Interp/PrimType.h

Removed: 




diff  --git a/clang/lib/AST/Interp/PrimType.h b/clang/lib/AST/Interp/PrimType.h
index f5f4f8e5c32d6..de4bf9bf802e0 100644
--- a/clang/lib/AST/Interp/PrimType.h
+++ b/clang/lib/AST/Interp/PrimType.h
@@ -81,35 +81,27 @@ inline bool isPrimitiveIntegral(PrimType Type) {
 /// Helper macro to simplify type switches.
 /// The macro implicitly exposes a type T in the scope of the inner block.
 #define TYPE_SWITCH_CASE(Name, B) \
-  case Name: { using T = PrimConv::T; do {B;} while(0); break; }
+  case Name: { using T = PrimConv::T; B; break; }
 #define TYPE_SWITCH(Expr, B)   
\
-  switch (Expr) {  
\
-TYPE_SWITCH_CASE(PT_Sint8, B)  
\
-TYPE_SWITCH_CASE(PT_Uint8, B)  
\
-TYPE_SWITCH_CASE(PT_Sint16, B) 
\
-TYPE_SWITCH_CASE(PT_Uint16, B) 
\
-TYPE_SWITCH_CASE(PT_Sint32, B) 
\
-TYPE_SWITCH_CASE(PT_Uint32, B) 
\
-TYPE_SWITCH_CASE(PT_Sint64, B) 
\
-TYPE_SWITCH_CASE(PT_Uint64, B) 
\
-TYPE_SWITCH_CASE(PT_Bool, B)   
\
-TYPE_SWITCH_CASE(PT_Ptr, B)
\
-  }
+  do { 
\
+switch (Expr) {
\
+  TYPE_SWITCH_CASE(PT_Sint8, B)
\
+  TYPE_SWITCH_CASE(PT_Uint8, B)
\
+  TYPE_SWITCH_CASE(PT_Sint16, B)   
\
+  TYPE_SWITCH_CASE(PT_Uint16, B)   
\
+  TYPE_SWITCH_CASE(PT_Sint32, B)   
\
+  TYPE_SWITCH_CASE(PT_Uint32, B)   
\
+  TYPE_SWITCH_CASE(PT_Sint64, B)   
\
+  TYPE_SWITCH_CASE(PT_Uint64, B)   
\
+  TYPE_SWITCH_CASE(PT_Bool, B) 
\
+  TYPE_SWITCH_CASE(PT_Ptr, B)  
\
+}  
\
+  } while (0)
 #define COMPOSITE_TYPE_SWITCH(Expr, B, D)  
\
-  switch (Expr) {  
\
-TYPE_SWITCH_CASE(PT_Ptr, B)
\
-default: do { D; } while(0); break;
\
-  }
-#define INT_TYPE_SWITCH(Expr, B)   
\
-  switch (Expr) {  
\
-TYPE_SWITCH_CASE(PT_Sint8, B)  
\
-TYPE_SWITCH_CASE(PT_Uint8, B)  
\
-TYPE_SWITCH_CASE(PT_Sint16, B) 
\
-TYPE_SWITCH_CASE(PT_Uint16, B) 
\
-TYPE_SWITCH_CASE(PT_Sint32, B) 
\
-TYPE_SWITCH_CASE(PT_Uint32, B) 
\
-TYPE_SWITCH_CASE(PT_Sint64, B) 
\
-TYPE_SWITCH_CASE(PT_Uint64, B) 
\
-default: llvm_unreachable("not an in

[PATCH] D117301: [clang][NFC] Wrap TYPE_SWITCH in "do while (0)" in the interpreter

2022-01-24 Thread Owen Pan 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 rG7cd441ff537e: [clang][NFC] Wrap TYPE_SWITCH in "do 
while (0)" in the interpreter (authored by owenpan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117301

Files:
  clang/lib/AST/Interp/PrimType.h


Index: clang/lib/AST/Interp/PrimType.h
===
--- clang/lib/AST/Interp/PrimType.h
+++ clang/lib/AST/Interp/PrimType.h
@@ -81,35 +81,27 @@
 /// Helper macro to simplify type switches.
 /// The macro implicitly exposes a type T in the scope of the inner block.
 #define TYPE_SWITCH_CASE(Name, B) \
-  case Name: { using T = PrimConv::T; do {B;} while(0); break; }
+  case Name: { using T = PrimConv::T; B; break; }
 #define TYPE_SWITCH(Expr, B)   
\
-  switch (Expr) {  
\
-TYPE_SWITCH_CASE(PT_Sint8, B)  
\
-TYPE_SWITCH_CASE(PT_Uint8, B)  
\
-TYPE_SWITCH_CASE(PT_Sint16, B) 
\
-TYPE_SWITCH_CASE(PT_Uint16, B) 
\
-TYPE_SWITCH_CASE(PT_Sint32, B) 
\
-TYPE_SWITCH_CASE(PT_Uint32, B) 
\
-TYPE_SWITCH_CASE(PT_Sint64, B) 
\
-TYPE_SWITCH_CASE(PT_Uint64, B) 
\
-TYPE_SWITCH_CASE(PT_Bool, B)   
\
-TYPE_SWITCH_CASE(PT_Ptr, B)
\
-  }
+  do { 
\
+switch (Expr) {
\
+  TYPE_SWITCH_CASE(PT_Sint8, B)
\
+  TYPE_SWITCH_CASE(PT_Uint8, B)
\
+  TYPE_SWITCH_CASE(PT_Sint16, B)   
\
+  TYPE_SWITCH_CASE(PT_Uint16, B)   
\
+  TYPE_SWITCH_CASE(PT_Sint32, B)   
\
+  TYPE_SWITCH_CASE(PT_Uint32, B)   
\
+  TYPE_SWITCH_CASE(PT_Sint64, B)   
\
+  TYPE_SWITCH_CASE(PT_Uint64, B)   
\
+  TYPE_SWITCH_CASE(PT_Bool, B) 
\
+  TYPE_SWITCH_CASE(PT_Ptr, B)  
\
+}  
\
+  } while (0)
 #define COMPOSITE_TYPE_SWITCH(Expr, B, D)  
\
-  switch (Expr) {  
\
-TYPE_SWITCH_CASE(PT_Ptr, B)
\
-default: do { D; } while(0); break;
\
-  }
-#define INT_TYPE_SWITCH(Expr, B)   
\
-  switch (Expr) {  
\
-TYPE_SWITCH_CASE(PT_Sint8, B)  
\
-TYPE_SWITCH_CASE(PT_Uint8, B)  
\
-TYPE_SWITCH_CASE(PT_Sint16, B) 
\
-TYPE_SWITCH_CASE(PT_Uint16, B) 
\
-TYPE_SWITCH_CASE(PT_Sint32, B) 
\
-TYPE_SWITCH_CASE(PT_Uint32, B) 
\
-TYPE_SWITCH_CASE(PT_Sint64, B) 
\
-TYPE_SWITCH_CASE(PT_Uint64, B) 
\
-default: llvm_unreachable("not an integer");   
\
-  }
+  do { 
\
+switch (Expr) {
\
+  TYPE_SWITCH_CASE(PT_Ptr, B)  
\
+  default: { D; break; }   
\
+}  
\
+  } while (0)
 #endif


Index: clang/lib/AST/Interp/PrimType.h
===
--- clang/lib/AST/Interp/PrimType.h
+++ clang/lib/AST/Interp/PrimType.h
@@ -81,35 +81,27 @@
 /// Helper macro to simplify type switches.
 /// The macro implicitly exposes a type T in the sco

[PATCH] D115523: [OpenCL] Set external linkage for block enqueue kernels

2022-01-24 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added a comment.

In D115523#3240870 , @yaxunl wrote:

> In D115523#3237857 , @Anastasia 
> wrote:
>
>> In D115523#3237410 , @yaxunl wrote:
>>
>>> It is possible that block kernels are defined and invoked in static 
>>> functions, therefore two block kernels in different TU's may have the same 
>>> name. Making such kernels external may cause duplicate symbols.
>>
>> Potentially we should append the name of the translation unit to all kernel 
>> wrapper names for the enqueued blocks to resolve this? For example, global 
>> constructors stubs are using such a similar naming scheme taken from the 
>> translation unit.
>>
>> But the kernel function in OpenCL has to be globally visible and many tools 
>> have been built with this assumption. Additionally, some toolchains might 
>> require the enqueued kernels to be globally visible as well in order to 
>> access them as an execution entry point.
>
> If we have to externalize such block kernels whose names are derived from 
> variables with internal linkage, we may need a way to make the block kernel 
> names unique.
>
> One approach would be use MD5 hash of the file path and compile options, 
> similar to 
> https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/Driver.cpp#L2623

Ok, however it might be enough to do what C++ handling does with global ctors. 
Should we file a bug for this for now?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115523

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


[clang] 50999e8 - [clang-format] Space between attribute closing parenthesis and qualified type colon.

2022-01-24 Thread Marek Kurdej via cfe-commits

Author: Marek Kurdej
Date: 2022-01-24T18:09:20+01:00
New Revision: 50999e82e8844615b1ae53edb9d56cdcace91b04

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

LOG: [clang-format] Space between attribute closing parenthesis and qualified 
type colon.

Fixes https://github.com/llvm/llvm-project/issues/35711.

Reviewed By: MyDeveloperDay, HazardyKnusperkeks, owenpan

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

Added: 


Modified: 
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/FormatTest.cpp

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index b9535f7965598..a8cd1e30f74e5 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -3339,6 +3339,9 @@ bool TokenAnnotator::spaceRequiredBefore(const 
AnnotatedLine &Line,
 if (Left.is(tok::ellipsis) && Right.is(tok::identifier) &&
 Line.First->is(Keywords.kw_import))
   return false;
+// Space in __attribute__((attr)) ::type.
+if (Left.is(TT_AttributeParen) && Right.is(tok::coloncolon))
+  return true;
 
 if (Left.is(tok::kw_operator))
   return Right.is(tok::coloncolon);

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 0ddbac48d716d..c4e0e14ce5bcd 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -10028,6 +10028,7 @@ TEST_F(FormatTest, UnderstandsAttributes) {
   verifyFormat("SomeType s __attribute__((unused)) (InitValue);");
   verifyFormat("a __attribute__((unused))\n"
"aaa(int i);");
+  verifyFormat("__attribute__((nodebug)) ::qualified_type f();");
   FormatStyle AfterType = getLLVMStyle();
   AfterType.AlwaysBreakAfterReturnType = FormatStyle::RTBS_All;
   verifyFormat("__attribute__((nodebug)) void\n"
@@ -10131,6 +10132,7 @@ TEST_F(FormatTest, UnderstandsSquareAttributes) {
   verifyFormat("class [[nodiscard]] f {\npublic:\n  f() {}\n}");
   verifyFormat("class [[deprecated(\"so sorry\")]] f {\npublic:\n  f() {}\n}");
   verifyFormat("class [[gnu::unused]] f {\npublic:\n  f() {}\n}");
+  verifyFormat("[[nodiscard]] ::qualified_type f();");
 
   // Make sure we do not mistake attributes for array subscripts.
   verifyFormat("int a() {}\n"



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


  1   2   3   >