[PATCH] D89935: [clangd] NFC: Update FIXME comment regarding lack of c/dtor support

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Both `SymbolKind` and `indexSymbolKindToSymbolKind` support constructors and
separate them into a different category from regular methods.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89935

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


Index: clang-tools-extra/clangd/FindSymbols.cpp
===
--- clang-tools-extra/clangd/FindSymbols.cpp
+++ clang-tools-extra/clangd/FindSymbols.cpp
@@ -179,8 +179,8 @@
 return llvm::None;
 
   index::SymbolInfo SymInfo = index::getSymbolInfo(&ND);
-  // FIXME: this is not classifying constructors, destructors and operators
-  //correctly (they're all "methods").
+  // FIXME: This does not classify operators correctly, they are cosnidered
+  // methods.
   SymbolKind SK = indexSymbolKindToSymbolKind(SymInfo.Kind);
 
   DocumentSymbol SI;


Index: clang-tools-extra/clangd/FindSymbols.cpp
===
--- clang-tools-extra/clangd/FindSymbols.cpp
+++ clang-tools-extra/clangd/FindSymbols.cpp
@@ -179,8 +179,8 @@
 return llvm::None;
 
   index::SymbolInfo SymInfo = index::getSymbolInfo(&ND);
-  // FIXME: this is not classifying constructors, destructors and operators
-  //correctly (they're all "methods").
+  // FIXME: This does not classify operators correctly, they are cosnidered
+  // methods.
   SymbolKind SK = indexSymbolKindToSymbolKind(SymInfo.Kind);
 
   DocumentSymbol SI;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89936: clang-tidy: adding "--clang-tidy-config=" to specify custom config file

[Hiral via Phabricator via cfe-commits]

Hiralo created this revision.
Hiralo added reviewers: alexfh, njames93, hokein, DmitryPolukhin.
Hiralo added a project: clang-tools-extra.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Hiralo requested review of this revision.

commit 5c98467825b128aabd04a770ac26905d856a3757
Author: Hiral Oza 
Date:   Wed Oct 21 23:30:22 2020 -0700

  clang-tidy: adding "--clang-tidy-config=" to specify custom config 
file.
  
  Let clang-tidy to read config from specified file.
  Example:
   $ clang-tidy --clang-tidy-config=/some/path/myTidyConfig --list-checks --
 ...this will read config from '/some/path/myTidyConfig'.
  
  May speed-up tidy runtime since now it will just look-up 
  instead of searching ".clang-tidy" in parent-dir(s).
  
  Directly specifying config path helps setting build dependencies.

Thank you in advance for your kind review.
-Hiral


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89936

Files:
  clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
  clang-tools-extra/clang-tidy/ClangTidyOptions.h
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

Index: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -73,6 +73,13 @@
 )"),
cl::init(""), cl::cat(ClangTidyCategory));
 
+static cl::opt ClangTidyConfig("clang-tidy-config", cl::desc(R"(
+Specify full path of .clang-tidy config file.
+For example, --clang-tidy-config=/some/path/myTidyConfig
+)"),
+cl::init(""),
+cl::cat(ClangTidyCategory));
+
 static cl::opt WarningsAsErrors("warnings-as-errors", cl::desc(R"(
 Upgrades warnings to errors. Same format as
 '-checks'.
@@ -279,6 +286,7 @@
 
   ClangTidyOptions DefaultOptions;
   DefaultOptions.Checks = DefaultChecks;
+  DefaultOptions.ClangTidyConfig = "";
   DefaultOptions.WarningsAsErrors = "";
   DefaultOptions.HeaderFilterRegex = HeaderFilter;
   DefaultOptions.SystemHeaders = SystemHeaders;
@@ -291,6 +299,8 @@
   ClangTidyOptions OverrideOptions;
   if (Checks.getNumOccurrences() > 0)
 OverrideOptions.Checks = Checks;
+  if (ClangTidyConfig.getNumOccurrences() > 0)
+OverrideOptions.ClangTidyConfig = ClangTidyConfig;
   if (WarningsAsErrors.getNumOccurrences() > 0)
 OverrideOptions.WarningsAsErrors = WarningsAsErrors;
   if (HeaderFilter.getNumOccurrences() > 0)
Index: clang-tools-extra/clang-tidy/ClangTidyOptions.h
===
--- clang-tools-extra/clang-tidy/ClangTidyOptions.h
+++ clang-tools-extra/clang-tidy/ClangTidyOptions.h
@@ -64,6 +64,9 @@
   /// Checks filter.
   llvm::Optional Checks;
 
+  /// Clang-tidy-config
+  llvm::Optional ClangTidyConfig;
+
   /// WarningsAsErrors filter.
   llvm::Optional WarningsAsErrors;
 
@@ -227,6 +230,8 @@
   /// Try to read configuration files from \p Directory using registered
   /// \c ConfigHandlers.
   llvm::Optional tryReadConfigFile(llvm::StringRef Directory);
+  llvm::Optional tryReadConfigFile(llvm::StringRef Path,
+  bool IsFile);
 
   llvm::StringMap CachedOptions;
   ClangTidyOptions OverrideOptions;
Index: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
@@ -109,6 +109,7 @@
 ClangTidyOptions ClangTidyOptions::getDefaults() {
   ClangTidyOptions Options;
   Options.Checks = "";
+  Options.ClangTidyConfig = "";
   Options.WarningsAsErrors = "";
   Options.HeaderFilterRegex = "";
   Options.SystemHeaders = false;
@@ -306,7 +307,41 @@
   << "...\n");
   assert(FS && "FS must be set.");
 
-  llvm::SmallString<128> AbsoluteFilePath(FileName);
+  llvm::SmallString<1024> AbsoluteFilePath(FileName);
+  if (!OverrideOptions.ClangTidyConfig.getValue().empty()) {
+AbsoluteFilePath.assign(OverrideOptions.ClangTidyConfig.getValue());
+if (FS->makeAbsolute(AbsoluteFilePath)) {
+  std::string Msg;
+  llvm::raw_string_ostream ErrStream(Msg);
+  ErrStream << " reading configuration from <" << AbsoluteFilePath
+<< "> can't make absolute path.\n";
+  llvm::report_fatal_error(ErrStream.str());
+}
+bool IsFile = false;
+bool IsLink = false;
+llvm::sys::fs::is_regular_file(Twine(AbsoluteFilePath), IsFile);
+llvm::sys::fs::is_symlink_file(Twine(AbsoluteFilePath), IsLink);
+if (!(IsFile || IsLink)) {
+  std::string Msg;
+  llvm::raw_string_ostream ErrStream(Msg);
+  ErrStream << " reading configuration from <" << AbsoluteFilePath
+<< "> file doesn't exist or not regular/symlink file.\n";
+  llvm::report_fatal_error(ErrS

[PATCH] D89849: Add option 'exceptions' to pragma clang fp

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff updated this revision to Diff 299887.
sepavloff added a comment.

Added ActOnPragmaFPExceptions


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89849

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/test/CodeGen/pragma-fp-exc.cpp
  clang/test/Parser/pragma-fp.cpp

Index: clang/test/Parser/pragma-fp.cpp
===
--- clang/test/Parser/pragma-fp.cpp
+++ clang/test/Parser/pragma-fp.cpp
@@ -1,14 +1,14 @@
 // RUN: %clang_cc1 -std=c++11 -verify %s
 
 void test_0(int *List, int Length) {
-/* expected-error@+1 {{missing option; expected 'contract' or 'reassociate'}} */
+/* expected-error@+1 {{missing option; expected 'contract', 'reassociate' or 'exceptions'}} */
 #pragma clang fp
   for (int i = 0; i < Length; i++) {
 List[i] = i;
   }
 }
 void test_1(int *List, int Length) {
-/* expected-error@+1 {{invalid option 'blah'; expected 'contract' or 'reassociate'}} */
+/* expected-error@+1 {{invalid option 'blah'; expected 'contract', 'reassociate' or 'exceptions'}} */
 #pragma clang fp blah
   for (int i = 0; i < Length; i++) {
 List[i] = i;
@@ -62,3 +62,14 @@
 #pragma clang fp contract(fast)
   }
 }
+
+void test_9(float *dest, float a, float b) {
+/* expected-error@+1 {{unexpected argument 'on' to '#pragma clang fp exceptions'; expected 'ignore', 'maytrap' or 'strict'}} */
+#pragma clang fp exceptions(on)
+  *dest = a + b;
+}
+
+void test_10(float *dest, float a, float b) {
+#pragma clang fp exceptions(maytrap) contract(fast) reassociate(on)
+  *dest = a + b;
+}
Index: clang/test/CodeGen/pragma-fp-exc.cpp
===
--- /dev/null
+++ clang/test/CodeGen/pragma-fp-exc.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm -o - %s | FileCheck --check-prefix=CHECK-DEF %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -ffp-exception-behavior=strict -emit-llvm -o - %s | FileCheck --check-prefix=CHECK-STRICT %s
+
+float func_01(float x, float y, float z) {
+  float res = x + y;
+  {
+#pragma clang fp exceptions(maytrap)
+res += z;
+  }
+  return res;
+}
+// CHECK-DEF-LABEL: @_Z7func_01fff
+// CHECK-DEF:   call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.ignore")
+// CHECK-DEF:   call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
+
+// CHECK-STRICT-LABEL: @_Z7func_01fff
+// CHECK-STRICT:   call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+// CHECK-STRICT:   call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -1016,6 +1016,11 @@
   CurFPFeatures = NewFPFeatures.applyOverrides(getLangOpts());
 }
 
+void Sema::ActOnPragmaFPExceptions(SourceLocation Loc,
+   LangOptions::FPExceptionModeKind FPE) {
+  setExceptionMode(Loc, FPE);
+}
+
 void Sema::PushNamespaceVisibilityAttr(const VisibilityAttr *Attr,
SourceLocation Loc) {
   // Visibility calculations will consider the namespace's visibility.
Index: clang/lib/Parse/ParsePragma.cpp
===
--- clang/lib/Parse/ParsePragma.cpp
+++ clang/lib/Parse/ParsePragma.cpp
@@ -2850,11 +2850,12 @@
 namespace {
 /// Used as the annotation value for tok::annot_pragma_fp.
 struct TokFPAnnotValue {
-  enum FlagKinds { Contract, Reassociate };
+  enum FlagKinds { Contract, Reassociate, Exceptions };
   enum FlagValues { On, Off, Fast };
 
-  FlagKinds FlagKind;
-  FlagValues FlagValue;
+  llvm::Optional ContractValue;
+  llvm::Optional ReassociateValue;
+  llvm::Optional ExceptionsValue;
 };
 } // end anonymous namespace
 
@@ -2871,6 +2872,7 @@
 return;
   }
 
+  auto *AnnotValue = new (PP.getPreprocessorAllocator()) TokFPAnnotValue;
   while (Tok.is(tok::identifier)) {
 IdentifierInfo *OptionInfo = Tok.getIdentifierInfo();
 
@@ -2879,6 +2881,7 @@
 OptionInfo->getName())
 .Case("contract", TokFPAnnotValue::Contract)
 .Case("reassociate", TokFPAnnotValue::Reassociate)
+.Case("exceptions", TokFPAnnotValue::Exceptions)
 .Default(None);
 if (!FlagKind) {
   PP.Diag(Tok.getLocation(), diag::err_pragma_fp_invalid_option)
@@ -2897,25 +2900,49 @@
 if (Tok.isNot(tok::identifier)) {
   P

[PATCH] D89849: Add option 'exceptions' to pragma clang fp

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added inline comments.



Comment at: clang/lib/Parse/ParsePragma.cpp:3041
+  if (AnnotValue->ExceptionsValue)
+Actions.setExceptionMode(Tok.getLocation(), *AnnotValue->ExceptionsValue);
   ConsumeAnnotationToken();

mibintc wrote:
> sepavloff wrote:
> > mibintc wrote:
> > > Did you consider adding ActOnPragmaFPExceptions? 
> > The existing method `setExceptionMode` does everything that we need. Do you 
> > think it should be renamed to `ActOnPragmaFPExceptions`? Are there any 
> > advantages for this?
> I think it's easier to understand if it's written to parallel the other 2 fp 
> pragmas that have similar semantics (contract and reassociate). 
It makes sense. This is a parser semantic action, they usually have names 
`ActOn...`. I didn't rename `setExceptionMode `, as in future we might need to 
set exception behavior from a place other than Parser.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89849

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

[PATCH] D89920: Export TemplateArgumentMatcher so clients defining custom matchers don't need to use the internal namespace

[Manuel Klimek via Phabricator via cfe-commits]

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

lg


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89920

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

[PATCH] D89935: [clangd] NFC: Update FIXME comment regarding lack of c/dtor support

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: clang-tools-extra/clangd/FindSymbols.cpp:182
   index::SymbolInfo SymInfo = index::getSymbolInfo(&ND);
-  // FIXME: this is not classifying constructors, destructors and operators
-  //correctly (they're all "methods").
+  // FIXME: This does not classify operators correctly, they are cosnidered
+  // methods.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89935

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

[PATCH] D89935: [clangd] NFC: Update FIXME comment regarding lack of c/dtor support

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 299891.
kbobyrev marked an inline comment as done.
kbobyrev added a comment.

Fix typo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89935

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


Index: clang-tools-extra/clangd/FindSymbols.cpp
===
--- clang-tools-extra/clangd/FindSymbols.cpp
+++ clang-tools-extra/clangd/FindSymbols.cpp
@@ -179,8 +179,8 @@
 return llvm::None;
 
   index::SymbolInfo SymInfo = index::getSymbolInfo(&ND);
-  // FIXME: this is not classifying constructors, destructors and operators
-  //correctly (they're all "methods").
+  // FIXME: This does not classify operators correctly, they are considered
+  // methods.
   SymbolKind SK = indexSymbolKindToSymbolKind(SymInfo.Kind);
 
   DocumentSymbol SI;


Index: clang-tools-extra/clangd/FindSymbols.cpp
===
--- clang-tools-extra/clangd/FindSymbols.cpp
+++ clang-tools-extra/clangd/FindSymbols.cpp
@@ -179,8 +179,8 @@
 return llvm::None;
 
   index::SymbolInfo SymInfo = index::getSymbolInfo(&ND);
-  // FIXME: this is not classifying constructors, destructors and operators
-  //correctly (they're all "methods").
+  // FIXME: This does not classify operators correctly, they are considered
+  // methods.
   SymbolKind SK = indexSymbolKindToSymbolKind(SymInfo.Kind);
 
   DocumentSymbol SI;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89935: [clangd] NFC: Update FIXME comment regarding lack of c/dtor support

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added inline comments.



Comment at: clang-tools-extra/clangd/FindSymbols.cpp:182
   index::SymbolInfo SymInfo = index::getSymbolInfo(&ND);
-  // FIXME: this is not classifying constructors, destructors and operators
-  //correctly (they're all "methods").
+  // FIXME: This does not classify operators correctly, they are cosnidered
+  // methods.

njames93 wrote:
> 
Oops, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89935

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

[PATCH] D89852: [clangd] Use SmallString instead of std::string in marshalling code; NFC

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

I'm a little concerned by this, not sure if `Expected` and `Optional` play 
nicely with rvo. If the compiler can't optimise it's potentially a 256 byte 
memcpy in the return, granted that would still be cheaper than dynamic 
allocation. However it would most likely be faster if the functions took a 
reference to the `SmallString` (or more likely `SmallVectorImpl`) and 
returned an `llvm::Error` or `bool`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89852

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

[PATCH] D89184: Support complex target features combinations

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added a comment.

a few minor style comments that I noticed




Comment at: clang/lib/CodeGen/CodeGenFunction.h:4705
+size_t SubexpressionStart = 0;
+for (size_t i = 0; i < FeatureList.size(); ++i) {
+  char CurrentToken = FeatureList[i];

(style)
```
for (size_t i = 0, e = FeatureList.size(); i < e; ++i) {
```



Comment at: clang/lib/CodeGen/CodeGenFunction.h:4713
+  SubexpressionStart = i + 1;
+InParentheses++;
+break;

(style)
```
++InParentheses;
```



Comment at: clang/lib/CodeGen/CodeGenFunction.h:4716
+  case ')':
+InParentheses--;
+assert(InParentheses >= 0 && "Parentheses are not in pair");

```
--InParentheses;
```




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

https://reviews.llvm.org/D89184

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

[PATCH] D89832: [CUDA] Extract CUDA version from cuda.h if version.txt is not found

[Evgeny Mankov via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChains/Cuda.cpp:76-77
+return CudaVersion::CUDA_102;
+  if (raw_version < 11010)
+return CudaVersion::CUDA_110;
+  return CudaVersion::LATEST;

Please, add `CudaVersion::CUDA_111` declaration in `Cuda.h` and a corresponding 
`if` here. 
Btw, `switch` is possible here. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89832

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

[PATCH] D89862: [clangd] Give the server information about client's remote index protocol version

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

Regarding versioning of grpc layer. In addition to including a version number 
in every request, looks like there's the concept of "versioned-services".

So we basically change the package name to be versioned, i.e. `package 
clang.clangd.remote.v1` and every time we make a breaking change, we increment 
the version number and start a new service (while keeping the old one).
Hopefully most of the core pieces will be re-usable, hence this will likely 
only end up adding a new `service` definition with possibly new reply/request 
types.

That might be more manageable than having a version in every request. It will 
also make handling a little bit easier, as dispatch will happen in grpc layer 
and server wouldn't have to perform conditional checks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89862

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

[PATCH] D89935: [clangd] NFC: Update FIXME comment regarding lack of c/dtor support

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

Am I missing something? We still have:

  case index::SymbolKind::Constructor:
  case index::SymbolKind::Destructor:
return SymbolKind::Constructor;

in Protocol.cpp. E.g. Constructors and Destructors are still classified badly. 
I suppose the bit around `they're all methods` are wrong though, maybe just 
drop that bit ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89935

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

[clang] be39a6f - [X86] Add User Interrupts(UINTR) instructions

[via cfe-commits]

Author: Tianqing Wang
Date: 2020-10-22T17:33:07+08:00
New Revision: be39a6fe6fc6c30186152863a7fac624e22262f7

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

LOG: [X86] Add User Interrupts(UINTR) instructions

For more details about these instructions, please refer to the latest
ISE document:
https://software.intel.com/en-us/download/intel-architecture-instruction-set-extensions-programming-reference.

Reviewed By: craig.topper

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

Added: 
clang/lib/Headers/uintrintrin.h
clang/test/CodeGen/X86/x86-uintr-builtins.c
llvm/test/CodeGen/X86/uintr-intrinsics.ll

Modified: 
clang/docs/ClangCommandLineReference.rst
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/BuiltinsX86_64.def
clang/include/clang/Driver/Options.td
clang/lib/Basic/Targets/X86.cpp
clang/lib/Basic/Targets/X86.h
clang/lib/Headers/CMakeLists.txt
clang/lib/Headers/cpuid.h
clang/lib/Headers/x86gprintrin.h
clang/test/Driver/x86-target-features.c
clang/test/Preprocessor/predefined-arch-macros.c
clang/test/Preprocessor/x86_target_features.c
llvm/docs/ReleaseNotes.rst
llvm/include/llvm/IR/IntrinsicsX86.td
llvm/include/llvm/Support/X86TargetParser.def
llvm/lib/Support/Host.cpp
llvm/lib/Support/X86TargetParser.cpp
llvm/lib/Target/X86/X86.td
llvm/lib/Target/X86/X86ISelLowering.cpp
llvm/lib/Target/X86/X86ISelLowering.h
llvm/lib/Target/X86/X86InstrInfo.td
llvm/lib/Target/X86/X86Subtarget.h
llvm/test/MC/Disassembler/X86/x86-64.txt
llvm/test/MC/X86/x86-64.s

Removed: 




diff  --git a/clang/docs/ClangCommandLineReference.rst 
b/clang/docs/ClangCommandLineReference.rst
index 86bc9f637ab7..6864f6058a3f 100644
--- a/clang/docs/ClangCommandLineReference.rst
+++ b/clang/docs/ClangCommandLineReference.rst
@@ -3335,6 +3335,8 @@ X86
 
 .. option:: -mtsxldtrk, -mno-tsxldtrk
 
+.. option:: -muintr, -mno-uintr
+
 .. option:: -mvaes, -mno-vaes
 
 .. option:: -mvpclmulqdq, -mno-vpclmulqdq

diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 0e2915ccf73a..56337177e060 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -200,7 +200,9 @@ X86 Support in Clang
   implies -mtune=. -mtune=generic is the default with no -march or -mtune
   specified.
 
-- Support for feature ``HRESET`` has been added.
+- Support for ``HRESET`` instructions has been added.
+
+- Support for ``UINTR`` instructions has been added.
 
 Internal API Changes
 

diff  --git a/clang/include/clang/Basic/BuiltinsX86_64.def 
b/clang/include/clang/Basic/BuiltinsX86_64.def
index f66ae78f7e81..3e186af82ff7 100644
--- a/clang/include/clang/Basic/BuiltinsX86_64.def
+++ b/clang/include/clang/Basic/BuiltinsX86_64.def
@@ -94,6 +94,12 @@ TARGET_BUILTIN(__builtin_ia32_cvtusi2sd64, "V2dV2dUOiIi", 
"ncV:128:", "avx512f")
 TARGET_BUILTIN(__builtin_ia32_cvtusi2ss64, "V4fV4fUOiIi", "ncV:128:", 
"avx512f")
 TARGET_BUILTIN(__builtin_ia32_directstore_u64, "vULi*ULi", "n", "movdiri")
 
+// UINTR
+TARGET_BUILTIN(__builtin_ia32_clui, "v", "n", "uintr")
+TARGET_BUILTIN(__builtin_ia32_stui, "v", "n", "uintr")
+TARGET_BUILTIN(__builtin_ia32_testui, "Uc", "n", "uintr")
+TARGET_BUILTIN(__builtin_ia32_senduipi, "vUWi", "n", "uintr")
+
 // AMX
 TARGET_BUILTIN(__builtin_ia32_tile_loadconfig, "vvC*", "n", "amx-tile")
 TARGET_BUILTIN(__builtin_ia32_tile_storeconfig, "vvC*", "n", "amx-tile")

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index fd2cf008cb18..1d930dbed9c4 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3321,6 +3321,8 @@ def mtbm : Flag<["-"], "mtbm">, 
Group;
 def mno_tbm : Flag<["-"], "mno-tbm">, Group;
 def mtsxldtrk : Flag<["-"], "mtsxldtrk">, Group;
 def mno_tsxldtrk : Flag<["-"], "mno-tsxldtrk">, Group;
+def muintr : Flag<["-"], "muintr">, Group;
+def mno_uintr : Flag<["-"], "mno-uintr">, Group;
 def mvaes : Flag<["-"], "mvaes">, Group;
 def mno_vaes : Flag<["-"], "mno-vaes">, Group;
 def mvpclmulqdq : Flag<["-"], "mvpclmulqdq">, Group;

diff  --git a/clang/lib/Basic/Targets/X86.cpp b/clang/lib/Basic/Targets/X86.cpp
index 9b607a3b3941..c8d96f887e90 100644
--- a/clang/lib/Basic/Targets/X86.cpp
+++ b/clang/lib/Basic/Targets/X86.cpp
@@ -310,6 +310,8 @@ bool 
X86TargetInfo::handleTargetFeatures(std::vector &Features,
   HasSERIALIZE = true;
 } else if (Feature == "+tsxldtrk") {
   HasTSXLDTRK = true;
+} else if (Feature == "+uintr") {
+  HasUINTR = true;
 }
 
 X86SSEEnum Level = llvm::StringSwitch(Feature)
@@ -726,6 +728,8 @@ void X86TargetInfo::getTargetDefines(const LangOptions 
&Opts,
 Builder.defineMacro("__SERIALIZE__");
   if (HasTSXLDT

[PATCH] D89301: [X86] Add User Interrupts(UINTR) instructions

[Pengfei Wang 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 rGbe39a6fe6fc6: [X86] Add User Interrupts(UINTR) instructions 
(authored by tianqing, committed by pengfei).

Changed prior to commit:
  https://reviews.llvm.org/D89301?vs=299317&id=299902#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89301

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/BuiltinsX86_64.def
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/X86.cpp
  clang/lib/Basic/Targets/X86.h
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Headers/cpuid.h
  clang/lib/Headers/uintrintrin.h
  clang/lib/Headers/x86gprintrin.h
  clang/test/CodeGen/X86/x86-uintr-builtins.c
  clang/test/Driver/x86-target-features.c
  clang/test/Preprocessor/predefined-arch-macros.c
  clang/test/Preprocessor/x86_target_features.c
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/IR/IntrinsicsX86.td
  llvm/include/llvm/Support/X86TargetParser.def
  llvm/lib/Support/Host.cpp
  llvm/lib/Support/X86TargetParser.cpp
  llvm/lib/Target/X86/X86.td
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/lib/Target/X86/X86InstrInfo.td
  llvm/lib/Target/X86/X86Subtarget.h
  llvm/test/CodeGen/X86/uintr-intrinsics.ll
  llvm/test/MC/Disassembler/X86/x86-64.txt
  llvm/test/MC/X86/x86-64.s

Index: llvm/test/MC/X86/x86-64.s
===
--- llvm/test/MC/X86/x86-64.s
+++ llvm/test/MC/X86/x86-64.s
@@ -2018,3 +2018,35 @@
 // CHECK: hreset
 // CHECK: encoding: [0xf3,0x0f,0x3a,0xf0,0xc0,0x01]
 hreset $1
+
+// CHECK: uiret
+// CHECK: encoding: [0xf3,0x0f,0x01,0xec]
+uiret
+
+// CHECK: clui
+// CHECK: encoding: [0xf3,0x0f,0x01,0xee]
+clui
+
+// CHECK: stui
+// CHECK: encoding: [0xf3,0x0f,0x01,0xef]
+stui
+
+// CHECK: testui
+// CHECK: encoding: [0xf3,0x0f,0x01,0xed]
+testui
+
+// CHECK: senduipi %rax
+// CHECK: encoding: [0xf3,0x0f,0xc7,0xf0]
+senduipi %rax
+
+// CHECK: senduipi %rdx
+// CHECK: encoding: [0xf3,0x0f,0xc7,0xf2]
+senduipi %rdx
+
+// CHECK: senduipi %r8
+// CHECK: encoding: [0xf3,0x41,0x0f,0xc7,0xf0]
+senduipi %r8
+
+// CHECK: senduipi %r13
+// CHECK: encoding: [0xf3,0x41,0x0f,0xc7,0xf5]
+senduipi %r13
Index: llvm/test/MC/Disassembler/X86/x86-64.txt
===
--- llvm/test/MC/Disassembler/X86/x86-64.txt
+++ llvm/test/MC/Disassembler/X86/x86-64.txt
@@ -715,3 +715,27 @@
 
 # CHECK: hreset $1
 0xf3 0x0f 0x3a 0xf0 0xc0 0x01
+
+# CHECK: uiret
+0xf3,0x0f,0x01,0xec
+
+# CHECK: clui
+0xf3,0x0f,0x01,0xee
+
+# CHECK: stui
+0xf3,0x0f,0x01,0xef
+
+# CHECK: testui
+0xf3,0x0f,0x01,0xed
+
+# CHECK: senduipi %rax
+0xf3,0x0f,0xc7,0xf0
+
+# CHECK: senduipi %rdx
+0xf3,0x0f,0xc7,0xf2
+
+# CHECK: senduipi %r8
+0xf3,0x41,0x0f,0xc7,0xf0
+
+# CHECK: senduipi %r13
+0xf3,0x41,0x0f,0xc7,0xf5
Index: llvm/test/CodeGen/X86/uintr-intrinsics.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/uintr-intrinsics.ll
@@ -0,0 +1,34 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+uintr | FileCheck %s --check-prefix=X64
+; RUN: llc < %s -mtriple=x86_64-linux-gnux32 -mattr=+uintr | FileCheck %s --check-prefix=X32
+
+define i8 @test_uintr(i64 %arg) {
+; X64-LABEL: test_uintr:
+; X64:   # %bb.0: # %entry
+; X64-NEXT:clui
+; X64-NEXT:stui
+; X64-NEXT:senduipi %rdi
+; X64-NEXT:testui
+; X64-NEXT:setb %al
+; X64-NEXT:retq
+
+; X32-LABEL: test_uintr:
+; X32:   # %bb.0: # %entry
+; X32-NEXT:clui
+; X32-NEXT:stui
+; X32-NEXT:senduipi %rdi
+; X32-NEXT:testui
+; X32-NEXT:setb %al
+; X32-NEXT:retq
+entry:
+  call void @llvm.x86.clui()
+  call void @llvm.x86.stui()
+  call void @llvm.x86.senduipi(i64 %arg)
+  %0 = call i8 @llvm.x86.testui()
+  ret i8 %0
+}
+
+declare void @llvm.x86.clui()
+declare void @llvm.x86.stui()
+declare i8 @llvm.x86.testui()
+declare void @llvm.x86.senduipi(i64 %arg)
Index: llvm/lib/Target/X86/X86Subtarget.h
===
--- llvm/lib/Target/X86/X86Subtarget.h
+++ llvm/lib/Target/X86/X86Subtarget.h
@@ -415,6 +415,9 @@
   bool HasAMXBF16 = false;
   bool HasAMXINT8 = false;
 
+  /// Processor supports User Level Interrupt instructions
+  bool HasUINTR = false;
+
   /// Processor has a single uop BEXTR implementation.
   bool HasFastBEXTR = false;
 
@@ -742,6 +745,7 @@
   bool hasHRESET() const { return HasHRESET; }
   bool hasSERIALIZE() const { return HasSERIALIZE; }
   bool hasTSXLDTRK() const { return HasTSXLDTRK; }
+  bool hasUINTR() const { return HasUINTR; }
   bool useRetpolineIndirectCalls() const { return UseRetpolineIndirectCalls; }
   bool useRetpoline

[PATCH] D89852: [clangd] Use SmallString instead of std::string in marshalling code; NFC

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

Sorry I don't really follow what we are gaining by changes from string to 
SmallString. Could you explain if I am missing something?

But I think it makes sense to get rid of Optionals in the Marshaller for 
`{Local,Remote}IndexRoot`, as there's no difference between `None` vs `empty` 
for them (I think).




Comment at: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp:58
+this->RemoteIndexRoot =
+llvm::SmallString<256>(llvm::sys::path::convert_to_slash(
+RemoteIndexRoot, llvm::sys::path::Style::windows));

we seem to be explicitly creating a smallstring out of std::string. and all of 
the usages of remoteindexroot seems to be just using a stringref. why are we 
doing this exactly? to make sure the addition of trailing separator below 
doesn't do reallocation ? If so this only happens once per each remote-index, 
right? Is it really worth all the hassle?

same for `LocalIndexRoot`.



Comment at: 
clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp:379
 return URIString.takeError();
-  Location.FileURI = Strings.save(*URIString).begin();
+  Location.FileURI = Strings.save(URIString->str()).begin();
   Location.Start = fromProtobuf(Message.start());

don't we end up creating a string here anyways?

(same below)



Comment at: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h:100
   /// be missing at the same time.
-  llvm::Optional RemoteIndexRoot;
-  llvm::Optional LocalIndexRoot;
+  llvm::Optional> RemoteIndexRoot;
+  llvm::Optional> LocalIndexRoot;

what's the difference between `None` and empty string here ? Can we just get 
rid of Optional?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89852

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

[PATCH] D89862: [clangd] Give the server information about client's remote index protocol version

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

What's the goal here?

There are at least 3 mechanisms we can use to mitigate client/server skew:

1. extending payloads in compatible way, so mismatched versions result in 
correct behavior (switching from proto3 to proto2 gives us more options here if 
it's possible)
2. signalling the version in-band as in this patch
3. defining a new incompatible protocol with a new name, as in versioned 
services

IME 1 is the most maintainable way to handle minor changes, while 3 is the most 
maintainable way to handle dramatic changes. Maybe there's an intermediate spot 
where 2 is the best option, but I'd like to understand concretely what scenario 
we might be solving.

OTOH, including/logging client version to make it easier to *debug* errors, 
without changing server logic, makes a lot of sense.
In that case we should use the VCS version (the version included in clangd 
--version) so we don't have another string we need to bump.




Comment at: clang-tools-extra/clangd/index/remote/Index.proto:63
   uint32 limit = 3;
+  string client_version = 4;
 }

rather than add this to every message, can we use ClientContext::AddMetadata to 
do this in a cross-cutting way?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89862

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

[PATCH] D89942: Disable LTO and LLD for bootstrap builds on systems unsupported by LLD

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

tbaeder created this revision.
tbaeder added a reviewer: serge-sans-paille.
Herald added subscribers: cfe-commits, pengfei, kristof.beyls, inglorion, 
mgorny.
Herald added a project: clang.
tbaeder requested review of this revision.

I tried doing a stage3 build on SystemZ/s390 but ran into the unfortunate 
problem that LLD does not support SystemZ, so the stage3 build can't use LLD 
and also can't use LTO.
Implementing the logic to only use LTO/LLD on non-SystemZ  system inside the 
`3-stage-base.cmake` was not possible however since none of the variables to 
check are set at that point. According to 
https://cmake.org/cmake/help/latest/manual/cmake.1.html#options, a cache file 
passed to `cmake -C` should consist of `set()` calls to populate the cache.

I'm not sure if the previous `if (APPLE)` check worked on apple systems at all. 
I'm also not sure if setting any of the `BOOTSTRAP_` variables inside a 
non-cache cmake file should happen at all.

I did test this patch on x86_64, s390, ppc64le and aarch64, but I'm not sure if 
SystemZ/s390 is the only problematic architecture and if there is a better way 
of checking whether we're on a non-lld-supported arch.

Anyway, happy to hear comments on this patch. Thanks!


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89942

Files:
  clang/CMakeLists.txt
  clang/cmake/caches/3-stage-base.cmake


Index: clang/cmake/caches/3-stage-base.cmake
===
--- clang/cmake/caches/3-stage-base.cmake
+++ clang/cmake/caches/3-stage-base.cmake
@@ -1,16 +1,6 @@
 set(CMAKE_BUILD_TYPE RELEASE CACHE STRING "")
 set(CLANG_ENABLE_BOOTSTRAP ON CACHE BOOL "")
 set(LLVM_BUILD_EXTERNAL_COMPILER_RT ON CACHE BOOL "")
-set(BOOTSTRAP_LLVM_ENABLE_LTO ON CACHE BOOL "")
-
-# Use LLD do have less requirements on system linker, unless we're on an apple
-# platform where the system compiler is to be prefered.
-if(APPLE)
-set(BOOTSTRAP_LLVM_ENABLE_LLD OFF CACHE BOOL "")
-else()
-set(BOOTSTRAP_LLVM_ENABLE_LLD ON CACHE BOOL "")
-endif()
-
 
 set(CLANG_BOOTSTRAP_TARGETS
   clang
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -656,6 +656,22 @@
   set(STAMP_DIR ${CMAKE_CURRENT_BINARY_DIR}/${NEXT_CLANG_STAGE}-stamps/)
   set(BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/${NEXT_CLANG_STAGE}-bins/)
 
+  # We want LLD for LTO, but lld does not support SystemZ, so disable
+  # LTO here and use the installed linker
+  if ("${LLVM_NATIVE_ARCH}" MATCHES "SystemZ")
+message(STATUS "Disabling LTO for stage3 builds since LLD does not support 
${LLVM_NATIVE_ARCH}")
+set(BOOTSTRAP_LLVM_ENABLE_LTO OFF CACHE BOOL "")
+  elseif(APPLE)
+# Use LLD do have less requirements on system linker, unless we're on an 
apple
+# platform where the system compiler is to be prefered.
+message(STATUS "Using system linker for stage3 builds on Apple")
+set(BOOTSTRAP_LLVM_ENABLE_LLD OFF CACHE BOOL "")
+  else()
+set(BOOTSTRAP_LLVM_ENABLE_LLD ON CACHE BOOL "")
+set(BOOTSTRAP_LLVM_ENABLE_LTO ON CACHE BOOL "")
+  endif()
+  message(STATUS "Stage3 builds: LLD: ${BOOTSTRAP_LLVM_ENABLE_LLD}. LTO: 
${BOOTSTRAP_LLVM_ENABLE_LTO}")
+
   if(BOOTSTRAP_LLVM_ENABLE_LLD)
 add_dependencies(clang-bootstrap-deps lld)
   endif()


Index: clang/cmake/caches/3-stage-base.cmake
===
--- clang/cmake/caches/3-stage-base.cmake
+++ clang/cmake/caches/3-stage-base.cmake
@@ -1,16 +1,6 @@
 set(CMAKE_BUILD_TYPE RELEASE CACHE STRING "")
 set(CLANG_ENABLE_BOOTSTRAP ON CACHE BOOL "")
 set(LLVM_BUILD_EXTERNAL_COMPILER_RT ON CACHE BOOL "")
-set(BOOTSTRAP_LLVM_ENABLE_LTO ON CACHE BOOL "")
-
-# Use LLD do have less requirements on system linker, unless we're on an apple
-# platform where the system compiler is to be prefered.
-if(APPLE)
-set(BOOTSTRAP_LLVM_ENABLE_LLD OFF CACHE BOOL "")
-else()
-set(BOOTSTRAP_LLVM_ENABLE_LLD ON CACHE BOOL "")
-endif()
-
 
 set(CLANG_BOOTSTRAP_TARGETS
   clang
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -656,6 +656,22 @@
   set(STAMP_DIR ${CMAKE_CURRENT_BINARY_DIR}/${NEXT_CLANG_STAGE}-stamps/)
   set(BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/${NEXT_CLANG_STAGE}-bins/)
 
+  # We want LLD for LTO, but lld does not support SystemZ, so disable
+  # LTO here and use the installed linker
+  if ("${LLVM_NATIVE_ARCH}" MATCHES "SystemZ")
+message(STATUS "Disabling LTO for stage3 builds since LLD does not support ${LLVM_NATIVE_ARCH}")
+set(BOOTSTRAP_LLVM_ENABLE_LTO OFF CACHE BOOL "")
+  elseif(APPLE)
+# Use LLD do have less requirements on system linker, unless we're on an apple
+# platform where the system compiler is to be prefered.
+message(STATUS "Using system linker for stage3 builds on Apple")
+set(BOOTSTRAP_LLVM_ENABLE_LLD OFF CACHE BO

[PATCH] D89935: [clangd] NFC: Update FIXME comment regarding lack of c/dtor support

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added a comment.

In D89935#2346648 , @kadircet wrote:

> Am I missing something? We still have:
>
>   case index::SymbolKind::Constructor:
>   case index::SymbolKind::Destructor:
> return SymbolKind::Constructor;
>
> in Protocol.cpp. E.g. Constructors and Destructors are still classified 
> badly. I suppose the bit around `they're all methods` are wrong though, maybe 
> just drop that bit ?

Yeah but the LSP `SymbolKind` which we are converting to does not have a 
destructor type, same thing with `CompletionItemKind`, so I guess we really do 
treat ctors and dtors the same way from the LSP perspective, aren't we?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89935

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

[PATCH] D89862: [clangd] Give the server information about client's remote index protocol version

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D89862#2346714 , @sammccall wrote:

> IME 1 is the most maintainable way to handle minor changes, while 3 is the 
> most maintainable way to handle dramatic changes.

(I realize I haven't backed this up - it's basically about how many codepaths 
you have to maintain for how long, and how complex they are. Happy to get into 
details if it's useful, but didn't want to derail too much)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89862

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

[PATCH] D89935: [clangd] NFC: Update FIXME comment regarding lack of c/dtor support

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

> Yeah but the LSP SymbolKind which we are converting to does not have a 
> destructor type, same thing with CompletionItemKind, so I guess we really do 
> treat ctors and dtors the same way from the LSP perspective, aren't we?

Yes, and that's what the previous fixme was saying. Now we are dropping that 
bit, but the code is still behaving badly?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89935

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

[PATCH] D89946: [clang] Suppress "follow-up" diagnostics on recovery call expressions.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added reviewers: rsmith, sammccall.
Herald added a subscriber: kristof.beyls.
Herald added a project: clang.
hokein requested review of this revision.

Because of typo-correction, the AST can be transformed, and the transformed
AST is marginally useful for diagnostics purpose, the following
diagnostics usually do harm than good (easily cause confusions).

Given the following code:

  void abcc();
  void test() {
if (abc());
// diagnostic 1 (for the typo-correction): the typo is correct to `abcc()`, 
so the code is treate as `if (abcc())` in AST perspective;
// diagnostic 2 (for mismatch type): we perform an type-analysis on `if`, 
discover the type is not match
  }

The secondary diagnostic "convertable to bool" is likely bogus to users.

The idea is to use RecoveryExpr (clang's dependent mechanism) to preserve the
recovery behavior but suppress all follow-up diagnostics.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89946

Files:
  clang/lib/Sema/SemaOverload.cpp
  clang/test/CXX/basic/basic.lookup/basic.lookup.argdep/p4.cpp
  clang/test/Modules/submodules-merge-defs.cpp
  clang/test/SemaCXX/enable_if.cpp
  clang/test/SemaCXX/typo-correction-delayed.cpp

Index: clang/test/SemaCXX/typo-correction-delayed.cpp
===
--- clang/test/SemaCXX/typo-correction-delayed.cpp
+++ clang/test/SemaCXX/typo-correction-delayed.cpp
@@ -209,6 +209,15 @@
 // expected-error-re@-1 {{use of undeclared identifier 'N'{{$
 }
 
+namespace noSecondaryDiags {
+void abcc(); // expected-note {{'abcc' declared here}}
+
+void test() {
+  // Verify the secondary diagnostic ".. convertiable to 'bool'" is suppressed.
+  if (abc()) {} // expected-error {{use of undeclared identifier 'abc'; did you mean 'abcc'?}}
+}
+}
+
 // PR 23285. This test must be at the end of the file to avoid additional,
 // unwanted diagnostics.
 // expected-error-re@+2 {{use of undeclared identifier 'uintmax_t'{{$
Index: clang/test/SemaCXX/enable_if.cpp
===
--- clang/test/SemaCXX/enable_if.cpp
+++ clang/test/SemaCXX/enable_if.cpp
@@ -356,9 +356,9 @@
   __attribute__((enable_if(num != 1, "")));
 }  // namespace ns
 
-using ns::Function; // expected-note 3{{declared here}}
+using ns::Function; // expected-note 2{{declared here}}
 void Run() {
-  Functioon(0); // expected-error{{use of undeclared identifier}} expected-error{{too few arguments}}
+  Functioon(0); // expected-error{{use of undeclared identifier}}
   Functioon(0, 1); // expected-error{{use of undeclared identifier}}
   Functioon(0, 1, 2); // expected-error{{use of undeclared identifier}}
 }
Index: clang/test/Modules/submodules-merge-defs.cpp
===
--- clang/test/Modules/submodules-merge-defs.cpp
+++ clang/test/Modules/submodules-merge-defs.cpp
@@ -18,7 +18,7 @@
 // expected-error-re@-1 {{missing '#include "{{.*}}-defs.h"'; 'A' must be declared}}
 // expected-note@defs.h:1 +{{here}}
 extern class A pre_a2;
-int pre_use_a = use_a(pre_a2); // expected-error 2{{'A' must be defined}} expected-error {{'use_a' must be declared}}
+int pre_use_a = use_a(pre_a2); // expected-error {{'use_a' must be declared}}
 // expected-note@defs.h:2 +{{here}}
 
 B::Inner2 pre_bi; // expected-error +{{must be declared}} expected-error +{{must be defined}}
Index: clang/test/CXX/basic/basic.lookup/basic.lookup.argdep/p4.cpp
===
--- clang/test/CXX/basic/basic.lookup/basic.lookup.argdep/p4.cpp
+++ clang/test/CXX/basic/basic.lookup/basic.lookup.argdep/p4.cpp
@@ -21,12 +21,10 @@
 }
 
 namespace C {
-  class C {}; // expected-note {{candidate constructor (the implicit copy constructor) not viable: no known conversion from 'B::B' to 'const C::C &' for 1st argument}}
+  class C {};
 #if __cplusplus >= 201103L // C++11 or later
-  // expected-note@-2 {{candidate constructor (the implicit move constructor) not viable: no known conversion from 'B::B' to 'C::C &&' for 1st argument}}
 #endif
-  void func(C); // expected-note {{'C::func' declared here}} \
-// expected-note {{passing argument to parameter here}}
+  void func(C); // expected-note {{'C::func' declared here}}
   C operator+(C,C);
   D::D operator+(D::D,D::D);
 }
@@ -38,13 +36,7 @@
 namespace Test {
   void test() {
 func(A::A());
-// FIXME: namespace-aware typo correction causes an extra, misleading
-// message in this case; some form of backtracking, diagnostic message
-// delaying, or argument checking before emitting diagnostics is needed to
-// avoid accepting and printing out a typo correction that proves to be
-// incorrect once argument-dependent lookup resolution has occurred.
-func(B::B()); // expected-error {{use of undeclared identifier 'func'; did you mean 'C::func'?}} \
-  // expected-err

[PATCH] D89946: [clang] Suppress "follow-up" diagnostics on recovery call expressions.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang/test/Modules/submodules-merge-defs.cpp:21
 extern class A pre_a2;
-int pre_use_a = use_a(pre_a2); // expected-error 2{{'A' must be defined}} 
expected-error {{'use_a' must be declared}}
+int pre_use_a = use_a(pre_a2); // expected-error {{'use_a' must be declared}}
 // expected-note@defs.h:2 +{{here}}

this (and the case below) are slight regressions, but I think the following 
diagnostics are not a big deal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89946

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

[PATCH] D87989: [Flang][Driver] Add infrastructure for basic frontend actions and file I/O

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87989

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

[PATCH] D89886: [clang-tidy] Fix redefinition of module in the same module.modulemap file

[Alexander Kornienko via Phabricator via cfe-commits]

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

Looks good!




Comment at: clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp:27
+// detection.
+if (!File->getName().endswith("module.modulemap"))
+  FilesToRecord.insert(File);

Looking at the relevant code I find special file names like "module.map", 
"module_private.map", "module.private.modulemap". Is this problem relevant for 
any of those?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89886

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

[PATCH] D89886: [clang-tidy] Fix redefinition of module in the same module.modulemap file

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

Ah, btw, any chance of adding a test for this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89886

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

[PATCH] D89380: [clang-tidy] Fix for cppcoreguidelines-prefer-member-initializer to handle classes declared in macros

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp:492
+
+#define MACRO1 struct InMacro1 { int i; InMacro1() { i = 0; } };
+// CHECK-MESSAGES: :[[@LINE-1]]:54: warning: 'i' should be initialized in a 
member initializer of the constructor 
[cppcoreguidelines-prefer-member-initializer]

baloghadamsoftware wrote:
> alexfh wrote:
> > Could you add tests where the field name comes from a macro argument and 
> > from token pasting? Something along the lines of:
> > 
> > ```
> > #define MACRO4(field) struct InMacro1 { int field; InMacro1() { field = 0; 
> > } }
> > 
> > MACRO4(q);
> > 
> > #define MACRO5(X) X
> > 
> > MACRO5(struct InMacro1 { int field; InMacro1() { field = 0; } });
> > 
> > #define MACRO6(field) struct InMacro1 { int qqq ## field; InMacro1() { qqq 
> > ## field = 0; } }
> > 
> > MACRO6(q);
> > ```
> It seems that handling of macro parameters in the `SourceManager` is totally 
> off. The location in these cases are the macro call itself, but the spelling 
> location is not the real location inside the macro, but in `MACRO4` it is the 
> location of the argument still in the macro call. The case with `MACRO6` is 
> even worse, because its spelling location is erroneous. So I could only added 
> some fixmes. However, it does not crash, at least. That is the main intention 
> of this particular patch.
It's impossible to correctly handle replacements in all cases involving macros 
(apart from expanding all macros while applying fixes ;). The usual strategy is 
to warn always, but only suggest replacements when we can be sufficiently 
confident in their validity. In this case we can either disable fixes when any 
part of the code being touched is in a macro or try to detect that the whole 
range being modified is sort of "on the same level of macro hell". The 
`Lexer::makeFileCharRange` is supposed to help with the latter (see 
clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp for an example of 
using it).


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

https://reviews.llvm.org/D89380

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

[PATCH] D89935: [clangd] NFC: Update FIXME comment regarding lack of c/dtor support

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added a comment.

I thought the point of the comment was us not handling it properly rather than 
LSP not supporting it (e.g. LSP does support `Operator` but we do not). Then, 
the comment about ctor and dtor being indistinguishable probably belongs to 
`Protocol.h/cpp` and `SymbolKind` there in particular?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89935

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

[clang-tools-extra] 37558fd - [clang-tidy] Add links to check docs in comments

[Alexander Kornienko via cfe-commits]

Author: Alexander Kornienko
Date: 2020-10-22T13:31:21+02:00
New Revision: 37558fd29ee0af2302c051b8e70543cfc3e7ca91

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

LOG: [clang-tidy] Add links to check docs in comments

Added: 


Modified: 
clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.h
clang-tools-extra/clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.h
clang-tools-extra/clang-tidy/google/DefaultArgumentsCheck.h
clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.h
clang-tools-extra/clang-tidy/google/ExplicitMakePairCheck.h
clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.h
clang-tools-extra/clang-tidy/google/IntegerTypesCheck.h
clang-tools-extra/clang-tidy/google/OverloadedUnaryAndCheck.h
clang-tools-extra/clang-tidy/google/TodoCommentCheck.h
clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.h
clang-tools-extra/clang-tidy/google/UsingNamespaceDirectiveCheck.h

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.h 
b/clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.h
index 72f96d094562..075b5f0ce450 100644
--- a/clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.h
+++ b/clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.h
@@ -25,6 +25,9 @@ namespace readability {
 /// This check is similar to `-Wold-style-cast`, but it suggests automated 
fixes
 /// in some cases. The reported locations should not be 
diff erent from the
 /// ones generated by `-Wold-style-cast`.
+///
+/// For the user-facing documentation see:
+/// 
http://clang.llvm.org/extra/clang-tidy/checks/google-readability-casting.html
 class AvoidCStyleCastsCheck : public ClangTidyCheck {
 public:
   AvoidCStyleCastsCheck(StringRef Name, ClangTidyContext *Context)

diff  --git 
a/clang-tools-extra/clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.h 
b/clang-tools-extra/clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.h
index 6a690f60da1c..a9989ee51bcf 100644
--- a/clang-tools-extra/clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.h
+++ b/clang-tools-extra/clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.h
@@ -18,6 +18,9 @@ namespace readability {
 
 // Check for underscores in the names of googletest tests, per
 // 
https://github.com/google/googletest/blob/master/googletest/docs/faq.md#why-should-test-suite-names-and-test-names-not-contain-underscore
+///
+/// For the user-facing documentation see:
+/// 
http://clang.llvm.org/extra/clang-tidy/checks/google-readability-avoid-underscore-in-googletest-name.html
 class AvoidUnderscoreInGoogletestNameCheck : public ClangTidyCheck {
 public:
   using ClangTidyCheck::ClangTidyCheck;

diff  --git a/clang-tools-extra/clang-tidy/google/DefaultArgumentsCheck.h 
b/clang-tools-extra/clang-tidy/google/DefaultArgumentsCheck.h
index 574965dc556a..089d463ff985 100644
--- a/clang-tools-extra/clang-tidy/google/DefaultArgumentsCheck.h
+++ b/clang-tools-extra/clang-tidy/google/DefaultArgumentsCheck.h
@@ -18,6 +18,9 @@ namespace google {
 /// Checks that default parameters are not given for virtual methods.
 ///
 /// See https://google.github.io/styleguide/cppguide.html#Default_Arguments
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/google-default-arguments.html
 class DefaultArgumentsCheck : public ClangTidyCheck {
 public:
   DefaultArgumentsCheck(StringRef Name, ClangTidyContext *Context)

diff  --git a/clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.h 
b/clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.h
index 721ad4efc979..573b0e18f90d 100644
--- a/clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.h
+++ b/clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.h
@@ -18,6 +18,9 @@ namespace google {
 /// Checks that all single-argument constructors are explicit.
 ///
 /// See https://google.github.io/styleguide/cppguide.html#Explicit_Constructors
+///
+/// For the user-facing documentation see:
+/// 
http://clang.llvm.org/extra/clang-tidy/checks/google-explicit-constructor.html
 class ExplicitConstructorCheck : public ClangTidyCheck {
 public:
   ExplicitConstructorCheck(StringRef Name, ClangTidyContext *Context)

diff  --git a/clang-tools-extra/clang-tidy/google/ExplicitMakePairCheck.h 
b/clang-tools-extra/clang-tidy/google/ExplicitMakePairCheck.h
index dae66c8afa8b..56ca5b20966e 100644
--- a/clang-tools-extra/clang-tidy/google/ExplicitMakePairCheck.h
+++ b/clang-tools-extra/clang-tidy/google/ExplicitMakePairCheck.h
@@ -22,6 +22,9 @@ namespace build {
 /// specified explicitly, and such use isn't intended in any case.
 ///
 /// Corresponding cpplint.py check name: 'build/explicit_make_pair'.
+///
+/// For the user-fa

[PATCH] D89946: [clang] Suppress "follow-up" diagnostics on recovery call expressions.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Cool, this makes sense to me. I hadn't realized this doesn't go through 
typoexpr, that's a bit unfortunate we can't address this just in one place.

This is a change of behavior with recoveryAST off, but this change basically 
only affects C++ so that's not a widely-used configuration.

I think the resulting AST is better as the errors there are visible.
Can we have a test specifically of the shape of the AST produced?




Comment at: clang/lib/Sema/SemaOverload.cpp:12729
 ///
 /// Returns true if new candidates were found.
 static ExprResult

while here, drop the stale "returns true" bit? The return type covers the 
contract well enough IMO



Comment at: clang/lib/Sema/SemaOverload.cpp:12786
 
   // Build an implicit member call if appropriate.  Just drop the
   // casts and such from the call, we don't really care.

nit: now member call -> member access as we never really go on to build a 
member call



Comment at: clang/lib/Sema/SemaOverload.cpp:12801
 
+  // Offering "follow-up" diagnostics on these recovery call expressions seems
+  // like a bad risk/reward tradeoff, e.g. following diagnostics on a

This seems a bit abrupt (because it's not obvious what the previous code is 
doing I guess).

Maybe

"We now have recovered a callee. However, building a real call may lead to 
incorrect
secondary diagnostics if our recovery wasn't correct.
We keep the recovery diagnostics and AST but suppress following diagnostics by
using RecoveryExpr"



Comment at: clang/lib/Sema/SemaOverload.cpp:12806
+  // using RecoveryExpr.
+  NewFn = SemaRef.CreateRecoveryExpr(NewFn.get()->getBeginLoc(),
+ NewFn.get()->getEndLoc(), {NewFn.get()});

I think giving the variable a new name here would be clearer



Comment at: clang/lib/Sema/SemaOverload.cpp:12811
+
   // This shouldn't cause an infinite loop because we're giving it
   // an expression with viable lookup results, which should never

comment is now out of date.



Comment at: clang/lib/Sema/SemaOverload.cpp:12814
   // end up here.
   return SemaRef.BuildCallExpr(/*Scope*/ nullptr, NewFn.get(), LParenLoc,
MultiExprArg(Args.data(), Args.size()),

This results in (IIUC):

```
CallExpr
>RecoveryExpr (indirection we inserted)
>>DeclRefExpr (callee)
>Arg1
```

Whereas when overload resolution fails, we create:
```
RecoveryExpr (call)
> OverloadExpr (callee)
> Arg1
```

I can see advantages for either, but is there a reason not to be consistent?
(Which I guess means emitting the first one here)



Comment at: clang/test/Modules/submodules-merge-defs.cpp:21
 extern class A pre_a2;
-int pre_use_a = use_a(pre_a2); // expected-error 2{{'A' must be defined}} 
expected-error {{'use_a' must be declared}}
+int pre_use_a = use_a(pre_a2); // expected-error {{'use_a' must be declared}}
 // expected-note@defs.h:2 +{{here}}

hokein wrote:
> this (and the case below) are slight regressions, but I think the following 
> diagnostics are not a big deal.
I agree, the secondary diagnostics aren't wrong but I don't think they're 
necessary either.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89946

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

[PATCH] D89801: [SystemZ][z/OS] Set short-enums as the default for z/OS

[Abhina Sree via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D89801

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

[PATCH] D89862: [clangd] Give the server information about client's remote index protocol version

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 299932.
kbobyrev marked an inline comment as done.
kbobyrev added a comment.

Use ClientContext::AddMetadata to propagate VCS version info.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89862

Files:
  clang-tools-extra/clangd/index/remote/Client.cpp
  clang-tools-extra/clangd/index/remote/Index.proto


Index: clang-tools-extra/clangd/index/remote/Index.proto
===
--- clang-tools-extra/clangd/index/remote/Index.proto
+++ clang-tools-extra/clangd/index/remote/Index.proto
@@ -22,7 +22,9 @@
   rpc Relations(RelationsRequest) returns (stream RelationsReply) {}
 }
 
-message LookupRequest { repeated string ids = 1; }
+message LookupRequest {
+  repeated string ids = 1;
+}
 
 // The response is a stream of symbol messages and the terminating message
 // indicating the end of stream.
Index: clang-tools-extra/clangd/index/remote/Client.cpp
===
--- clang-tools-extra/clangd/index/remote/Client.cpp
+++ clang-tools-extra/clangd/index/remote/Client.cpp
@@ -15,6 +15,7 @@
 #include "marshalling/Marshalling.h"
 #include "support/Logger.h"
 #include "support/Trace.h"
+#include "clang/Basic/Version.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 
@@ -40,6 +41,7 @@
 const auto RPCRequest = ProtobufMarshaller->toProtobuf(Request);
 SPAN_ATTACH(Tracer, "Request", RPCRequest.DebugString());
 grpc::ClientContext Context;
+Context.AddMetadata("version", clang::getClangToolFullVersion("clangd"));
 std::chrono::system_clock::time_point Deadline =
 std::chrono::system_clock::now() + DeadlineWaitingTime;
 Context.set_deadline(Deadline);


Index: clang-tools-extra/clangd/index/remote/Index.proto
===
--- clang-tools-extra/clangd/index/remote/Index.proto
+++ clang-tools-extra/clangd/index/remote/Index.proto
@@ -22,7 +22,9 @@
   rpc Relations(RelationsRequest) returns (stream RelationsReply) {}
 }
 
-message LookupRequest { repeated string ids = 1; }
+message LookupRequest {
+  repeated string ids = 1;
+}
 
 // The response is a stream of symbol messages and the terminating message
 // indicating the end of stream.
Index: clang-tools-extra/clangd/index/remote/Client.cpp
===
--- clang-tools-extra/clangd/index/remote/Client.cpp
+++ clang-tools-extra/clangd/index/remote/Client.cpp
@@ -15,6 +15,7 @@
 #include "marshalling/Marshalling.h"
 #include "support/Logger.h"
 #include "support/Trace.h"
+#include "clang/Basic/Version.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 
@@ -40,6 +41,7 @@
 const auto RPCRequest = ProtobufMarshaller->toProtobuf(Request);
 SPAN_ATTACH(Tracer, "Request", RPCRequest.DebugString());
 grpc::ClientContext Context;
+Context.AddMetadata("version", clang::getClangToolFullVersion("clangd"));
 std::chrono::system_clock::time_point Deadline =
 std::chrono::system_clock::now() + DeadlineWaitingTime;
 Context.set_deadline(Deadline);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89862: [clangd] Give the server information about client's remote index protocol version

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 299933.
kbobyrev added a comment.

Remove formatting change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89862

Files:
  clang-tools-extra/clangd/index/remote/Client.cpp


Index: clang-tools-extra/clangd/index/remote/Client.cpp
===
--- clang-tools-extra/clangd/index/remote/Client.cpp
+++ clang-tools-extra/clangd/index/remote/Client.cpp
@@ -15,6 +15,7 @@
 #include "marshalling/Marshalling.h"
 #include "support/Logger.h"
 #include "support/Trace.h"
+#include "clang/Basic/Version.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 
@@ -40,6 +41,7 @@
 const auto RPCRequest = ProtobufMarshaller->toProtobuf(Request);
 SPAN_ATTACH(Tracer, "Request", RPCRequest.DebugString());
 grpc::ClientContext Context;
+Context.AddMetadata("version", clang::getClangToolFullVersion("clangd"));
 std::chrono::system_clock::time_point Deadline =
 std::chrono::system_clock::now() + DeadlineWaitingTime;
 Context.set_deadline(Deadline);


Index: clang-tools-extra/clangd/index/remote/Client.cpp
===
--- clang-tools-extra/clangd/index/remote/Client.cpp
+++ clang-tools-extra/clangd/index/remote/Client.cpp
@@ -15,6 +15,7 @@
 #include "marshalling/Marshalling.h"
 #include "support/Logger.h"
 #include "support/Trace.h"
+#include "clang/Basic/Version.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 
@@ -40,6 +41,7 @@
 const auto RPCRequest = ProtobufMarshaller->toProtobuf(Request);
 SPAN_ATTACH(Tracer, "Request", RPCRequest.DebugString());
 grpc::ClientContext Context;
+Context.AddMetadata("version", clang::getClangToolFullVersion("clangd"));
 std::chrono::system_clock::time_point Deadline =
 std::chrono::system_clock::now() + DeadlineWaitingTime;
 Context.set_deadline(Deadline);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89862: [clangd] Give the server information about client's remote index protocol version

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added a comment.

In D89862#2346645 , @kadircet wrote:

> Regarding versioning of grpc layer. In addition to including a version number 
> in every request, looks like there's the concept of "versioned-services".
>
> So we basically change the package name to be versioned, i.e. `package 
> clang.clangd.remote.v1` and every time we make a breaking change, we 
> increment the version number and start a new service (while keeping the old 
> one).
> Hopefully most of the core pieces will be re-usable, hence this will likely 
> only end up adding a new `service` definition with possibly new reply/request 
> types.
>
> That might be more manageable than having a version in every request. It will 
> also make handling a little bit easier, as dispatch will happen in grpc layer 
> and server wouldn't have to perform conditional checks.

Good point! @sammccall should we do the package versioning and update to 
mitigate potential breaking changes?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89862

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

[PATCH] D89852: [clangd] Use SmallString instead of std::string in marshalling code; NFC

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 299934.
kbobyrev marked 3 inline comments as done.
kbobyrev added a comment.

Get rid of llvm::Optional in local and remote paths.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89852

Files:
  clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
  clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h

Index: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
===
--- clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
+++ clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
@@ -95,8 +95,8 @@
   /// of them can be missing (if the machines are different they don't know each
   /// other's specifics and will only do one-way translation), but both can not
   /// be missing at the same time.
-  llvm::Optional RemoteIndexRoot;
-  llvm::Optional LocalIndexRoot;
+  std::string RemoteIndexRoot;
+  std::string LocalIndexRoot;
   llvm::BumpPtrAllocator Arena;
   llvm::UniqueStringSaver Strings;
 };
Index: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
===
--- clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
+++ clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
@@ -56,17 +56,17 @@
 assert(llvm::sys::path::is_absolute(RemoteIndexRoot));
 this->RemoteIndexRoot = llvm::sys::path::convert_to_slash(
 RemoteIndexRoot, llvm::sys::path::Style::windows);
-llvm::StringRef Path(*this->RemoteIndexRoot);
+llvm::StringRef Path(this->RemoteIndexRoot);
 if (!Path.endswith(PosixSeparator))
-  *this->RemoteIndexRoot += PosixSeparator;
+  this->RemoteIndexRoot += PosixSeparator;
   }
   if (!LocalIndexRoot.empty()) {
 assert(llvm::sys::path::is_absolute(LocalIndexRoot));
 this->LocalIndexRoot = llvm::sys::path::convert_to_slash(
 LocalIndexRoot, llvm::sys::path::Style::windows);
-llvm::StringRef Path(*this->LocalIndexRoot);
+llvm::StringRef Path(this->LocalIndexRoot);
 if (!Path.endswith(PosixSeparator))
-  *this->LocalIndexRoot += PosixSeparator;
+  this->LocalIndexRoot += PosixSeparator;
   }
   assert(!RemoteIndexRoot.empty() || !LocalIndexRoot.empty());
 }
@@ -83,7 +83,7 @@
 
 llvm::Expected
 Marshaller::fromProtobuf(const FuzzyFindRequest *Message) {
-  assert(RemoteIndexRoot);
+  assert(!RemoteIndexRoot.empty());
   clangd::FuzzyFindRequest Result;
   Result.Query = Message->query();
   for (const auto &Scope : Message->scopes())
@@ -93,7 +93,7 @@
 Result.Limit = Message->limit();
   Result.RestrictForCodeCompletion = Message->restricted_for_code_completion();
   for (const auto &Path : Message->proximity_paths()) {
-llvm::SmallString<256> LocalPath = llvm::StringRef(*RemoteIndexRoot);
+llvm::SmallString<256> LocalPath = llvm::StringRef(RemoteIndexRoot);
 llvm::sys::path::append(LocalPath, Path);
 // FuzzyFindRequest requires proximity paths to have platform-native format
 // in order for SymbolIndex to process the query correctly.
@@ -202,7 +202,7 @@
 }
 
 FuzzyFindRequest Marshaller::toProtobuf(const clangd::FuzzyFindRequest &From) {
-  assert(LocalIndexRoot);
+  assert(!LocalIndexRoot.empty());
   FuzzyFindRequest RPCRequest;
   RPCRequest.set_query(From.Query);
   for (const auto &Scope : From.Scopes)
@@ -213,7 +213,7 @@
   RPCRequest.set_restricted_for_code_completion(From.RestrictForCodeCompletion);
   for (const auto &Path : From.ProximityPaths) {
 llvm::SmallString<256> RelativePath = llvm::StringRef(Path);
-if (llvm::sys::path::replace_path_prefix(RelativePath, *LocalIndexRoot, ""))
+if (llvm::sys::path::replace_path_prefix(RelativePath, LocalIndexRoot, ""))
   RPCRequest.add_proximity_paths(llvm::sys::path::convert_to_slash(
   RelativePath, llvm::sys::path::Style::windows));
   }
@@ -301,20 +301,20 @@
 
 llvm::Expected
 Marshaller::relativePathToURI(llvm::StringRef RelativePath) {
-  assert(LocalIndexRoot);
+  assert(!LocalIndexRoot.empty());
   assert(RelativePath == llvm::sys::path::convert_to_slash(RelativePath));
   if (RelativePath.empty())
 return error("Empty relative path.");
   if (llvm::sys::path::is_absolute(RelativePath, llvm::sys::path::Style::posix))
 return error("RelativePath '{0}' is absolute.", RelativePath);
-  llvm::SmallString<256> FullPath = llvm::StringRef(*LocalIndexRoot);
+  llvm::SmallString<256> FullPath = llvm::StringRef(LocalIndexRoot);
   llvm::sys::path::append(FullPath, RelativePath);
   auto Result = URI::createFile(FullPath);
   return Result.toString();
 }
 
 llvm::Expected Marshaller::uriToRelativePath(llvm::StringRef URI) {
-  assert(RemoteIndexRoot);
+  assert(!RemoteIndexRoot.empty());
   auto ParsedURI = URI::parse(URI);
   if (!ParsedURI)
 return ParsedURI.takeError();
@@ -326,9 +326,9 @@
   i

[PATCH] D85697: [clang-tidy] Add cppcoreguidelines-prefer-scoped-enums

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-scoped-enums.cpp:165
+
+enum class OpaqueScopedEnumWithFixedUnderlyingType : unsigned;

janosbenjaminantal wrote:
> aaron.ballman wrote:
> > njames93 wrote:
> > > janosbenjaminantal wrote:
> > > > aaron.ballman wrote:
> > > > > What should happen in cases like:
> > > > > ```
> > > > > // A case where the user has manually added a prefix they want; it
> > > > > // seems like fixing this to use a scoped enumeration changes the
> > > > > // expected way you interface with the enumeration by name.
> > > > > namespace EnumPrefix {
> > > > > enum Whatever {
> > > > >   One,
> > > > >   Two
> > > > > };
> > > > > }
> > > > > 
> > > > > // Another form of the same idea above.
> > > > > struct AnotherPrefix {
> > > > >   enum Whatever {
> > > > > One,
> > > > > Two
> > > > >   };
> > > > > };
> > > > > 
> > > > > // What about use in class hierarchies?
> > > > > // Header file the user controls
> > > > > enum SomeEnum {
> > > > >   One,
> > > > >   Two
> > > > > };
> > > > > 
> > > > > struct SomeType {
> > > > >   virtual void func(SomeEnum E) = 0; // Changing this may break the 
> > > > > hierarchy
> > > > > };
> > > > > 
> > > > > // Here's another situation where even warning is a bit suspect
> > > > > enum E : int;
> > > > > extern void func(E);
> > > > > ```
> > > > I think there is no good solution for the first two cases. We cannot be 
> > > > sure whether it is meant to be a prefix or not. It is just a very 
> > > > educated guess (if the the scope contains only one enum, then it is 
> > > > very like just a prefix, but might not be true in every cases). I don't 
> > > > have a very strong opinion about this, but I think this check can be 
> > > > useful even without supporting this case. Do you think it is a very 
> > > > common solution?
> > > > 
> > > > For the third one, I think if the fix is applied for every files, then 
> > > > it won't be a problem. If the fix cannot be applied to all of the files 
> > > > (e.g.: in case of a lib where the users can inherit from the classes 
> > > > defined in the lib) it will definitely break the hierarchy, however I 
> > > > don't think we can do anything about this. I am not totally sure, but I 
> > > > think this a problem for a lot of other checks too, e.g.: 
> > > > google-explicit-constructor or readability-identifier-naming. However I 
> > > > am afraid I misunderstood something, so let me in that case.
> > > > 
> > > > The last case with the extern function is a perfect place to suppress 
> > > > the warning in my opinion. How it could be decided, whether the user 
> > > > can change the function or not? Therefore I would keep the warning in 
> > > > that case and trust the users to suppress it if necessary.
> > > Maybe the check should only flag enumerations that appear at TU level, 
> > > any other enumeration is kind of scoped already.
> > >  Do you think it is a very common solution?
> > 
> > I think it likely is -- it was a common tactic used in pre-C++11 code bases 
> > (for instance, Clang uses this quite often, as in: 
> > https://github.com/llvm/llvm-project/blob/master/clang/include/clang/AST/Decl.h#L837)
> > 
> > > For the third one, I think if the fix is applied for every files, then it 
> > > won't be a problem. 
> > 
> > Agreed -- the scenario I was thinking about is when the virtual hierarchy 
> > is part of a published interface, like in a plugin system. Changing the 
> > base declaration may be easy, but you may not even *see* the other derived 
> > classes. I'm not certain it's a solvable issue.
> > 
> > > The last case with the extern function is a perfect place to suppress the 
> > > warning in my opinion.
> > 
> > Agreed.
> > 
> > > Maybe the check should only flag enumerations that appear at TU level, 
> > > any other enumeration is kind of scoped already.
> > 
> > I was sort of wondering the same thing, but perhaps instead of TU level, 
> > I'd go with TU or anonymous namespace so that we still diagnose things like 
> > these:
> > ```
> > namespace {
> > enum E { One }; // Seems reasonable to suggest scoping
> > namespace {
> > enum F { Two }; // Same here even though there are nested namespaces
> > }
> > }
> > ```
> I think the scoped vs unscoped enumeration is not about the scope itself, 
> more likely the conversion rules applied to them: the unscoped enumerations 
> are easily, even mistakenly convertible to/from `int`s. The core guideline 
> reasons with this: 
> https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Renum-class
> 
> From that point of view, it is reasonable to convert the enums that are in 
> their own namespaces also. Of course, it would be nice to eliminate the extra 
> scope, but I don't think we can detect it. From my understanding, the only we 
> could do is just check whether and enumeration is the only thing in the 
> namespace, and if yes,

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

[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 aside from a request for a comment to be added. Thank you!




Comment at: clang/lib/Sema/SemaDeclAttr.cpp:3706
+
+if (!Result || !Notes.empty()) {
+  Diag(E->getBeginLoc(), diag::err_attribute_argument_n_type)

Tyker wrote:
> aaron.ballman wrote:
> > I'm surprised that the presence of notes alone would mean the attribute 
> > argument has an error and should be skipped. Are there circumstances under 
> > which `Result` is true but `Notes` is non-empty?
> AFAIK
> Result means the expression can be folded to a constant.
> Note.empty() means the expression is a valid constant expression in the 
> current language mode.
> 
> the difference is observable in code like:
> ```
> constexpr int foldable_but_invalid() {
>   int *A = new int(0);
>   return *A;
> }
> 
> [[clang::annotate("", foldable_but_invalid())]] void f1() {}
> ```
> foldable_but_invalid retruns a constant but any constant evaluation of this 
> function is invalid because it doesn't desallocate A.
> with !Notes.empty() this fails, without it no errors occurs.
> 
> i think it is desirable that attributes don't diverge from the language mode.
> I added a test for this.
Ah, thank you for explaining that! Can you add a comment to the code to make 
that clear for others who may run across it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88645

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

[PATCH] D83088: Introduce CfgTraits abstraction

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

In D83088#2346322 , @mehdi_amini wrote:

> In D83088#2345540 , @nhaehnle wrote:
>
>> David, I don't think this is appropriate here. Let's take the discussion to 
>> llvm-dev.
>
> Seems like David asked to revert in the meantime?

-1 to reverting, which will just make the history messier with no tangible 
benefit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088

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

[clang] e4b4543 - [Clang] [TableGen] Clean up !if(!eq(bool, 1) and related booleans

[Paul C. Anagnostopoulos via cfe-commits]

Author: Paul C. Anagnostopoulos
Date: 2020-10-22T09:29:15-04:00
New Revision: e4b4543ff0c83b6f1d80064e2dcd22b2bb0bfab6

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

LOG: [Clang] [TableGen] Clean up !if(!eq(bool, 1) and related booleans

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

Added: 


Modified: 
clang/include/clang/Basic/arm_mve.td
clang/include/clang/Basic/arm_mve_defs.td

Removed: 




diff  --git a/clang/include/clang/Basic/arm_mve.td 
b/clang/include/clang/Basic/arm_mve.td
index 25daae2a0a25..94f76dd729b0 100644
--- a/clang/include/clang/Basic/arm_mve.td
+++ b/clang/include/clang/Basic/arm_mve.td
@@ -406,7 +406,7 @@ def vabdq: Intrinsic {
+  bit wantXVariant = 1> {
   defm "" : IntrinsicMX<
   Vector, (args Vector:$a, Vector:$b, Predicate:$pred),
   !con((IRInt $a, $b),
@@ -415,7 +415,7 @@ multiclass VectorVectorArithmetic {
+  bit wantXVariant = 1> {
   defm "" : IntrinsicMXNameOverride<
   Vector, (args Vector:$a, unpromoted:$b, Predicate:$pred),
   !con((IRInt $a, (splat $b)),
@@ -451,7 +451,7 @@ let params = T.Usual in {
 }
 
 multiclass DblVectorVectorArithmetic {
+ bit wantXVariant = 1> {
   defm "" : IntrinsicMX<
   DblVector, (args Vector:$a, Vector:$b, DblPredicate:$pred),
   !con((IRInt $a, $b),
@@ -460,7 +460,7 @@ multiclass DblVectorVectorArithmetic {
+ bit wantXVariant = 1> {
   defm "" : IntrinsicMXNameOverride<
   DblVector, (args Vector:$a, unpromoted:$b, DblPredicate:$pred),
   !con((IRInt $a, (splat 
$b)),

diff  --git a/clang/include/clang/Basic/arm_mve_defs.td 
b/clang/include/clang/Basic/arm_mve_defs.td
index 4038a18027f8..1a090c08cc85 100644
--- a/clang/include/clang/Basic/arm_mve_defs.td
+++ b/clang/include/clang/Basic/arm_mve_defs.td
@@ -519,7 +519,7 @@ class NameOverride {
 // polymorph 
diff erently (typically because the type of the inactive
 // parameter can be used as a disambiguator if it's present).
 multiclass IntrinsicMX {
@@ -532,7 +532,7 @@ multiclass IntrinsicMX) in {
+  if wantXVariant then {
 // The _x variant leaves off that parameter, and simply uses an
 // undef value of the same type.
 
@@ -546,7 +546,7 @@ multiclass IntrinsicMX {
@@ -556,7 +556,7 @@ multiclass IntrinsicMXNameOverride) in {
+  if wantXVariant then {
 def "_x" # nameSuffix:
   Intrinsic,
   NameOverride {



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

[PATCH] D89893: [Clang] [TableGen] Clean up !if(!eq(boolean, 1) and related booleans

[Paul C. Anagnostopoulos 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 rGe4b4543ff0c8: [Clang] [TableGen] Clean up !if(!eq(bool, 1) 
and related booleans (authored by Paul-C-Anagnostopoulos).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89893

Files:
  clang/include/clang/Basic/arm_mve.td
  clang/include/clang/Basic/arm_mve_defs.td


Index: clang/include/clang/Basic/arm_mve_defs.td
===
--- clang/include/clang/Basic/arm_mve_defs.td
+++ clang/include/clang/Basic/arm_mve_defs.td
@@ -519,7 +519,7 @@
 // polymorph differently (typically because the type of the inactive
 // parameter can be used as a disambiguator if it's present).
 multiclass IntrinsicMX {
@@ -532,7 +532,7 @@
 let pnt = pnt_m;
   }
 
-  foreach unusedVar = !if(!eq(wantXVariant, 1), [1], []) in {
+  if wantXVariant then {
 // The _x variant leaves off that parameter, and simply uses an
 // undef value of the same type.
 
@@ -546,7 +546,7 @@
 // Same as above, but with an additional parameter 'basename' which overrides
 // the C intrinsic base name
 multiclass IntrinsicMXNameOverride {
@@ -556,7 +556,7 @@
 let pnt = pnt_m;
   }
 
-  foreach unusedVar = !if(!eq(wantXVariant, 1), [1], []) in {
+  if wantXVariant then {
 def "_x" # nameSuffix:
   Intrinsic,
   NameOverride {
Index: clang/include/clang/Basic/arm_mve.td
===
--- clang/include/clang/Basic/arm_mve.td
+++ clang/include/clang/Basic/arm_mve.td
@@ -406,7 +406,7 @@
 }
 
 multiclass VectorVectorArithmetic {
+  bit wantXVariant = 1> {
   defm "" : IntrinsicMX<
   Vector, (args Vector:$a, Vector:$b, Predicate:$pred),
   !con((IRInt $a, $b),
@@ -415,7 +415,7 @@
 
 multiclass VectorScalarArithmetic {
+  bit wantXVariant = 1> {
   defm "" : IntrinsicMXNameOverride<
   Vector, (args Vector:$a, unpromoted:$b, Predicate:$pred),
   !con((IRInt $a, (splat $b)),
@@ -451,7 +451,7 @@
 }
 
 multiclass DblVectorVectorArithmetic {
+ bit wantXVariant = 1> {
   defm "" : IntrinsicMX<
   DblVector, (args Vector:$a, Vector:$b, DblPredicate:$pred),
   !con((IRInt $a, $b),
@@ -460,7 +460,7 @@
 
 multiclass DblVectorScalarArithmetic {
+ bit wantXVariant = 1> {
   defm "" : IntrinsicMXNameOverride<
   DblVector, (args Vector:$a, unpromoted:$b, DblPredicate:$pred),
   !con((IRInt $a, (splat 
$b)),


Index: clang/include/clang/Basic/arm_mve_defs.td
===
--- clang/include/clang/Basic/arm_mve_defs.td
+++ clang/include/clang/Basic/arm_mve_defs.td
@@ -519,7 +519,7 @@
 // polymorph differently (typically because the type of the inactive
 // parameter can be used as a disambiguator if it's present).
 multiclass IntrinsicMX {
@@ -532,7 +532,7 @@
 let pnt = pnt_m;
   }
 
-  foreach unusedVar = !if(!eq(wantXVariant, 1), [1], []) in {
+  if wantXVariant then {
 // The _x variant leaves off that parameter, and simply uses an
 // undef value of the same type.
 
@@ -546,7 +546,7 @@
 // Same as above, but with an additional parameter 'basename' which overrides
 // the C intrinsic base name
 multiclass IntrinsicMXNameOverride {
@@ -556,7 +556,7 @@
 let pnt = pnt_m;
   }
 
-  foreach unusedVar = !if(!eq(wantXVariant, 1), [1], []) in {
+  if wantXVariant then {
 def "_x" # nameSuffix:
   Intrinsic,
   NameOverride {
Index: clang/include/clang/Basic/arm_mve.td
===
--- clang/include/clang/Basic/arm_mve.td
+++ clang/include/clang/Basic/arm_mve.td
@@ -406,7 +406,7 @@
 }
 
 multiclass VectorVectorArithmetic {
+  bit wantXVariant = 1> {
   defm "" : IntrinsicMX<
   Vector, (args Vector:$a, Vector:$b, Predicate:$pred),
   !con((IRInt $a, $b),
@@ -415,7 +415,7 @@
 
 multiclass VectorScalarArithmetic {
+  bit wantXVariant = 1> {
   defm "" : IntrinsicMXNameOverride<
   Vector, (args Vector:$a, unpromoted:$b, Predicate:$pred),
   !con((IRInt $a, (splat $b)),
@@ -451,7 +451,7 @@
 }
 
 multiclass DblVectorVectorArithmetic {
+ bit wantXVariant = 1> {
   defm "" : IntrinsicMX<
   DblVector, (args Vector:$a, Vector:$b, DblPredicate:$pred),
   !con((IRInt $a, $b),
@@ -460,7 +460,7 @@
 
 multiclass DblVectorScalarArithmetic {
+ bit wantXVariant = 1> {
   defm "" : IntrinsicMXNameOverride<
   DblVector, (args Vector:$a, unpromoted:$b, DblPredicate:$pred),
   !con((IRInt $a, (splat $b)),
___
cfe-commits mailing list

[PATCH] D89862: [clangd] Give the server information about client's remote index protocol version

[Sam McCall via Phabricator via cfe-commits]

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

In D89862#2346967 , @kbobyrev wrote:

> In D89862#2346645 , @kadircet wrote:
>
>> Regarding versioning of grpc layer. In addition to including a version 
>> number in every request, looks like there's the concept of 
>> "versioned-services".
>>
>> So we basically change the package name to be versioned, i.e. `package 
>> clang.clangd.remote.v1` and every time we make a breaking change, we 
>> increment the version number and start a new service (while keeping the old 
>> one).
>> Hopefully most of the core pieces will be re-usable, hence this will likely 
>> only end up adding a new `service` definition with possibly new 
>> reply/request types.
>>
>> That might be more manageable than having a version in every request. It 
>> will also make handling a little bit easier, as dispatch will happen in grpc 
>> layer and server wouldn't have to perform conditional checks.
>
> Good point! @sammccall should we do the package versioning and update to 
> mitigate potential breaking changes?

Yep, tacking "v1" on this preemptively SGTM.

Note this itself is a breaking change :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89862

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

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

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Missing langref changes for new intrinsic.


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

https://reviews.llvm.org/D89959

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

[PATCH] D89785: [clangd] Add basic support for attributes (selection, hover)

[Sam McCall via Phabricator via cfe-commits]

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

Only deoptimize selection for *explicit* attributes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89785

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/AST.h
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/ASTTests.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang-tools-extra/clangd/unittests/SelectionTests.cpp

Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -415,7 +415,19 @@
 template  class Container> class A {};
 A<[[V^ector]]> a;
   )cpp",
-   "TemplateArgumentLoc"}};
+   "TemplateArgumentLoc"},
+
+  // Attributes
+  {R"cpp(
+void f(int * __attribute__(([[no^nnull]])) );
+  )cpp",
+   "NonNullAttr"},
+
+  {R"cpp(
+// Digraph syntax for attributes to avoid accidental annotations.
+class <:[gsl::Owner([[in^t]])]:> X{};
+  )cpp",
+   "BuiltinTypeLoc"}};
 
   for (const Case &C : Cases) {
 trace::TestTracer Tracer;
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1991,6 +1991,16 @@
 HI.NamespaceScope = "ObjC::"; // FIXME: fix it
 HI.Definition = "char data";
   }},
+  {
+  R"cpp(
+   void foo(int * __attribute__(([[non^null]], noescape)) );
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "nonnull";
+HI.Kind = index::SymbolKind::Unknown; // FIXME: no suitable value
+HI.Definition = "__attribute__((nonnull()))";
+HI.Documentation = ""; // FIXME
+  }},
   };
 
   // Create a tiny index, so tests above can verify documentation is fetched.
Index: clang-tools-extra/clangd/unittests/ASTTests.cpp
===
--- clang-tools-extra/clangd/unittests/ASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ASTTests.cpp
@@ -11,8 +11,11 @@
 #include "Annotations.h"
 #include "ParsedAST.h"
 #include "TestTU.h"
+#include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/Attr.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
+#include "clang/Basic/AttrKinds.h"
 #include "clang/Basic/SourceManager.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
@@ -25,6 +28,8 @@
 namespace clang {
 namespace clangd {
 namespace {
+using testing::Contains;
+using testing::Each;
 
 TEST(GetDeducedType, KwAutoExpansion) {
   struct Test {
@@ -206,6 +211,38 @@
 }
   }
 }
+
+MATCHER_P(attrKind, K, "") { return arg->getKind() == K; }
+
+MATCHER(implicitAttr, "") { return arg->isImplicit(); }
+
+TEST(ClangdAST, GetAttributes) {
+  const char *Code = R"cpp(
+class X{};
+class [[nodiscard]] Y{};
+void f(int * a, int * __attribute__((nonnull)) b);
+void foo(bool c) {
+  if (c)
+[[unlikely]] return;
+}
+  )cpp";
+  ParsedAST AST = TestTU::withCode(Code).build();
+  auto DeclAttrs = [&](llvm::StringRef Name) {
+return getAttributes(DynTypedNode::create(findUnqualifiedDecl(AST, Name)));
+  };
+  ASSERT_THAT(DeclAttrs("X"), Each(implicitAttr()));
+  ASSERT_THAT(DeclAttrs("Y"), Contains(attrKind(attr::WarnUnusedResult)));
+  ASSERT_THAT(DeclAttrs("f"), Each(implicitAttr()));
+  ASSERT_THAT(DeclAttrs("a"), Each(implicitAttr()));
+  ASSERT_THAT(DeclAttrs("b"), Contains(attrKind(attr::NonNull)));
+
+  Stmt *FooBody = cast(findDecl(AST, "foo")).getBody();
+  IfStmt *FooIf = cast(cast(FooBody)->body_front());
+  ASSERT_THAT(getAttributes(DynTypedNode::create(*FooIf)),
+  Each(implicitAttr()));
+  ASSERT_THAT(getAttributes(DynTypedNode::create(*FooIf->getThen())),
+  Contains(attrKind(attr::Unlikely)));
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -182,6 +182,12 @@
   ST.commonAncestor()) {
 if (NodeKind)
   *NodeKind = N->ASTNode.getNodeKind();
+// Attributes don't target decls, look at the
+// thing it's attached to.
+// We still report the original NodeKind!
+// This makes the `override` hack work.
+ 

[PATCH] D89960: Testing the use of arc.

[Zahira Ammarguellat via Phabricator via cfe-commits]

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

Change-Id: I94cb7ac295acc8de9aa2c60bb31a1f5ee7d86fde


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89960

Files:
  clang/lib/Parse/ParseCXXInlineMethods.cpp


Index: clang/lib/Parse/ParseCXXInlineMethods.cpp
===
--- clang/lib/Parse/ParseCXXInlineMethods.cpp
+++ clang/lib/Parse/ParseCXXInlineMethods.cpp
@@ -404,6 +404,7 @@
   if (Tok.is(tok::eof) && Tok.getEofData() == Param)
 ConsumeAnyToken();
 } else if (HasUnparsed) {
+  // This is a test to use arc.
   assert(Param->hasInheritedDefaultArg());
   FunctionDecl *Old = cast(LM.Method)->getPreviousDecl();
   ParmVarDecl *OldParam = Old->getParamDecl(I);


Index: clang/lib/Parse/ParseCXXInlineMethods.cpp
===
--- clang/lib/Parse/ParseCXXInlineMethods.cpp
+++ clang/lib/Parse/ParseCXXInlineMethods.cpp
@@ -404,6 +404,7 @@
   if (Tok.is(tok::eof) && Tok.getEofData() == Param)
 ConsumeAnyToken();
 } else if (HasUnparsed) {
+  // This is a test to use arc.
   assert(Param->hasInheritedDefaultArg());
   FunctionDecl *Old = cast(LM.Method)->getPreviousDecl();
   ParmVarDecl *OldParam = Old->getParamDecl(I);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89651: [clang-tidy] Add bugprone-suspicious-memory-comparison check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D89651#2342367 , @gbencze wrote:

> In D89651#2338266 , @njames93 wrote:
>
>> Should point out there is already a check for cert-oop57-cpp, added in 
>> D72488 . Do these play nice with each other 
>> or should they perhaps be merged or share code?
>
> I would certainly expect some duplicate warnings with there two checks, but 
> as far as I can tell that check does not currently warn on using `memcmp` 
> with non-standard-layout types. 
> I personally would leave the exp42 and flp37 parts of this check as separate, 
> because those are useful in C as well. But maybe it'd be better to move the 
> warning for non-standard-layout types from here to the existing one.

The thrust of the idea behind OOP57-CPP is that you shouldn't use C functions 
to do work that would normally be done via a C++ overloaded operator. e.g., 
don't call `memcmp()` on two structs, define the appropriate set of relational 
and equality operators for the type instead. It's required that you do this for 
non-trivial types, but it's aspirational to do it for all types. I don't think 
the proposed check should relate to OOP57-CPP all that much -- I see it more 
closely relating to EXP62-CPP instead: 
https://wiki.sei.cmu.edu/confluence/display/cplusplus/EXP62-CPP.+Do+not+access+the+bits+of+an+object+representation+that+are+not+part+of+the+object%27s+value+representation
 except that this check is currently only about comparisons and not accessing, 
so it's not quite perfect coverage.




Comment at: 
clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.cpp:45
+
+  for (unsigned int i = 0; i < 2; ++i) {
+const Expr *ArgExpr = CE->getArg(i);

The lint warning here is actually correct, which is a lovely change of pace.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.cpp:70
+  if (ComparedBits.hasValue() && *ComparedBits >= PointeeSize &&
+  !Ctx.hasUniqueObjectRepresentations(PointeeQualifiedType)) {
+diag(CE->getBeginLoc(),

Note that this may produce false positives as the list of objects with unique 
representations is not complete. For instance, it doesn't handle _Complex or 
_Atomic types, etc.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.cpp:73
+ "comparing object representation of type %0 which does not have "
+ "unique object representations; consider comparing the values "
+ "manually")

unique object representations -> a unique object representation

WDYT about:
consider comparing the values manually -> consider comparing %select{the 
values|the members of the object}0 manually

to make it more clear that these cases are different:
```
memcmp(&some_float, &some_other_float, sizeof(float));
memcmp(&some_struct, &some_other_struct, sizeof(some_struct));
```
The use of "values" make it sound a bit like the user should be able to do `if 
(some_struct == some_other_struct)` to me.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison-32bits.cpp:2
+// RUN: %check_clang_tidy %s bugprone-suspicious-memory-comparison %t \
+// RUN: -- -- -target x86_64-unknown-unknown -m32
+

Wouldn't picking a 32-bit target suffice instead of `-m32`? e.g., `-target 
i386-unknown-unknown`



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.c:121
+  memcmp(&a, &b, sizeof(int)); // no-warning: not comparing entire object
+  memcmp(&a, &b, 2 * sizeof(int));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing object representation 
of

Just to make it obvious, I think a test like this would also be handy:
```
struct Whatever {
  int i[2];
  char c;
};

struct Whatever one, two;
memcmp(&one, &two, 2 * sizeof(int)); // Shouldn't warn either
```
which brings up a pathological case that I have no idea how it should behave:
```
struct Whatever {
  int i[2];
  int : 0; // What now?!
};

struct Whatever one, two;
memcmp(&one, &two, 2 * sizeof(int)); // Warn? Don't Warn? Cry?
```



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.c:198
+}
+
+struct PaddingAfterUnion {

How about a test where everything lines up perfectly for bit-fields?
```
struct Whatever {
  int i : 10;
  int j : 10;
  int k : 10;
  int l : 2;
};
_Static_assert(sizeof(struct Whatever) == sizeof(int));
```



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.cpp:229
+  // consider comparing the values manually
+}
+} // namespace alignment

Another case we should be careful of is template instantiation

[PATCH] D89961: [libTooling] Add function to Transformer to create a no-op edit.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
ymandel added reviewers: tdl-g, gribozavr2.
Herald added a project: clang.
ymandel requested review of this revision.

This functionality is commonly needed in clang tidy checks (based on
transformer) that only print warnings, without suggesting any edits. The no-op
edit allows the user to associate a diagnostic message with a source location.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89961

Files:
  clang/include/clang/Tooling/Transformer/RewriteRule.h
  clang/lib/Tooling/Transformer/RewriteRule.cpp
  clang/unittests/Tooling/TransformerTest.cpp


Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -426,6 +426,14 @@
   testRule(makeRule(returnStmt().bind("return"), noEdits()), Input, Input);
 }
 
+TEST_F(TransformerTest, NoopEdit) {
+  using transformer::node;
+  using transformer::noopEdit;
+  std::string Input = "int f(int x) { return x; }";
+  testRule(makeRule(returnStmt().bind("return"), noopEdit(node("return"))),
+   Input, Input);
+}
+
 TEST_F(TransformerTest, IfBound2Args) {
   using transformer::ifBound;
   std::string Input = "int f(int x) { return x; }";
Index: clang/lib/Tooling/Transformer/RewriteRule.cpp
===
--- clang/lib/Tooling/Transformer/RewriteRule.cpp
+++ clang/lib/Tooling/Transformer/RewriteRule.cpp
@@ -73,6 +73,24 @@
   };
 }
 
+EditGenerator transformer::noopEdit(RangeSelector Anchor) {
+  return [Anchor = std::move(Anchor)](const MatchResult &Result)
+ -> Expected> {
+Expected Range = Anchor(Result);
+if (!Range)
+  return Range.takeError();
+// In case the range is inside a macro expansion, map the location back to 
a
+// "real" source location.
+SourceLocation Begin =
+Result.SourceManager->getSpellingLoc(Range->getBegin());
+Edit E;
+// Implicitly, leave `E.Replacement` as the empty string.
+E.Kind = EditKind::Range;
+E.Range = CharSourceRange::getCharRange(Begin, Begin);
+return SmallVector{E};
+  };
+}
+
 EditGenerator
 transformer::flattenVector(SmallVector Generators) {
   if (Generators.size() == 1)
Index: clang/include/clang/Tooling/Transformer/RewriteRule.h
===
--- clang/include/clang/Tooling/Transformer/RewriteRule.h
+++ clang/include/clang/Tooling/Transformer/RewriteRule.h
@@ -107,7 +107,7 @@
   TextGenerator Replacement;
   TextGenerator Note;
   // Not all transformations will want or need to attach metadata and therefore
-  // should not be requierd to do so.
+  // should not be required to do so.
   AnyGenerator Metadata = [](const ast_matchers::MatchFinder::MatchResult &)
   -> llvm::Expected {
 return llvm::Expected(llvm::Any());
@@ -131,6 +131,11 @@
 /// Generates no edits.
 inline EditGenerator noEdits() { return editList({}); }
 
+/// Generates a single, no-op edit anchored at the start location of the
+/// specified range. A `noopEdit` may be preferred over `noEdits` to associate 
a
+/// diagnostic `Explanation` with the rule.
+EditGenerator noopEdit(RangeSelector Anchor);
+
 /// Version of `ifBound` specialized to `ASTEdit`.
 inline EditGenerator ifBound(std::string ID, ASTEdit TrueEdit,
  ASTEdit FalseEdit) {


Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -426,6 +426,14 @@
   testRule(makeRule(returnStmt().bind("return"), noEdits()), Input, Input);
 }
 
+TEST_F(TransformerTest, NoopEdit) {
+  using transformer::node;
+  using transformer::noopEdit;
+  std::string Input = "int f(int x) { return x; }";
+  testRule(makeRule(returnStmt().bind("return"), noopEdit(node("return"))),
+   Input, Input);
+}
+
 TEST_F(TransformerTest, IfBound2Args) {
   using transformer::ifBound;
   std::string Input = "int f(int x) { return x; }";
Index: clang/lib/Tooling/Transformer/RewriteRule.cpp
===
--- clang/lib/Tooling/Transformer/RewriteRule.cpp
+++ clang/lib/Tooling/Transformer/RewriteRule.cpp
@@ -73,6 +73,24 @@
   };
 }
 
+EditGenerator transformer::noopEdit(RangeSelector Anchor) {
+  return [Anchor = std::move(Anchor)](const MatchResult &Result)
+ -> Expected> {
+Expected Range = Anchor(Result);
+if (!Range)
+  return Range.takeError();
+// In case the range is inside a macro expansion, map the location back to a
+// "real" source location.
+SourceLocation Begin =
+Result.SourceManager->getSpellingLoc(Range->getBegin());
+Edit E;
+// Implicitly, leave `E.Replacement` as the empty string.
+E.Kind = EditKind::Range;
+E.Range = Ch

[PATCH] D72553: [clang-tidy] Add performance-prefer-preincrement check

[Aaron Ballman via Phabricator via cfe-commits]

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

In D72553#1815497 , @njames93 wrote:

> could this do with an alias into performance?

If it was 1997, I'd say yes, but I am not aware of any optimizer that does so 
poorly with pre/post increment to suggest this is actually a 
performance-related check still today. Do we have any evidence that at O1 
 or higher this has any impact 
whatsoever on performance? If not, then I'd say the alias shouldn't be there 
(because the check could be noisy on correct code bases).




Comment at: 
clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp:56
+CheckFactories.registerCheck(
+"performance-prefer-pre-increment");
 CheckFactories.registerCheck(

I think the name is a bit unfortunate -- the check handles more than just 
incrementing. But there's not really a good neutral word like "crement" that 
covers both actions. "Adjustment" sort of comes close, but would be kind of 
awful for a check name. Alternative ideas are welcome here.



Comment at: 
clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.cpp:21
+namespace {
+AST_MATCHER_P(UnaryOperator, isOperator, UnaryOperatorKind, Opcode) {
+  return Node.getOpcode() == Opcode;

Oh, neat, we have `unaryOperator(hasOperatorName("++"))` but that doesn't quite 
cut it here. :-D



Comment at: 
clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.cpp:37-38
+void PreferPreIncrementCheck::registerMatchers(MatchFinder *Finder) {
+  // Ignore all unary ops with a parent decl or expr, those use the value
+  // returned. Reordering those would change the behaviour of the expression.
+  // FIXME: Add any more parents which could use the result of the operation.

Overloaded operators are a similar concern about changing the behavior of the 
expression. I think it's reasonably safe to assume that pre and post 
increment/decrement will typically do the same operations, but perhaps we 
should skip any overloaded operators that aren't idiomatic or aren't paired? 
e.g.,
```
struct S {
  S operator++(int);
}; // Only post-fix available, don't diagnose

struct T {
  int& operator++();
  int operator++(int);
}; // Not idiomatic return types, don't diagnose

struct U {
  S& operator++();
  T operator++(int);
}; // Oh god, this just hurts, don't diagnose
```



Comment at: 
clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.cpp:42
+  unless(anyOf(hasParent(decl()), hasParent(expr()),
+   hasParent(returnStmt()), hasParent(cxxThrowExpr(;
+  auto BoundExpr = expr().bind("ignore");

I think `hasParent(cxxThrowExpr())` is already covered by `hasParent(expr())`, 
right?

Also, this may not be quite right -- what about a case like this: `sizeof(i++)` 
where there's no performance reason to prefer one or the other?



Comment at: 
clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.cpp:88
+   bool IsIncrement, bool WarnOnly) {
+  // Warn for all occurances, but don't fix macro usage.
+  DiagnosticBuilder Diag =

Typo: occurances -> occurrences



Comment at: 
clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.cpp:90
+  DiagnosticBuilder Diag =
+  diag(Range.getBegin(), "Use pre-%0 instead of post-%0")
+  << (IsIncrement ? "increment" : "decrement");

clang-tidy diagnostics are not capitalized like that -- also, this diagnostic 
doesn't really say what's wrong with the user's code. How about: `pre-%0 may 
have better performance characteristics than post-%0` or something along those 
lines?



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:199-202
+- New alias :doc:`performance-prefer-pre-increment
+  ` to
+  :doc:`llvm-prefer-pre-increment
+  ` was added.

lebedev.ri wrote:
> njames93 wrote:
> > lebedev.ri wrote:
> > > Are we **really** **really** sure this is the correct relation direction?
> > > This isn't an llvm-specific guideline that may be applicable to other 
> > > code,
> > > but a known generic C++ guideline that llvm coding guide follows.
> > You're probably right, I added this to llvm first, then thought about 
> > alias. Which module should its primary be
> > I'd say performancepersonally. Cppcoreguidelines has [[ 
> > https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#enforcement-8
> >  | 1 little note ]] about it but I dont think that justifies putting the 
> > check in there.
> I would agree with `performance` module.
> Putting it in cppcoreguidelines would be the same - why there, it's not an 
> Cppcoreguidelines invention?
I'm rather opposed to putting it 

[PATCH] D89899: [CodeGen] Implement [[likely]] and [[unlikely]] for the iteration statements

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thank you for this!




Comment at: clang/include/clang/Basic/AttrDocs.td:1773
 
-At the moment the attributes only have effect when used in an ``if``, ``else``,
-or ``switch`` statement.
+Below some usage examples likelihood attributes and their effect:
 

How about: `Below are some example usages of the likelihood attributes and 
their effects:`



Comment at: clang/include/clang/Basic/AttrDocs.td:1854
+  for(int i = 0; i != size; ++i) [[likely]] {
+...   // The loop the likely path of execution
+  }

The loop the likely -> The loop is the likely

Also, mentioning that this applies to range-based for loops may not be a bad 
thing either.



Comment at: clang/include/clang/Basic/AttrDocs.td:1861
+
+  do [[unlikely]] {   // The loop will always iterate once
+...   // The likelihood attribute affects the likelihood of the

Oye, I don't know how I feel about this. According to the standard 
(http://eel.is/c++draft/stmt.pre#1), the unlikely attribute appertains to the 
compound statement, not the expression within the while loop and that compound 
statement is required to be entered at least once so this just feels really 
unintuitive to me. WDYT?



Comment at: clang/include/clang/Basic/AttrDocs.td:1866
+  while(true) [[unlikely]] {
+...   // The attribute has no effect
+  }   // Clang elides comparison and generates an infinite loop

This is a bit of an edge case where I can't tell whether we should or should 
not diagnose. On the one hand, the user wrote something and we think it's 
meaningless, which we would usually diagnose as being an ignored attribute so 
the user can maybe react to it. On the other hand, "has no effect" is perhaps 
not the same as "ignored", as with `i + 0` (it's not really ignored but you 
would expect it to have no effect either).



Comment at: clang/include/clang/Basic/AttrDocs.td:1867
+...   // The attribute has no effect
+  }   // Clang elides comparison and generates an infinite loop
+

elides comparison -> elides the comparison



Comment at: clang/include/clang/Basic/AttrDocs.td:1871
+...   // The attribute has no effect
+  } while(0); // Clang elides comparison and generates no loop
+

elides comparison -> elides the comparison



Comment at: clang/lib/CodeGen/CodeGenFunction.h:1410
 
+  /// Calculate the branch weight for PGO data or the likelihood attribe.
+  /// The function tries to get the weight of \ref createProfileWeightsForLoop.

attribe -> attribute


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89899

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

[PATCH] D89910: [AIX] Let alloca return 16 bytes alignment

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L updated this revision to Diff 299975.
Xiangling_L marked 3 inline comments as done.
Xiangling_L added a comment.

Fixed the testcase formatting issues;
Fixed the `BIGGEST_ALIGNMENT` value in testcases;


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

https://reviews.llvm.org/D89910

Files:
  clang/lib/Basic/Targets/PPC.h
  clang/test/CodeGen/aix_alloca_align.c
  clang/test/Preprocessor/init-ppc.c
  clang/test/Preprocessor/init-ppc64.c


Index: clang/test/Preprocessor/init-ppc64.c
===
--- clang/test/Preprocessor/init-ppc64.c
+++ clang/test/Preprocessor/init-ppc64.c
@@ -666,7 +666,7 @@
 // PPC64-AIX:#define _LP64 1
 // PPC64-AIX:#define _POWER 1
 // PPC64-AIX:#define __64BIT__ 1
-// PPC64-AIX:#define __BIGGEST_ALIGNMENT__ 8
+// PPC64-AIX:#define __BIGGEST_ALIGNMENT__ 16
 // PPC64-AIX:#define __BIG_ENDIAN__ 1
 // PPC64-AIX:#define __BYTE_ORDER__ __ORDER_BIG_ENDIAN__
 // PPC64-AIX:#define __CHAR16_TYPE__ unsigned short
Index: clang/test/Preprocessor/init-ppc.c
===
--- clang/test/Preprocessor/init-ppc.c
+++ clang/test/Preprocessor/init-ppc.c
@@ -398,7 +398,7 @@
 // PPC-AIX:#define _LONG_LONG 1
 // PPC-AIX-NOT:#define _LP64 1
 // PPC-AIX:#define _POWER 1
-// PPC-AIX:#define __BIGGEST_ALIGNMENT__ 8
+// PPC-AIX:#define __BIGGEST_ALIGNMENT__ 16
 // PPC-AIX:#define __BIG_ENDIAN__ 1
 // PPC-AIX:#define __BYTE_ORDER__ __ORDER_BIG_ENDIAN__
 // PPC-AIX:#define __CHAR16_TYPE__ unsigned short
Index: clang/test/CodeGen/aix_alloca_align.c
===
--- /dev/null
+++ clang/test/CodeGen/aix_alloca_align.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -triple=powerpc-ibm-aix-xcoff -S -emit-llvm < %s | \
+// RUN:   FileCheck --check-prefix=32BIT %s
+
+// RUN: %clang_cc1 -triple=powerpc64-ibm-aix-xcoff -S -emit-llvm < %s | \
+// RUN:   FileCheck --check-prefix=64BIT %s
+
+typedef __SIZE_TYPE__ size_t;
+extern void *alloca(size_t __size) __attribute__((__nothrow__));
+
+void foo() {
+  char *ptr1 = (char *)alloca(sizeof(char) * 9);
+}
+
+// 32BIT: %0 = alloca i8, i32 9, align 16
+// 64BIT: %0 = alloca i8, i64 9, align 16
Index: clang/lib/Basic/Targets/PPC.h
===
--- clang/lib/Basic/Targets/PPC.h
+++ clang/lib/Basic/Targets/PPC.h
@@ -370,7 +370,6 @@
   SizeType = UnsignedLong;
   PtrDiffType = SignedLong;
   IntPtrType = SignedLong;
-  SuitableAlign = 64;
   LongDoubleWidth = 64;
   LongDoubleAlign = DoubleAlign = 32;
   LongDoubleFormat = &llvm::APFloat::IEEEdouble();
@@ -409,7 +408,6 @@
 if (Triple.isOSAIX()) {
   // TODO: Set appropriate ABI for AIX platform.
   DataLayout = "E-m:a-i64:64-n32:64";
-  SuitableAlign = 64;
   LongDoubleWidth = 64;
   LongDoubleAlign = DoubleAlign = 32;
   LongDoubleFormat = &llvm::APFloat::IEEEdouble();


Index: clang/test/Preprocessor/init-ppc64.c
===
--- clang/test/Preprocessor/init-ppc64.c
+++ clang/test/Preprocessor/init-ppc64.c
@@ -666,7 +666,7 @@
 // PPC64-AIX:#define _LP64 1
 // PPC64-AIX:#define _POWER 1
 // PPC64-AIX:#define __64BIT__ 1
-// PPC64-AIX:#define __BIGGEST_ALIGNMENT__ 8
+// PPC64-AIX:#define __BIGGEST_ALIGNMENT__ 16
 // PPC64-AIX:#define __BIG_ENDIAN__ 1
 // PPC64-AIX:#define __BYTE_ORDER__ __ORDER_BIG_ENDIAN__
 // PPC64-AIX:#define __CHAR16_TYPE__ unsigned short
Index: clang/test/Preprocessor/init-ppc.c
===
--- clang/test/Preprocessor/init-ppc.c
+++ clang/test/Preprocessor/init-ppc.c
@@ -398,7 +398,7 @@
 // PPC-AIX:#define _LONG_LONG 1
 // PPC-AIX-NOT:#define _LP64 1
 // PPC-AIX:#define _POWER 1
-// PPC-AIX:#define __BIGGEST_ALIGNMENT__ 8
+// PPC-AIX:#define __BIGGEST_ALIGNMENT__ 16
 // PPC-AIX:#define __BIG_ENDIAN__ 1
 // PPC-AIX:#define __BYTE_ORDER__ __ORDER_BIG_ENDIAN__
 // PPC-AIX:#define __CHAR16_TYPE__ unsigned short
Index: clang/test/CodeGen/aix_alloca_align.c
===
--- /dev/null
+++ clang/test/CodeGen/aix_alloca_align.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -triple=powerpc-ibm-aix-xcoff -S -emit-llvm < %s | \
+// RUN:   FileCheck --check-prefix=32BIT %s
+
+// RUN: %clang_cc1 -triple=powerpc64-ibm-aix-xcoff -S -emit-llvm < %s | \
+// RUN:   FileCheck --check-prefix=64BIT %s
+
+typedef __SIZE_TYPE__ size_t;
+extern void *alloca(size_t __size) __attribute__((__nothrow__));
+
+void foo() {
+  char *ptr1 = (char *)alloca(sizeof(char) * 9);
+}
+
+// 32BIT: %0 = alloca i8, i32 9, align 16
+// 64BIT: %0 = alloca i8, i64 9, align 16
Index: clang/lib/Basic/Targets/PPC.h
===
--- clang/lib/Basic/Targets/PPC.h
+++ clang/lib/Basic/Targets/PPC.h
@@ -370,7 +370,6 @@
   SizeType = UnsignedLong;
   PtrDiffTy

[PATCH] D89886: [clang-tidy] Fix redefinition of module in the same module.modulemap file

[Dmitry Polukhin via Phabricator via cfe-commits]

DmitryPolukhin updated this revision to Diff 299977.
DmitryPolukhin added a comment.

Added all module map file names


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89886

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


Index: clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
===
--- clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
+++ clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
@@ -13,13 +13,22 @@
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Serialization/ASTReader.h"
 
+#define DEBUG_TYPE "clang-tidy"
+
 namespace clang {
 namespace tooling {
 
 class ExpandModularHeadersPPCallbacks::FileRecorder {
 public:
   /// Records that a given file entry is needed for replaying callbacks.
-  void addNecessaryFile(const FileEntry *File) { FilesToRecord.insert(File); }
+  void addNecessaryFile(const FileEntry *File) {
+// Don't record modulemap files because it breaks same file detection.
+if (!(File->getName().endswith("module.modulemap") ||
+  File->getName().endswith("module.private.modulemap") ||
+  File->getName().endswith("module.map") ||
+  File->getName().endswith("module_private.map")))
+  FilesToRecord.insert(File);
+  }
 
   /// Records content for a file and adds it to the FileSystem.
   void recordFileContent(const FileEntry *File,
@@ -44,8 +53,8 @@
   /// `FilesToRecord` should be empty.
   void checkAllFilesRecorded() {
 for (auto FileEntry : FilesToRecord)
-  llvm::errs() << "Did not record contents for input file: "
-   << FileEntry->getName() << "\n";
+  LLVM_DEBUG(llvm::dbgs() << "Did not record contents for input file: "
+  << FileEntry->getName() << "\n");
   }
 
 private:


Index: clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
===
--- clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
+++ clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
@@ -13,13 +13,22 @@
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Serialization/ASTReader.h"
 
+#define DEBUG_TYPE "clang-tidy"
+
 namespace clang {
 namespace tooling {
 
 class ExpandModularHeadersPPCallbacks::FileRecorder {
 public:
   /// Records that a given file entry is needed for replaying callbacks.
-  void addNecessaryFile(const FileEntry *File) { FilesToRecord.insert(File); }
+  void addNecessaryFile(const FileEntry *File) {
+// Don't record modulemap files because it breaks same file detection.
+if (!(File->getName().endswith("module.modulemap") ||
+  File->getName().endswith("module.private.modulemap") ||
+  File->getName().endswith("module.map") ||
+  File->getName().endswith("module_private.map")))
+  FilesToRecord.insert(File);
+  }
 
   /// Records content for a file and adds it to the FileSystem.
   void recordFileContent(const FileEntry *File,
@@ -44,8 +53,8 @@
   /// `FilesToRecord` should be empty.
   void checkAllFilesRecorded() {
 for (auto FileEntry : FilesToRecord)
-  llvm::errs() << "Did not record contents for input file: "
-   << FileEntry->getName() << "\n";
+  LLVM_DEBUG(llvm::dbgs() << "Did not record contents for input file: "
+  << FileEntry->getName() << "\n");
   }
 
 private:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89886: [clang-tidy] Fix redefinition of module in the same module.modulemap file

[Dmitry Polukhin via Phabricator via cfe-commits]

DmitryPolukhin marked an inline comment as done.
DmitryPolukhin added a comment.

In D89886#2346851 , @alexfh wrote:

> Ah, btw, any chance of adding a test for this?

Oh, I was not able to create small reproducer that without including large 
Apple Frameworks with modules :( My hypothesis that it is side effect of module 
cache that triggers module load before it is referenced from sources. I tested 
it on reproducer from PR47839 + my real internal example.




Comment at: clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp:27
+// detection.
+if (!File->getName().endswith("module.modulemap"))
+  FilesToRecord.insert(File);

alexfh wrote:
> Looking at the relevant code I find special file names like "module.map", 
> "module_private.map", "module.private.modulemap". Is this problem relevant 
> for any of those?
I think it is relevant for these files too, added them all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89886

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

[PATCH] D89184: Support complex target features combinations

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

dexonsmith added a comment.

In D89184#2346453 , @pengfei wrote:

> LGTM. But I suggest you waiting for 1 or 2 days to see if other reviewers 
> object.

Given that @echristo marked this as needing changes I would suggest waiting / 
reaching out to confirm the concerns were addressed.


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

https://reviews.llvm.org/D89184

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

[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:237
+static void getHungarianNotationDefaultConfig(
+std::shared_ptr HNOption) {
+

It seems like this function should take `HNOption` as a reference rather than a 
`shared_ptr`.



Comment at: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h:87
+HungarianPrefixOption HungarianPrefixOpt;
+std::shared_ptr
+HungarianNotationOption;

I'd like to avoid using a `shared_ptr` here if we can avoid it -- do we expect 
this to be super expensive to copy by value (I would imagine it'll be an 
expensive copy but we don't make copies all that often, but maybe my intuition 
is wrong)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86671

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

[PATCH] D89920: Export TemplateArgumentMatcher so clients defining custom matchers don't need to use the internal namespace

[David Van Cleve via Phabricator via cfe-commits]

davidvancleve added a comment.

Hi klimek, any more action needed on my part to land this? This is my first 
LLVM change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89920

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

[PATCH] D89903: [CodeGen] Crash instead of generating broken code with self-capturing __block var

[ille via Phabricator via cfe-commits]

ille added a comment.

> We do not actually support allocation failure for a lot of things around 
> blocks. I don't think the copy-helper functions even have a way to propagate 
> out a failure in copying a field. I have never seen any code in the wild that 
> would handle Block_copy returning a null pointer. Effectively it is assumed 
> to not happen.

Fair – although if that's the case, perhaps xnu should not be using blocks.  I 
suppose failures of small allocations like this are rare enough that the issue 
hasn't come up in practice, and same with the null blocks under ARC.

> It seems somewhat unlikely to me that anyone would actually write code like 
> your example without copying the block and potentially triggering the 
> `__block` variable to be moved to the heap, which is why I think pre-moving 
> the variable might be acceptable.

Also fair.  I can implement this, but I do think there should be an associated 
warning flag.

> With all that said, I agree that crashing and/or just not drilling into the 
> variable is not acceptable.

The patch I submitted switches from not drilling into the variable to crashing. 
 Are you saying I should submit a more thorough fix rather than trying to land 
this first?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89903

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

[PATCH] D71726: Let clang atomic builtins fetch add/sub support floating point types

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.
Herald added a subscriber: dexonsmith.

ping


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

https://reviews.llvm.org/D71726

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

[PATCH] D89814: [TableGen] Change !getop and !setop to !getdagop and !setdagop

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle accepted this revision.
nhaehnle 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/D89814/new/

https://reviews.llvm.org/D89814

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

[PATCH] D89913: SourceManager: Encapsulate line number mapping into SrcMgr::LineOffsetMapping

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

dexonsmith updated this revision to Diff 299989.
dexonsmith added a comment.
Herald added a subscriber: mgorny.

Fix an off-by-one bug in `LineOffsetMapping::LineOffsetMapping`, add unit 
tests, and clang-format.


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

https://reviews.llvm.org/D89913

Files:
  clang/include/clang/Basic/SourceManager.h
  clang/lib/Basic/SourceManager.cpp
  clang/lib/Lex/ScratchBuffer.cpp
  clang/unittests/Basic/CMakeLists.txt
  clang/unittests/Basic/LineOffsetMappingTest.cpp

Index: clang/unittests/Basic/LineOffsetMappingTest.cpp
===
--- /dev/null
+++ clang/unittests/Basic/LineOffsetMappingTest.cpp
@@ -0,0 +1,62 @@
+//===- unittests/Basic/LineOffsetMappingTest.cpp - Test LineOffsetMapping -===//
+//
+// 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 "clang/Basic/SourceManager.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace clang::SrcMgr;
+using namespace llvm;
+
+namespace {
+
+TEST(LineOffsetMappingTest, empty) {
+  LineOffsetMapping Mapping;
+  EXPECT_FALSE(Mapping);
+
+#if !defined(NDEBUG) && GTEST_HAS_DEATH_TEST
+  EXPECT_DEATH((void)Mapping.getLines(), "Assertion");
+#endif
+}
+
+TEST(LineOffsetMappingTest, construct) {
+  llvm::BumpPtrAllocator Alloc;
+  unsigned Offsets[] = {0, 10, 20};
+  LineOffsetMapping Mapping(Offsets, Alloc);
+  EXPECT_EQ(3u, Mapping.size());
+  EXPECT_EQ(0u, Mapping[0]);
+  EXPECT_EQ(10u, Mapping[1]);
+  EXPECT_EQ(20u, Mapping[2]);
+
+#if !defined(NDEBUG) && GTEST_HAS_DEATH_TEST
+  EXPECT_DEATH((void)Mapping[3], "Assertion");
+#endif
+}
+
+TEST(LineOffsetMappingTest, get) {
+  llvm::BumpPtrAllocator Alloc;
+  StringRef Source = "first line\n"
+ "second line\n";
+  auto Mapping = LineOffsetMapping::get(MemoryBufferRef(Source, ""), Alloc);
+  EXPECT_EQ(3u, Mapping.size());
+  EXPECT_EQ(0u, Mapping[0]);
+  EXPECT_EQ(11u, Mapping[1]);
+  EXPECT_EQ(23u, Mapping[2]);
+}
+
+TEST(LineOffsetMappingTest, getMissingFinalNewline) {
+  llvm::BumpPtrAllocator Alloc;
+  StringRef Source = "first line\n"
+ "second line";
+  auto Mapping = LineOffsetMapping::get(MemoryBufferRef(Source, ""), Alloc);
+  EXPECT_EQ(2u, Mapping.size());
+  EXPECT_EQ(0u, Mapping[0]);
+  EXPECT_EQ(11u, Mapping[1]);
+}
+
+} // end namespace
Index: clang/unittests/Basic/CMakeLists.txt
===
--- clang/unittests/Basic/CMakeLists.txt
+++ clang/unittests/Basic/CMakeLists.txt
@@ -7,6 +7,7 @@
   DiagnosticTest.cpp
   FileEntryTest.cpp
   FileManagerTest.cpp
+  LineOffsetMappingTest.cpp
   SourceManagerTest.cpp
   )
 
Index: clang/lib/Lex/ScratchBuffer.cpp
===
--- clang/lib/Lex/ScratchBuffer.cpp
+++ clang/lib/Lex/ScratchBuffer.cpp
@@ -40,7 +40,7 @@
 auto *ContentCache = const_cast(
 SourceMgr.getSLocEntry(SourceMgr.getFileID(BufferStartLoc))
  .getFile().getContentCache());
-ContentCache->SourceLineCache = nullptr;
+ContentCache->SourceLineCache = SrcMgr::LineOffsetMapping();
   }
 
   // Prefix the token with a \n, so that it looks like it is the first thing on
Index: clang/lib/Basic/SourceManager.cpp
===
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -1198,10 +1198,10 @@
   const char *Buf = MemBuf->getBufferStart();
   // See if we just calculated the line number for this FilePos and can use
   // that to lookup the start of the line instead of searching for it.
-  if (LastLineNoFileIDQuery == FID &&
-  LastLineNoContentCache->SourceLineCache != nullptr &&
-  LastLineNoResult < LastLineNoContentCache->NumLines) {
-unsigned *SourceLineCache = LastLineNoContentCache->SourceLineCache;
+  if (LastLineNoFileIDQuery == FID && LastLineNoContentCache->SourceLineCache &&
+  LastLineNoResult < LastLineNoContentCache->SourceLineCache.size()) {
+const unsigned *SourceLineCache =
+LastLineNoContentCache->SourceLineCache.begin();
 unsigned LineStart = SourceLineCache[LastLineNoResult - 1];
 unsigned LineEnd = SourceLineCache[LastLineNoResult];
 if (FilePos >= LineStart && FilePos < LineEnd) {
@@ -1272,6 +1272,11 @@
   if (Invalid)
 return;
 
+  FI->SourceLineCache = LineOffsetMapping::get(*Buffer, Alloc);
+}
+
+LineOffsetMapping LineOffsetMapping::get(llvm::MemoryBufferRef Buffer,
+ llvm::BumpPtrAllocator &Alloc) {
   // Find the file offsets of all of the *physical* source lines.  This does
   // not look at trigraphs, escaped newlines, or anything else tricky.
   SmallVector LineOffs

[PATCH] D89897: [AIX] Emit error for -G option on AIX

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L marked 3 inline comments as done.
Xiangling_L added inline comments.



Comment at: clang/test/Driver/aix-err-options.c:1
+// RUN: %clang -target powerpc32-ibm-aix-xcoff -### -S -emit-llvm -G 0 2>&1 %s 
| \
+// RUN:   FileCheck -check-prefix=CHECK32 %s

hubert.reinterpretcast wrote:
> Does this need a call to `not`?
Using `not` will fail the testcase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89897

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

[PATCH] D89897: [AIX] Emit error for -G option on AIX

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L updated this revision to Diff 21.
Xiangling_L marked an inline comment as done.
Xiangling_L added a comment.

Fix the formatting issues;


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

https://reviews.llvm.org/D89897

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/aix-err-options.c
  llvm/CMakeLists.txt


Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -930,7 +930,7 @@
 
   # Modules should be built with -G, so we can use runtime linking with
   # plugins.
-  string(APPEND CMAKE_MODULE_LINKER_FLAGS " -G")
+  string(APPEND CMAKE_MODULE_LINKER_FLAGS " -Wl,-G")
 
   # Also set the correct flags for building shared libraries.
   string(APPEND CMAKE_SHARED_LINKER_FLAGS " -shared")
Index: clang/test/Driver/aix-err-options.c
===
--- /dev/null
+++ clang/test/Driver/aix-err-options.c
@@ -0,0 +1,7 @@
+// RUN: %clang -target powerpc32-ibm-aix-xcoff -### -S -emit-llvm -G 0 2>&1 %s 
| \
+// RUN:   FileCheck --check-prefix=CHECK32 %s
+// RUN: %clang -target powerpc64-ibm-aix-xcoff -### -S -emit-llvm -G 0 2>&1 %s 
| \
+// RUN:   FileCheck --check-prefix=CHECK64 %s
+
+// CHECK32: error: unsupported option '-G' for target 'powerpc32-ibm-aix-xcoff'
+// CHECK64: error: unsupported option '-G' for target 'powerpc64-ibm-aix-xcoff'
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4958,6 +4958,11 @@
   if (D.CCGenDiagnostics)
 CmdArgs.push_back("-disable-pragma-debug-crash");
 
+  if (RawTriple.isOSAIX())
+if (Arg *A = Args.getLastArg(options::OPT_G))
+  D.Diag(diag::err_drv_unsupported_opt_for_target)
+  << A->getSpelling() << RawTriple.str();
+
   bool UseSeparateSections = isUseSeparateSections(Triple);
 
   if (Args.hasFlag(options::OPT_ffunction_sections,


Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -930,7 +930,7 @@
 
   # Modules should be built with -G, so we can use runtime linking with
   # plugins.
-  string(APPEND CMAKE_MODULE_LINKER_FLAGS " -G")
+  string(APPEND CMAKE_MODULE_LINKER_FLAGS " -Wl,-G")
 
   # Also set the correct flags for building shared libraries.
   string(APPEND CMAKE_SHARED_LINKER_FLAGS " -shared")
Index: clang/test/Driver/aix-err-options.c
===
--- /dev/null
+++ clang/test/Driver/aix-err-options.c
@@ -0,0 +1,7 @@
+// RUN: %clang -target powerpc32-ibm-aix-xcoff -### -S -emit-llvm -G 0 2>&1 %s | \
+// RUN:   FileCheck --check-prefix=CHECK32 %s
+// RUN: %clang -target powerpc64-ibm-aix-xcoff -### -S -emit-llvm -G 0 2>&1 %s | \
+// RUN:   FileCheck --check-prefix=CHECK64 %s
+
+// CHECK32: error: unsupported option '-G' for target 'powerpc32-ibm-aix-xcoff'
+// CHECK64: error: unsupported option '-G' for target 'powerpc64-ibm-aix-xcoff'
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4958,6 +4958,11 @@
   if (D.CCGenDiagnostics)
 CmdArgs.push_back("-disable-pragma-debug-crash");
 
+  if (RawTriple.isOSAIX())
+if (Arg *A = Args.getLastArg(options::OPT_G))
+  D.Diag(diag::err_drv_unsupported_opt_for_target)
+  << A->getSpelling() << RawTriple.str();
+
   bool UseSeparateSections = isUseSeparateSections(Triple);
 
   if (Args.hasFlag(options::OPT_ffunction_sections,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D82756: Port some floating point options to new option marshalling infrastructure

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 26.
jansvoboda11 added a comment.

Rebase onto master.


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

https://reviews.llvm.org/D82756

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/complex-math.c
  clang/test/CodeGen/fast-math.c
  clang/test/CodeGen/finite-math.c
  clang/test/CodeGen/fp-floatcontrol-stack.cpp
  clang/test/CodeGen/fp-function-attrs.cpp
  clang/test/CodeGen/fp-options-to-fast-math-flags.c
  clang/test/CodeGen/fpconstrained.c
  clang/test/CodeGen/fpconstrained.cpp
  clang/test/CodeGen/libcalls.c
  clang/test/CodeGenOpenCL/no-signed-zeros.cl
  clang/test/CodeGenOpenCL/relaxed-fpmath.cl
  clang/test/Driver/opencl.cl
  clang/test/Headers/nvptx_device_math_sin.c
  clang/test/Headers/nvptx_device_math_sin.cpp
  clang/test/SemaOpenCL/fp-options.cl
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -443,7 +443,8 @@
 OS << "OPTION(";
 WriteOptRecordFields(OS, R);
 OS << ")\n";
-if (!isa(R.getValueInit("MarshallingKind")))
+if (!isa(R.getValueInit("MarshallingKind")) &&
+!R.getValueAsString("KeyPath").empty())
   OptsWithMarshalling.push_back(MarshallingKindInfo::create(R));
   }
   OS << "#endif // OPTION\n";
Index: clang/test/SemaOpenCL/fp-options.cl
===
--- clang/test/SemaOpenCL/fp-options.cl
+++ clang/test/SemaOpenCL/fp-options.cl
@@ -1,4 +1,4 @@
 // RUN: %clang_cc1 %s -finclude-default-header -triple spir-unknown-unknown -emit-pch -o %t.pch
-// RUN: %clang_cc1 %s -finclude-default-header -cl-no-signed-zeros -triple spir-unknown-unknown -include-pch %t.pch -fsyntax-only -verify
+// RUN: %clang_cc1 %s -finclude-default-header -fno-signed-zeros -triple spir-unknown-unknown -include-pch %t.pch -fsyntax-only -verify
 // expected-no-diagnostics
 
Index: clang/test/Headers/nvptx_device_math_sin.cpp
===
--- clang/test/Headers/nvptx_device_math_sin.cpp
+++ clang/test/Headers/nvptx_device_math_sin.cpp
@@ -1,8 +1,15 @@
 // REQUIRES: nvptx-registered-target
 // RUN: %clang_cc1 -x c++ -internal-isystem %S/Inputs/include -fopenmp -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc
 // RUN: %clang_cc1 -x c++ -include __clang_openmp_device_functions.h -internal-isystem %S/../../lib/Headers/openmp_wrappers -internal-isystem %S/Inputs/include -fopenmp -triple nvptx64-nvidia-cuda -aux-triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s --check-prefix=SLOW
-// RUN: %clang_cc1 -x c++ -internal-isystem %S/Inputs/include -fopenmp -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc -ffast-math -ffp-contract=fast
-// RUN: %clang_cc1 -x c++ -include __clang_openmp_device_functions.h -internal-isystem %S/../../lib/Headers/openmp_wrappers -internal-isystem %S/Inputs/include -fopenmp -triple nvptx64-nvidia-cuda -aux-triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - -ffast-math -ffp-contract=fast | FileCheck %s --check-prefix=FAST
+// RUN: %clang_cc1 -x c++ -internal-isystem %S/Inputs/include -fopenmp -triple powerpc64le-unknown-unknown \
+// RUN:   -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc -ffast-math -ffinite-math-only \
+// RUN:   -menable-no-infs -menable-no-nans -fno-signed-zeros -freciprocal-math -menable-unsafe-fp-math \
+// RUN:   -fapprox-func -mreassociate -ffp-contract=fast
+// RUN: %clang_cc1 -x c++ -include __clang_openmp_device_functions.h -internal-isystem %S/../../lib/Headers/openmp_wrappers \
+// RUN:   -internal-isystem %S/Inputs/include -fopenmp -triple nvptx64-nvidia-cuda -aux-triple powerpc64le-unknown-unknown \
+// RUN:   -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc \
+// RUN:   -o - -ffast-math -ffinite-math-only -menable-no-infs -menable-no-nans -fno-signed-zeros -freciprocal-math \
+// RUN:   -menable-unsafe-fp-math -fapprox-func -mreassociate -ffp-contract=fast | FileCheck %s --check-prefix=FAST
 // expected-no-diagnostics
 
 #include 
Index: clang/test/Headers/nvptx_device_math_sin.c
===
--- clang/test/Headers/nvptx_device_math_sin.c
+++ clang/test/Headers/nvptx_device_math_sin.c
@@ -1,8 +1,15 @@
 // REQUIRES: nvptx-registered-target
 // RUN: %clang_cc1 -x c -internal-isystem %S/Inputs/include

[PATCH] D89913: SourceManager: Encapsulate line number mapping into SrcMgr::LineOffsetMapping

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

dexonsmith updated this revision to Diff 27.
dexonsmith added a comment.

Add the unittest (`constructTwo`) that caught the off-by-one bug, somehow 
missed in the last update.


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

https://reviews.llvm.org/D89913

Files:
  clang/include/clang/Basic/SourceManager.h
  clang/lib/Basic/SourceManager.cpp
  clang/lib/Lex/ScratchBuffer.cpp
  clang/unittests/Basic/CMakeLists.txt
  clang/unittests/Basic/LineOffsetMappingTest.cpp

Index: clang/unittests/Basic/LineOffsetMappingTest.cpp
===
--- /dev/null
+++ clang/unittests/Basic/LineOffsetMappingTest.cpp
@@ -0,0 +1,79 @@
+//===- unittests/Basic/LineOffsetMappingTest.cpp - Test LineOffsetMapping -===//
+//
+// 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 "clang/Basic/SourceManager.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace clang::SrcMgr;
+using namespace llvm;
+
+namespace {
+
+TEST(LineOffsetMappingTest, empty) {
+  LineOffsetMapping Mapping;
+  EXPECT_FALSE(Mapping);
+
+#if !defined(NDEBUG) && GTEST_HAS_DEATH_TEST
+  EXPECT_DEATH((void)Mapping.getLines(), "Assertion");
+#endif
+}
+
+TEST(LineOffsetMappingTest, construct) {
+  BumpPtrAllocator Alloc;
+  unsigned Offsets[] = {0, 10, 20};
+  LineOffsetMapping Mapping(Offsets, Alloc);
+  EXPECT_EQ(3u, Mapping.size());
+  EXPECT_EQ(0u, Mapping[0]);
+  EXPECT_EQ(10u, Mapping[1]);
+  EXPECT_EQ(20u, Mapping[2]);
+
+#if !defined(NDEBUG) && GTEST_HAS_DEATH_TEST
+  EXPECT_DEATH((void)Mapping[3], "Assertion");
+#endif
+}
+
+TEST(LineOffsetMappingTest, constructTwo) {
+  // Confirm allocation size is big enough, convering an off-by-one bug.
+  BumpPtrAllocator Alloc;
+  unsigned Offsets1[] = {0, 10};
+  unsigned Offsets2[] = {0, 20};
+  LineOffsetMapping Mapping1(Offsets1, Alloc);
+  LineOffsetMapping Mapping2(Offsets2, Alloc);
+
+  // Need to check Mapping1 *after* building Mapping2.
+  EXPECT_EQ(2u, Mapping1.size());
+  EXPECT_EQ(0u, Mapping1[0]);
+  EXPECT_EQ(10u, Mapping1[1]);
+  EXPECT_EQ(2u, Mapping2.size());
+  EXPECT_EQ(0u, Mapping2[0]);
+  EXPECT_EQ(20u, Mapping2[1]);
+}
+
+TEST(LineOffsetMappingTest, get) {
+  BumpPtrAllocator Alloc;
+  StringRef Source = "first line\n"
+ "second line\n";
+  auto Mapping = LineOffsetMapping::get(MemoryBufferRef(Source, ""), Alloc);
+  EXPECT_EQ(3u, Mapping.size());
+  EXPECT_EQ(0u, Mapping[0]);
+  EXPECT_EQ(11u, Mapping[1]);
+  EXPECT_EQ(23u, Mapping[2]);
+}
+
+TEST(LineOffsetMappingTest, getMissingFinalNewline) {
+  BumpPtrAllocator Alloc;
+  StringRef Source = "first line\n"
+ "second line";
+  auto Mapping = LineOffsetMapping::get(MemoryBufferRef(Source, ""), Alloc);
+  EXPECT_EQ(2u, Mapping.size());
+  EXPECT_EQ(0u, Mapping[0]);
+  EXPECT_EQ(11u, Mapping[1]);
+}
+
+} // end namespace
Index: clang/unittests/Basic/CMakeLists.txt
===
--- clang/unittests/Basic/CMakeLists.txt
+++ clang/unittests/Basic/CMakeLists.txt
@@ -7,6 +7,7 @@
   DiagnosticTest.cpp
   FileEntryTest.cpp
   FileManagerTest.cpp
+  LineOffsetMappingTest.cpp
   SourceManagerTest.cpp
   )
 
Index: clang/lib/Lex/ScratchBuffer.cpp
===
--- clang/lib/Lex/ScratchBuffer.cpp
+++ clang/lib/Lex/ScratchBuffer.cpp
@@ -40,7 +40,7 @@
 auto *ContentCache = const_cast(
 SourceMgr.getSLocEntry(SourceMgr.getFileID(BufferStartLoc))
  .getFile().getContentCache());
-ContentCache->SourceLineCache = nullptr;
+ContentCache->SourceLineCache = SrcMgr::LineOffsetMapping();
   }
 
   // Prefix the token with a \n, so that it looks like it is the first thing on
Index: clang/lib/Basic/SourceManager.cpp
===
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -1198,10 +1198,10 @@
   const char *Buf = MemBuf->getBufferStart();
   // See if we just calculated the line number for this FilePos and can use
   // that to lookup the start of the line instead of searching for it.
-  if (LastLineNoFileIDQuery == FID &&
-  LastLineNoContentCache->SourceLineCache != nullptr &&
-  LastLineNoResult < LastLineNoContentCache->NumLines) {
-unsigned *SourceLineCache = LastLineNoContentCache->SourceLineCache;
+  if (LastLineNoFileIDQuery == FID && LastLineNoContentCache->SourceLineCache &&
+  LastLineNoResult < LastLineNoContentCache->SourceLineCache.size()) {
+const unsigned *SourceLineCache =
+LastLineNoContentCache->SourceLineCache.begin();
 unsigned LineStart = SourceLineCache[LastLineNoResult - 1];
 unsigned LineEnd = SourceLineCache

[PATCH] D89903: [CodeGen] Crash instead of generating broken code with self-capturing __block var

[ille via Phabricator via cfe-commits]

ille updated this revision to Diff 28.
ille added a comment.

Move the check to cover the atomic case as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89903

Files:
  clang/lib/CodeGen/CGDecl.cpp
  clang/test/CodeGenCXX/block-capture-own-init.cpp


Index: clang/test/CodeGenCXX/block-capture-own-init.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/block-capture-own-init.cpp
@@ -0,0 +1,9 @@
+// RUN: not %clang_cc1 -x c++ -std=c++11 -fblocks -emit-llvm %s 2>&1 | 
FileCheck %s
+
+void test_aggregate_captured_by_own_init() {
+  struct foo { int a[100]; };
+  extern foo get_foo(foo *(^)());
+  // CHECK: error: cannot compile this aggregate initialized with potentially 
self-capturing block yet
+  __block foo f = get_foo(^{ return &f; });
+}
+
Index: clang/lib/CodeGen/CGDecl.cpp
===
--- clang/lib/CodeGen/CGDecl.cpp
+++ clang/lib/CodeGen/CGDecl.cpp
@@ -1913,6 +1913,11 @@
 return;
   }
   case TEK_Aggregate:
+if (capturedByInit) {
+  // TODO: how can we delay here if D is captured by its initializer?
+  CGM.ErrorUnsupported(
+  init, "aggregate initialized with potentially self-capturing block");
+}
 if (type->isAtomicType()) {
   EmitAtomicInit(const_cast(init), lvalue);
 } else {
@@ -1921,7 +1926,6 @@
 Overlap = AggValueSlot::DoesNotOverlap;
   else if (auto *FD = dyn_cast(D))
 Overlap = getOverlapForFieldInit(FD);
-  // TODO: how can we delay here if D is captured by its initializer?
   EmitAggExpr(init, AggValueSlot::forLValue(
 lvalue, *this, AggValueSlot::IsDestructed,
 AggValueSlot::DoesNotNeedGCBarriers,


Index: clang/test/CodeGenCXX/block-capture-own-init.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/block-capture-own-init.cpp
@@ -0,0 +1,9 @@
+// RUN: not %clang_cc1 -x c++ -std=c++11 -fblocks -emit-llvm %s 2>&1 | FileCheck %s
+
+void test_aggregate_captured_by_own_init() {
+  struct foo { int a[100]; };
+  extern foo get_foo(foo *(^)());
+  // CHECK: error: cannot compile this aggregate initialized with potentially self-capturing block yet
+  __block foo f = get_foo(^{ return &f; });
+}
+
Index: clang/lib/CodeGen/CGDecl.cpp
===
--- clang/lib/CodeGen/CGDecl.cpp
+++ clang/lib/CodeGen/CGDecl.cpp
@@ -1913,6 +1913,11 @@
 return;
   }
   case TEK_Aggregate:
+if (capturedByInit) {
+  // TODO: how can we delay here if D is captured by its initializer?
+  CGM.ErrorUnsupported(
+  init, "aggregate initialized with potentially self-capturing block");
+}
 if (type->isAtomicType()) {
   EmitAtomicInit(const_cast(init), lvalue);
 } else {
@@ -1921,7 +1926,6 @@
 Overlap = AggValueSlot::DoesNotOverlap;
   else if (auto *FD = dyn_cast(D))
 Overlap = getOverlapForFieldInit(FD);
-  // TODO: how can we delay here if D is captured by its initializer?
   EmitAggExpr(init, AggValueSlot::forLValue(
 lvalue, *this, AggValueSlot::IsDestructed,
 AggValueSlot::DoesNotNeedGCBarriers,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89910: [AIX] Let alloca return 16 bytes alignment

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/CodeGen/aix_alloca_align.c:11
+void foo() {
+  char *ptr1 = (char *)alloca(sizeof(char) * 9);
+}

I'm not entirely sure, but can we try for size 32 and see if we get 16?


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

https://reviews.llvm.org/D89910

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

[clang] b2524eb - [HIP] Fix HIP rounding math intrinsics

[Aaron En Ye Shi via cfe-commits]

Author: Aaron En Ye Shi
Date: 2020-10-22T15:57:09Z
New Revision: b2524eb9445a4487115c8f94fd946d2c4c95f652

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

LOG: [HIP] Fix HIP rounding math intrinsics

The __ocml_*_rte_f32 and __ocml_*_rte_f64 functions are not
available if OCML_BASIC_ROUNDED_OPERATIONS is not defined.

Reviewed By: b-sumner, yaxunl

Fixes: SWDEV-257235

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

Added: 


Modified: 
clang/lib/Headers/__clang_hip_math.h

Removed: 




diff  --git a/clang/lib/Headers/__clang_hip_math.h 
b/clang/lib/Headers/__clang_hip_math.h
index f2365e8844fe..14d91c66b352 100644
--- a/clang/lib/Headers/__clang_hip_math.h
+++ b/clang/lib/Headers/__clang_hip_math.h
@@ -547,102 +547,117 @@ float __expf(float __x) { return 
__ocml_native_exp_f32(__x); }
 #if defined OCML_BASIC_ROUNDED_OPERATIONS
 __DEVICE__
 float __fadd_rd(float __x, float __y) { return __ocml_add_rtn_f32(__x, __y); }
-#endif
 __DEVICE__
 float __fadd_rn(float __x, float __y) { return __ocml_add_rte_f32(__x, __y); }
-#if defined OCML_BASIC_ROUNDED_OPERATIONS
 __DEVICE__
 float __fadd_ru(float __x, float __y) { return __ocml_add_rtp_f32(__x, __y); }
-
 __DEVICE__
 float __fadd_rz(float __x, float __y) { return __ocml_add_rtz_f32(__x, __y); }
+#else
+__DEVICE__
+float __fadd_rn(float __x, float __y) { return __x + __y; }
+#endif
 
+#if defined OCML_BASIC_ROUNDED_OPERATIONS
 __DEVICE__
 float __fdiv_rd(float __x, float __y) { return __ocml_div_rtn_f32(__x, __y); }
-#endif
 __DEVICE__
 float __fdiv_rn(float __x, float __y) { return __ocml_div_rte_f32(__x, __y); }
-#if defined OCML_BASIC_ROUNDED_OPERATIONS
 __DEVICE__
 float __fdiv_ru(float __x, float __y) { return __ocml_div_rtp_f32(__x, __y); }
-
 __DEVICE__
 float __fdiv_rz(float __x, float __y) { return __ocml_div_rtz_f32(__x, __y); }
+#else
+__DEVICE__
+float __fdiv_rn(float __x, float __y) { return __x / __y; }
 #endif
+
 __DEVICE__
 float __fdividef(float __x, float __y) { return __x / __y; }
+
 #if defined OCML_BASIC_ROUNDED_OPERATIONS
 __DEVICE__
 float __fmaf_rd(float __x, float __y, float __z) {
   return __ocml_fma_rtn_f32(__x, __y, __z);
 }
-#endif
 __DEVICE__
 float __fmaf_rn(float __x, float __y, float __z) {
   return __ocml_fma_rte_f32(__x, __y, __z);
 }
-#if defined OCML_BASIC_ROUNDED_OPERATIONS
 __DEVICE__
 float __fmaf_ru(float __x, float __y, float __z) {
   return __ocml_fma_rtp_f32(__x, __y, __z);
 }
-
 __DEVICE__
 float __fmaf_rz(float __x, float __y, float __z) {
   return __ocml_fma_rtz_f32(__x, __y, __z);
 }
+#else
+__DEVICE__
+float __fmaf_rn(float __x, float __y, float __z) {
+  return __ocml_fma_f32(__x, __y, __z);
+}
+#endif
 
+#if defined OCML_BASIC_ROUNDED_OPERATIONS
 __DEVICE__
 float __fmul_rd(float __x, float __y) { return __ocml_mul_rtn_f32(__x, __y); }
-#endif
 __DEVICE__
 float __fmul_rn(float __x, float __y) { return __ocml_mul_rte_f32(__x, __y); }
-#if defined OCML_BASIC_ROUNDED_OPERATIONS
 __DEVICE__
 float __fmul_ru(float __x, float __y) { return __ocml_mul_rtp_f32(__x, __y); }
-
 __DEVICE__
 float __fmul_rz(float __x, float __y) { return __ocml_mul_rtz_f32(__x, __y); }
-
+#else
 __DEVICE__
-float __frcp_rd(float __x) { return __llvm_amdgcn_rcp_f32(__x); }
+float __fmul_rn(float __x, float __y) { return __x * __y; }
 #endif
-__DEVICE__
-float __frcp_rn(float __x) { return __llvm_amdgcn_rcp_f32(__x); }
+
 #if defined OCML_BASIC_ROUNDED_OPERATIONS
 __DEVICE__
-float __frcp_ru(float __x) { return __llvm_amdgcn_rcp_f32(__x); }
-
+float __frcp_rd(float __x) { return __ocml_div_rtn_f32(1.0f, __x); }
+__DEVICE__
+float __frcp_rn(float __x) { return __ocml_div_rte_f32(1.0f, __x); }
 __DEVICE__
-float __frcp_rz(float __x) { return __llvm_amdgcn_rcp_f32(__x); }
+float __frcp_ru(float __x) { return __ocml_div_rtp_f32(1.0f, __x); }
+__DEVICE__
+float __frcp_rz(float __x) { return __ocml_div_rtz_f32(1.0f, __x); }
+#else
+__DEVICE__
+float __frcp_rn(float __x) { return 1.0f / __x; }
 #endif
+
 __DEVICE__
 float __frsqrt_rn(float __x) { return __llvm_amdgcn_rsq_f32(__x); }
+
 #if defined OCML_BASIC_ROUNDED_OPERATIONS
 __DEVICE__
 float __fsqrt_rd(float __x) { return __ocml_sqrt_rtn_f32(__x); }
-#endif
 __DEVICE__
 float __fsqrt_rn(float __x) { return __ocml_sqrt_rte_f32(__x); }
-#if defined OCML_BASIC_ROUNDED_OPERATIONS
 __DEVICE__
 float __fsqrt_ru(float __x) { return __ocml_sqrt_rtp_f32(__x); }
-
 __DEVICE__
 float __fsqrt_rz(float __x) { return __ocml_sqrt_rtz_f32(__x); }
+#else
+__DEVICE__
+float __fsqrt_rn(float __x) { return __ocml_native_sqrt_f32(__x); }
+#endif
 
+#if defined OCML_BASIC_ROUNDED_OPERATIONS
 __DEVICE__
 float __fsub_rd(float __x, float __y) { return __ocml_sub_rtn_f32(__x, __y); }
-#endif
 __DEVICE__
 float __fsub_rn(float __x, float __y) { return __ocml_sub_rte_f32(__x, _

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Hmm... so I got distracted the last few days with a handful of small SEMA 
issues that I believe to be solved, so I'm going back to my codegen issues.

It seems that my problem is that we don't actually mangle calling-convention in 
a pointer types.  The result is:
64 bit: https://godbolt.org/z/nhnoG9
32 bit: https://godbolt.org/z/eK6jha

As you can see, the, only 32 bit-windows actually gets that right in that it 
differentiates between the two calling-conventions on the operator-func-ptr.  
The result is the other 3 platforms all get the AST right, but don't generate 
separate implementations for them.  MY implementation is still wrong, since 
something else odd happens with lambdas on 32 bits, but I'd like to solve these 
problems as well.

I'll work on the Lambda issue to at least get windows 32 bit right (the only 
place that will have multiple operator-func-ptr in this case), but I'll 
eventually need to solve the issue with the differing calling conventions for 
the MSVC compat implementation that is a follow up to this. Any suggestions on 
where that could fit in?


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

https://reviews.llvm.org/D89559

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

[PATCH] D89897: [AIX] Emit error for -G option on AIX

[Hubert Tong via Phabricator via cfe-commits]

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

LGTM; thanks!




Comment at: clang/test/Driver/aix-err-options.c:1
+// RUN: %clang -target powerpc32-ibm-aix-xcoff -### -S -emit-llvm -G 0 2>&1 %s 
| \
+// RUN:   FileCheck -check-prefix=CHECK32 %s

Xiangling_L wrote:
> hubert.reinterpretcast wrote:
> > Does this need a call to `not`?
> Using `not` will fail the testcase.
Got it; didn't know `-###` has such an effect on the return code.


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

https://reviews.llvm.org/D89897

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

[PATCH] D89372: [OpenCL] Remove unused extensions

[Marco Antognini via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa779a169931c: [OpenCL] Remove unused extensions (authored by 
mantognini).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89372

Files:
  clang/include/clang/Basic/OpenCLExtensions.def
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Basic/Targets/NVPTX.h
  clang/test/Misc/amdgcn.languageOptsOpenCL.cl
  clang/test/Misc/nvptx.languageOptsOpenCL.cl
  clang/test/Misc/r600.languageOptsOpenCL.cl
  clang/test/SemaOpenCL/extension-version.cl

Index: clang/test/SemaOpenCL/extension-version.cl
===
--- clang/test/SemaOpenCL/extension-version.cl
+++ clang/test/SemaOpenCL/extension-version.cl
@@ -34,16 +34,6 @@
 #endif
 #pragma OPENCL EXTENSION cl_khr_int64_extended_atomics: enable
 
-#ifndef cl_khr_gl_sharing
-#error "Missing cl_khr_gl_sharing define"
-#endif
-#pragma OPENCL EXTENSION cl_khr_gl_sharing: enable
-
-#ifndef cl_khr_icd
-#error "Missing cl_khr_icd define"
-#endif
-#pragma OPENCL EXTENSION cl_khr_icd: enable
-
 // Core features in CL 1.1
 
 #ifndef cl_khr_byte_addressable_store
@@ -86,15 +76,6 @@
 // expected-warning@-2{{OpenCL extension 'cl_khr_local_int32_extended_atomics' is core feature or supported optional core feature - ignoring}}
 #endif
 
-#if (defined(__OPENCL_C_VERSION__) && __OPENCL_C_VERSION__ < 110)
-// Deprecated abvoe 1.0
-#ifndef cl_khr_select_fprounding_mode
-#error "Missing cl_khr_select_fp_rounding_mode define"
-#endif
-#pragma OPENCL EXTENSION cl_khr_select_fprounding_mode: enable
-#endif
-
-
 // Core feature in CL 1.2
 #ifndef cl_khr_fp64
 #error "Missing cl_khr_fp64 define"
@@ -113,24 +94,6 @@
 // expected-warning@-2{{OpenCL extension 'cl_khr_3d_image_writes' is core feature or supported optional core feature - ignoring}}
 #endif
 
-#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 110)
-#ifndef cl_khr_gl_event
-#error "Missing cl_khr_gl_event define"
-#endif
-#else
-// expected-warning@+2{{unsupported OpenCL extension 'cl_khr_gl_event' - ignoring}}
-#endif
-#pragma OPENCL EXTENSION cl_khr_gl_event : enable
-
-#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 110)
-#ifndef cl_khr_d3d10_sharing
-#error "Missing cl_khr_d3d10_sharing define"
-#endif
-#else
-// expected-warning@+2{{unsupported OpenCL extension 'cl_khr_d3d10_sharing' - ignoring}}
-#endif
-#pragma OPENCL EXTENSION cl_khr_d3d10_sharing : enable
-
 #if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 110)
 #ifndef cles_khr_int64
 #error "Missing cles_khr_int64 define"
@@ -140,60 +103,6 @@
 #endif
 #pragma OPENCL EXTENSION cles_khr_int64 : enable
 
-#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 120)
-#ifndef cl_khr_context_abort
-#error "Missing cl_context_abort define"
-#endif
-#else
-// expected-warning@+2{{unsupported OpenCL extension 'cl_khr_context_abort' - ignoring}}
-#endif
-#pragma OPENCL EXTENSION cl_khr_context_abort : enable
-
-#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 120)
-#ifndef cl_khr_d3d11_sharing
-#error "Missing cl_khr_d3d11_sharing define"
-#endif
-#else
-// expected-warning@+2{{unsupported OpenCL extension 'cl_khr_d3d11_sharing' - ignoring}}
-#endif
-#pragma OPENCL EXTENSION cl_khr_d3d11_sharing : enable
-
-#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 120)
-#ifndef cl_khr_dx9_media_sharing
-#error "Missing cl_khr_dx9_media_sharing define"
-#endif
-#else
-// expected-warning@+2{{unsupported OpenCL extension 'cl_khr_dx9_media_sharing' - ignoring}}
-#endif
-#pragma OPENCL EXTENSION cl_khr_dx9_media_sharing : enable
-
-#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 120)
-#ifndef cl_khr_image2d_from_buffer
-#error "Missing cl_khr_image2d_from_buffer define"
-#endif
-#else
-// expected-warning@+2{{unsupported OpenCL extension 'cl_khr_image2d_from_buffer' - ignoring}}
-#endif
-#pragma OPENCL EXTENSION cl_khr_image2d_from_buffer : enable
-
-#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 120)
-#ifndef cl_khr_initialize_memory
-#error "Missing cl_khr_initialize_memory define"
-#endif
-#else
-// expected-warning@+2{{unsupported OpenCL extension 'cl_khr_initialize_memory' - ignoring}}
-#endif
-#pragma OPENCL EXTENSION cl_khr_initialize_memory : enable
-
-#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 120)
-#ifndef cl_khr_gl_depth_images
-#error "Missing cl_khr_gl_depth_images define"
-#endif
-#else
-// expected-warning@+2{{unsupported OpenCL extension 'cl_khr_gl_depth_images' - ignoring}}
-#endif
-#pragma OPENCL EXTENSION cl_khr_gl_depth_images : enable
-
 #if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 120)
 #ifndef cl_khr_gl_msaa_sharing
 #error "Missing cl_khr_gl_msaa_sharing defi

[clang] a779a16 - [OpenCL] Remove unused extensions

[Marco Antognini via cfe-commits]

Author: Marco Antognini
Date: 2020-10-22T17:01:31+01:00
New Revision: a779a169931c0738bf43dc50fc545c1e88597e92

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

LOG: [OpenCL] Remove unused extensions

Many non-language extensions are defined but also unused. This patch
removes them with their tests as they do not require compiler support.

The cl_khr_select_fprounding_mode extension is also removed because it
has been deprecated since OpenCL 1.1 and Clang doesn't have any specific
support for it.

The cl_khr_context_abort extension is only referred to in "The OpenCL
Specification", version 1.2 and 2.0, in Table 4.3, but no specification
is provided in "The OpenCL Extension Specification" for these versions.
Because it is both unused in Clang and lacks specification, this
extension is removed.

The following extensions are platform extensions that bring new OpenCL
APIs but do not impact the kernel language nor require compiler support.
They are therefore removed.

- cl_khr_gl_sharing, introduced in OpenCL 1.0

- cl_khr_icd, introduced in OpenCL 1.2

- cl_khr_gl_event, introduced in OpenCL 1.1
Note: this extension adds a new API to create cl_event but it also
specifies that these can only be used by clEnqueueAcquireGLObjects.
Hence, they cannot be used on the device side and the extension does
not impact the kernel language.

- cl_khr_d3d10_sharing, introduced in OpenCL 1.1

- cl_khr_d3d11_sharing, introduced in OpenCL 1.2

- cl_khr_dx9_media_sharing, introduced in OpenCL 1.2

- cl_khr_image2d_from_buffer, introduced in OpenCL 1.2

- cl_khr_initialize_memory, introduced in OpenCL 1.2

- cl_khr_gl_depth_images, introduced in OpenCL 1.2
Note: this extension is related to cl_khr_depth_images but only the
latter adds new features to the kernel language.

- cl_khr_spir, introduced in OpenCL 1.2

- cl_khr_egl_event, introduced in OpenCL 1.2
Note: this extension adds a new API to create cl_event but it also
specifies that these can only be used by clEnqueueAcquire* API
functions. Hence, they cannot be used on the device side and the
extension does not impact the kernel language.

- cl_khr_egl_image, introduced in OpenCL 1.2

- cl_khr_terminate_context, introduced in OpenCL 1.2

The minimum required OpenCL version used in OpenCLExtensions.def for
these extensions is not always correct. Removing these address that
issue.

Reviewed By: Anastasia

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

Added: 


Modified: 
clang/include/clang/Basic/OpenCLExtensions.def
clang/lib/Basic/Targets/AMDGPU.h
clang/lib/Basic/Targets/NVPTX.h
clang/test/Misc/amdgcn.languageOptsOpenCL.cl
clang/test/Misc/nvptx.languageOptsOpenCL.cl
clang/test/Misc/r600.languageOptsOpenCL.cl
clang/test/SemaOpenCL/extension-version.cl

Removed: 




diff  --git a/clang/include/clang/Basic/OpenCLExtensions.def 
b/clang/include/clang/Basic/OpenCLExtensions.def
index 1ae36b32fb0a..d67cb3ff019b 100644
--- a/clang/include/clang/Basic/OpenCLExtensions.def
+++ b/clang/include/clang/Basic/OpenCLExtensions.def
@@ -23,6 +23,16 @@
 //   core - minimum OpenCL version when the extension becomes optional core
 //  feature or core feature. ~0U indicates not a core feature or an
 //  optional core feature.
+//
+// As per The OpenCL Extension Specification, Section 1.2, in this file, an
+// extension is defined if and only it either:
+//  * affects the OpenCL language semantics or its syntax,
+//  * adds built-in functions to the language.
+//
+// For such an extension, a preprocessor #define that matches the extension
+// name must be created and a #pragma is required if and only if the
+// compilation flow is impacted, e.g. due to a 
diff erence of syntax or
+// semantics in the language compared to the core standard.
 
 #ifndef OPENCLEXT_INTERNAL
 #ifndef OPENCLEXT
@@ -34,8 +44,6 @@
 
 // OpenCL 1.0.
 OPENCLEXT_INTERNAL(cl_khr_3d_image_writes, 100, 200)
-// fprounding mode is special since it is not mentioned beyond 1.0
-OPENCLEXT_INTERNAL(cl_khr_select_fprounding_mode, 100, 110)
 OPENCLEXT_INTERNAL(cl_khr_byte_addressable_store, 100, 110)
 OPENCLEXT_INTERNAL(cl_khr_fp16, 100, ~0U)
 OPENCLEXT_INTERNAL(cl_khr_fp64, 100, 120)
@@ -45,35 +53,19 @@ OPENCLEXT_INTERNAL(cl_khr_local_int32_base_atomics, 100, 
110)
 OPENCLEXT_INTERNAL(cl_khr_local_int32_extended_atomics, 100, 110)
 OPENCLEXT_INTERNAL(cl_khr_int64_base_atomics, 100, ~0U)
 OPENCLEXT_INTERNAL(cl_khr_int64_extended_atomics, 100, ~0U)
-OPENCLEXT_INTERNAL(cl_khr_gl_sharing, 100, ~0U)
-OPENCLEXT_INTERNAL(cl_khr_icd, 100, ~0U)
-
-// OpenCL 1.1.
-OPENCLEXT_INTERNAL(cl_khr_gl_event, 110, ~0U)
-OPENCLEXT_INTERNAL(cl_khr_d3d10_sharing, 110, ~0U)
 
 // EMBEDDED_PROFILE
 OPENCLEXT_INTERNAL(cles_khr_int64, 110, ~0U)
 
 // OpenCL 1.2.
-OPENCLEXT_INTERNAL

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Mangling more calling conventions when mangling function types in Itanium 
(except at the top level) is the right thing to do.  There's already a place to 
do so in the mangling.  We just haven't done this yet because a lot of those 
calling convention are supported by other compilers, so we need to coordinate.  
So you need to check out what other compilers do (GCC, ICC) and imitate them if 
they're already setting a reasonable default; otherwise, I think there's a 
standard algorithm for generating these.

Separately, the MSVC mangler should support Clang's CCs if there's a reasonable 
extensible rule there.  I've never been able to figure out the MVSC mangling 
grammar.


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

https://reviews.llvm.org/D89559

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

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D89559#2347642 , @rjmccall wrote:

> Mangling more calling conventions when mangling function types in Itanium 
> (except at the top level) is the right thing to do.  There's already a place 
> to do so in the mangling.  We just haven't done this yet because a lot of 
> those calling convention are supported by other compilers, so we need to 
> coordinate.  So you need to check out what other compilers do (GCC, ICC) and 
> imitate them if they're already setting a reasonable default; otherwise, I 
> think there's a standard algorithm for generating these.
>
> Separately, the MSVC mangler should support Clang's CCs if there's a 
> reasonable extensible rule there.  I've never been able to figure out the 
> MVSC mangling grammar.

Same here :)

I remember there being places to mangle calling convention, I was just 
surprised they didn't happen in the function type here.  GCC doesn't seem to 
support any of the calling conventions in 64 bit, but I'll see if ICC has 
selected a mangling scheme.


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

https://reviews.llvm.org/D89559

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

[PATCH] D87989: [Flang][Driver] Add infrastructure for basic frontend actions and file I/O

[Sourabh Singh Tomar via Phabricator via cfe-commits]

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

Thanks for your patience. LGTM! at least the part I reviewed. Though I would 
vote for having another approval(From some senior folks in community)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87989

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

[PATCH] D89971: [OpenMP][CUDA] Add missing overload for `remquo(float,float,int*)`

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert created this revision.
jdoerfert added reviewers: JonChesterfield, ye-luo, tra, yaxunl.
Herald added subscribers: guansong, bollu.
Herald added a project: clang.
jdoerfert requested review of this revision.
Herald added a subscriber: sstefan1.

Reported by Colleen Bertoni  after running the OvO test
suite: https://github.com/TApplencourt/OvO/

The template overload is still hidden behind an ifdef for OpenMP. In the
future we probably want to remove the ifdef but that requires further
testing.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89971

Files:
  clang/lib/Headers/__clang_cuda_cmath.h


Index: clang/lib/Headers/__clang_cuda_cmath.h
===
--- clang/lib/Headers/__clang_cuda_cmath.h
+++ clang/lib/Headers/__clang_cuda_cmath.h
@@ -174,6 +174,9 @@
 __DEVICE__ float sqrt(float __x) { return ::sqrtf(__x); }
 __DEVICE__ float tan(float __x) { return ::tanf(__x); }
 __DEVICE__ float tanh(float __x) { return ::tanhf(__x); }
+__DEVICE__ float remquo(float __n, float __d, int *__q) {
+  return ::remquof(__n, __d, __q);
+}
 
 // Notably missing above is nexttoward.  We omit it because
 // libdevice doesn't provide an implementation, and we don't want to be in the


Index: clang/lib/Headers/__clang_cuda_cmath.h
===
--- clang/lib/Headers/__clang_cuda_cmath.h
+++ clang/lib/Headers/__clang_cuda_cmath.h
@@ -174,6 +174,9 @@
 __DEVICE__ float sqrt(float __x) { return ::sqrtf(__x); }
 __DEVICE__ float tan(float __x) { return ::tanf(__x); }
 __DEVICE__ float tanh(float __x) { return ::tanhf(__x); }
+__DEVICE__ float remquo(float __n, float __d, int *__q) {
+  return ::remquof(__n, __d, __q);
+}
 
 // Notably missing above is nexttoward.  We omit it because
 // libdevice doesn't provide an implementation, and we don't want to be in the
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Tyker via Phabricator via cfe-commits]

Tyker added a comment.

In D88645#2347050 , @aaron.ballman 
wrote:

> LGTM aside from a request for a comment to be added. Thank you!

do you mean an RFC on llvm-dev/cfe-dev ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88645

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

[PATCH] D89971: [OpenMP][CUDA] Add missing overload for `remquo(float,float,int*)`

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

This should fix the last OvO math test (I think C++11) we fail. While those 
tests are "simple" they are fairly exhaustive and it's a good sign to pass them 
;)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89971

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

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D88645#2347725 , @Tyker wrote:

> In D88645#2347050 , @aaron.ballman 
> wrote:
>
>> LGTM aside from a request for a comment to be added. Thank you!
>
> do you mean an RFC on llvm-dev/cfe-dev ?

Oh gosh no! I just meant I was asking for a comment to be added in 
SemaDeclAttr.td before you commit. Sorry for the confusion! :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88645

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

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

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D88645#2347725 , @Tyker wrote:

> In D88645#2347050 , @aaron.ballman 
> wrote:
>
>> LGTM aside from a request for a comment to be added. Thank you!
>
> do you mean an RFC on llvm-dev/cfe-dev ?

He means this review comment he made requesting a comment in the code: >Ah, 
thank you for explaining that! Can you add a comment to the code to make that 
clear for others who may run across it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88645

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

[PATCH] D89971: [OpenMP][CUDA] Add missing overload for `remquo(float,float,int*)`

[Jon Chesterfield via Phabricator via cfe-commits]

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

Change obviously good. Am I right in reading this as all of OvO then passes for 
trunk, nvptx?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89971

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

[PATCH] D89832: [CUDA] Extract CUDA version from cuda.h if version.txt is not found

[Justin Lebar via Phabricator via cfe-commits]

jlebar accepted this revision.
jlebar added a comment.

LGTM modulo emankov's comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89832

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

[PATCH] D89971: [OpenMP][CUDA] Add missing overload for `remquo(float,float,int*)`

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D89971#2347762 , @JonChesterfield 
wrote:

> Change obviously good. Am I right in reading this as all of OvO then passes 
> for trunk, nvptx?

C++11 math tests on one system. These are system dependent to some degree so I 
would not claim it will always work, but it looks promising. and yes, nvptx, 
though nothing really cares about that, it just resolves down to __nv calls 
instead of __xyz calls at the end of the day.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89971

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

[PATCH] D89972: Add pipeline model for HiSilicon's TSV110

[Elvina Yakubova via Phabricator via cfe-commits]

Elvina created this revision.
Elvina added reviewers: bryanpkc, kristof.beyls, t.p.northover, SjoerdMeijer.
Elvina added projects: LLVM, clang.
Herald added subscribers: cfe-commits, jfb, hiraditya.
Elvina requested review of this revision.

This patch adds the scheduling and cost model for TSV110.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89972

Files:
  clang/test/Driver/aarch64-cpus.c
  llvm/lib/Target/AArch64/AArch64.td
  llvm/lib/Target/AArch64/AArch64SchedTSV110.td
  llvm/lib/Target/AArch64/AArch64SchedTSV110Details.td
  llvm/test/CodeGen/AArch64/machine-combiner-madd.ll
  llvm/test/CodeGen/AArch64/preferred-function-alignment.ll

Index: llvm/test/CodeGen/AArch64/preferred-function-alignment.ll
===
--- llvm/test/CodeGen/AArch64/preferred-function-alignment.ll
+++ llvm/test/CodeGen/AArch64/preferred-function-alignment.ll
@@ -20,6 +20,7 @@
 ; RUN: llc -mtriple=aarch64-unknown-linux -mcpu=thunderxt88 < %s | FileCheck --check-prefixes=ALIGN3,CHECK %s
 ; RUN: llc -mtriple=aarch64-unknown-linux -mcpu=thunderx2t99 < %s | FileCheck --check-prefixes=ALIGN3,CHECK %s
 ; RUN: llc -mtriple=aarch64-unknown-linux -mcpu=thunderx3t110 < %s | FileCheck --check-prefixes=ALIGN4,CHECK %s
+; RUN: llc -mtriple=aarch64-unknown-linux -mcpu=tsv110 < %s | FileCheck --check-prefixes=ALIGN4,CHECK %s
 ; RUN: llc -mtriple=aarch64-unknown-linux -mcpu=exynos-m3 < %s | FileCheck --check-prefixes=ALIGN5,CHECK %s
 
 define void @test() {
Index: llvm/test/CodeGen/AArch64/machine-combiner-madd.ll
===
--- llvm/test/CodeGen/AArch64/machine-combiner-madd.ll
+++ llvm/test/CodeGen/AArch64/machine-combiner-madd.ll
@@ -7,6 +7,7 @@
 ; RUN: llc -mtriple=aarch64-linux-gnu -mcpu=kryo   < %s | FileCheck %s
 ; RUN: llc -mtriple=aarch64-linux-gnu -mcpu=thunderx2t99 < %s | FileCheck %s
 ; RUN: llc -mtriple=aarch64-linux-gnu -mcpu=thunderx3t110 < %s | FileCheck %s
+; RUN: llc -mtriple=aarch64-linux-gnu -mcpu=tsv110 < %s | FileCheck %s
 
 ; Make sure that inst-combine fuses the multiply add in the addressing mode of
 ; the load.
Index: llvm/lib/Target/AArch64/AArch64SchedTSV110Details.td
===
--- /dev/null
+++ llvm/lib/Target/AArch64/AArch64SchedTSV110Details.td
@@ -0,0 +1,637 @@
+//==- AArch64SchedTSV110Details.td - TSV110 Scheduling Defs -*- tablegen -*-==//
+//
+// 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
+//
+//===--===//
+//
+// This file defines the uop and latency details for the machine model for the
+// Huawei TSV110 subtarget.
+//
+//===--===//
+
+// Contains all of the TSV110 specific SchedWriteRes types. The approach
+// below is to define a generic SchedWriteRes for every combination of
+// latency and microOps. The naming conventions is to use a prefix, one field
+// for latency, and one or more microOp count/type designators.
+//   Prefix: TSV110Wr
+//	 Latency: #cyc
+//   MicroOp Count/Types: #(ALU|AB|MDU|FSU1|FSU2|LdSt|ALUAB|F|FLdSt)
+//
+// e.g. TSV110Wr_6cyc_1ALU_6MDU_4LdSt means the total latency is 6 and there are
+//  1 micro-ops to be issued down one ALU pipe, six MDU pipes and four LdSt pipes.
+//
+
+//===--===//
+// Define Generic 1 micro-op types
+
+def TSV110Wr_1cyc_1AB: SchedWriteRes<[TSV110UnitAB]>{ let Latency = 1; }
+def TSV110Wr_1cyc_1ALU   : SchedWriteRes<[TSV110UnitALU]>   { let Latency = 1; }
+def TSV110Wr_1cyc_1ALUAB : SchedWriteRes<[TSV110UnitALUAB]> { let Latency = 1; }
+def TSV110Wr_1cyc_1MDU   : SchedWriteRes<[TSV110UnitMDU]>   { let Latency = 1; }
+def TSV110Wr_1cyc_1FSU1  : SchedWriteRes<[TSV110UnitFSU1]>  { let Latency = 1; }
+def TSV110Wr_1cyc_1FSU2  : SchedWriteRes<[TSV110UnitFSU2]>  { let Latency = 1; }
+def TSV110Wr_1cyc_1LdSt  : SchedWriteRes<[TSV110UnitLdSt]>  { let Latency = 1; }
+
+def TSV110Wr_2cyc_1AB: SchedWriteRes<[TSV110UnitAB]>{ let Latency = 2; }
+def TSV110Wr_2cyc_1ALU   : SchedWriteRes<[TSV110UnitALU]>   { let Latency = 2; }
+def TSV110Wr_2cyc_1LdSt  : SchedWriteRes<[TSV110UnitLdSt]>  { let Latency = 2; }
+def TSV110Wr_2cyc_1MDU   : SchedWriteRes<[TSV110UnitMDU]>   { let Latency = 2; }
+def TSV110Wr_2cyc_1FSU1  : SchedWriteRes<[TSV110UnitFSU1]>  { let Latency = 2; }
+def TSV110Wr_2cyc_1F : SchedWriteRes<[TSV110UnitF]> { let Latency = 2; }
+
+def TSV110Wr_3cyc_1F : SchedWriteRes<[TSV110UnitF]> { let Latency = 3; }
+def TSV110Wr_3cyc_1FSU1  : SchedWriteRes<[TSV110UnitFSU1]>  { let Latency = 3; }
+def TSV110Wr_3cyc_1MDU   : SchedWriteRes<[TSV110UnitMDU]>   { let Latency = 3; }
+
+def TSV110W

[PATCH] D89832: [CUDA] Extract CUDA version from cuda.h if version.txt is not found

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/lib/Driver/ToolChains/Cuda.cpp:76-77
+return CudaVersion::CUDA_102;
+  if (raw_version < 11010)
+return CudaVersion::CUDA_110;
+  return CudaVersion::LATEST;

emankov wrote:
> Please, add `CudaVersion::CUDA_111` declaration in `Cuda.h` and a 
> corresponding `if` here. 
> Btw, `switch` is possible here. 
It does not serve any purpose here. 102/110 were added when clang was only 
accepting specific versions. Now that it will accept any newer version,  
Arguably it's 102/101 that should be gone until we implement the new 
functionality. All of that would out of scope for this patch.

As for the switch, it would only work to match exact versions encoded in the 
CUDA headers, including updates, patches, special private builds etc. I do not 
have access to all of those versions, so I can not enumerate all of them. Range 
checking is more robust.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89832

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

[PATCH] D87989: [Flang][Driver] Add infrastructure for basic frontend actions and file I/O

[Richard Barton via Phabricator via cfe-commits]

richard.barton.arm accepted this revision.
richard.barton.arm added a comment.

I'm happy to accept this revision based on @SouraVX 's code review and the fact 
that Andrzej has sent multiple RFCs covering the clang-side changes, including 
the Options flags (latest here 
http://lists.llvm.org/pipermail/cfe-dev/2020-October/067079.html). I think this 
code is good enough to commit.

Thanks for the code and reviews!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87989

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

[PATCH] D89974: [driver][CUDA] Use CMake's FindCUDA as default --cuda-path.

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur created this revision.
Meinersbur added reviewers: jdoerfert, RaviNarayanaswamy, ye-luo, jhuber6, 
phosek, grokos, tambre, rnk, sylvestre.ledru, gtbercea, tra, yaxunl, Hahnfeld, 
jdenny.
Meinersbur added a project: clang.
Herald added subscribers: openmp-commits, dang, ormris, mgorny.
Herald added a project: OpenMP.
Meinersbur requested review of this revision.
Herald added a subscriber: sstefan1.

Add the introspection result from `find_package(CUDA)` as the first candidate 
when looking for a CUDA installation. This allows specifying the path to the 
CUDA installation when configuring LLVM instead of in every invocation of 
clang. Clang and libomptarget both use CMake's introspection to determine the 
CUDA environment, in particular to determine the default and supported 
architectures for OpenMP. libomptarget's regression tests assume that clang 
will find the CUDA installation itself without passing --cuda-path. Ideally, 
these are consistent. Making Clang also use the CMake introspection was the 
preferred solution from the OpenMP Multi-Company conference call over adding 
--cuda-path to every regression test.

To not make the driver regression test dependendent on the build configuration, 
a new flag `--cuda-path-ignore-cmake` is added which causes the  
CUDA_TOOLKIT_ROOT_PATH to not be added to the list of installation candidates.

Some notes:

- Both libomptarget and clang have a workaround for 
http://bugs.debian.org/882505 (CUDA on Debian and Ubuntu installations with 
/usr/lib/cuda prefix instead of /usr), but implemented very differently. Also 
see D40453  and D55588 
). In my Ubuntu LTS versions 18.04 and 20.04, 
this seem to be fixed as /usr/lib/cuda still exists but only contain  the files 
`version.txt` and `libdevice.10.bc` (though as symbolic link directory), which 
happen to be the files Clang looks for when searching a CUDA installation. I 
assume this is a compatibility workaround to not break Clang's compatibility 
workaround. libomptarget's implementation was to overwrite 
CUDA_TOOLKIT_ROOT_DIR after invoking FindCUDA, which is an evil thing to do. I 
replaced it with a `PATHS "/usr/lib/cuda"` so FindCUDA can check that path 
itself for older distributions which indeed have CUDA installed there.
- find_package(CUDA) is deprecated by find_package(CUDAToolkit) in CMake 3.17. 
The required version for LLVM currently is CMake 3.13.4


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89974

Files:
  clang/CMakeLists.txt
  clang/include/clang/Config/config.h.cmake
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Cuda.cpp
  clang/test/Driver/cuda-detect-path.cu
  clang/test/Driver/cuda-detect.cu
  openmp/libomptarget/cmake/Modules/LibomptargetGetDependencies.cmake

Index: openmp/libomptarget/cmake/Modules/LibomptargetGetDependencies.cmake
===
--- openmp/libomptarget/cmake/Modules/LibomptargetGetDependencies.cmake
+++ openmp/libomptarget/cmake/Modules/LibomptargetGetDependencies.cmake
@@ -112,10 +112,12 @@
 
 # Looking for CUDA...
 
-if (CUDA_TOOLKIT_ROOT_DIR)
-  set(LIBOMPTARGET_CUDA_TOOLKIT_ROOT_DIR_PRESET TRUE)
-endif()
-find_package(CUDA QUIET)
+
+# How CUDA is detected should be consistent with Clang.
+#
+# /usr/lib/cuda is a special case for Debian/Ubuntu to have nvidia-cuda-toolkit
+# work out of the box. More info on http://bugs.debian.org/882505
+find_package(CUDA QUIET PATHS "/usr/lib/cuda")
 
 # Try to get the highest Nvidia GPU architecture the system supports
 if (CUDA_FOUND)
@@ -221,31 +223,3 @@
   LIBOMPTARGET_DEP_VEO_FOUND
   LIBOMPTARGET_DEP_VEO_INCLUDE_DIRS)
 
-# Looking for CUDA libdevice subdirectory
-#
-# Special case for Debian/Ubuntu to have nvidia-cuda-toolkit work
-# out of the box. More info on http://bugs.debian.org/882505
-
-
-set(LIBOMPTARGET_CUDA_LIBDEVICE_SUBDIR nvvm/libdevice)
-
-# Don't alter CUDA_TOOLKIT_ROOT_DIR if the user specified it, if a value was
-# already cached for it, or if it already has libdevice.  Otherwise, on
-# Debian/Ubuntu, look where the nvidia-cuda-toolkit package normally installs
-# libdevice.
-if (NOT LIBOMPTARGET_CUDA_TOOLKIT_ROOT_DIR_PRESET AND
-NOT EXISTS
-  "${CUDA_TOOLKIT_ROOT_DIR}/${LIBOMPTARGET_CUDA_LIBDEVICE_SUBDIR}")
-  find_program(LSB_RELEASE lsb_release)
-  if (LSB_RELEASE)
-execute_process(COMMAND ${LSB_RELEASE} -is
-  OUTPUT_VARIABLE LSB_RELEASE_ID
-  OUTPUT_STRIP_TRAILING_WHITESPACE)
-set(candidate_dir /usr/lib/cuda)
-if ((LSB_RELEASE_ID STREQUAL "Debian" OR LSB_RELEASE_ID STREQUAL "Ubuntu")
-AND EXISTS "${candidate_dir}/${LIBOMPTARGET_CUDA_LIBDEVICE_SUBDIR}")
-  set(CUD

[PATCH] D89971: [OpenMP][CUDA] Add missing overload for `remquo(float,float,int*)`

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

> The template overload

function overload.

> is still hidden behind an ifdef for OpenMP. In the
> future we probably want to remove the ifdef but that requires further
> testing.

I don't think it's the case.  I've just ran `clang++ -x cuda /dev/null  
--cuda-host-only -dD -E ` and I do see the definitions for `tanh` just above 
your change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89971

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

[PATCH] D89974: [driver][CUDA] Use CMake's FindCUDA as default --cuda-path.

[Artem Belevich via Phabricator via cfe-commits]

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

One concern I have is that the path we configure during clang's build is not 
necessarily the right choice for the user of clang we build. It's likely that 
the clang in the end will be used on a completely different machine.
E.g. official clang builds can not ever provide the same CUDA path for *all* 
users who end up using them. Requiring the rest to use a special option to make 
clang work again looks like an overall usability regression to me.

I think the default should still let clang search for CUDA or require the user 
to provide correct CUDA path.  "Use CUDA path discovered by CMake at build 
time" should be a non-default configuration option if/when it's needed and 
appropriate.

Alternatively, I would not object to appending cmake-discovered path to the 
default search list, or enforcing it for OpenMP builds only if that's what 
OpenMP needs (though I still don't see how that's going to be a good default 
for all/most users).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89974

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

[PATCH] D82756: Port some floating point options to new option marshalling infrastructure

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: clang/test/CodeGen/fp-function-attrs.cpp:2
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -ffast-math -ffinite-math-only 
-menable-unsafe-fp-math \
+// RUN:   -menable-no-infs -menable-no-nans -fno-signed-zeros 
-freciprocal-math \
+// RUN:   -fapprox-func -mreassociate -ffp-contract=fast -emit-llvm -o - %s | 
FileCheck %s

jansvoboda11 wrote:
> dang wrote:
> > Anastasia wrote:
> > > dang wrote:
> > > > Anastasia wrote:
> > > > > dang wrote:
> > > > > > Anastasia wrote:
> > > > > > > Not clear why do you need to pass these extra flags now?
> > > > > > Previously passing -ffast-math to CC1 implied all these other 
> > > > > > flags. I am trying to make CC1 option parsing as simple as 
> > > > > > possible, so that we can then make it easy to generate a command 
> > > > > > line from a CompilerInvocation instance. You can refer to [[ 
> > > > > > http://lists.llvm.org/pipermail/cfe-dev/2020-May/065421.html | 
> > > > > > http://lists.llvm.org/pipermail/cfe-dev/2020-May/065421.html ]] for 
> > > > > > more details on why we want to be able to do this
> > > > > Just to understand, there used to be implied flags and it made the 
> > > > > manual command line use of clang more compact and easy... Is the idea 
> > > > > now to change those compound flags such that individul flags always 
> > > > > need to be passed?
> > > > > 
> > > > > Although I thought you are still adding the implicit flags:
> > > > > 
> > > > >   {options::OPT_cl_fast_relaxed_math,
> > > > >[&](const Arg *Arg) {
> > > > >  RenderArg(Arg);
> > > > > 
> > > > >  
> > > > > CmdArgs.push_back(GetArgString(options::OPT_cl_mad_enable));
> > > > >  CmdArgs.push_back(GetArgString(options::OPT_ffast_math));
> > > > >  
> > > > > CmdArgs.push_back(GetArgString(options::OPT_ffinite_math_only));
> > > > >  CmdArgs.push_back(
> > > > >  GetArgString(options::OPT_menable_unsafe_fp_math));
> > > > >  
> > > > > CmdArgs.push_back(GetArgString(options::OPT_mreassociate));
> > > > >  
> > > > > CmdArgs.push_back(GetArgString(options::OPT_menable_no_nans));
> > > > >  CmdArgs.push_back(
> > > > >  GetArgString(options::OPT_menable_no_infinities));
> > > > >  
> > > > > CmdArgs.push_back(GetArgString(options::OPT_fno_signed_zeros));
> > > > >  
> > > > > CmdArgs.push_back(GetArgString(options::OPT_freciprocal_math));
> > > > >  
> > > > > CmdArgs.push_back(GetArgString(options::OPT_fapprox_func));
> > > > >}}
> > > > > 
> > > > > Do I just misunderstand something?
> > > > The command line of the driver doesn't change. This patch only affects 
> > > > what CC1 understands, now CC1 doesn't know anymore that 
> > > > `-cl-fast-relaxed-math` implies all these other options so the driver 
> > > > is responsible for specifying them when it constructs the CC1 command 
> > > > line.
> > > > 
> > > > To summarize, the clang driver command line isn't affected by this 
> > > > patch and it shouldn't be so let me know if something is wrong there. 
> > > > However, manually constructed `clang -cc1` invocations need to specify 
> > > > the all the implied flags manually now.
> > > Yes I understand, however, I am wondering whether this is intuitive 
> > > because it seems the behavior of clang with `-cc1` and without will be 
> > > different if the same `-cl-fast-relaxed-math` flag is passed.
> > > 
> > > I also find adding all the flags manually is too verbode if 
> > > `-cl-fast-relaxed-math` assumes to enable all the extra setting.
> > My understanding is that `-cc1` is an internal interface, so end-users 
> > should never use `-cc1` directly and/or rely on itss interface. It is 
> > already the case that flags mean very different things to the driver and 
> > `-cc1` for example "--target=" and "-triple". Furthermore, this impacted 
> > very few tests which leads me to believe that few compiler developers 
> > actually rely on this behavior.
> > 
> > Do you think this would be a major inconvenience to compiler developers to 
> > have to manually expand it out?
> Hi @Anastasia, I'll be taking over this patch. I agree with Daniel that 
> `-cc1` is an internal interface that doesn't need to match the public driver 
> interface.
> The current approach is by far the simplest to get command-line option 
> marshaling working.
> 
> What are your thoughts?
Sorry for the delay.

> My understanding is that -cc1 is an internal interface, so end-users should 
> never use -cc1 directly and/or rely on itss interface. 

This is true in practice but there are developers that use `-cc1` too. My main 
concern is however that the internal testing gets more complicated - with so 
many more flags to be added that can also be easy to miss.

Is there any compromise we could find?  


CHANGES SINCE LAST ACTION
  https://reviews.

[PATCH] D89910: [AIX] Let alloca return 16 bytes alignment

[Xiangling Liao via Phabricator via cfe-commits]

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



Comment at: clang/test/CodeGen/aix_alloca_align.c:11
+void foo() {
+  char *ptr1 = (char *)alloca(sizeof(char) * 9);
+}

hubert.reinterpretcast wrote:
> I'm not entirely sure, but can we try for size 32 and see if we get 16?
Sure, I will add this in.


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

https://reviews.llvm.org/D89910

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

[PATCH] D89910: [AIX] Let alloca return 16 bytes alignment

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L updated this revision to Diff 300043.
Xiangling_L marked an inline comment as done.
Xiangling_L added a comment.

Add one case to the test;


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

https://reviews.llvm.org/D89910

Files:
  clang/lib/Basic/Targets/PPC.h
  clang/test/CodeGen/aix_alloca_align.c
  clang/test/Preprocessor/init-ppc.c
  clang/test/Preprocessor/init-ppc64.c


Index: clang/test/Preprocessor/init-ppc64.c
===
--- clang/test/Preprocessor/init-ppc64.c
+++ clang/test/Preprocessor/init-ppc64.c
@@ -666,7 +666,7 @@
 // PPC64-AIX:#define _LP64 1
 // PPC64-AIX:#define _POWER 1
 // PPC64-AIX:#define __64BIT__ 1
-// PPC64-AIX:#define __BIGGEST_ALIGNMENT__ 8
+// PPC64-AIX:#define __BIGGEST_ALIGNMENT__ 16
 // PPC64-AIX:#define __BIG_ENDIAN__ 1
 // PPC64-AIX:#define __BYTE_ORDER__ __ORDER_BIG_ENDIAN__
 // PPC64-AIX:#define __CHAR16_TYPE__ unsigned short
Index: clang/test/Preprocessor/init-ppc.c
===
--- clang/test/Preprocessor/init-ppc.c
+++ clang/test/Preprocessor/init-ppc.c
@@ -398,7 +398,7 @@
 // PPC-AIX:#define _LONG_LONG 1
 // PPC-AIX-NOT:#define _LP64 1
 // PPC-AIX:#define _POWER 1
-// PPC-AIX:#define __BIGGEST_ALIGNMENT__ 8
+// PPC-AIX:#define __BIGGEST_ALIGNMENT__ 16
 // PPC-AIX:#define __BIG_ENDIAN__ 1
 // PPC-AIX:#define __BYTE_ORDER__ __ORDER_BIG_ENDIAN__
 // PPC-AIX:#define __CHAR16_TYPE__ unsigned short
Index: clang/test/CodeGen/aix_alloca_align.c
===
--- /dev/null
+++ clang/test/CodeGen/aix_alloca_align.c
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple=powerpc-ibm-aix-xcoff -S -emit-llvm < %s | \
+// RUN:   FileCheck --check-prefix=32BIT %s
+
+// RUN: %clang_cc1 -triple=powerpc64-ibm-aix-xcoff -S -emit-llvm < %s | \
+// RUN:   FileCheck --check-prefix=64BIT %s
+
+typedef __SIZE_TYPE__ size_t;
+extern void *alloca(size_t __size) __attribute__((__nothrow__));
+
+void foo() {
+  char *ptr1 = (char *)alloca(sizeof(char) * 9);
+  char *ptr2 = (char *)alloca(sizeof(char) * 32);
+}
+
+// 32BIT: %0 = alloca i8, i32 9, align 16
+// 32BIT: %1 = alloca i8, i32 32, align 16
+
+// 64BIT: %0 = alloca i8, i64 9, align 16
+// 64BIT: %1 = alloca i8, i64 32, align 16
Index: clang/lib/Basic/Targets/PPC.h
===
--- clang/lib/Basic/Targets/PPC.h
+++ clang/lib/Basic/Targets/PPC.h
@@ -370,7 +370,6 @@
   SizeType = UnsignedLong;
   PtrDiffType = SignedLong;
   IntPtrType = SignedLong;
-  SuitableAlign = 64;
   LongDoubleWidth = 64;
   LongDoubleAlign = DoubleAlign = 32;
   LongDoubleFormat = &llvm::APFloat::IEEEdouble();
@@ -409,7 +408,6 @@
 if (Triple.isOSAIX()) {
   // TODO: Set appropriate ABI for AIX platform.
   DataLayout = "E-m:a-i64:64-n32:64";
-  SuitableAlign = 64;
   LongDoubleWidth = 64;
   LongDoubleAlign = DoubleAlign = 32;
   LongDoubleFormat = &llvm::APFloat::IEEEdouble();


Index: clang/test/Preprocessor/init-ppc64.c
===
--- clang/test/Preprocessor/init-ppc64.c
+++ clang/test/Preprocessor/init-ppc64.c
@@ -666,7 +666,7 @@
 // PPC64-AIX:#define _LP64 1
 // PPC64-AIX:#define _POWER 1
 // PPC64-AIX:#define __64BIT__ 1
-// PPC64-AIX:#define __BIGGEST_ALIGNMENT__ 8
+// PPC64-AIX:#define __BIGGEST_ALIGNMENT__ 16
 // PPC64-AIX:#define __BIG_ENDIAN__ 1
 // PPC64-AIX:#define __BYTE_ORDER__ __ORDER_BIG_ENDIAN__
 // PPC64-AIX:#define __CHAR16_TYPE__ unsigned short
Index: clang/test/Preprocessor/init-ppc.c
===
--- clang/test/Preprocessor/init-ppc.c
+++ clang/test/Preprocessor/init-ppc.c
@@ -398,7 +398,7 @@
 // PPC-AIX:#define _LONG_LONG 1
 // PPC-AIX-NOT:#define _LP64 1
 // PPC-AIX:#define _POWER 1
-// PPC-AIX:#define __BIGGEST_ALIGNMENT__ 8
+// PPC-AIX:#define __BIGGEST_ALIGNMENT__ 16
 // PPC-AIX:#define __BIG_ENDIAN__ 1
 // PPC-AIX:#define __BYTE_ORDER__ __ORDER_BIG_ENDIAN__
 // PPC-AIX:#define __CHAR16_TYPE__ unsigned short
Index: clang/test/CodeGen/aix_alloca_align.c
===
--- /dev/null
+++ clang/test/CodeGen/aix_alloca_align.c
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple=powerpc-ibm-aix-xcoff -S -emit-llvm < %s | \
+// RUN:   FileCheck --check-prefix=32BIT %s
+
+// RUN: %clang_cc1 -triple=powerpc64-ibm-aix-xcoff -S -emit-llvm < %s | \
+// RUN:   FileCheck --check-prefix=64BIT %s
+
+typedef __SIZE_TYPE__ size_t;
+extern void *alloca(size_t __size) __attribute__((__nothrow__));
+
+void foo() {
+  char *ptr1 = (char *)alloca(sizeof(char) * 9);
+  char *ptr2 = (char *)alloca(sizeof(char) * 32);
+}
+
+// 32BIT: %0 = alloca i8, i32 9, align 16
+// 32BIT: %1 = alloca i8, i32 32, align 16
+
+// 64BIT: %0 = alloca i8, i64 9, align 16
+// 64BIT: %1 = alloca i8, i64 32, align 16
Index: clang/lib/Basi

[PATCH] D69903: [Basic] Introduce PODSourceLocation, NFCI

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

dexonsmith added a comment.

In D69903#2342011 , @miyuki wrote:

> In D69903#2340020 , @dexonsmith 
> wrote:
>
>> An alternative would be to update the unions to an `AlignedCharArrayUnion` 
>> and use `SourceLocation` directly. WDYT?
>
> So, say, in `DeclarationNameLoc`, I would add `AlignedCharArrayUnion` as 
> follows:
>
>   union {
> llvm::AlignedCharArrayUnion CXXLitOpName> UninitStorage;
> struct NT NamedType;
> struct CXXOpName CXXOperatorName;
> struct CXXLitOpName CXXLiteralOperatorName;
>   };
>
> And change the constructor of `DeclarationNameLoc` to default-construct 
> `UninitStorage`, i.e.:
>
>   DeclarationNameLoc() : UninitStorage() {
> memset(UninitStorage.buffer, 0, sizeof(UninitStorage.buffer));
>   }
>
> After that, I can use `SourceLocation` in `DeclarationNameLoc` directly.
>
> Do I understand your idea correctly?

Not quite, I wasn't thinking of wrapping the `AlignedCharArrayUnion` in a 
`union`, I was thinking of using it *as* the union.

Here's one way you could do it. In the first commit, you can change:

  union {
struct NT NamedType;
struct CXXOpName CXXOperatorName;
struct CXXLitOpName CXXLiteralOperatorName;
  };

to something like:

  AlignedCharArrayUnion NameLoc;

This will mean updating users of `NamedType`, `CXXOperatorName`, and 
`CXXLiteralOperatorName` to go through the new `NameLoc` field. If there are a 
lot of them, you might want to start with a prep commit that adds:

  NT &getNamedType() { return NamedType; }
  CXXOpName &getCXXOperatorName() { return CXXOperatorName; }
  CXXLitOpName &getCXXLiteralOperatorName() { return CXXLiteralOperatorName; }

and change all the users to go through the function, then when you land the 
change to `AlignedCharArrayUnion` you just have to update those three 
functions. But if there aren't many users it may not be worth it.

Once you've created `NameLoc` (using an `AlignedCharArrayUnion`), a follow-up 
commit can change `CXXOpName` and `CXXLitOpName` to use a `SourceLocation` 
instead of a `PODSourceLocation`, and then you can delete `PODSourceLocation` 
once you've handled all of its users.


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

https://reviews.llvm.org/D69903

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

[PATCH] D89974: [driver][CUDA] Use CMake's FindCUDA as default --cuda-path.

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

In D89974#2347938 , @tra wrote:

> One concern I have is that the path we configure during clang's build is not 
> necessarily the right choice for the user of clang we build. It's likely that 
> the clang in the end will be used on a completely different machine.
> E.g. official clang builds can not ever provide the same CUDA path for *all* 
> users who end up using them. Requiring the rest to use a special option to 
> make clang work again looks like an overall usability regression to me.

Not quite, as far as I understand the change it's only one of the searched 
paths.

> I think the default should still let clang search for CUDA or require the 
> user to provide correct CUDA path.  "Use CUDA path discovered by CMake at 
> build time" should be a non-default configuration option if/when it's needed 
> and appropriate.

I agree here. It's definitely surprising to make it the *first* path because 
`module load`ing another CUDA version and putting it into `PATH` is not 
recognized anymore.




Comment at: clang/test/Driver/cuda-detect.cu:7
+// RUN: %clang -v --target=i386-unknown-linux --cuda-path-ignore-cmake  \
+// RUN:   --sysroot=%S/no-cuda-there--cuda-path-ignore-env 2>&1 | FileCheck %s 
-check-prefix NOCUDA
+// RUN: %clang -v --target=i386-apple-macosx --cuda-path-ignore-cmake \

There's now a space missing before `--cuda-path-ignore-env`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89974

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

[PATCH] D89974: [driver][CUDA] Use CMake's FindCUDA as default --cuda-path.

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

In D89974#2347979 , @Hahnfeld wrote:

> In D89974#2347938 , @tra wrote:
>
>> I think the default should still let clang search for CUDA or require the 
>> user to provide correct CUDA path.  "Use CUDA path discovered by CMake at 
>> build time" should be a non-default configuration option if/when it's needed 
>> and appropriate.
>
> I agree here. It's definitely surprising to make it the *first* path because 
> `module load`ing another CUDA version and putting it into `PATH` is not 
> recognized anymore.

Also note that `GCC_INSTALL_PREFIX` works differently (empty by default) and 
must be set explicitly during configure to make the build embed a path into the 
final executable. Maybe that's the better way to go?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89974

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

[PATCH] D89974: [driver][CUDA] Use CMake's FindCUDA as default --cuda-path.

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D89974#2347979 , @Hahnfeld wrote:

> In D89974#2347938 , @tra wrote:
>
>> One concern I have is that the path we configure during clang's build is not 
>> necessarily the right choice for the user of clang we build. It's likely 
>> that the clang in the end will be used on a completely different machine.
>> E.g. official clang builds can not ever provide the same CUDA path for *all* 
>> users who end up using them. Requiring the rest to use a special option to 
>> make clang work again looks like an overall usability regression to me.
>
> Not quite, as far as I understand the change it's only one of the searched 
> paths.

Right. My mistake. I misread the code and the excess of 
`--cuda-path-ignore-cmake` in the tests convinced me that it replaced the 
default search paths.

The point about the path unlikely to be a good default guess still stands. It 
will also introduce surprising behavior when clang built on a machine with CUDA 
installed will behave differently compared to clang built on a machine w/o 
CUDA. IMO clang built from the same sources with the same build configuration 
options should behave the same way, regardless of what's been installed on the 
build host during the build.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89974

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

[clang-tools-extra] 156e8b3 - clang/Basic: Remove ContentCache::getRawBuffer, NFC

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

Author: Duncan P. N. Exon Smith
Date: 2020-10-22T14:00:44-04:00
New Revision: 156e8b37024abd8630666e0ae8a251b80299ed1d

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

LOG: clang/Basic: Remove ContentCache::getRawBuffer, NFC

Replace `ContentCache::getRawBuffer` with `getBufferDataIfLoaded` and
`getBufferIfLoaded`, excising another accessor for the underlying
`MemoryBuffer*` in favour of `StringRef` and `MemoryBufferRef`.

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
clang/include/clang/Basic/SourceManager.h
clang/lib/Basic/SourceManager.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp 
b/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
index f20182bd25e6..4299baedd79f 100644
--- a/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
+++ b/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
@@ -30,12 +30,12 @@ class ExpandModularHeadersPPCallbacks::FileRecorder {
   return;
 
 // FIXME: Why is this happening? We might be losing contents here.
-if (!ContentCache.getRawBuffer())
+llvm::Optional Data = ContentCache.getBufferDataIfLoaded();
+if (!Data)
   return;
 
 InMemoryFs.addFile(File->getName(), /*ModificationTime=*/0,
-   llvm::MemoryBuffer::getMemBufferCopy(
-   ContentCache.getRawBuffer()->getBuffer()));
+   llvm::MemoryBuffer::getMemBufferCopy(*Data));
 // Remove the file from the set of necessary files.
 FilesToRecord.erase(File);
   }

diff  --git a/clang/include/clang/Basic/SourceManager.h 
b/clang/include/clang/Basic/SourceManager.h
index 65c3de5a1e41..4b84ae132c3e 100644
--- a/clang/include/clang/Basic/SourceManager.h
+++ b/clang/include/clang/Basic/SourceManager.h
@@ -194,9 +194,23 @@ namespace SrcMgr {
 /// this content cache.  This is used for performance analysis.
 llvm::MemoryBuffer::BufferKind getMemoryBufferKind() const;
 
-/// Get the underlying buffer, returning NULL if the buffer is not
-/// yet available.
-const llvm::MemoryBuffer *getRawBuffer() const { return Buffer.get(); }
+/// Return the buffer, only if it has been loaded.
+/// specified FileID, returning None if it's not yet loaded.
+///
+/// \param FID The file ID whose contents will be returned.
+llvm::Optional getBufferIfLoaded() const {
+  if (Buffer)
+return Buffer->getMemBufferRef();
+  return None;
+}
+
+/// Return a StringRef to the source buffer data, only if it has already
+/// been loaded.
+llvm::Optional getBufferDataIfLoaded() const {
+  if (Buffer)
+return Buffer->getBuffer();
+  return None;
+}
 
 /// Set the buffer.
 void setBuffer(std::unique_ptr B) {
@@ -207,10 +221,10 @@ namespace SrcMgr {
 /// Set the buffer to one that's not owned (or to nullptr).
 ///
 /// \pre Buffer cannot already be set.
-void setUnownedBuffer(const llvm::MemoryBuffer *B) {
+void setUnownedBuffer(llvm::Optional B) {
   assert(!Buffer && "Expected to be called right after construction");
   if (B)
-setBuffer(llvm::MemoryBuffer::getMemBuffer(B->getMemBufferRef()));
+setBuffer(llvm::MemoryBuffer::getMemBuffer(*B));
 }
 
 // If BufStr has an invalid BOM, returns the BOM name; otherwise, returns

diff  --git a/clang/lib/Basic/SourceManager.cpp 
b/clang/lib/Basic/SourceManager.cpp
index f61b4e2a2281..9592b92f59bc 100644
--- a/clang/lib/Basic/SourceManager.cpp
+++ b/clang/lib/Basic/SourceManager.cpp
@@ -155,7 +155,7 @@ ContentCache::getBufferOrNone(DiagnosticsEngine &Diag, 
FileManager &FM,
 
   // Check that the file's size is the same as in the file entry (which may
   // have come from a stat cache).
-  if (getRawBuffer()->getBufferSize() != (size_t)ContentsEntry->getSize()) {
+  if (Buffer->getBufferSize() != (size_t)ContentsEntry->getSize()) {
 if (Diag.isDiagnosticInFlight())
   Diag.SetDelayedDiagnostic(diag::err_file_modified,
 ContentsEntry->getName());
@@ -363,7 +363,7 @@ void SourceManager::initializeForReplay(const SourceManager 
&Old) {
 Clone->BufferOverridden = Cache->BufferOverridden;
 Clone->IsFileVolatile = Cache->IsFileVolatile;
 Clone->IsTransient = Cache->IsTransient;
-Clone->setUnownedBuffer(Cache->getRawBuffer());
+Clone->setUnownedBuffer(Cache->getBufferIfLoaded());
 return Clone;
   };
 
@@ -476,7 +476,8 @@ const SrcMgr::ContentCache *
 SourceManager::getFakeContentCacheForRecovery() const {
   if (!FakeCon

[PATCH] D89974: [driver][CUDA] Use CMake's FindCUDA as default --cuda-path.

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur updated this revision to Diff 300046.
Meinersbur edited the summary of this revision.
Meinersbur added a comment.

Re-add accidentally removed space


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89974

Files:
  clang/CMakeLists.txt
  clang/include/clang/Config/config.h.cmake
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Cuda.cpp
  clang/test/Driver/cuda-detect-path.cu
  clang/test/Driver/cuda-detect.cu
  openmp/libomptarget/cmake/Modules/LibomptargetGetDependencies.cmake

Index: openmp/libomptarget/cmake/Modules/LibomptargetGetDependencies.cmake
===
--- openmp/libomptarget/cmake/Modules/LibomptargetGetDependencies.cmake
+++ openmp/libomptarget/cmake/Modules/LibomptargetGetDependencies.cmake
@@ -112,10 +112,12 @@
 
 # Looking for CUDA...
 
-if (CUDA_TOOLKIT_ROOT_DIR)
-  set(LIBOMPTARGET_CUDA_TOOLKIT_ROOT_DIR_PRESET TRUE)
-endif()
-find_package(CUDA QUIET)
+
+# How CUDA is detected should be consistent with Clang.
+#
+# /usr/lib/cuda is a special case for Debian/Ubuntu to have nvidia-cuda-toolkit
+# work out of the box. More info on http://bugs.debian.org/882505
+find_package(CUDA QUIET PATHS "/usr/lib/cuda")
 
 # Try to get the highest Nvidia GPU architecture the system supports
 if (CUDA_FOUND)
@@ -221,31 +223,3 @@
   LIBOMPTARGET_DEP_VEO_FOUND
   LIBOMPTARGET_DEP_VEO_INCLUDE_DIRS)
 
-# Looking for CUDA libdevice subdirectory
-#
-# Special case for Debian/Ubuntu to have nvidia-cuda-toolkit work
-# out of the box. More info on http://bugs.debian.org/882505
-
-
-set(LIBOMPTARGET_CUDA_LIBDEVICE_SUBDIR nvvm/libdevice)
-
-# Don't alter CUDA_TOOLKIT_ROOT_DIR if the user specified it, if a value was
-# already cached for it, or if it already has libdevice.  Otherwise, on
-# Debian/Ubuntu, look where the nvidia-cuda-toolkit package normally installs
-# libdevice.
-if (NOT LIBOMPTARGET_CUDA_TOOLKIT_ROOT_DIR_PRESET AND
-NOT EXISTS
-  "${CUDA_TOOLKIT_ROOT_DIR}/${LIBOMPTARGET_CUDA_LIBDEVICE_SUBDIR}")
-  find_program(LSB_RELEASE lsb_release)
-  if (LSB_RELEASE)
-execute_process(COMMAND ${LSB_RELEASE} -is
-  OUTPUT_VARIABLE LSB_RELEASE_ID
-  OUTPUT_STRIP_TRAILING_WHITESPACE)
-set(candidate_dir /usr/lib/cuda)
-if ((LSB_RELEASE_ID STREQUAL "Debian" OR LSB_RELEASE_ID STREQUAL "Ubuntu")
-AND EXISTS "${candidate_dir}/${LIBOMPTARGET_CUDA_LIBDEVICE_SUBDIR}")
-  set(CUDA_TOOLKIT_ROOT_DIR "${candidate_dir}" CACHE PATH
-  "Toolkit location." FORCE)
-endif()
-  endif()
-endif()
Index: clang/test/Driver/cuda-detect.cu
===
--- clang/test/Driver/cuda-detect.cu
+++ clang/test/Driver/cuda-detect.cu
@@ -3,19 +3,18 @@
 // REQUIRES: nvptx-registered-target
 //
 // Check that we properly detect CUDA installation.
-// RUN: %clang -v --target=i386-unknown-linux \
+// RUN: %clang -v --target=i386-unknown-linux --cuda-path-ignore-cmake  \
 // RUN:   --sysroot=%S/no-cuda-there --cuda-path-ignore-env 2>&1 | FileCheck %s -check-prefix NOCUDA
-// RUN: %clang -v --target=i386-apple-macosx \
+// RUN: %clang -v --target=i386-apple-macosx --cuda-path-ignore-cmake \
 // RUN:   --sysroot=%S/no-cuda-there --cuda-path-ignore-env 2>&1 | FileCheck %s -check-prefix NOCUDA
-// RUN: %clang -v --target=x86_64-unknown-linux \
+// RUN: %clang -v --target=x86_64-unknown-linux --cuda-path-ignore-cmake \
 // RUN:   --sysroot=%S/no-cuda-there --cuda-path-ignore-env 2>&1 | FileCheck %s -check-prefix NOCUDA
-// RUN: %clang -v --target=x86_64-apple-macosx \
+// RUN: %clang -v --target=x86_64-apple-macosx --cuda-path-ignore-cmake \
 // RUN:   --sysroot=%S/no-cuda-there --cuda-path-ignore-env 2>&1 | FileCheck %s -check-prefix NOCUDA
 
-
-// RUN: %clang -v --target=i386-unknown-linux \
+// RUN: %clang -v --target=i386-unknown-linux --cuda-path-ignore-cmake \
 // RUN:   --sysroot=%S/Inputs/CUDA --cuda-path-ignore-env 2>&1 | FileCheck %s
-// RUN: %clang -v --target=i386-apple-macosx \
+// RUN: %clang -v --target=i386-apple-macosx --cuda-path-ignore-cmake \
 // RUN:   --sysroot=%S/Inputs/CUDA --cuda-path-ignore-env 2>&1 | FileCheck %s
 
 // RUN: %clang -v --target=i386-unknown-linux \
@@ -24,23 +23,23 @@
 // RUN:   --cuda-path=%S/Inputs/CUDA/usr/local/cuda 2>&1 | FileCheck %s
 
 // Check that we don't find a CUDA installation without libdevice ...
-// RUN: %clang -v --target=i386-unknown-linux \
+// RUN: %clang -v --target=i386-unknown-linux --cuda-path-ignore-cmake \
 // RUN:   --sysroot=%S/Inputs/CUDA-nolibdevice --cuda-path-ignore-env 2>&1 | FileCheck %s -check-prefix NOCUDA
-// RUN: %clang -v --target=i386-apple-macosx \
+// RUN: %clang -v 

[PATCH] D89445: clang/Basic: Remove ContentCache::getRawBuffer, NFC

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rG156e8b37024a: clang/Basic: Remove 
ContentCache::getRawBuffer, NFC (authored by dexonsmith).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89445

Files:
  clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
  clang/include/clang/Basic/SourceManager.h
  clang/lib/Basic/SourceManager.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1468,7 +1468,7 @@
 if (PP->getHeaderSearchInfo()
 .getHeaderSearchOpts()
 .ValidateASTInputFilesContent) {
-  auto *MemBuff = Cache->getRawBuffer();
+  auto MemBuff = Cache->getBufferIfLoaded();
   if (MemBuff)
 ContentHash = hash_value(MemBuff->getBuffer());
   else
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -1542,7 +1542,7 @@
   = SourceMgr.getOrCreateContentCache(File, isSystem(FileCharacter));
 if (OverriddenBuffer && !ContentCache->BufferOverridden &&
 ContentCache->ContentsEntry == ContentCache->OrigEntry &&
-!ContentCache->getRawBuffer()) {
+!ContentCache->getBufferIfLoaded()) {
   auto Buffer = ReadBuffer(SLocEntryCursor, File->getName());
   if (!Buffer)
 return true;
Index: clang/lib/Basic/SourceManager.cpp
===
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -155,7 +155,7 @@
 
   // Check that the file's size is the same as in the file entry (which may
   // have come from a stat cache).
-  if (getRawBuffer()->getBufferSize() != (size_t)ContentsEntry->getSize()) {
+  if (Buffer->getBufferSize() != (size_t)ContentsEntry->getSize()) {
 if (Diag.isDiagnosticInFlight())
   Diag.SetDelayedDiagnostic(diag::err_file_modified,
 ContentsEntry->getName());
@@ -363,7 +363,7 @@
 Clone->BufferOverridden = Cache->BufferOverridden;
 Clone->IsFileVolatile = Cache->IsFileVolatile;
 Clone->IsTransient = Cache->IsTransient;
-Clone->setUnownedBuffer(Cache->getRawBuffer());
+Clone->setUnownedBuffer(Cache->getBufferIfLoaded());
 return Clone;
   };
 
@@ -476,7 +476,8 @@
 SourceManager::getFakeContentCacheForRecovery() const {
   if (!FakeContentCacheForRecovery) {
 FakeContentCacheForRecovery = std::make_unique();
-FakeContentCacheForRecovery->setUnownedBuffer(getFakeBufferForRecovery());
+FakeContentCacheForRecovery->setUnownedBuffer(
+getFakeBufferForRecovery()->getMemBufferRef());
   }
   return FakeContentCacheForRecovery.get();
 }
@@ -751,10 +752,7 @@
   if (!SLoc.isFile() || MyInvalid)
 return None;
 
-  if (const llvm::MemoryBuffer *Buf =
-  SLoc.getFile().getContentCache()->getRawBuffer())
-return Buf->getBuffer();
-  return None;
+  return SLoc.getFile().getContentCache()->getBufferDataIfLoaded();
 }
 
 llvm::Optional SourceManager::getBufferDataOrNone(FileID FID) const {
Index: clang/include/clang/Basic/SourceManager.h
===
--- clang/include/clang/Basic/SourceManager.h
+++ clang/include/clang/Basic/SourceManager.h
@@ -194,9 +194,23 @@
 /// this content cache.  This is used for performance analysis.
 llvm::MemoryBuffer::BufferKind getMemoryBufferKind() const;
 
-/// Get the underlying buffer, returning NULL if the buffer is not
-/// yet available.
-const llvm::MemoryBuffer *getRawBuffer() const { return Buffer.get(); }
+/// Return the buffer, only if it has been loaded.
+/// specified FileID, returning None if it's not yet loaded.
+///
+/// \param FID The file ID whose contents will be returned.
+llvm::Optional getBufferIfLoaded() const {
+  if (Buffer)
+return Buffer->getMemBufferRef();
+  return None;
+}
+
+/// Return a StringRef to the source buffer data, only if it has already
+/// been loaded.
+llvm::Optional getBufferDataIfLoaded() const {
+  if (Buffer)
+return Buffer->getBuffer();
+  return None;
+}
 
 /// Set the buffer.
 void setBuffer(std::unique_ptr B) {
@@ -207,10 +221,10 @@
 /// Set the buffer to one that's not owned (or to nullptr).
 ///
 /// \pre Buffer cannot already be set.
-void setUnownedBuffer(const llvm::MemoryBuffer *B) {
+void setUnownedBuffer(llvm::Optional B) {
   assert(!Buffer && "Expected to be called right after construction");
   if (B)
-setBuffer(llvm::MemoryBuff

[PATCH] D69844: [clang][Basic] Integrate SourceLocation with FoldingSet, NFCI

[Mikhail Maltsev via Phabricator via cfe-commits]

miyuki updated this revision to Diff 300044.
miyuki retitled this revision from "[Basic] Integrate SourceLocation and 
SourceRange with FoldingSet, NFCI" to "[clang][Basic] Integrate SourceLocation 
with FoldingSet, NFCI".
miyuki edited the summary of this revision.
miyuki added a comment.

Addressed review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69844

Files:
  clang/include/clang/Basic/SourceLocation.h
  clang/lib/Analysis/PathDiagnostic.cpp
  clang/lib/Basic/SourceLocation.cpp


Index: clang/lib/Basic/SourceLocation.cpp
===
--- clang/lib/Basic/SourceLocation.cpp
+++ clang/lib/Basic/SourceLocation.cpp
@@ -15,6 +15,7 @@
 #include "clang/Basic/PrettyStackTrace.h"
 #include "clang/Basic/SourceManager.h"
 #include "llvm/ADT/DenseMapInfo.h"
+#include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/MemoryBuffer.h"
@@ -45,6 +46,11 @@
   return llvm::DenseMapInfo::getHashValue(ID);
 }
 
+void llvm::FoldingSetTrait::Profile(
+const SourceLocation &X, llvm::FoldingSetNodeID &ID) {
+  ID.AddInteger(X.ID);
+}
+
 void SourceLocation::print(raw_ostream &OS, const SourceManager &SM)const{
   if (!isValid()) {
 OS << "";
Index: clang/lib/Analysis/PathDiagnostic.cpp
===
--- clang/lib/Analysis/PathDiagnostic.cpp
+++ clang/lib/Analysis/PathDiagnostic.cpp
@@ -1083,9 +1083,9 @@
 
//===--===//
 
 void PathDiagnosticLocation::Profile(llvm::FoldingSetNodeID &ID) const {
-  ID.AddInteger(Range.getBegin().getRawEncoding());
-  ID.AddInteger(Range.getEnd().getRawEncoding());
-  ID.AddInteger(Loc.getRawEncoding());
+  ID.Add(Range.getBegin());
+  ID.Add(Range.getEnd());
+  ID.Add(static_cast(Loc));
 }
 
 void PathDiagnosticPiece::Profile(llvm::FoldingSetNodeID &ID) const {
@@ -1095,8 +1095,8 @@
   ID.AddInteger((unsigned) getDisplayHint());
   ArrayRef Ranges = getRanges();
   for (const auto &I : Ranges) {
-ID.AddInteger(I.getBegin().getRawEncoding());
-ID.AddInteger(I.getEnd().getRawEncoding());
+ID.Add(I.getBegin());
+ID.Add(I.getEnd());
   }
 }
 
Index: clang/include/clang/Basic/SourceLocation.h
===
--- clang/include/clang/Basic/SourceLocation.h
+++ clang/include/clang/Basic/SourceLocation.h
@@ -26,6 +26,9 @@
 
 template  struct DenseMapInfo;
 
+class FoldingSetNodeID;
+template  struct FoldingSetTrait;
+
 } // namespace llvm
 
 namespace clang {
@@ -87,6 +90,7 @@
   friend class ASTReader;
   friend class ASTWriter;
   friend class SourceManager;
+  friend class llvm::FoldingSetTrait;
 
   unsigned ID = 0;
 
@@ -501,6 +505,10 @@
 }
   };
 
+  template <> struct FoldingSetTrait {
+static void Profile(const clang::SourceLocation &X, FoldingSetNodeID &ID);
+  };
+
   // Teach SmallPtrSet how to handle SourceLocation.
   template<>
   struct PointerLikeTypeTraits {


Index: clang/lib/Basic/SourceLocation.cpp
===
--- clang/lib/Basic/SourceLocation.cpp
+++ clang/lib/Basic/SourceLocation.cpp
@@ -15,6 +15,7 @@
 #include "clang/Basic/PrettyStackTrace.h"
 #include "clang/Basic/SourceManager.h"
 #include "llvm/ADT/DenseMapInfo.h"
+#include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/MemoryBuffer.h"
@@ -45,6 +46,11 @@
   return llvm::DenseMapInfo::getHashValue(ID);
 }
 
+void llvm::FoldingSetTrait::Profile(
+const SourceLocation &X, llvm::FoldingSetNodeID &ID) {
+  ID.AddInteger(X.ID);
+}
+
 void SourceLocation::print(raw_ostream &OS, const SourceManager &SM)const{
   if (!isValid()) {
 OS << "";
Index: clang/lib/Analysis/PathDiagnostic.cpp
===
--- clang/lib/Analysis/PathDiagnostic.cpp
+++ clang/lib/Analysis/PathDiagnostic.cpp
@@ -1083,9 +1083,9 @@
 //===--===//
 
 void PathDiagnosticLocation::Profile(llvm::FoldingSetNodeID &ID) const {
-  ID.AddInteger(Range.getBegin().getRawEncoding());
-  ID.AddInteger(Range.getEnd().getRawEncoding());
-  ID.AddInteger(Loc.getRawEncoding());
+  ID.Add(Range.getBegin());
+  ID.Add(Range.getEnd());
+  ID.Add(static_cast(Loc));
 }
 
 void PathDiagnosticPiece::Profile(llvm::FoldingSetNodeID &ID) const {
@@ -1095,8 +1095,8 @@
   ID.AddInteger((unsigned) getDisplayHint());
   ArrayRef Ranges = getRanges();
   for (const auto &I : Ranges) {
-ID.AddInteger(I.getBegin().getRawEncoding());
-ID.AddInteger(I.getEnd().getRawEncoding());
+ID.Add(I.getBegin());
+ID.Add(I.getEnd());
   }
 }
 
Index: clang/include/clang/Basic/SourceLocation.h
===

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a subscriber: majnemer.
erichkeane added a comment.

Turns out this patch: 
https://github.com/llvm/llvm-project/commit/2e1e0491b7098fcfe01945e8f62cafe1fcb3cf36
 is my problem. The issue has to do deducing a 'local' type in the return type. 
 As a result, we choose to mangle any type 'containing' auto as 'auto'.

So ours demangles as: ##public: __thiscall `void __cdecl 
usage(void)'::`1'operator (void)const ##
MSVC of course demangles as: ##public: __thiscall 
::operator double 
(__cdecl*)(int,float,double)(void)const ##

To me, it seems that the ResultType->getContainedAutoType is perhaps in the 
wrong here (also of interest, the dyn_cast, for a function that already returns 
AutoType?)? I'm not sure how to workaround this.  I can of course create an 
awkward reproducer of this (https://godbolt.org/z/KWPTTh), but I don't have a 
good idea on how to better handle this.  @majnemer I see you were the initial 
committer of this, and would like to see if you have any ideas?


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

https://reviews.llvm.org/D89559

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

[PATCH] D82756: Port some floating point options to new option marshalling infrastructure

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia resigned from this revision.
Anastasia added a comment.

Just to clarify aside from the concern I have raised regarding internal testing 
I am not in any strong opposition of this feature. So if the community decides 
that it is more important to have this feature than to keep the tests short I 
am fine with this. However, due to the limited time I will not be able to 
continue reviewing this change as I don't want to block the progress due to my 
priorities.


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

https://reviews.llvm.org/D82756

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

[PATCH] D89974: [driver][CUDA] Use CMake's FindCUDA as default --cuda-path.

[Raul Tambre via Phabricator via cfe-commits]

tambre added a comment.

I echo @tra's concerns.
Having an option for a vendor to append/prepend a toolkit search location seems 
useful, but currently this seems more like a workaround for an explicit CUDA 
toolkit path not being passed to the testsuite.
Shortcomings in the testrunner don't seem like a reason to introduce new 
build-time configured default search paths into the driver.

Aside, here's an overview of CMake+CUDA:

- FindCUDA is deprecated since 3.10 and obsolete since 3.17. Replaced by native 
language support and FindCUDAToolkit 
.
- CMake supports Clang as a CUDA compiler since 3.18.
- CMake supports separable compilation with Clang since 3.19.
- CMake doesn't support CUDA with Clang on Windows 
. I hope to work on it 
for 3.20.

Ideally LLVM would use the native CMake CUDA support and the testsuite would 
use the CUDA toolkit found by CMake where necessary.
Users would be able to override the selected toolkit by setting 
`CUDAToolkit_ROOT` when configuring Clang.
Unfortunately this would require CMake 3.18, so this'll have to wait a bit.

If there are any questions/issues on the CMake side let me know. I maintain 
Clang and CUDA support in CMake.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89974

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