[PATCH] D114080: [SYCL] Diagnose uses of zero length arrays

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

I hope that someone not from Intel will take a look as well. If this won't 
happen for reasonable amount of time, I'll submit this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114080

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

[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

[Henry Linjamäki via Phabricator via cfe-commits]

linjamaki added a comment.

Assuming that this patch is ready to land. @tra or @yaxunl, could you please 
commit this patch to the LLVM for us? Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110622

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

[PATCH] D102669: [analyzer][ctu] Fix wrong 'multiple definitions' errors caused by space characters in lookup names when parsing the ctu index file

[Ella Ma via Phabricator via cfe-commits]

OikawaKirie added a comment.

It seems that it is not this patch that triggers the problem, which is similar 
to D75665 .
IMO it is the problem of on-demand-parsing, but I do not have a Mac M1 
 device to reproduce this bug.
Maybe we can just land this patch by restricting the test case to be executed 
only on Linux, just as what D75665  does 
(rG5cc18516c483 
 vs 
rG97e07d0c352c 
), and 
leave the problem for future fixes.

Could you please do the update as provided below and land this patch again? 
@steakhal or other reviewers?




Comment at: clang/test/Analysis/ctu-lookup-name-with-space.cpp:13
+// RUN:   -verify %s
+
+void importee();

Adding this line here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102669

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

[PATCH] D115879: [clang-format] extern with new line brace without indentation

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1282-1300
 if (FormatTok->Tok.is(tok::string_literal)) {
   nextToken();
   if (FormatTok->Tok.is(tok::l_brace)) {
+if (Style.BraceWrapping.AfterExternBlock) {
+  addUnwrappedLine();
+}
 if (!Style.IndentExternBlock) {

This `case` is kind of messy and some cleanup may be in order. A related 
question: why do we make `IEBS_AfterExternBlock` depend on 
`BraceWrapping.AfterExternBlock`? Can't we only have true/false for 
`IndentExternBlock` and let the user set it independent of 
`BraceWrapping.AfterExternBlock`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115879

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

[PATCH] D112903: [C++20] [Module] Fix bug47116 and implement [module.interface]/p6

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu marked 5 inline comments as done.
ChuanqiXu added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:1622
 
+// [module.interface]p6:
+// A redeclaration of an entity X is implicitly exported if X was introduced by

aaron.ballman wrote:
> 
I didn't address this since it belongs to C++20 instead of C++2b.


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

https://reviews.llvm.org/D112903

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

[PATCH] D115879: [clang-format] extern with new line brace without indentation

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:3819
+  Style.IndentExternBlock = FormatStyle::IEBS_NoIndent;
+  verifyFormat("extern \"C\"\n{ /*13*/\n}", Style);
+  verifyFormat("extern \"C\"\n{\n"

curdeius wrote:
> I'd prefer that you split strings after `\n`. I find it easier to read that 
> way.
> The same below.
It's everywhere in this function. We should fix them either now or in a 
separate patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115879

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

[PATCH] D112903: [C++20] [Module] Fix bug47116 and implement [module.interface]/p6

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu updated this revision to Diff 395045.
ChuanqiXu added a comment.

Address comments


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

https://reviews.llvm.org/D112903

Files:
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/DeclBase.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/CXX/module/module.interface/p2-2.cpp
  clang/test/CXX/module/module.interface/p6.cpp

Index: clang/test/CXX/module/module.interface/p6.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.interface/p6.cpp
@@ -0,0 +1,93 @@
+// The test is check we couldn't export a redeclaration which isn't exported previously and
+// check it is OK to redeclare no matter exported nor not if is the previous declaration is exported.
+// RUN: %clang_cc1 -std=c++20 %s -verify
+
+export module X;
+
+struct S { // expected-note {{previous declaration is here}}
+  int n;
+};
+typedef S S;
+export typedef S S; // OK, does not redeclare an entity
+export struct S;// expected-error {{cannot export redeclaration 'S' here since the previous declaration is not exported}}
+
+namespace A {
+struct X; // expected-note {{previous declaration is here}}
+export struct Y;
+} // namespace A
+
+namespace A {
+export struct X; // expected-error {{cannot export redeclaration 'X' here since the previous declaration is not exported}}
+export struct Y; // OK
+struct Z;// expected-note {{previous declaration is here}}
+export struct Z; // expected-error {{cannot export redeclaration 'Z' here since the previous declaration is not exported}}
+} // namespace A
+
+namespace A {
+struct B;// expected-note {{previous declaration is here}}
+struct C {}; // expected-note {{previous declaration is here}}
+} // namespace A
+
+namespace A {
+export struct B {}; // expected-error {{cannot export redeclaration 'B' here since the previous declaration is not exported}}
+export struct C;// expected-error {{cannot export redeclaration 'C' here since the previous declaration is not exported}}
+} // namespace A
+
+template 
+struct TemplS; // expected-note {{previous declaration is here}}
+
+export template 
+struct TemplS {}; // expected-error {{cannot export redeclaration 'TemplS' here since the previous declaration is not exported}}
+
+template 
+struct TemplS2; // expected-note {{previous declaration is here}}
+
+export template 
+struct TemplS2 {}; // expected-error {{cannot export redeclaration 'TemplS2' here since the previous declaration is not exported}}
+
+void baz();// expected-note {{previous declaration is here}}
+export void baz(); // expected-error {{cannot export redeclaration 'baz' here since the previous declaration is not exported}}
+
+namespace A {
+export void foo();
+void bar();// expected-note {{previous declaration is here}}
+export void bar(); // expected-error {{cannot export redeclaration 'bar' here since the previous declaration is not exported}}
+void f1(); // expected-note {{previous declaration is here}}
+} // namespace A
+
+// OK
+//
+// [module.interface]/p6
+// A redeclaration of an entity X is implicitly exported if X was introduced by an exported declaration
+void A::foo();
+
+// The compiler couldn't export A::f1() here since A::f1() is declared above without exported.
+// See [module.interface]/p6 for details.
+export void A::f1(); // expected-error {{cannot export redeclaration 'f1' here since the previous declaration is not exported}}
+
+template 
+void TemplFunc(); // expected-note {{previous declaration is here}}
+
+export template 
+void TemplFunc() { // expected-error {{cannot export redeclaration 'TemplFunc' here since the previous declaration is not exported}}
+}
+
+namespace A {
+template 
+void TemplFunc2(); // expected-note {{previous declaration is here}}
+export template 
+void TemplFunc2() {} // expected-error {{cannot export redeclaration 'TemplFunc2' here since the previous declaration is not exported}}
+template 
+void TemplFunc3(); // expected-note {{previous declaration is here}}
+} // namespace A
+
+export template 
+void A::TemplFunc3() {} // expected-error {{cannot export redeclaration 'TemplFunc3' here since the previous declaration is not exported}}
+
+int var;// expected-note {{previous declaration is here}}
+export int var; // expected-error {{cannot export redeclaration 'var' here since the previous declaration is not exported}}
+
+template 
+T TemplVar; // expected-note {{previous declaration is here}}
+export template 
+T TemplVar; // expected-error {{cannot export redeclaration 'TemplVar' here since the previous declaration is not exported}}
Index: clang/test/CXX/module/module.interface/p2-2.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.interface/p2-2.cpp
@@ 

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

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng added a comment.

@asb

> Are you saying that there won't be a new ISA manual release (using whatever 
> naming scheme) that incorporates the ratified versions of bitmanip, crypto, 
> vector etc?

Oh, I guess I using some word too strong there, here should have some newer 
release in future, but we don't know the time-frame and release version scheme, 
maybe I should checked that with other foundation folks, and back to post the 
updated info here.

That already become a practical problem to GNU toolchain side since upstream 
GNU toolchain support those extensions who already frozen, and could enable 
that without version and `-meanble-experimental-extension`, but for Clang/LLVM 
side, it's still guarded by `-meanble-experimental-extension` and require 
explicitly version, so it's not urgent issue for Clang/LLVM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113237

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

[clang] c50a4b3 - [Modules] Incorrect ODR detection for unresolved using type

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2021-12-17T10:37:40+08:00
New Revision: c50a4b3f97497c27ad62797080b52f501dac5e38

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

LOG: [Modules] Incorrect ODR detection for unresolved using type

Implement `getUnresolvedUsingType()` and don't create a new
`UnresolvedUsingType` when there is already canonical declaration.

This solved an incorrect ODR detection in modules for uresolved using
type.

Reviewed By: rjmccall

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

Added: 
clang/test/Modules/Inputs/odr_using_dependent_name/X.cppm
clang/test/Modules/Inputs/odr_using_dependent_name/foo.h
clang/test/Modules/odr_using_dependent_name.cppm

Modified: 
clang/include/clang/AST/ASTContext.h
clang/include/clang/AST/TypeProperties.td
clang/lib/AST/ASTContext.cpp

Removed: 




diff  --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index e861cd6ae3466..ebe8fc8faf910 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -1564,6 +1564,9 @@ class ASTContext : public RefCountedBase {
 
   QualType getEnumType(const EnumDecl *Decl) const;
 
+  QualType
+  getUnresolvedUsingType(const UnresolvedUsingTypenameDecl *Decl) const;
+
   QualType getInjectedClassNameType(CXXRecordDecl *Decl, QualType TST) const;
 
   QualType getAttributedType(attr::Kind attrKind,

diff  --git a/clang/include/clang/AST/TypeProperties.td 
b/clang/include/clang/AST/TypeProperties.td
index b7730a0f32dcf..19325d0c1fb23 100644
--- a/clang/include/clang/AST/TypeProperties.td
+++ b/clang/include/clang/AST/TypeProperties.td
@@ -358,7 +358,7 @@ let Class = UnresolvedUsingType in {
   }
 
   def : Creator<[{
-return ctx.getTypeDeclType(cast(declaration));
+return 
ctx.getUnresolvedUsingType(cast(declaration));
   }]>;
 }
 

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index b6e1966bf980c..22020119b9f50 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -4568,9 +4568,7 @@ QualType ASTContext::getTypeDeclTypeSlow(const TypeDecl 
*Decl) const {
 assert(Enum->isFirstDecl() && "enum has previous declaration");
 return getEnumType(Enum);
   } else if (const auto *Using = dyn_cast(Decl)) {
-Type *newType = new (*this, TypeAlignment) UnresolvedUsingType(Using);
-Decl->TypeForDecl = newType;
-Types.push_back(newType);
+return getUnresolvedUsingType(Using);
   } else
 llvm_unreachable("TypeDecl without a type?");
 
@@ -4619,6 +4617,22 @@ QualType ASTContext::getEnumType(const EnumDecl *Decl) 
const {
   return QualType(newType, 0);
 }
 
+QualType ASTContext::getUnresolvedUsingType(
+const UnresolvedUsingTypenameDecl *Decl) const {
+  if (Decl->TypeForDecl)
+return QualType(Decl->TypeForDecl, 0);
+
+  if (const UnresolvedUsingTypenameDecl *CanonicalDecl =
+  Decl->getCanonicalDecl())
+if (CanonicalDecl->TypeForDecl)
+  return QualType(Decl->TypeForDecl = CanonicalDecl->TypeForDecl, 0);
+
+  Type *newType = new (*this, TypeAlignment) UnresolvedUsingType(Decl);
+  Decl->TypeForDecl = newType;
+  Types.push_back(newType);
+  return QualType(newType, 0);
+}
+
 QualType ASTContext::getAttributedType(attr::Kind attrKind,
QualType modifiedType,
QualType equivalentType) {

diff  --git a/clang/test/Modules/Inputs/odr_using_dependent_name/X.cppm 
b/clang/test/Modules/Inputs/odr_using_dependent_name/X.cppm
new file mode 100644
index 0..ccf246017a497
--- /dev/null
+++ b/clang/test/Modules/Inputs/odr_using_dependent_name/X.cppm
@@ -0,0 +1,3 @@
+module;
+#include "foo.h"
+export module X;

diff  --git a/clang/test/Modules/Inputs/odr_using_dependent_name/foo.h 
b/clang/test/Modules/Inputs/odr_using_dependent_name/foo.h
new file mode 100644
index 0..2c02912b2257e
--- /dev/null
+++ b/clang/test/Modules/Inputs/odr_using_dependent_name/foo.h
@@ -0,0 +1,9 @@
+template 
+struct bar {
+  using Ty = int;
+};
+template 
+struct foo : public bar {
+  using typename bar::Ty;
+  void baz(Ty);
+};

diff  --git a/clang/test/Modules/odr_using_dependent_name.cppm 
b/clang/test/Modules/odr_using_dependent_name.cppm
new file mode 100644
index 0..8d7ea9f57bedc
--- /dev/null
+++ b/clang/test/Modules/odr_using_dependent_name.cppm
@@ -0,0 +1,9 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: %clang -std=c++20 %S/Inputs/odr_using_dependent_name/X.cppm 
--precompile -o %t/X.pcm
+// RUN: %clang -std=c++20 -I%S/Inputs/odr_using_dependent_name 
-fprebuilt-module-path=%t %s --precompile -c
+// expected-no-diagnostics
+module;
+#include "foo.h"
+export module Y;
+import X;

[PATCH] D115792: [Modules] Incorrect ODR detection for unresolved using type

[Chuanqi Xu 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 rGc50a4b3f9749: [Modules] Incorrect ODR detection for 
unresolved using type (authored by ChuanqiXu).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115792

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/TypeProperties.td
  clang/lib/AST/ASTContext.cpp
  clang/test/Modules/Inputs/odr_using_dependent_name/X.cppm
  clang/test/Modules/Inputs/odr_using_dependent_name/foo.h
  clang/test/Modules/odr_using_dependent_name.cppm

Index: clang/test/Modules/odr_using_dependent_name.cppm
===
--- /dev/null
+++ clang/test/Modules/odr_using_dependent_name.cppm
@@ -0,0 +1,9 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: %clang -std=c++20 %S/Inputs/odr_using_dependent_name/X.cppm --precompile -o %t/X.pcm
+// RUN: %clang -std=c++20 -I%S/Inputs/odr_using_dependent_name -fprebuilt-module-path=%t %s --precompile -c
+// expected-no-diagnostics
+module;
+#include "foo.h"
+export module Y;
+import X;
Index: clang/test/Modules/Inputs/odr_using_dependent_name/foo.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/odr_using_dependent_name/foo.h
@@ -0,0 +1,9 @@
+template 
+struct bar {
+  using Ty = int;
+};
+template 
+struct foo : public bar {
+  using typename bar::Ty;
+  void baz(Ty);
+};
Index: clang/test/Modules/Inputs/odr_using_dependent_name/X.cppm
===
--- /dev/null
+++ clang/test/Modules/Inputs/odr_using_dependent_name/X.cppm
@@ -0,0 +1,3 @@
+module;
+#include "foo.h"
+export module X;
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -4568,9 +4568,7 @@
 assert(Enum->isFirstDecl() && "enum has previous declaration");
 return getEnumType(Enum);
   } else if (const auto *Using = dyn_cast(Decl)) {
-Type *newType = new (*this, TypeAlignment) UnresolvedUsingType(Using);
-Decl->TypeForDecl = newType;
-Types.push_back(newType);
+return getUnresolvedUsingType(Using);
   } else
 llvm_unreachable("TypeDecl without a type?");
 
@@ -4619,6 +4617,22 @@
   return QualType(newType, 0);
 }
 
+QualType ASTContext::getUnresolvedUsingType(
+const UnresolvedUsingTypenameDecl *Decl) const {
+  if (Decl->TypeForDecl)
+return QualType(Decl->TypeForDecl, 0);
+
+  if (const UnresolvedUsingTypenameDecl *CanonicalDecl =
+  Decl->getCanonicalDecl())
+if (CanonicalDecl->TypeForDecl)
+  return QualType(Decl->TypeForDecl = CanonicalDecl->TypeForDecl, 0);
+
+  Type *newType = new (*this, TypeAlignment) UnresolvedUsingType(Decl);
+  Decl->TypeForDecl = newType;
+  Types.push_back(newType);
+  return QualType(newType, 0);
+}
+
 QualType ASTContext::getAttributedType(attr::Kind attrKind,
QualType modifiedType,
QualType equivalentType) {
Index: clang/include/clang/AST/TypeProperties.td
===
--- clang/include/clang/AST/TypeProperties.td
+++ clang/include/clang/AST/TypeProperties.td
@@ -358,7 +358,7 @@
   }
 
   def : Creator<[{
-return ctx.getTypeDeclType(cast(declaration));
+return ctx.getUnresolvedUsingType(cast(declaration));
   }]>;
 }
 
Index: clang/include/clang/AST/ASTContext.h
===
--- clang/include/clang/AST/ASTContext.h
+++ clang/include/clang/AST/ASTContext.h
@@ -1564,6 +1564,9 @@
 
   QualType getEnumType(const EnumDecl *Decl) const;
 
+  QualType
+  getUnresolvedUsingType(const UnresolvedUsingTypenameDecl *Decl) const;
+
   QualType getInjectedClassNameType(CXXRecordDecl *Decl, QualType TST) const;
 
   QualType getAttributedType(attr::Kind attrKind,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng added a comment.

@jrtc27

> Outside of the I/F/D special cases, where F/D don't really matter and I2p0 is 
> just I2p1Zicsr2p0_Zifencei2p0,, I thought the new policy was that ratified 
> extensions would never be changed, only new extensions published, and thus 
> version numbers are basically irrelevant other than to distinguish ratified 
> from pre-ratified?

The `I2p0` and `I2p1` is the most problem now, and new policy seems intend to 
add new ratified ext. rather than extend or change, but I guess I just be more 
conservative here, since the `I` isn't expect to be changed before it change.

And we are trying to support *real* multi-version on clang/LLVM (e.g. try to 
support v 0.10 and 1.0 and emit right version on ELF attr.), so we are headache 
on this issue for a while, but this might not issue to upstream LLVM I guess.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113237

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

[PATCH] D102669: [analyzer][ctu] Fix wrong 'multiple definitions' errors caused by space characters in lookup names when parsing the ctu index file

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

reverted in 770ef94097c02205b3ec9e902f1d6a9c99b5189c 
. thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102669

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

[clang] 770ef94 - Revert "[analyzer][ctu] Fix wrong 'multiple definitions' errors caused by space characters in lookup names when parsing the ctu index file"

[Nico Weber via cfe-commits]

Author: Nico Weber
Date: 2021-12-16T20:46:51-05:00
New Revision: 770ef94097c02205b3ec9e902f1d6a9c99b5189c

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

LOG: Revert "[analyzer][ctu] Fix wrong 'multiple definitions' errors caused by 
space characters in lookup names when parsing the ctu index file"

This reverts commit 333d66b09494b7ebc1a89f2befa79128a56f77e3.
Breaks tests on macOS, see comments on https://reviews.llvm.org/D102669

Added: 


Modified: 
clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
clang/include/clang/Basic/DiagnosticCrossTUKinds.td
clang/lib/CrossTU/CrossTranslationUnit.cpp
clang/test/Analysis/Inputs/ctu-import.c.externalDefMap.ast-dump.txt
clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.ast-dump.txt
clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt

clang/test/Analysis/Inputs/plist-macros-with-expansion-ctu.c.externalDefMap.txt
clang/test/Analysis/ctu-inherited-default-ctor.cpp
clang/test/Analysis/func-mapping-test.cpp
clang/unittests/CrossTU/CrossTranslationUnitTest.cpp

Removed: 
clang/test/Analysis/Inputs/ctu-lookup-name-with-space.cpp
clang/test/Analysis/ctu-lookup-name-with-space.cpp



diff  --git a/clang/docs/analyzer/user-docs/CrossTranslationUnit.rst 
b/clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
index 0976c9a5b67a1..0e0691b95d5d6 100644
--- a/clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
+++ b/clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
@@ -81,12 +81,12 @@ of `foo.cpp`:
   $
 
 The next step is to create a CTU index file which holds the `USR` name and 
location of external definitions in the
-source files in format `: `:
+source files:
 
 .. code-block:: bash
 
   $ clang-extdef-mapping -p . foo.cpp
-  9:c:@F@foo# /path/to/your/project/foo.cpp
+  c:@F@foo# /path/to/your/project/foo.cpp
   $ clang-extdef-mapping -p . foo.cpp > externalDefMap.txt
 
 We have to modify `externalDefMap.txt` to contain the name of the `.ast` files 
instead of the source files:
@@ -278,12 +278,12 @@ The `invocation list`:
 
 We'd like to analyze `main.cpp` and discover the division by zero bug.
 As we are using On-demand mode, we only need to create a CTU index file which 
holds the `USR` name and location of
-external definitions in the source files in format `: 
`:
+external definitions in the source files:
 
 .. code-block:: bash
 
   $ clang-extdef-mapping -p . foo.cpp
-  9:c:@F@foo# /path/to/your/project/foo.cpp
+  c:@F@foo# /path/to/your/project/foo.cpp
   $ clang-extdef-mapping -p . foo.cpp > externalDefMap.txt
 
 Now everything is available for the CTU analysis.

diff  --git a/clang/include/clang/Basic/DiagnosticCrossTUKinds.td 
b/clang/include/clang/Basic/DiagnosticCrossTUKinds.td
index e6ea1956f98a6..4277a3173203a 100644
--- a/clang/include/clang/Basic/DiagnosticCrossTUKinds.td
+++ b/clang/include/clang/Basic/DiagnosticCrossTUKinds.td
@@ -12,8 +12,8 @@ def err_ctu_error_opening : Error<
   "error opening '%0': required by the CrossTU functionality">;
 
 def err_extdefmap_parsing : Error<
-  "error parsing index file: '%0' line: %1 ': ' "
-  "format expected">;
+  "error parsing index file: '%0' line: %1 'UniqueID filename' format "
+  "expected">;
 
 def err_multiple_def_index : Error<
   "multiple definitions are found for the same key in index ">;

diff  --git a/clang/lib/CrossTU/CrossTranslationUnit.cpp 
b/clang/lib/CrossTU/CrossTranslationUnit.cpp
index cbe07acb76fb1..0aecad491eccb 100644
--- a/clang/lib/CrossTU/CrossTranslationUnit.cpp
+++ b/clang/lib/CrossTU/CrossTranslationUnit.cpp
@@ -149,35 +149,6 @@ std::error_code IndexError::convertToErrorCode() const {
   return std::error_code(static_cast(Code), *Category);
 }
 
-/// Parse one line of the input CTU index file.
-///
-/// @param[in]  LineRef The input CTU index item in format
-/// ": ".
-/// @param[out] LookupName  The lookup name in format ":".
-/// @param[out] FilePathThe file path "".
-static bool parseCrossTUIndexItem(StringRef LineRef, StringRef ,
-  StringRef ) {
-  // `LineRef` is ": " now.
-
-  size_t USRLength = 0;
-  if (LineRef.consumeInteger(10, USRLength))
-return false;
-  assert(USRLength && "USRLength should be greater than zero.");
-
-  if (!LineRef.consume_front(":"))
-return false;
-
-  // `LineRef` is now just " ".
-
-  // Check LookupName length out of bound and incorrect delimiter.
-  if (USRLength >= LineRef.size() || ' ' != LineRef[USRLength])
-return false;
-
-  LookupName = LineRef.substr(0, USRLength);
-  FilePath = LineRef.substr(USRLength + 1);
-  return true;
-}
-
 llvm::Expected>
 parseCrossTUIndex(StringRef IndexPath) {
   std::ifstream ExternalMapFile{std::string(IndexPath)};
@@ -189,23 

[PATCH] D102669: [analyzer][ctu] Fix wrong 'multiple definitions' errors caused by space characters in lookup names when parsing the ctu index file

[Ella Ma via Phabricator via cfe-commits]

OikawaKirie added a comment.

Could you please revert this on my behalf? I currently have no idea to fix this 
problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102669

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

[PATCH] D115893: [WebAssembly] Support clang -fwasm-exceptions for bitcode

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp:37
 
-// Emscripten's asm.js-style exception handling
-cl::opt
-WasmEnableEmEH("enable-emscripten-cxx-exceptions",
-   cl::desc("WebAssembly Emscripten-style exception handling"),
-   cl::init(false));
-
-// Emscripten's asm.js-style setjmp/longjmp handling
-cl::opt WasmEnableEmSjLj(
-"enable-emscripten-sjlj",
-cl::desc("WebAssembly Emscripten-style setjmp/longjmp handling"),
-cl::init(false));
-
-// Exception handling using wasm EH instructions
-cl::opt WasmEnableEH("wasm-enable-eh",
-   cl::desc("WebAssembly exception handling"),
-   cl::init(false));
-
-// setjmp/longjmp handling using wasm EH instrutions
-cl::opt WasmEnableSjLj("wasm-enable-sjlj",
- cl::desc("WebAssembly setjmp/longjmp handling"),
- cl::init(false));
+extern cl::opt WasmEnableEmEH;   // asm.js-style EH
+extern cl::opt WasmEnableEmSjLj; // asm.js-style SjLJ

aheejin wrote:
> dschuff wrote:
> > We could put these declarations in WebAssemblyUtilities.h; then they 
> > wouldn't have to be duplicated here and in WebAssemblyMCAsmInfo.cpp
> Done. But as a result they are not within `WebAssembly` namespace, which I 
> think is actually better because we have been cluttering the `llvm` namespace 
> so far. And I needed to attach `WebAssembly::` to all the usages of these 
> variables in other places. In this file I just used `using WebAssembly::***`, 
> because there are too many usages of these options.
 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115893

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

[PATCH] D102669: [analyzer][ctu] Fix wrong 'multiple definitions' errors caused by space characters in lookup names when parsing the ctu index file

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

This breaks tests on macOS: http://45.33.8.238/macm1/23920/step_7.txt

Please take a look and revert for now if it takes a while to fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102669

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

[PATCH] D115021: [funcatts] Rewrite callsite operand handling in memory access inference

[Philip Reames via Phabricator via cfe-commits]

reames added a comment.

I'm starting to split this into individual pieces with the writeonly dependence 
removed.  The first one handles indirect calls, and is posted as D115916 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115021

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

[clang] 4625b84 - [WebAssembly] Support clang -fwasm-exceptions for bitcode

[Heejin Ahn via cfe-commits]

Author: Heejin Ahn
Date: 2021-12-16T16:49:24-08:00
New Revision: 4625b848793f8cfa4affac8d03b9f498b3e477fe

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

LOG: [WebAssembly] Support clang -fwasm-exceptions for bitcode

This supports bitcode compilation using `clang -fwasm-exceptions`.

---

The current situation:

Currently the backend requires two options for Wasm EH:
`-wasm-enable-eh` and `-exception-model=wasm`. Wasm SjLj requires two
options as well: `-wasm-enable-sjlj` and `-exception-model=wasm`. When
using Wasm EH via Emscripten, you only need to pass `-fwasm-exceptions`,
and these options will be added within the clang driver. This
description will focus on the case of Wasm EH going forward, but Wasm
SjLj's case is similar.

When you pass `-fwasm-exceptions` to emcc and clang driver, the clang
driver adds these options to the command line that calls the clang
frontend (`clang -cc1`): `-mllvm -wasm-enable-eh` and
`-exception-model=wasm`. `-wasm-enable-eh` is prefixed with `-mllvm`, so
it is passed as is to the backend. But `-exception-model` is parsed and
processed within the clang frontend and stored in `LangOptions` class.
This info is later transferred to `TargetOptions` class, and then
eventually passed to `MCAsmInfo` class. All LLVM code queries this
`MCAsmInfo` to get the exception model.

---

Problem:

The problem is the whole `LangOptions` processing is bypassed when
compiling bitcode, so the information transfer of `LangOptions` ->
`TargetOptions` -> `MCAsmInfo` does not happen. They are all set to
`ExceptionHandling::None`, which is the default value.

---

What other targets do, and why we can't do the same:

Other targets support bitcode compilation by the clang driver, but they
can do that by using different triples. For example, X86 target supports
multiple triples, each of which has its own subclass of `MCAsmInfo`, so
it can hardcode the appropriate exception model within those subclasses'
constructors. But we don't have separate triples for each exception
mode: none, emscripten, and wasm.

---

What this CL does:

If we can figure out whether `-wasm-enable-eh` is passed to the backend,
we can programatically set the exception model from the backend, rather
than requiring it to be passed.

So we check `WasmEnableEH` and `WasmEnableSjLj` variables, which are
`cl::opt` for `-wasm-enable-eh` and `-wasm-enable-sjlj`, in
`WebAssemblyMCAsmInfo` constructor, and if either of them is set, we set
`MCAsmInfo.ExceptionType` to Wasm. `TargetOptions` cannot be updated
there, so we make sure they are the same later.

Fixes https://github.com/emscripten-core/emscripten/issues/15712.

Reviewed By: dschuff

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

Added: 
clang/test/CodeGen/WebAssembly/wasm-eh.ll

Modified: 
llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCAsmInfo.cpp
llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.cpp
llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.h
llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
llvm/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp
llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp

Removed: 




diff  --git a/clang/test/CodeGen/WebAssembly/wasm-eh.ll 
b/clang/test/CodeGen/WebAssembly/wasm-eh.ll
new file mode 100644
index 0..de06887423183
--- /dev/null
+++ b/clang/test/CodeGen/WebAssembly/wasm-eh.ll
@@ -0,0 +1,38 @@
+; REQUIRES: webassembly-registered-target
+; RUN: %clang %s -target wasm32-unknown-unknown -fwasm-exceptions -c -S -o - | 
FileCheck %s
+
+; This tests whether clang driver can take -fwasm-exceptions and compile 
bitcode
+; files using Wasm EH.
+
+; CHECK-LABEL: test
+; CHECK: try
+; CHECK:   call foo
+; CHECK: catch __cpp_exception
+; CHECK: end
+define void @test() personality i8* bitcast (i32 (...)* 
@__gxx_wasm_personality_v0 to i8*) {
+entry:
+  invoke void @foo()
+  to label %try.cont unwind label %catch.dispatch
+
+catch.dispatch:   ; preds = %entry
+  %0 = catchswitch within none [label %catch.start] unwind to caller
+
+catch.start:  ; preds = %catch.dispatch
+  %1 = catchpad within %0 [i8* null]
+  %2 = call i8* @llvm.wasm.get.exception(token %1)
+  %3 = call i32 @llvm.wasm.get.ehselector(token %1)
+  %4 = call i8* @__cxa_begin_catch(i8* %2) #2 [ "funclet"(token %1) ]
+  call void @__cxa_end_catch() [ "funclet"(token %1) ]
+  catchret from %1 to label %try.cont
+
+try.cont: ; preds = %entry, 
%catch.start
+  ret void
+}
+
+declare void @foo()
+declare i32 @__gxx_wasm_personality_v0(...)
+declare i8* @llvm.wasm.get.exception(token)
+declare i32 

[PATCH] D115893: [WebAssembly] Support clang -fwasm-exceptions for bitcode

[Heejin Ahn 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 rG4625b848793f: [WebAssembly] Support clang -fwasm-exceptions 
for bitcode (authored by aheejin).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115893

Files:
  clang/test/CodeGen/WebAssembly/wasm-eh.ll
  llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCAsmInfo.cpp
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.cpp
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.h
  llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp

Index: llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
===
--- llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
@@ -14,6 +14,7 @@
 #include "WebAssemblyTargetMachine.h"
 #include "MCTargetDesc/WebAssemblyMCTargetDesc.h"
 #include "TargetInfo/WebAssemblyTargetInfo.h"
+#include "Utils/WebAssemblyUtilities.h"
 #include "WebAssembly.h"
 #include "WebAssemblyMachineFunctionInfo.h"
 #include "WebAssemblyTargetObjectFile.h"
@@ -24,6 +25,7 @@
 #include "llvm/CodeGen/RegAllocRegistry.h"
 #include "llvm/CodeGen/TargetPassConfig.h"
 #include "llvm/IR/Function.h"
+#include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/TargetRegistry.h"
 #include "llvm/Target/TargetOptions.h"
 #include "llvm/Transforms/Scalar.h"
@@ -33,28 +35,6 @@
 
 #define DEBUG_TYPE "wasm"
 
-// Emscripten's asm.js-style exception handling
-cl::opt
-WasmEnableEmEH("enable-emscripten-cxx-exceptions",
-   cl::desc("WebAssembly Emscripten-style exception handling"),
-   cl::init(false));
-
-// Emscripten's asm.js-style setjmp/longjmp handling
-cl::opt WasmEnableEmSjLj(
-"enable-emscripten-sjlj",
-cl::desc("WebAssembly Emscripten-style setjmp/longjmp handling"),
-cl::init(false));
-
-// Exception handling using wasm EH instructions
-cl::opt WasmEnableEH("wasm-enable-eh",
-   cl::desc("WebAssembly exception handling"),
-   cl::init(false));
-
-// setjmp/longjmp handling using wasm EH instrutions
-cl::opt WasmEnableSjLj("wasm-enable-sjlj",
- cl::desc("WebAssembly setjmp/longjmp handling"),
- cl::init(false));
-
 // A command-line option to keep implicit locals
 // for the purpose of testing with lit/llc ONLY.
 // This produces output which is not valid WebAssembly, and is not supported
@@ -368,7 +348,23 @@
   return nullptr; // No reg alloc
 }
 
-static void basicCheckForEHAndSjLj(const TargetMachine *TM) {
+using WebAssembly::WasmEnableEH;
+using WebAssembly::WasmEnableEmEH;
+using WebAssembly::WasmEnableEmSjLj;
+using WebAssembly::WasmEnableSjLj;
+
+static void basicCheckForEHAndSjLj(TargetMachine *TM) {
+  // Before checking, we make sure TargetOptions.ExceptionModel is the same as
+  // MCAsmInfo.ExceptionsType. Normally these have to be the same, because clang
+  // stores the exception model info in LangOptions, which is later transferred
+  // to TargetOptions and MCAsmInfo. But when clang compiles bitcode directly,
+  // clang's LangOptions is not used and thus the exception model info is not
+  // correctly transferred to TargetOptions and MCAsmInfo, so we make sure we
+  // have the correct exception model in in WebAssemblyMCAsmInfo constructor.
+  // But in this case TargetOptions is still not updated, so we make sure they
+  // are the same.
+  TM->Options.ExceptionModel = TM->getMCAsmInfo()->getExceptionHandlingType();
+
   // Basic Correctness checking related to -exception-model
   if (TM->Options.ExceptionModel != ExceptionHandling::None &&
   TM->Options.ExceptionModel != ExceptionHandling::Wasm)
Index: llvm/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp
===
--- llvm/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp
@@ -40,9 +40,6 @@
" instruction output for test purposes only."),
   cl::init(false));
 
-extern cl::opt WasmEnableEmEH;
-extern cl::opt WasmEnableEmSjLj;
-
 static void removeRegisterOperands(const MachineInstr *MI, MCInst );
 
 MCSymbol *
@@ -112,7 +109,8 @@
 
   bool InvokeDetected = false;
   auto *WasmSym = Printer.getMCSymbolForFunction(
-  F, WasmEnableEmEH || WasmEnableEmSjLj, Signature.get(), InvokeDetected);
+  F, WebAssembly::WasmEnableEmEH || WebAssembly::WasmEnableEmSjLj,
+  Signature.get(), InvokeDetected);
   WasmSym->setSignature(Signature.get());
   

[PATCH] D115893: [WebAssembly] Support clang -fwasm-exceptions for bitcode

[Heejin Ahn via Phabricator via cfe-commits]

aheejin updated this revision to Diff 395025.
aheejin added a comment.

Remove an unnecessary include


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115893

Files:
  clang/test/CodeGen/WebAssembly/wasm-eh.ll
  llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCAsmInfo.cpp
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.cpp
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.h
  llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp

Index: llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
===
--- llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
@@ -14,6 +14,7 @@
 #include "WebAssemblyTargetMachine.h"
 #include "MCTargetDesc/WebAssemblyMCTargetDesc.h"
 #include "TargetInfo/WebAssemblyTargetInfo.h"
+#include "Utils/WebAssemblyUtilities.h"
 #include "WebAssembly.h"
 #include "WebAssemblyMachineFunctionInfo.h"
 #include "WebAssemblyTargetObjectFile.h"
@@ -24,6 +25,7 @@
 #include "llvm/CodeGen/RegAllocRegistry.h"
 #include "llvm/CodeGen/TargetPassConfig.h"
 #include "llvm/IR/Function.h"
+#include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/TargetRegistry.h"
 #include "llvm/Target/TargetOptions.h"
 #include "llvm/Transforms/Scalar.h"
@@ -33,28 +35,6 @@
 
 #define DEBUG_TYPE "wasm"
 
-// Emscripten's asm.js-style exception handling
-cl::opt
-WasmEnableEmEH("enable-emscripten-cxx-exceptions",
-   cl::desc("WebAssembly Emscripten-style exception handling"),
-   cl::init(false));
-
-// Emscripten's asm.js-style setjmp/longjmp handling
-cl::opt WasmEnableEmSjLj(
-"enable-emscripten-sjlj",
-cl::desc("WebAssembly Emscripten-style setjmp/longjmp handling"),
-cl::init(false));
-
-// Exception handling using wasm EH instructions
-cl::opt WasmEnableEH("wasm-enable-eh",
-   cl::desc("WebAssembly exception handling"),
-   cl::init(false));
-
-// setjmp/longjmp handling using wasm EH instrutions
-cl::opt WasmEnableSjLj("wasm-enable-sjlj",
- cl::desc("WebAssembly setjmp/longjmp handling"),
- cl::init(false));
-
 // A command-line option to keep implicit locals
 // for the purpose of testing with lit/llc ONLY.
 // This produces output which is not valid WebAssembly, and is not supported
@@ -368,7 +348,23 @@
   return nullptr; // No reg alloc
 }
 
-static void basicCheckForEHAndSjLj(const TargetMachine *TM) {
+using WebAssembly::WasmEnableEH;
+using WebAssembly::WasmEnableEmEH;
+using WebAssembly::WasmEnableEmSjLj;
+using WebAssembly::WasmEnableSjLj;
+
+static void basicCheckForEHAndSjLj(TargetMachine *TM) {
+  // Before checking, we make sure TargetOptions.ExceptionModel is the same as
+  // MCAsmInfo.ExceptionsType. Normally these have to be the same, because clang
+  // stores the exception model info in LangOptions, which is later transferred
+  // to TargetOptions and MCAsmInfo. But when clang compiles bitcode directly,
+  // clang's LangOptions is not used and thus the exception model info is not
+  // correctly transferred to TargetOptions and MCAsmInfo, so we make sure we
+  // have the correct exception model in in WebAssemblyMCAsmInfo constructor.
+  // But in this case TargetOptions is still not updated, so we make sure they
+  // are the same.
+  TM->Options.ExceptionModel = TM->getMCAsmInfo()->getExceptionHandlingType();
+
   // Basic Correctness checking related to -exception-model
   if (TM->Options.ExceptionModel != ExceptionHandling::None &&
   TM->Options.ExceptionModel != ExceptionHandling::Wasm)
Index: llvm/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp
===
--- llvm/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp
@@ -40,9 +40,6 @@
" instruction output for test purposes only."),
   cl::init(false));
 
-extern cl::opt WasmEnableEmEH;
-extern cl::opt WasmEnableEmSjLj;
-
 static void removeRegisterOperands(const MachineInstr *MI, MCInst );
 
 MCSymbol *
@@ -112,7 +109,8 @@
 
   bool InvokeDetected = false;
   auto *WasmSym = Printer.getMCSymbolForFunction(
-  F, WasmEnableEmEH || WasmEnableEmSjLj, Signature.get(), InvokeDetected);
+  F, WebAssembly::WasmEnableEmEH || WebAssembly::WasmEnableEmSjLj,
+  Signature.get(), InvokeDetected);
   WasmSym->setSignature(Signature.get());
   Printer.addSignature(std::move(Signature));
   WasmSym->setType(wasm::WASM_SYMBOL_TYPE_FUNCTION);
Index: llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp

[PATCH] D115893: [WebAssembly] Support clang -fwasm-exceptions for bitcode

[Heejin Ahn via Phabricator via cfe-commits]

aheejin added inline comments.



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp:37
 
-// Emscripten's asm.js-style exception handling
-cl::opt
-WasmEnableEmEH("enable-emscripten-cxx-exceptions",
-   cl::desc("WebAssembly Emscripten-style exception handling"),
-   cl::init(false));
-
-// Emscripten's asm.js-style setjmp/longjmp handling
-cl::opt WasmEnableEmSjLj(
-"enable-emscripten-sjlj",
-cl::desc("WebAssembly Emscripten-style setjmp/longjmp handling"),
-cl::init(false));
-
-// Exception handling using wasm EH instructions
-cl::opt WasmEnableEH("wasm-enable-eh",
-   cl::desc("WebAssembly exception handling"),
-   cl::init(false));
-
-// setjmp/longjmp handling using wasm EH instrutions
-cl::opt WasmEnableSjLj("wasm-enable-sjlj",
- cl::desc("WebAssembly setjmp/longjmp handling"),
- cl::init(false));
+extern cl::opt WasmEnableEmEH;   // asm.js-style EH
+extern cl::opt WasmEnableEmSjLj; // asm.js-style SjLJ

dschuff wrote:
> We could put these declarations in WebAssemblyUtilities.h; then they wouldn't 
> have to be duplicated here and in WebAssemblyMCAsmInfo.cpp
Done. But as a result they are not within `WebAssembly` namespace, which I 
think is actually better because we have been cluttering the `llvm` namespace 
so far. And I needed to attach `WebAssembly::` to all the usages of these 
variables in other places. In this file I just used `using WebAssembly::***`, 
because there are too many usages of these options.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115893

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

[PATCH] D115893: [WebAssembly] Support clang -fwasm-exceptions for bitcode

[Heejin Ahn via Phabricator via cfe-commits]

aheejin updated this revision to Diff 395024.
aheejin marked 2 inline comments as done.
aheejin added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115893

Files:
  clang/test/CodeGen/WebAssembly/wasm-eh.ll
  llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCAsmInfo.cpp
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.cpp
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.h
  llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp

Index: llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
===
--- llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
@@ -14,6 +14,7 @@
 #include "WebAssemblyTargetMachine.h"
 #include "MCTargetDesc/WebAssemblyMCTargetDesc.h"
 #include "TargetInfo/WebAssemblyTargetInfo.h"
+#include "Utils/WebAssemblyUtilities.h"
 #include "WebAssembly.h"
 #include "WebAssemblyMachineFunctionInfo.h"
 #include "WebAssemblyTargetObjectFile.h"
@@ -24,6 +25,7 @@
 #include "llvm/CodeGen/RegAllocRegistry.h"
 #include "llvm/CodeGen/TargetPassConfig.h"
 #include "llvm/IR/Function.h"
+#include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/TargetRegistry.h"
 #include "llvm/Target/TargetOptions.h"
 #include "llvm/Transforms/Scalar.h"
@@ -33,28 +35,6 @@
 
 #define DEBUG_TYPE "wasm"
 
-// Emscripten's asm.js-style exception handling
-cl::opt
-WasmEnableEmEH("enable-emscripten-cxx-exceptions",
-   cl::desc("WebAssembly Emscripten-style exception handling"),
-   cl::init(false));
-
-// Emscripten's asm.js-style setjmp/longjmp handling
-cl::opt WasmEnableEmSjLj(
-"enable-emscripten-sjlj",
-cl::desc("WebAssembly Emscripten-style setjmp/longjmp handling"),
-cl::init(false));
-
-// Exception handling using wasm EH instructions
-cl::opt WasmEnableEH("wasm-enable-eh",
-   cl::desc("WebAssembly exception handling"),
-   cl::init(false));
-
-// setjmp/longjmp handling using wasm EH instrutions
-cl::opt WasmEnableSjLj("wasm-enable-sjlj",
- cl::desc("WebAssembly setjmp/longjmp handling"),
- cl::init(false));
-
 // A command-line option to keep implicit locals
 // for the purpose of testing with lit/llc ONLY.
 // This produces output which is not valid WebAssembly, and is not supported
@@ -368,7 +348,23 @@
   return nullptr; // No reg alloc
 }
 
-static void basicCheckForEHAndSjLj(const TargetMachine *TM) {
+using WebAssembly::WasmEnableEH;
+using WebAssembly::WasmEnableEmEH;
+using WebAssembly::WasmEnableEmSjLj;
+using WebAssembly::WasmEnableSjLj;
+
+static void basicCheckForEHAndSjLj(TargetMachine *TM) {
+  // Before checking, we make sure TargetOptions.ExceptionModel is the same as
+  // MCAsmInfo.ExceptionsType. Normally these have to be the same, because clang
+  // stores the exception model info in LangOptions, which is later transferred
+  // to TargetOptions and MCAsmInfo. But when clang compiles bitcode directly,
+  // clang's LangOptions is not used and thus the exception model info is not
+  // correctly transferred to TargetOptions and MCAsmInfo, so we make sure we
+  // have the correct exception model in in WebAssemblyMCAsmInfo constructor.
+  // But in this case TargetOptions is still not updated, so we make sure they
+  // are the same.
+  TM->Options.ExceptionModel = TM->getMCAsmInfo()->getExceptionHandlingType();
+
   // Basic Correctness checking related to -exception-model
   if (TM->Options.ExceptionModel != ExceptionHandling::None &&
   TM->Options.ExceptionModel != ExceptionHandling::Wasm)
Index: llvm/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp
===
--- llvm/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp
@@ -40,9 +40,6 @@
" instruction output for test purposes only."),
   cl::init(false));
 
-extern cl::opt WasmEnableEmEH;
-extern cl::opt WasmEnableEmSjLj;
-
 static void removeRegisterOperands(const MachineInstr *MI, MCInst );
 
 MCSymbol *
@@ -112,7 +109,8 @@
 
   bool InvokeDetected = false;
   auto *WasmSym = Printer.getMCSymbolForFunction(
-  F, WasmEnableEmEH || WasmEnableEmSjLj, Signature.get(), InvokeDetected);
+  F, WebAssembly::WasmEnableEmEH || WebAssembly::WasmEnableEmSjLj,
+  Signature.get(), InvokeDetected);
   WasmSym->setSignature(Signature.get());
   Printer.addSignature(std::move(Signature));
   WasmSym->setType(wasm::WASM_SYMBOL_TYPE_FUNCTION);
Index: 

[PATCH] D102669: [analyzer][ctu] Fix wrong 'multiple definitions' errors caused by space characters in lookup names when parsing the ctu index file

[Ella Ma via Phabricator via cfe-commits]

OikawaKirie added a comment.

I do not know how this error happens. Maybe we can currently revert this patch 
an have another try in the future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102669

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

[PATCH] D102669: [analyzer][ctu] Fix wrong 'multiple definitions' errors caused by space characters in lookup names when parsing the ctu index file

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

This commit seems to have caused a test to fail: 
https://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/26118/testReport/

Can you fix the failure or revert the patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102669

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

[PATCH] D114639: Raise the minimum Visual Studio version to VS2019

[Stella Stamenova via Phabricator via cfe-commits]

stella.stamenova added a comment.

In D114639#3192823 , 
@stella.stamenova wrote:

> In D114639#3191752 , @RKSimon wrote:
>
>> @rnk @stella.stamenova How long do you think it will be before you can 
>> update your buildbots to VS2019 or VS2022 please?
>
> The mlir buildbot was updated already. The lldb buildbot will be done this 
> week - there's a test change that needs to happen in lldb before we can 
> update the buildbot. I'll let you know once it's done.

I am declaring victory for both the mlir and lldb buildbots.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114639

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

[PATCH] D115893: [WebAssembly] Support clang -fwasm-exceptions for bitcode

[Derek Schuff via Phabricator via cfe-commits]

dschuff accepted this revision.
dschuff added inline comments.
This revision is now accepted and ready to land.



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp:37
 
-// Emscripten's asm.js-style exception handling
-cl::opt
-WasmEnableEmEH("enable-emscripten-cxx-exceptions",
-   cl::desc("WebAssembly Emscripten-style exception handling"),
-   cl::init(false));
-
-// Emscripten's asm.js-style setjmp/longjmp handling
-cl::opt WasmEnableEmSjLj(
-"enable-emscripten-sjlj",
-cl::desc("WebAssembly Emscripten-style setjmp/longjmp handling"),
-cl::init(false));
-
-// Exception handling using wasm EH instructions
-cl::opt WasmEnableEH("wasm-enable-eh",
-   cl::desc("WebAssembly exception handling"),
-   cl::init(false));
-
-// setjmp/longjmp handling using wasm EH instrutions
-cl::opt WasmEnableSjLj("wasm-enable-sjlj",
- cl::desc("WebAssembly setjmp/longjmp handling"),
- cl::init(false));
+extern cl::opt WasmEnableEmEH;   // asm.js-style EH
+extern cl::opt WasmEnableEmSjLj; // asm.js-style SjLJ

We could put these declarations in WebAssemblyUtilities.h; then they wouldn't 
have to be duplicated here and in WebAssemblyMCAsmInfo.cpp



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp:357
+  // Before checking, we make sure TargetOptions.ExceptionModel is the same as
+  // MCAsmInfo.ExceptionsType. Normally when these have to be the same, because
+  // clang stores the exception model info in LangOptions, which is later




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115893

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

[PATCH] D115410: [llvm][test] rewrite callbr to use i rather than X constraint NFC

[James Y Knight via Phabricator via cfe-commits]

jyknight added inline comments.



Comment at: llvm/test/tools/llvm-diff/callbr.ll:28-29
 entry:
-  callbr void asm sideeffect "", "X,X,~{dirflag},~{fpsr},~{flags}"(i8* 
blockaddress(@foo, %t_no), i8* blockaddress(@foo, %return))
+  callbr void asm sideeffect "", "i,i,~{dirflag},~{fpsr},~{flags}"(i8* 
blockaddress(@foo, %t_no), i8* blockaddress(@foo, %return))
   to label %asm.fallthrough [label %return, label %t_no]
 

nickdesaulniers wrote:
> pengfei wrote:
> > jyknight wrote:
> > > pengfei wrote:
> > > > If my above assumption is true, I think we can't replace the `X` with 
> > > > `i` here.
> > > > Besides, I'm confused on the indirect labels list. Are they the labels 
> > > > of `bar` or `foo`?
> > > I don't see a a problem with using "i" everywhere -- all blockaddress are 
> > > going to be immediate values no matter whether they're an indirect target 
> > > or not.
> > > 
> > > The indirect labels list is only referring to labels in the current 
> > > function.
> > > 
> > > This test is confusing, but, it is a test for llvm-diff, so that's okay 
> > > or maybe even intended. (It can't actually possibly jump to @bar:%return 
> > > or @bar:%t_no, because nothing ever gets the address of those labels. It 
> > > does get the similarly-named labels in @foo, but it can't jump to those 
> > > either, since they're in a different function.)
> > Thanks for the explanation. My point is the test3 above intended to use `X` 
> > to indicate the destination is not in the indirect labels list. For 
> > consistency, we should use `X` here too, since the @foo:%return etc. are 
> > not in the list either. Or we don't need to use `X` in test3.
> The "indirect destination list" for the `callbr` in `@bar` is the `[label 
> %return, label %t_no]`. Both operands have corresponding `blockaddress` 
> arguments. So they //should not// use `X` in this case.
I don't see why the correct constraint to use should be related at all to 
whether the blockaddress argument corresponds to a label in the indirect label 
list or not.

Using something other than "X" should probably always be preferred, since 
presumably the instruction you're emitting has requirements. (Unless of course 
you don't actually use the argument, or only use it in a comment, or something 
like that...in which case "X" is fine.)

But, FTR, in this test, the blockaddress is for a label in a //different// 
function ("@foo") than the function we're in ("@bar"), which is what pengfei 
was pointing out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115410

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

[PATCH] D115903: [clang-format] Extra spaces surrounding arrow in templated member call in variable decl

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:1682
Current.NestingLevel == 0 &&
-   !Current.Previous->is(tok::kw_operator)) {
+   !Current.Previous->isOneOf(tok::kw_operator, tok::identifier)) {
   // not auto operator->() -> xxx;

curdeius wrote:
> Won't it break lambdas with an identifier (e.g. macro) before the arrow? E.g.:
> ```
> auto lmbd = [] NOEXCEPT -> int {
>   return 0;
> };
> ```
it doesn't seem so, maybe one of the other rules is catching that

auto lmbd = [] NOEXCEPT -> int { return 0; };



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115903

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

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Paul Kirth via Phabricator via cfe-commits]

paulkirth created this revision.
Herald added subscribers: abrachet, ormris, dexonsmith, wenlei, phosek, 
hiraditya, mgorny.
paulkirth updated this revision to Diff 394995.
paulkirth added a comment.
paulkirth updated this revision to Diff 394997.
paulkirth added reviewers: phosek, mcgrathr, leonardchan, lebedev.ri.
paulkirth published this revision for review.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

[misexpect] Fix Typo


paulkirth added a comment.

Fix bad diff in arctool


paulkirth added a comment.

Ready for review


Reimplements MisExpect diagnostics to reconstruct its original checking
methodology only using MD_prof branch_weights metadata.

New checks rely on 2 invariants:

1. for frontend instrumentation, MD_prof branch_weights will always be 
populated before llvm.expect intrinsics are lowered.

2. for IR and sample profiling, llvm.expect intrinsics will always be lowered 
before branch_weights are populated from the IR profiles.

These invariants allow the checking to assume how the existing branch
weights are populated depending on the profiling method used, and emit
the correct diagnostics. If these invariants are ever invalidated, the
MisExpect related checks would need to be updated, potentially by
re-introducing MD_misexpect metadata, and ensuring it always will be
transformed the same way as branch_weights in other optimization passes.

Frontend based profiling is now enabled without using LLVM Args, by
introducing a new CodeGen option, and checking if the -Wmisexpect flag
has been passed on the command line.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115907

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Profile/Inputs/misexpect-branch.proftext
  clang/test/Profile/misexpect-branch-cold.c
  clang/test/Profile/misexpect-branch-unpredictable.c
  clang/test/Profile/misexpect-branch.c
  llvm/docs/MisExpect.rst
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/include/llvm/Transforms/Utils/MisExpect.h
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/Transforms/IPO/SampleProfile.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/lib/Transforms/Utils/CMakeLists.txt
  llvm/lib/Transforms/Utils/MisExpect.cpp
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/test/Transforms/PGOProfile/misexpect-branch-correct.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-stripped.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-unpredictable.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch-default.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch.ll

Index: llvm/test/Transforms/PGOProfile/misexpect-switch.ll
===
--- /dev/null
+++ llvm/test/Transforms/PGOProfile/misexpect-switch.ll
@@ -0,0 +1,290 @@
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch.proftext -o %t.profdata
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch-correct.proftext -o %t.c.profdata
+
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pgo-warn-misexpect 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pgo-warn-misexpect -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; New PM
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -S 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; RUN: opt < 

[PATCH] D115693: [Try2][InstrProf] Attach debug info to counters

[Ellis Hoag 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 rG58d9c1aec88d: [Try2][InstrProf] Attach debug info to 
counters (authored by ellis).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115693

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  compiler-rt/include/profile/InstrProfData.inc
  compiler-rt/lib/profile/InstrProfiling.c
  compiler-rt/lib/profile/InstrProfilingMerge.c
  compiler-rt/lib/profile/InstrProfilingWriter.c
  llvm/include/llvm/ProfileData/InstrProf.h
  llvm/include/llvm/ProfileData/InstrProfData.inc
  llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
  llvm/lib/ProfileData/InstrProf.cpp
  llvm/lib/ProfileData/InstrProfWriter.cpp
  llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/test/Instrumentation/InstrProfiling/debug-info-correlate.ll

Index: llvm/test/Instrumentation/InstrProfiling/debug-info-correlate.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/InstrProfiling/debug-info-correlate.ll
@@ -0,0 +1,68 @@
+; RUN: opt < %s -instrprof -debug-info-correlate -S > %t.ll
+; RUN: FileCheck < %t.ll --implicit-check-not "{{__llvm_prf_data|__llvm_prf_names}}" %s
+; RUN: %llc_dwarf -O0 -filetype=obj < %t.ll | llvm-dwarfdump - | FileCheck --implicit-check-not "{{DW_TAG|NULL}}" %s --check-prefix CHECK-DWARF
+
+; REQUIRES: system-linux
+
+@__profn_foo = private constant [3 x i8] c"foo"
+; CHECK:  @__profc_foo =
+; CHECK-SAME: !dbg ![[EXPR:[0-9]+]]
+
+; CHECK:  ![[EXPR]] = !DIGlobalVariableExpression(var: ![[GLOBAL:[0-9]+]]
+; CHECK:  ![[GLOBAL]] = {{.*}} !DIGlobalVariable(name: "__profc_foo"
+; CHECK-SAME: scope: ![[SCOPE:[0-9]+]]
+; CHECK-SAME: annotations: ![[ANNOTATIONS:[0-9]+]]
+; CHECK:  ![[SCOPE]] = {{.*}} !DISubprogram(name: "foo"
+; CHECK:  ![[ANNOTATIONS]] = !{![[NAME:[0-9]+]], ![[HASH:[0-9]+]], ![[COUNTERS:[0-9]+]]}
+; CHECK:  ![[NAME]] = !{!"Function Name", !"foo"}
+; CHECK:  ![[HASH]] = !{!"CFG Hash", i64 12345678}
+; CHECK:  ![[COUNTERS]] = !{!"Num Counters", i32 2}
+
+define void @_Z3foov() !dbg !12 {
+  call void @llvm.instrprof.increment(i8* getelementptr inbounds ([3 x i8], [3 x i8]* @__profn_foo, i32 0, i32 0), i64 12345678, i32 2, i32 0)
+  ret void
+}
+
+declare void @llvm.instrprof.increment(i8*, i64, i32, i32)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3, !4, !5, !6, !7, !8, !9, !10}
+!llvm.ident = !{!11}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 14.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "debug-info-correlate.cpp", directory: "")
+!2 = !{i32 7, !"Dwarf Version", i32 4}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{i32 1, !"wchar_size", i32 4}
+!5 = !{i32 1, !"branch-target-enforcement", i32 0}
+!6 = !{i32 1, !"sign-return-address", i32 0}
+!7 = !{i32 1, !"sign-return-address-all", i32 0}
+!8 = !{i32 1, !"sign-return-address-with-bkey", i32 0}
+!9 = !{i32 7, !"uwtable", i32 1}
+!10 = !{i32 7, !"frame-pointer", i32 1}
+!11 = !{!"clang version 14.0.0"}
+!12 = distinct !DISubprogram(name: "foo", linkageName: "_Z3foov", scope: !13, file: !13, line: 1, type: !14, scopeLine: 1, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !16)
+!13 = !DIFile(filename: "debug-info-correlate.cpp", directory: "")
+!14 = !DISubroutineType(types: !15)
+!15 = !{null}
+!16 = !{}
+
+; CHECK-DWARF: DW_TAG_compile_unit
+; CHECK-DWARF:   DW_TAG_subprogram
+; CHECK-DWARF: DW_AT_name	("foo")
+; CHECK-DWARF: DW_TAG_variable
+; CHECK-DWARF:   DW_AT_name	("__profc_foo")
+; CHECK-DWARF:   DW_AT_type	({{.*}} "Profile Data Type")
+; CHECK-DWARF:   DW_TAG_LLVM_annotation
+; CHECK-DWARF: DW_AT_name	("Function Name")
+; CHECK-DWARF: DW_AT_const_value	("foo")
+; CHECK-DWARF:   DW_TAG_LLVM_annotation
+; CHECK-DWARF: DW_AT_name	("CFG Hash")
+; CHECK-DWARF: DW_AT_const_value	(12345678)
+; CHECK-DWARF:   DW_TAG_LLVM_annotation
+; CHECK-DWARF: DW_AT_name	("Num Counters")
+; CHECK-DWARF: DW_AT_const_value	(2)
+; CHECK-DWARF:   NULL
+; CHECK-DWARF: NULL
+; CHECK-DWARF:   DW_TAG_unspecified_type
+; CHECK-DWARF:   NULL
Index: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
===
--- llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
+++ llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
@@ -291,6 +291,8 @@
 // Command line option to specify the name of the function for CFG dump
 // Defined in Analysis/BlockFrequencyInfo.cpp:  -view-bfi-func-name=
 extern cl::opt ViewBlockFreqFuncName;
+
+extern cl::opt 

[clang] 58d9c1a - [Try2][InstrProf] Attach debug info to counters

[Ellis Hoag via cfe-commits]

Author: Ellis Hoag
Date: 2021-12-16T14:20:30-08:00
New Revision: 58d9c1aec88d5d4c783643df057d87f6b0c9f693

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

LOG: [Try2][InstrProf] Attach debug info to counters

Add the llvm flag `-debug-info-correlate` to attach debug info to 
instrumentation counters so we can correlate raw profile data to their 
functions. Raw profiles are dumped as `.proflite` files. The next diff enables 
`llvm-profdata` to consume `.proflite` and debug info files to produce a normal 
`.profdata` profile.

Part of the "lightweight instrumentation" work: 
https://groups.google.com/g/llvm-dev/c/r03Z6JoN7d4

The original diff https://reviews.llvm.org/D114565 was reverted because of the 
`Instrumentation/InstrProfiling/debug-info-correlate.ll` test, which is fixed 
in this commit.

Reviewed By: kyulee

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

Added: 
llvm/test/Instrumentation/InstrProfiling/debug-info-correlate.ll

Modified: 
clang/lib/CodeGen/BackendUtil.cpp
compiler-rt/include/profile/InstrProfData.inc
compiler-rt/lib/profile/InstrProfiling.c
compiler-rt/lib/profile/InstrProfilingMerge.c
compiler-rt/lib/profile/InstrProfilingWriter.c
llvm/include/llvm/ProfileData/InstrProf.h
llvm/include/llvm/ProfileData/InstrProfData.inc
llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
llvm/lib/ProfileData/InstrProf.cpp
llvm/lib/ProfileData/InstrProfWriter.cpp
llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index 510f3911939cf..49a278f1a09fc 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -94,10 +94,16 @@ using namespace llvm;
   llvm::PassPluginLibraryInfo get##Ext##PluginInfo();
 #include "llvm/Support/Extension.def"
 
+namespace llvm {
+extern cl::opt DebugInfoCorrelate;
+}
+
 namespace {
 
 // Default filename used for profile generation.
-static constexpr StringLiteral DefaultProfileGenName = "default_%m.profraw";
+std::string getDefaultProfileGenName() {
+  return DebugInfoCorrelate ? "default_%m.proflite" : "default_%m.profraw";
+}
 
 class EmitAssemblyHelper {
   DiagnosticsEngine 
@@ -886,7 +892,7 @@ void EmitAssemblyHelper::CreatePasses(legacy::PassManager 
,
 if (!CodeGenOpts.InstrProfileOutput.empty())
   PMBuilder.PGOInstrGen = CodeGenOpts.InstrProfileOutput;
 else
-  PMBuilder.PGOInstrGen = std::string(DefaultProfileGenName);
+  PMBuilder.PGOInstrGen = getDefaultProfileGenName();
   }
   if (CodeGenOpts.hasProfileIRUse()) {
 PMBuilder.PGOInstrUse = CodeGenOpts.ProfileInstrumentUsePath;
@@ -1231,7 +1237,7 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
   if (CodeGenOpts.hasProfileIRInstr())
 // -fprofile-generate.
 PGOOpt = PGOOptions(CodeGenOpts.InstrProfileOutput.empty()
-? std::string(DefaultProfileGenName)
+? getDefaultProfileGenName()
 : CodeGenOpts.InstrProfileOutput,
 "", "", PGOOptions::IRInstr, PGOOptions::NoCSAction,
 CodeGenOpts.DebugInfoForProfiling);
@@ -1269,13 +1275,13 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
  "Cannot run CSProfileGen pass with ProfileGen or SampleUse "
  " pass");
   PGOOpt->CSProfileGenFile = CodeGenOpts.InstrProfileOutput.empty()
- ? std::string(DefaultProfileGenName)
+ ? getDefaultProfileGenName()
  : CodeGenOpts.InstrProfileOutput;
   PGOOpt->CSAction = PGOOptions::CSIRInstr;
 } else
   PGOOpt = PGOOptions("",
   CodeGenOpts.InstrProfileOutput.empty()
-  ? std::string(DefaultProfileGenName)
+  ? getDefaultProfileGenName()
   : CodeGenOpts.InstrProfileOutput,
   "", PGOOptions::NoAction, PGOOptions::CSIRInstr,
   CodeGenOpts.DebugInfoForProfiling);

diff  --git a/compiler-rt/include/profile/InstrProfData.inc 
b/compiler-rt/include/profile/InstrProfData.inc
index 008b8dde5820a..44719126b5965 100644
--- a/compiler-rt/include/profile/InstrProfData.inc
+++ b/compiler-rt/include/profile/InstrProfData.inc
@@ -653,15 +653,17 @@ serializeValueProfDataFrom(ValueProfRecordClosure 
*Closure,
 
 /* Profile version is always of type uint64_t. Reserve the upper 8 bits in the
  * version for other variants of profile. We set the lowest bit of the upper 8
- * bits (i.e. bit 56) to 1 to indicate if this is an 

[PATCH] D115903: [clang-format] Extra spaces surrounding arrow in templated member call in variable decl

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:1682
Current.NestingLevel == 0 &&
-   !Current.Previous->is(tok::kw_operator)) {
+   !Current.Previous->isOneOf(tok::kw_operator, tok::identifier)) {
   // not auto operator->() -> xxx;

Won't it break lambdas with an identifier (e.g. macro) before the arrow? E.g.:
```
auto lmbd = [] NOEXCEPT -> int {
  return 0;
};
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115903

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

[PATCH] D115903: [clang-format] Extra spaces surrounding arrow in templated member call in variable decl

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: curdeius, HazardyKnusperkeks, owenpan, lichray.
MyDeveloperDay added projects: clang, clang-format.
MyDeveloperDay requested review of this revision.

https://github.com/llvm/llvm-project/issues/43196

Fixes #43196

-> is incorrectly interpreted as a TrailingReturnArrow if we've seen an auto

  auto p = new A;
  auto x = p -> foo<1>();


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115903

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -6719,6 +6719,8 @@
 
   // Not trailing return types.
   verifyFormat("void f() { auto a = b->c(); }");
+  verifyFormat("auto a = p->foo();");
+  verifyFormat("int a = p->foo();");
 }
 
 TEST_F(FormatTest, DeductionGuides) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1679,7 +1679,7 @@
   Current.setType(TT_LambdaArrow);
 } else if (Current.is(tok::arrow) && AutoFound && Line.MustBeDeclaration &&
Current.NestingLevel == 0 &&
-   !Current.Previous->is(tok::kw_operator)) {
+   !Current.Previous->isOneOf(tok::kw_operator, tok::identifier)) {
   // not auto operator->() -> xxx;
   Current.setType(TT_TrailingReturnArrow);
 } else if (Current.is(tok::arrow) && Current.Previous &&


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -6719,6 +6719,8 @@
 
   // Not trailing return types.
   verifyFormat("void f() { auto a = b->c(); }");
+  verifyFormat("auto a = p->foo();");
+  verifyFormat("int a = p->foo();");
 }
 
 TEST_F(FormatTest, DeductionGuides) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1679,7 +1679,7 @@
   Current.setType(TT_LambdaArrow);
 } else if (Current.is(tok::arrow) && AutoFound && Line.MustBeDeclaration &&
Current.NestingLevel == 0 &&
-   !Current.Previous->is(tok::kw_operator)) {
+   !Current.Previous->isOneOf(tok::kw_operator, tok::identifier)) {
   // not auto operator->() -> xxx;
   Current.setType(TT_TrailingReturnArrow);
 } else if (Current.is(tok::arrow) && Current.Previous &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115693: [Try2][InstrProf] Attach debug info to counters

[Ellis Hoag via Phabricator via cfe-commits]

ellis updated this revision to Diff 394982.
ellis added a comment.

Use `std::string` instead of `Twine` to pass around the default raw profile 
name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115693

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  compiler-rt/include/profile/InstrProfData.inc
  compiler-rt/lib/profile/InstrProfiling.c
  compiler-rt/lib/profile/InstrProfilingMerge.c
  compiler-rt/lib/profile/InstrProfilingWriter.c
  llvm/include/llvm/ProfileData/InstrProf.h
  llvm/include/llvm/ProfileData/InstrProfData.inc
  llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
  llvm/lib/ProfileData/InstrProf.cpp
  llvm/lib/ProfileData/InstrProfWriter.cpp
  llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/test/Instrumentation/InstrProfiling/debug-info-correlate.ll

Index: llvm/test/Instrumentation/InstrProfiling/debug-info-correlate.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/InstrProfiling/debug-info-correlate.ll
@@ -0,0 +1,68 @@
+; RUN: opt < %s -instrprof -debug-info-correlate -S > %t.ll
+; RUN: FileCheck < %t.ll --implicit-check-not "{{__llvm_prf_data|__llvm_prf_names}}" %s
+; RUN: %llc_dwarf -O0 -filetype=obj < %t.ll | llvm-dwarfdump - | FileCheck --implicit-check-not "{{DW_TAG|NULL}}" %s --check-prefix CHECK-DWARF
+
+; REQUIRES: system-linux
+
+@__profn_foo = private constant [3 x i8] c"foo"
+; CHECK:  @__profc_foo =
+; CHECK-SAME: !dbg ![[EXPR:[0-9]+]]
+
+; CHECK:  ![[EXPR]] = !DIGlobalVariableExpression(var: ![[GLOBAL:[0-9]+]]
+; CHECK:  ![[GLOBAL]] = {{.*}} !DIGlobalVariable(name: "__profc_foo"
+; CHECK-SAME: scope: ![[SCOPE:[0-9]+]]
+; CHECK-SAME: annotations: ![[ANNOTATIONS:[0-9]+]]
+; CHECK:  ![[SCOPE]] = {{.*}} !DISubprogram(name: "foo"
+; CHECK:  ![[ANNOTATIONS]] = !{![[NAME:[0-9]+]], ![[HASH:[0-9]+]], ![[COUNTERS:[0-9]+]]}
+; CHECK:  ![[NAME]] = !{!"Function Name", !"foo"}
+; CHECK:  ![[HASH]] = !{!"CFG Hash", i64 12345678}
+; CHECK:  ![[COUNTERS]] = !{!"Num Counters", i32 2}
+
+define void @_Z3foov() !dbg !12 {
+  call void @llvm.instrprof.increment(i8* getelementptr inbounds ([3 x i8], [3 x i8]* @__profn_foo, i32 0, i32 0), i64 12345678, i32 2, i32 0)
+  ret void
+}
+
+declare void @llvm.instrprof.increment(i8*, i64, i32, i32)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3, !4, !5, !6, !7, !8, !9, !10}
+!llvm.ident = !{!11}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 14.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "debug-info-correlate.cpp", directory: "")
+!2 = !{i32 7, !"Dwarf Version", i32 4}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{i32 1, !"wchar_size", i32 4}
+!5 = !{i32 1, !"branch-target-enforcement", i32 0}
+!6 = !{i32 1, !"sign-return-address", i32 0}
+!7 = !{i32 1, !"sign-return-address-all", i32 0}
+!8 = !{i32 1, !"sign-return-address-with-bkey", i32 0}
+!9 = !{i32 7, !"uwtable", i32 1}
+!10 = !{i32 7, !"frame-pointer", i32 1}
+!11 = !{!"clang version 14.0.0"}
+!12 = distinct !DISubprogram(name: "foo", linkageName: "_Z3foov", scope: !13, file: !13, line: 1, type: !14, scopeLine: 1, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !16)
+!13 = !DIFile(filename: "debug-info-correlate.cpp", directory: "")
+!14 = !DISubroutineType(types: !15)
+!15 = !{null}
+!16 = !{}
+
+; CHECK-DWARF: DW_TAG_compile_unit
+; CHECK-DWARF:   DW_TAG_subprogram
+; CHECK-DWARF: DW_AT_name	("foo")
+; CHECK-DWARF: DW_TAG_variable
+; CHECK-DWARF:   DW_AT_name	("__profc_foo")
+; CHECK-DWARF:   DW_AT_type	({{.*}} "Profile Data Type")
+; CHECK-DWARF:   DW_TAG_LLVM_annotation
+; CHECK-DWARF: DW_AT_name	("Function Name")
+; CHECK-DWARF: DW_AT_const_value	("foo")
+; CHECK-DWARF:   DW_TAG_LLVM_annotation
+; CHECK-DWARF: DW_AT_name	("CFG Hash")
+; CHECK-DWARF: DW_AT_const_value	(12345678)
+; CHECK-DWARF:   DW_TAG_LLVM_annotation
+; CHECK-DWARF: DW_AT_name	("Num Counters")
+; CHECK-DWARF: DW_AT_const_value	(2)
+; CHECK-DWARF:   NULL
+; CHECK-DWARF: NULL
+; CHECK-DWARF:   DW_TAG_unspecified_type
+; CHECK-DWARF:   NULL
Index: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
===
--- llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
+++ llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
@@ -291,6 +291,8 @@
 // Command line option to specify the name of the function for CFG dump
 // Defined in Analysis/BlockFrequencyInfo.cpp:  -view-bfi-func-name=
 extern cl::opt ViewBlockFreqFuncName;
+
+extern cl::opt DebugInfoCorrelate;
 } // namespace llvm
 
 static cl::opt
@@ -467,8 +469,9 @@
 

[PATCH] D115901: [OpenMP][NFC] update status for 5.1 'fail' atomic extension

[Chi Chun Chen via Phabricator via cfe-commits]

cchen accepted this revision.
cchen 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/D115901/new/

https://reviews.llvm.org/D115901

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

[PATCH] D115879: [clang-format] extern with new line brace without indentation

[Marek Kurdej via Phabricator via cfe-commits]

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



Comment at: clang/unittests/Format/FormatTest.cpp:3815
+
+  Style.IndentExternBlock = FormatStyle::IEBS_AfterExternBlock;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;

This line is unnecessary and misleading (copied from above?), as 
`IndentExternBlock` is overridden at line 3818. Please delete.



Comment at: clang/unittests/Format/FormatTest.cpp:3819
+  Style.IndentExternBlock = FormatStyle::IEBS_NoIndent;
+  verifyFormat("extern \"C\"\n{ /*13*/\n}", Style);
+  verifyFormat("extern \"C\"\n{\n"

I'd prefer that you split strings after `\n`. I find it easier to read that way.
The same below.



Comment at: clang/unittests/Format/FormatTest.cpp:3825
+
+  Style.IndentExternBlock = FormatStyle::IEBS_AfterExternBlock;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;

Ditto. Remove.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115879

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

[PATCH] D115103: Leak Sanitizer port to Windows

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added inline comments.



Comment at: compiler-rt/lib/lsan/lsan_allocator.h:52-53
 
-#if defined(__mips64) || defined(__aarch64__) || defined(__i386__) || \
-defined(__arm__) || SANITIZER_RISCV64 || defined(__hexagon__)
+#if defined(__mips64) || defined(__aarch64__) || defined(_M_ARM64) || \
+defined(__i386__) || defined(_M_IX86) || defined(__arm__) ||  \
+defined(_M_ARM) || SANITIZER_RISCV64 || defined(__hexagon__)





Comment at: compiler-rt/lib/lsan/lsan_allocator.h:69
 using PrimaryAllocator = PrimaryAllocatorASVT;
-#elif defined(__x86_64__) || defined(__powerpc64__) || defined(__s390x__)
-# if SANITIZER_FUCHSIA
+#elif defined(__x86_64__) || defined(_M_X64) || defined(__powerpc64__) || \
+defined(__s390x__)





Comment at: compiler-rt/lib/lsan/lsan_common.cpp:248-253
+#  if defined(__x86_64__) || defined(_M_X64)
   // Accept only canonical form user-space addresses.
   return ((p >> 47) == 0);
 #  elif defined(__mips64)
   return ((p >> 40) == 0);
+#  elif defined(__aarch64__) || defined(_M_ARM64)

same


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

https://reviews.llvm.org/D115103

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

[PATCH] D72326: [clang-format] Rebased on master: Add option to specify explicit config file

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/Format.cpp:3231
+return Style;
+  } else if (ParserErrorCode != ParseError::Success) {
+return make_string_error("Error reading " + ConfigFile + ": " +

else after return

But I prefer a `switch` on `ParseErrorCode`.



Comment at: clang/lib/Format/Format.cpp:3273
 
+  llvm::SmallVector FilesToLookFor;
+  // User provided clang-format file using -style=file:/path/to/format/file

Why move that, it it's not used here?



Comment at: clang/lib/Format/Format.cpp:2743
+bool IsSuitable = true;
+auto Style = LoadConfigFile(StyleNameFile, FS, );
+if (Style && !IsSuitable) {

MyDeveloperDay wrote:
> pass Style in and rename return value
Why did you not follow your own comment?


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

https://reviews.llvm.org/D72326

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

[PATCH] D115902: [OPENMP]Look through member function call base during implicit DSA analysis.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev created this revision.
ABataev added reviewers: jdoerfert, mikerice, ddpagan.
Herald added subscribers: guansong, yaxunl.
ABataev requested review of this revision.
Herald added a subscriber: sstefan1.
Herald added a project: clang.

Need to look through the base of the member function calls at the DSA
analysis stage to correctly capture implicit class instances.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115902

Files:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/task_member_call_codegen.cpp
  clang/test/OpenMP/taskloop_codegen.cpp

Index: clang/test/OpenMP/taskloop_codegen.cpp
===
--- clang/test/OpenMP/taskloop_codegen.cpp
+++ clang/test/OpenMP/taskloop_codegen.cpp
@@ -238,8 +238,8 @@
 // CHECK-LABEL: taskloop_with_class
 void taskloop_with_class() {
   St s1;
-  // CHECK: [[TD:%.+]] = call i8* @__kmpc_omp_task_alloc(%struct.ident_t* @{{.+}}, i32 [[GTID:%.+]], i32 1, i64 80, i64 8, i32 (i32, i8*)* bitcast (i32 (i32, [[TD_TYPE:%.+]]*)* @{{.+}} to i32 (i32, i8*)*))
-  // CHECK: call void @__kmpc_taskloop(%struct.ident_t* @{{.+}}, i32 [[GTID]], i8* [[TD]], i32 1, i64* %{{.+}}, i64* %{{.+}}, i64 %{{.+}}, i32 1, i32 0, i64 0, i8* null)
+  // CHECK: [[TD:%.+]] = call i8* @__kmpc_omp_task_alloc(%struct.ident_t* @{{.+}}, i32 [[GTID:%.+]], i32 1, i64 88, i64 8, i32 (i32, i8*)* bitcast (i32 (i32, [[TD_TYPE:%.+]]*)* @{{.+}} to i32 (i32, i8*)*))
+  // CHECK: call void @__kmpc_taskloop(%struct.ident_t* @{{.+}}, i32 [[GTID]], i8* [[TD]], i32 1, i64* %{{.+}}, i64* %{{.+}}, i64 %{{.+}}, i32 1, i32 0, i64 0, i8* bitcast (void ([[TD_TYPE]]*, [[TD_TYPE]]*, i32)* @{{.+}} to i8*))
 #pragma omp taskloop
   for (St s = St(); s < s1; s += 1) {
   }
Index: clang/test/OpenMP/task_member_call_codegen.cpp
===
--- /dev/null
+++ clang/test/OpenMP/task_member_call_codegen.cpp
@@ -0,0 +1,319 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --function-signature --include-generated-funcs --replace-value-regex "__omp_offloading_[0-9a-z]+_[0-9a-z]+" "reduction_size[.].+[.]" "pl_cond[.].+[.|,]" --prefix-filecheck-ir-name _
+// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -x c++ -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK1
+// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-apple-darwin10 -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-apple-darwin10 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s --check-prefix=CHECK2
+// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -fopenmp-enable-irbuilder -x c++ -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK3
+// RUN: %clang_cc1 -fopenmp -fopenmp-enable-irbuilder -x c++ -triple x86_64-apple-darwin10 -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-enable-irbuilder -x c++ -triple x86_64-apple-darwin10 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s --check-prefix=CHECK4
+
+// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp-simd -x c++ -emit-llvm %s -o - | FileCheck %s --implicit-check-not="{{__kmpc|__tgt}}"
+// RUN: %clang_cc1 -fopenmp-simd -x c++ -triple x86_64-apple-darwin10 -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -x c++ -triple x86_64-apple-darwin10 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s --implicit-check-not="{{__kmpc|__tgt}}"
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+
+class a {
+public:
+  void b();
+};
+void c() {
+  a d;
+#pragma omp task
+  d.b();
+}
+#endif
+// CHECK1-LABEL: define {{[^@]+}}@_Z1cv
+// CHECK1-SAME: () #[[ATTR0:[0-9]+]] {
+// CHECK1-NEXT:  entry:
+// CHECK1-NEXT:[[D:%.*]] = alloca [[CLASS_A:%.*]], align 1
+// CHECK1-NEXT:[[AGG_CAPTURED:%.*]] = alloca [[STRUCT_ANON:%.*]], align 1
+// CHECK1-NEXT:[[TMP0:%.*]] = call i32 @__kmpc_global_thread_num(%struct.ident_t* @[[GLOB1:[0-9]+]])
+// CHECK1-NEXT:[[TMP1:%.*]] = call i8* @__kmpc_omp_task_alloc(%struct.ident_t* @[[GLOB1]], i32 [[TMP0]], i32 1, i64 48, i64 1, i32 (i32, i8*)* bitcast (i32 (i32, %struct.kmp_task_t_with_privates*)* @.omp_task_entry. to i32 (i32, i8*)*))
+// CHECK1-NEXT:[[TMP2:%.*]] = bitcast i8* [[TMP1]] to %struct.kmp_task_t_with_privates*
+// CHECK1-NEXT:[[TMP3:%.*]] = getelementptr inbounds [[STRUCT_KMP_TASK_T_WITH_PRIVATES:%.*]], %struct.kmp_task_t_with_privates* [[TMP2]], i32 0, i32 0
+// CHECK1-NEXT:[[TMP4:%.*]] = getelementptr inbounds [[STRUCT_KMP_TASK_T_WITH_PRIVATES]], %struct.kmp_task_t_with_privates* [[TMP2]], i32 0, i32 1
+// CHECK1-NEXT:[[TMP5:%.*]] = getelementptr inbounds [[STRUCT__KMP_PRIVATES_T:%.*]], %struct..kmp_privates.t* [[TMP4]], i32 0, i32 0
+// CHECK1-NEXT:[[TMP6:%.*]] = call i32 @__kmpc_omp_task(%struct.ident_t* @[[GLOB1]], i32 [[TMP0]], i8* [[TMP1]])
+// CHECK1-NEXT:ret void
+//
+//
+// CHECK1-LABEL: define {{[^@]+}}@.omp_task_privates_map.
+// CHECK1-SAME: 

[PATCH] D115901: [OpenMP][NFC] update status for 5.1 'fail' atomic extension

[Deepak Eachempati via Phabricator via cfe-commits]

dreachem created this revision.
dreachem added reviewers: jdoerfert, koops, cchen.
Herald added subscribers: guansong, yaxunl.
dreachem requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

Update status for the atomic 'fail' clause to "worked on".


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115901

Files:
  clang/docs/OpenMPSupport.rst


Index: clang/docs/OpenMPSupport.rst
===
--- clang/docs/OpenMPSupport.rst
+++ clang/docs/OpenMPSupport.rst
@@ -266,7 +266,7 @@
 
+==+==+==+===+
 | atomic extension | 'compare' clause on atomic construct  
   | :good:`worked on`| 
  |
 
+--+--+--+---+
-| atomic extension | 'fail' clause on atomic construct 
   | :none:`unclaimed`| 
  |
+| atomic extension | 'fail' clause on atomic construct 
   | :part:`worked on`| 
  |
 
+--+--+--+---+
 | base language| C++ attribute specifier syntax
   | :good:`done` | D105648 
  |
 
+--+--+--+---+


Index: clang/docs/OpenMPSupport.rst
===
--- clang/docs/OpenMPSupport.rst
+++ clang/docs/OpenMPSupport.rst
@@ -266,7 +266,7 @@
 +==+==+==+===+
 | atomic extension | 'compare' clause on atomic construct | :good:`worked on`|   |
 +--+--+--+---+
-| atomic extension | 'fail' clause on atomic construct| :none:`unclaimed`|   |
+| atomic extension | 'fail' clause on atomic construct| :part:`worked on`|   |
 +--+--+--+---+
 | base language| C++ attribute specifier syntax   | :good:`done` | D105648   |
 +--+--+--+---+
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111100: enable plugins for clang-tidy

[Jameson Nash via Phabricator via cfe-commits]

vtjnash added a comment.

Thanks! I will work on making those changes




Comment at: clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp:24-25
+
+  //void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  //void check(const ast_matchers::MatchFinder::MatchResult ) override;
+

aaron.ballman wrote:
> Shouldn't these functions be overloaded? We don't need it to be particularly 
> functional, but the plugin should demonstrate that it works and can be run by 
> clang-tidy (not just loaded and listed as a check).
I figured that is guaranteed by the C++ linker, if it can successfully list the 
check, so I didn't think it seemed essential to test for that also. I put these 
here mostly just to help anyone copying the file to getting started with 
adopting it to their use case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

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

[PATCH] D112881: [clang-tidy] Allow disabling integer to floating-point narrowing conversions for cppcoreguidelines-narrowing-conversions

[Paul Altin via Phabricator via cfe-commits]

paulaltin added a comment.

> Thanks! I landed this as 9198d04c06b561cd78d9407cedd50f7b995ee6d8 
>  this 
> morning (sorry for the slight delay in getting this landed).

No worries, thanks @aaron.ballman!


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

https://reviews.llvm.org/D112881

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

[PATCH] D115865: [clang-format] add support for branch attribute macros

[MyDeveloperDay 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 rG2b671c3fe0d6: [clang-format] add support for branch 
attribute macros (authored by MyDeveloperDay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115865

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -22274,6 +22274,42 @@
"  return 29;\n"
"}",
Style);
+
+  verifyFormat("if (argc > 5) [[unlikely]]\n"
+   "  return 29;\n",
+   Style);
+  verifyFormat("if (argc > 5) [[likely]]\n"
+   "  return 29;\n",
+   Style);
+
+  Style.AttributeMacros.push_back("UNLIKELY");
+  Style.AttributeMacros.push_back("LIKELY");
+  verifyFormat("if (argc > 5) UNLIKELY\n"
+   "  return 29;\n",
+   Style);
+
+  verifyFormat("if (argc > 5) UNLIKELY {\n"
+   "  return 29;\n"
+   "}",
+   Style);
+  verifyFormat("if (argc > 5) UNLIKELY {\n"
+   "  return 29;\n"
+   "} else [[likely]] {\n"
+   "  return 42;\n"
+   "}\n",
+   Style);
+  verifyFormat("if (argc > 5) UNLIKELY {\n"
+   "  return 29;\n"
+   "} else LIKELY {\n"
+   "  return 42;\n"
+   "}\n",
+   Style);
+  verifyFormat("if (argc > 5) [[unlikely]] {\n"
+   "  return 29;\n"
+   "} else LIKELY {\n"
+   "  return 42;\n"
+   "}\n",
+   Style);
 }
 
 TEST_F(FormatTest, PenaltyIndentedWhitespace) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2163,6 +2163,9 @@
 nextToken();
   if (FormatTok->Tok.is(tok::l_paren))
 parseParens();
+  // handle  AttributeMacro  if (x) UNLIKELY
+  if (FormatTok->is(TT_AttributeMacro))
+nextToken();
   // handle [[likely]] / [[unlikely]]
   if (FormatTok->is(tok::l_square) && tryToParseSimpleAttribute())
 parseSquare();
@@ -2182,6 +2185,9 @@
   }
   if (FormatTok->Tok.is(tok::kw_else)) {
 nextToken();
+// handle  AttributeMacro  else UNLIKELY
+if (FormatTok->is(TT_AttributeMacro))
+  nextToken();
 // handle [[likely]] / [[unlikely]]
 if (FormatTok->Tok.is(tok::l_square) && tryToParseSimpleAttribute())
   parseSquare();


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -22274,6 +22274,42 @@
"  return 29;\n"
"}",
Style);
+
+  verifyFormat("if (argc > 5) [[unlikely]]\n"
+   "  return 29;\n",
+   Style);
+  verifyFormat("if (argc > 5) [[likely]]\n"
+   "  return 29;\n",
+   Style);
+
+  Style.AttributeMacros.push_back("UNLIKELY");
+  Style.AttributeMacros.push_back("LIKELY");
+  verifyFormat("if (argc > 5) UNLIKELY\n"
+   "  return 29;\n",
+   Style);
+
+  verifyFormat("if (argc > 5) UNLIKELY {\n"
+   "  return 29;\n"
+   "}",
+   Style);
+  verifyFormat("if (argc > 5) UNLIKELY {\n"
+   "  return 29;\n"
+   "} else [[likely]] {\n"
+   "  return 42;\n"
+   "}\n",
+   Style);
+  verifyFormat("if (argc > 5) UNLIKELY {\n"
+   "  return 29;\n"
+   "} else LIKELY {\n"
+   "  return 42;\n"
+   "}\n",
+   Style);
+  verifyFormat("if (argc > 5) [[unlikely]] {\n"
+   "  return 29;\n"
+   "} else LIKELY {\n"
+   "  return 42;\n"
+   "}\n",
+   Style);
 }
 
 TEST_F(FormatTest, PenaltyIndentedWhitespace) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2163,6 +2163,9 @@
 nextToken();
   if (FormatTok->Tok.is(tok::l_paren))
 parseParens();
+  // handle  AttributeMacro  if (x) UNLIKELY
+  if (FormatTok->is(TT_AttributeMacro))
+nextToken();
   // handle [[likely]] / [[unlikely]]
   if (FormatTok->is(tok::l_square) && tryToParseSimpleAttribute())
 parseSquare();
@@ -2182,6 +2185,9 @@
   }
   if (FormatTok->Tok.is(tok::kw_else)) {
 nextToken();
+// handle  AttributeMacro  else UNLIKELY
+if 

[clang] 2b671c3 - [clang-format] add support for branch attribute macros

[via cfe-commits]

Author: mydeveloperday
Date: 2021-12-16T20:36:25Z
New Revision: 2b671c3fe0d6f6517dc5396e73d28285d6fe6223

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

LOG: [clang-format] add support for branch attribute macros

https://github.com/llvm/llvm-project/issues/49184

clang-format doesn't handle the use of AttributeMacros where `[[unlikely]]` / 
`[[likely]]` could be used in `if` statements

This was not covered in the original commit {{D80144}}

Fixes #49184

Reviewed By: curdeius, owenpan

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

Added: 


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

Removed: 




diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp 
b/clang/lib/Format/UnwrappedLineParser.cpp
index 2f27bf912fa2f..78405f23e4003 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -2163,6 +2163,9 @@ void UnwrappedLineParser::parseIfThenElse() {
 nextToken();
   if (FormatTok->Tok.is(tok::l_paren))
 parseParens();
+  // handle  AttributeMacro  if (x) UNLIKELY
+  if (FormatTok->is(TT_AttributeMacro))
+nextToken();
   // handle [[likely]] / [[unlikely]]
   if (FormatTok->is(tok::l_square) && tryToParseSimpleAttribute())
 parseSquare();
@@ -2182,6 +2185,9 @@ void UnwrappedLineParser::parseIfThenElse() {
   }
   if (FormatTok->Tok.is(tok::kw_else)) {
 nextToken();
+// handle  AttributeMacro  else UNLIKELY
+if (FormatTok->is(TT_AttributeMacro))
+  nextToken();
 // handle [[likely]] / [[unlikely]]
 if (FormatTok->Tok.is(tok::l_square) && tryToParseSimpleAttribute())
   parseSquare();

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 37d08d2d91292..2eb10609646a6 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -22274,6 +22274,42 @@ TEST_F(FormatTest, LikelyUnlikely) {
"  return 29;\n"
"}",
Style);
+
+  verifyFormat("if (argc > 5) [[unlikely]]\n"
+   "  return 29;\n",
+   Style);
+  verifyFormat("if (argc > 5) [[likely]]\n"
+   "  return 29;\n",
+   Style);
+
+  Style.AttributeMacros.push_back("UNLIKELY");
+  Style.AttributeMacros.push_back("LIKELY");
+  verifyFormat("if (argc > 5) UNLIKELY\n"
+   "  return 29;\n",
+   Style);
+
+  verifyFormat("if (argc > 5) UNLIKELY {\n"
+   "  return 29;\n"
+   "}",
+   Style);
+  verifyFormat("if (argc > 5) UNLIKELY {\n"
+   "  return 29;\n"
+   "} else [[likely]] {\n"
+   "  return 42;\n"
+   "}\n",
+   Style);
+  verifyFormat("if (argc > 5) UNLIKELY {\n"
+   "  return 29;\n"
+   "} else LIKELY {\n"
+   "  return 42;\n"
+   "}\n",
+   Style);
+  verifyFormat("if (argc > 5) [[unlikely]] {\n"
+   "  return 29;\n"
+   "} else LIKELY {\n"
+   "  return 42;\n"
+   "}\n",
+   Style);
 }
 
 TEST_F(FormatTest, PenaltyIndentedWhitespace) {



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

[PATCH] D115456: Implement on-demand TLS initialization for Microsoft CXX ABI

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

Is this a new feature in MSVC? Seems like it might 

 be. If so, should it be predicated on `isCompatibleWithMSVC(1925)` or 
something?


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

https://reviews.llvm.org/D115456

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

[PATCH] D115792: [Modules] Incorrect ODR detection for unresolved using type

[John McCall via Phabricator via cfe-commits]

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

LGTM, thanks.


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

https://reviews.llvm.org/D115792

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

[PATCH] D115893: [WebAssembly] Support clang -fwasm-exceptions for bitcode

[Heejin Ahn via Phabricator via cfe-commits]

aheejin updated this revision to Diff 394951.
aheejin added a comment.

Revert a function name change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115893

Files:
  clang/test/CodeGen/WebAssembly/wasm-eh.ll
  llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCAsmInfo.cpp
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp

Index: llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
===
--- llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
@@ -24,6 +24,7 @@
 #include "llvm/CodeGen/RegAllocRegistry.h"
 #include "llvm/CodeGen/TargetPassConfig.h"
 #include "llvm/IR/Function.h"
+#include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/TargetRegistry.h"
 #include "llvm/Target/TargetOptions.h"
 #include "llvm/Transforms/Scalar.h"
@@ -33,27 +34,10 @@
 
 #define DEBUG_TYPE "wasm"
 
-// Emscripten's asm.js-style exception handling
-cl::opt
-WasmEnableEmEH("enable-emscripten-cxx-exceptions",
-   cl::desc("WebAssembly Emscripten-style exception handling"),
-   cl::init(false));
-
-// Emscripten's asm.js-style setjmp/longjmp handling
-cl::opt WasmEnableEmSjLj(
-"enable-emscripten-sjlj",
-cl::desc("WebAssembly Emscripten-style setjmp/longjmp handling"),
-cl::init(false));
-
-// Exception handling using wasm EH instructions
-cl::opt WasmEnableEH("wasm-enable-eh",
-   cl::desc("WebAssembly exception handling"),
-   cl::init(false));
-
-// setjmp/longjmp handling using wasm EH instrutions
-cl::opt WasmEnableSjLj("wasm-enable-sjlj",
- cl::desc("WebAssembly setjmp/longjmp handling"),
- cl::init(false));
+extern cl::opt WasmEnableEmEH;   // asm.js-style EH
+extern cl::opt WasmEnableEmSjLj; // asm.js-style SjLJ
+extern cl::opt WasmEnableEH; // EH using Wasm EH instructions
+extern cl::opt WasmEnableSjLj;   // SjLj using Wasm EH instructions
 
 // A command-line option to keep implicit locals
 // for the purpose of testing with lit/llc ONLY.
@@ -368,7 +352,18 @@
   return nullptr; // No reg alloc
 }
 
-static void basicCheckForEHAndSjLj(const TargetMachine *TM) {
+static void basicCheckForEHAndSjLj(TargetMachine *TM) {
+  // Before checking, we make sure TargetOptions.ExceptionModel is the same as
+  // MCAsmInfo.ExceptionsType. Normally when these have to be the same, because
+  // clang stores the exception model info in LangOptions, which is later
+  // transferred to TargetOptions and MCAsmInfo. But when clang compiles bitcode
+  // directly, clang's LangOptions is not used and thus the exception model info
+  // is not correctly transferred to TargetOptions and MCAsmInfo, so we make
+  // sure we have the correct exception model in in WebAssemblyMCAsmInfo
+  // constructor. But in this case TargetOptions is still not updated, so we
+  // make sure they are the same.
+  TM->Options.ExceptionModel = TM->getMCAsmInfo()->getExceptionHandlingType();
+
   // Basic Correctness checking related to -exception-model
   if (TM->Options.ExceptionModel != ExceptionHandling::None &&
   TM->Options.ExceptionModel != ExceptionHandling::Wasm)
Index: llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.cpp
===
--- llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.cpp
+++ llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.cpp
@@ -18,6 +18,29 @@
 #include "llvm/MC/MCContext.h"
 using namespace llvm;
 
+// Exception handling & setjmp-longjmp handling related options. These are
+// defined here to be shared between WebAssembly and its subdirectories.
+
+// Emscripten's asm.js-style exception handling
+cl::opt
+WasmEnableEmEH("enable-emscripten-cxx-exceptions",
+   cl::desc("WebAssembly Emscripten-style exception handling"),
+   cl::init(false));
+// Emscripten's asm.js-style setjmp/longjmp handling
+cl::opt WasmEnableEmSjLj(
+"enable-emscripten-sjlj",
+cl::desc("WebAssembly Emscripten-style setjmp/longjmp handling"),
+cl::init(false));
+// Exception handling using wasm EH instructions
+cl::opt WasmEnableEH("wasm-enable-eh",
+   cl::desc("WebAssembly exception handling"),
+   cl::init(false));
+// setjmp/longjmp handling using wasm EH instrutions
+cl::opt WasmEnableSjLj("wasm-enable-sjlj",
+ cl::desc("WebAssembly setjmp/longjmp handling"),
+ cl::init(false));
+
+// Function names in libc++abi and libunwind
 const char *const WebAssembly::CxaBeginCatchFn = "__cxa_begin_catch";
 const char *const WebAssembly::CxaRethrowFn = "__cxa_rethrow";
 const char *const 

[PATCH] D109885: [MLIR][[amdgpu-arch]][OpenMP] Remove direct dependency on /opt/rocm

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

In D109885#3198340 , @JonChesterfield 
wrote:

> In D109885#3198296 , @arsenm wrote:
>
>> This isn't a feature, it's the introduction of a bug.
>
> It would regress people depending on the implicit pickup of /opt/rocm, 
> leaving them with a straightforward workaround of setting the cmake variable.

Exactly. Builds that don't find their dependencies in the default location 
without doing anything are bad builds. It adds extra work and discovery a 
casual builder doesn't actually care about.

> The inverse, where we look in /opt/rocm unless that's overridden (perhaps by 
> the empty string), achieves much the same effect without breaking anyone 
> using trunk with a rocm install.

Exactly, this is how the search process is supposed to work. HINTS are at the 
bottom of the search hierarchy. The mechanisms for finding a specific version 
win out over the basic hint to the default

> I'm very mistrusting of mixing a rocm toolchain with a trunk toolchain as 
> they're both quite keen on runpath and LD_LIBRARY_PATH to find internal 
> components. That makes it very easy to accidentally mix the two together into 
> something that doesn't work so personal preference is to rip out the 
> /opt/rocm search path for HSA entirely and encourage people to build it from 
> source.

There are a number of cmake crimes going on in both of these, but this isn't 
one of them. LD_LIBRARY_PATH should not be used for any builds to find 
components.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109885

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

[PATCH] D115893: [WebAssembly] Support clang -fwasm-exceptions for bitcode

[Heejin Ahn via Phabricator via cfe-commits]

aheejin created this revision.
aheejin added a reviewer: dschuff.
Herald added subscribers: wingo, ecnelises, pengfei, sunfish, hiraditya, 
jgravelle-google, sbc100.
aheejin requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

This supports bitcode compilation using `clang -fwasm-exceptions`.

---

The current situation:

Currently the backend requires two options for Wasm EH:
`-wasm-enable-eh` and `-exception-model=wasm`. Wasm SjLj requires two
options as well: `-wasm-enable-sjlj` and `-exception-model=wasm`. When
using Wasm EH via Emscripten, you only need to pass `-fwasm-exceptions`,
and these options will be added within the clang driver. This
description will focus on the case of Wasm EH going forward, but Wasm
SjLj's case is similar.

When you pass `-fwasm-exceptions` to emcc and clang driver, the clang
driver adds these options to the command line that calls the clang
frontend (`clang -cc1`): `-mllvm -wasm-enable-eh` and
`-exception-model=wasm`. `-wasm-enable-eh` is prefixed with `-mllvm`, so
it is passed as is to the backend. But `-exception-model` is parsed and
processed within the clang frontend and stored in `LangOptions` class
backend via `LangOptions` class. This info is later transferred to
`TargetOptions` class, and then eventually passed to `MCAsmInfo` class.
All LLVM code queries this `MCAsmInfo` to get the exception model. (
(e.g., here)

---

Problem:

The problem is the whole `LangOptions` processing is bypassed when
compiling bitcode, so the information transfer of `LangOptions` ->
`TargetOptions` -> `MCAsmInfo` does not happen. They are all set to
`ExceptionHandling::None`, which is the default value.

---

What other targets do, and why we can't do the same:

Other targets support bitcode compilation by the clang driver, but they
can do that by using different triples. For example, X86 target supports
multiple triples, each of which has its own subclass of `MCAsmInfo`, so
it can hardcode the appropriate exception model within those subclasses'
constructors. But we don't have separate triples for each exception
mode: none, emscripten, ans wasm.

---

What this CL does:

If we can figure out whether `-wasm-enable-eh` is passed to the backend,
we can programatically set the exception model from the backend, rather
than requiring it to be passed.

So we check `WasmEnableEH` and `WasmEnableSjLj` variables, which are
`cl::opt` for `-wasm-enable-eh` and `-wasm-enable-sjlj`, in
`WebAssemblyMCAsmInfo` constructor, and if either of them is set, we set
`MCAsmInfo.ExceptionType` to Wasm. `TargetOptions` cannot be updated
there, so we make sure they are the same later.

Fixes https://github.com/emscripten-core/emscripten/issues/15712.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115893

Files:
  clang/test/CodeGen/WebAssembly/wasm-eh.ll
  llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCAsmInfo.cpp
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp

Index: llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
===
--- llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
@@ -24,6 +24,7 @@
 #include "llvm/CodeGen/RegAllocRegistry.h"
 #include "llvm/CodeGen/TargetPassConfig.h"
 #include "llvm/IR/Function.h"
+#include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/TargetRegistry.h"
 #include "llvm/Target/TargetOptions.h"
 #include "llvm/Transforms/Scalar.h"
@@ -33,27 +34,10 @@
 
 #define DEBUG_TYPE "wasm"
 
-// Emscripten's asm.js-style exception handling
-cl::opt
-WasmEnableEmEH("enable-emscripten-cxx-exceptions",
-   cl::desc("WebAssembly Emscripten-style exception handling"),
-   cl::init(false));
-
-// Emscripten's asm.js-style setjmp/longjmp handling
-cl::opt WasmEnableEmSjLj(
-"enable-emscripten-sjlj",
-cl::desc("WebAssembly Emscripten-style setjmp/longjmp handling"),
-cl::init(false));
-
-// Exception handling using wasm EH instructions
-cl::opt WasmEnableEH("wasm-enable-eh",
-   cl::desc("WebAssembly exception handling"),
-   cl::init(false));
-
-// setjmp/longjmp handling using wasm EH instrutions
-cl::opt WasmEnableSjLj("wasm-enable-sjlj",
- cl::desc("WebAssembly setjmp/longjmp handling"),
- cl::init(false));
+extern cl::opt WasmEnableEmEH;   // asm.js-style EH
+extern cl::opt WasmEnableEmSjLj; // asm.js-style SjLJ
+extern cl::opt WasmEnableEH; // EH using Wasm EH instructions
+extern cl::opt WasmEnableSjLj;   // SjLj using Wasm EH instructions
 
 // A command-line option to keep implicit locals
 // for the purpose of testing with lit/llc ONLY.
@@ -368,7 +352,18 @@
   return nullptr; // No reg alloc
 }
 
-static void basicCheckForEHAndSjLj(const 

[PATCH] D109885: [MLIR][[amdgpu-arch]][OpenMP] Remove direct dependency on /opt/rocm

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added inline comments.



Comment at: clang/tools/amdgpu-arch/CMakeLists.txt:13
+
+find_package(hsa-runtime64 QUIET 1.2.0 HINTS ${CMAKE_INSTALL_PREFIX} PATH 
${ROCM_PATH})
 if (NOT ${hsa-runtime64_FOUND})

arsenm wrote:
> I also think CMAKE_INSTALL_PREFIX in the search hints is bad practice. I 
> don't recall ever seeing a project do this. Depending on the install path for 
> anything leads to flaky builds
This was copied from the amdgpu plugin. I can't remember where I copied that 
from. Alternatives welcome - what's the proper way to indicate 'this library? 
it's probably next to all the other llvm libraries'


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109885

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

[PATCH] D109885: [MLIR][[amdgpu-arch]][OpenMP] Remove direct dependency on /opt/rocm

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

In D109885#3198296 , @arsenm wrote:

> This isn't a feature, it's the introduction of a bug.

It would regress people depending on the implicit pickup of /opt/rocm, leaving 
them with a straightforward workaround of setting the cmake variable.

The inverse, where we look in /opt/rocm unless that's overridden (perhaps by 
the empty string), achieves much the same effect without breaking anyone using 
trunk with a rocm install.

I'm very mistrusting of mixing a rocm toolchain with a trunk toolchain as 
they're both quite keen on runpath and LD_LIBRARY_PATH to find internal 
components. That makes it very easy to accidentally mix the two together into 
something that doesn't work so personal preference is to rip out the /opt/rocm 
search path for HSA entirely and encourage people to build it from source.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109885

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

[PATCH] D109885: [MLIR][[amdgpu-arch]][OpenMP] Remove direct dependency on /opt/rocm

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



Comment at: clang/tools/amdgpu-arch/CMakeLists.txt:13
+
+find_package(hsa-runtime64 QUIET 1.2.0 HINTS ${CMAKE_INSTALL_PREFIX} PATH 
${ROCM_PATH})
 if (NOT ${hsa-runtime64_FOUND})

I also think CMAKE_INSTALL_PREFIX in the search hints is bad practice. I don't 
recall ever seeing a project do this. Depending on the install path for 
anything leads to flaky builds


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109885

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

[PATCH] D109885: [MLIR][[amdgpu-arch]][OpenMP] Remove direct dependency on /opt/rocm

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

In D109885#3198290 , @JonChesterfield 
wrote:

> In D109885#3198232 , @arsenm wrote:
>
>> Dropping the default install location from the default search hint is 
>> entirely unreasonable
>
> That seems to be the feature request, though I haven't gone through the jira 
> web to work out what's going on here.

This isn't a feature, it's the introduction of a bug. It degrades the usability 
of the build. If you want an explicit version, that's what the search order 
process is for.

> We can either have opt-in to somewhere other than CMAKE_INSTALL_PREFIX, which 
> I think Ethan's suggestion does (not clear whether PATH between the two 
> variables is semantically important), or we could have opt-out for people who 
> want to be sure they aren't picking up something in /opt/rocm.

This isn't how search paths work. By default, the fallback should always find 
the default location


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109885

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

[PATCH] D109885: [MLIR][[amdgpu-arch]][OpenMP] Remove direct dependency on /opt/rocm

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

In D109885#3198232 , @arsenm wrote:

> Dropping the default install location from the default search hint is 
> entirely unreasonable

That seems to be the feature request, though I haven't gone through the jira 
web to work out what's going on here.

We can either have opt-in to somewhere other than CMAKE_INSTALL_PREFIX, which I 
think Ethan's suggestion does (not clear whether PATH between the two variables 
is semantically important), or we could have opt-out for people who want to be 
sure they aren't picking up something in /opt/rocm.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109885

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

[PATCH] D111100: enable plugins for clang-tidy

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp:392-394
+  cl::Option *load_opt = cl::getRegisteredOptions().lookup("load");
+  if (load_opt)
+load_opt->addCategory(ClangTidyCategory);





Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:421-424
+If you want to develop this out-of-tree, the steps above are the largely same.
+External to the clang-tidy build system, put all of the new code into a single
+shared library. Build and link it against llvm, while allowing some symbols to
+be undefined during linking, almost exactly as you would define a clang plugin.

I wordsmithed a bit and this should be similar to what you already had. I did 
remove the "while allowing some symbols to be undefined during linking" bit 
because I wasn't certain what that was about.



Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:426-427
+
+Then we can run it by passing `-load` to `clang-tidy`, in addition to the name
+of our new checks.
+





Comment at: clang-tools-extra/docs/clang-tidy/index.rst:223
+ Load the dynamic object ``plugin``. This
+ object should register new static 
analyzer or clang-tidy passes. Once loaded, the object
+ will add new command line options to run





Comment at: clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp:24-25
+
+  //void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  //void check(const ast_matchers::MatchFinder::MatchResult ) override;
+

Shouldn't these functions be overloaded? We don't need it to be particularly 
functional, but the plugin should demonstrate that it works and can be run by 
clang-tidy (not just loaded and listed as a check).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

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

[PATCH] D109885: [MLIR][[amdgpu-arch]][OpenMP] Remove direct dependency on /opt/rocm

[Matt Arsenault via Phabricator via cfe-commits]

arsenm requested changes to this revision.
arsenm added a comment.

Dropping the default install location from the default search hint is entirely 
unreasonable


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109885

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

[PATCH] D109885: [MLIR][[amdgpu-arch]][OpenMP] Remove direct dependency on /opt/rocm

[Ethan Stewart via Phabricator via cfe-commits]

estewart08 added a comment.

In D109885#3194819 , @ronlieb wrote:

> @estewart08 thoughts on a good CMAKE variable to allow users to define 
> equivalent of /opt/rocm  ?   and not use environment variable inside the 
> cmake file.

I would be ok with the following, without the check for ENV{ROCM_PATH}. The 
user has the option to set -DROCM_PATH=$ROCM_PATH or -DCMAKE_PREFIX_PATH.

  find_package(hsa-runtime64 QUIET 1.2.0 HINTS ${CMAKE_INSTALL_PREFIX} PATH 
${ROCM_PATH})


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109885

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

[PATCH] D112453: [Sema] When dereferencing a pointer of dependent type, infer the result type.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:10546-10548
+  if (PointeeTy->isDependentType()) {
+return true;
+  }





Comment at: clang/lib/Sema/SemaExpr.cpp:13824
 SourceLocation OpLoc) {
-  if (Op->isTypeDependent())
+  if (Op->isTypeDependent() && !Op->getType()->isPointerType())
 return S.Context.DependentTy;

One thing that makes me a bit uncomfortable is that the logic for these used to 
be far more understandable when it was just checking for a dependent type. Now 
we need to sprinkle "and not a pointer type" in places, but it's not 
particularly clear as to why for a naïve reader of the code.

I wonder if we want some sort of type predicate `isDeferrablyDependentType()` 
or something along those lines, or whether that's not really plausible? 
Basically, I'm hoping to find a way that, as a code reviewer, I can more easily 
spot places where `isTypeDependent()` should really be caring about type 
dependent pointers as a special case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112453

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

[libunwind] 1c4867e - [libunwind] Provide a way to conveniently install libunwind headers

[Louis Dionne via cfe-commits]

Author: PoYao Chang
Date: 2021-12-16T13:32:40-05:00
New Revision: 1c4867e6fc507fe6e81cfc0e3c7148307b4b7433

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

LOG: [libunwind] Provide a way to conveniently install libunwind headers

This adds a CMake option (defaults to OFF to not be intrusive) to activate
2 new targets `install-unwind-headers` and `install-unwind-headers-stripped`.
So, for example:

  cmake -S runtimes -B build -G Ninja \
-DLLVM_ENABLE_RUNTIMES='libunwind' \
-DLIBUNWIND_INSTALL_HEADERS=ON

And then, `ninja -C build install-unwind` would install headers in addition
to good ol' dylibs and archives, i.e., targets `install-unwind*` `DEPENDS`
on `install-unwind-headers*`. On the other hand,
`ninja -C build install-unwind-headers` gives you headers only.

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

Added: 
libunwind/include/CMakeLists.txt

Modified: 
libunwind/CMakeLists.txt
libunwind/src/CMakeLists.txt

Removed: 




diff  --git a/libunwind/CMakeLists.txt b/libunwind/CMakeLists.txt
index a63dc453ffb6c..eb478e4e7730c 100644
--- a/libunwind/CMakeLists.txt
+++ b/libunwind/CMakeLists.txt
@@ -78,6 +78,7 @@ option(LIBUNWIND_INCLUDE_TESTS "Build the libunwind tests." 
${LLVM_INCLUDE_TESTS
 option(LIBUNWIND_IS_BAREMETAL "Build libunwind for baremetal targets." OFF)
 option(LIBUNWIND_USE_FRAME_HEADER_CACHE "Cache frame headers for unwinding. 
Requires locking dl_iterate_phdr." OFF)
 option(LIBUNWIND_REMEMBER_HEAP_ALLOC "Use heap instead of the stack for 
.cfi_remember_state." OFF)
+option(LIBUNWIND_INSTALL_HEADERS "Install the libunwind headers." OFF)
 
 set(LIBUNWIND_LIBDIR_SUFFIX "${LLVM_LIBDIR_SUFFIX}" CACHE STRING
 "Define suffix of library directory name (32/64)")
@@ -372,7 +373,7 @@ endif()
 # Setup Source Code
 
#===
 
-include_directories(include)
+add_subdirectory(include)
 
 add_subdirectory(src)
 

diff  --git a/libunwind/include/CMakeLists.txt 
b/libunwind/include/CMakeLists.txt
new file mode 100644
index 0..c3bb1dd0f69fa
--- /dev/null
+++ b/libunwind/include/CMakeLists.txt
@@ -0,0 +1,31 @@
+set(files
+__libunwind_config.h
+libunwind.h
+mach-o/compact_unwind_encoding.h
+unwind_arm_ehabi.h
+unwind_itanium.h
+unwind.h
+)
+
+add_library(unwind-headers INTERFACE)
+target_include_directories(unwind-headers INTERFACE 
${CMAKE_CURRENT_SOURCE_DIR})
+
+if(LIBUNWIND_INSTALL_HEADERS)
+  foreach(file ${files})
+get_filename_component(dir ${file} DIRECTORY)
+install(FILES ${file}
+  DESTINATION "include/${dir}"
+  COMPONENT unwind-headers
+  PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ
+)
+  endforeach()
+
+  if(NOT CMAKE_CONFIGURATION_TYPES)
+add_custom_target(install-unwind-headers
+  DEPENDS unwind-headers
+  COMMAND "${CMAKE_COMMAND}"
+  -DCMAKE_INSTALL_COMPONENT=unwind-headers
+  -P "${LIBUNWIND_BINARY_DIR}/cmake_install.cmake")
+add_custom_target(install-unwind-headers-stripped DEPENDS 
install-unwind-headers)
+  endif()
+endif()

diff  --git a/libunwind/src/CMakeLists.txt b/libunwind/src/CMakeLists.txt
index 41513ddfff1f5..710198550a061 100644
--- a/libunwind/src/CMakeLists.txt
+++ b/libunwind/src/CMakeLists.txt
@@ -133,7 +133,8 @@ if (LIBUNWIND_ENABLE_SHARED)
   else()
 target_compile_options(unwind_shared PRIVATE -fno-rtti)
   endif()
-  target_link_libraries(unwind_shared PRIVATE ${LIBUNWIND_LIBRARIES})
+  target_link_libraries(unwind_shared PRIVATE ${LIBUNWIND_LIBRARIES}
+  PRIVATE unwind-headers)
   set_target_properties(unwind_shared
 PROPERTIES
   CXX_EXTENSIONS OFF
@@ -160,7 +161,8 @@ if (LIBUNWIND_ENABLE_STATIC)
   else()
 target_compile_options(unwind_static PRIVATE -fno-rtti)
   endif()
-  target_link_libraries(unwind_static PRIVATE ${LIBUNWIND_LIBRARIES})
+  target_link_libraries(unwind_static PRIVATE ${LIBUNWIND_LIBRARIES}
+  PRIVATE unwind-headers)
   set_target_properties(unwind_static
 PROPERTIES
   CXX_EXTENSIONS OFF
@@ -207,4 +209,8 @@ if (NOT CMAKE_CONFIGURATION_TYPES AND 
LIBUNWIND_INSTALL_LIBRARY)
 -DCMAKE_INSTALL_COMPONENT=unwind
 -DCMAKE_INSTALL_DO_STRIP=1
 -P "${LIBUNWIND_BINARY_DIR}/cmake_install.cmake")
+  if(LIBUNWIND_INSTALL_HEADERS)
+add_dependencies(install-unwind install-unwind-headers)
+add_dependencies(install-unwind-stripped install-unwind-headers-stripped)
+  endif()
 endif()



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

[PATCH] D115456: Implement on-demand TLS initialization for Microsoft CXX ABI

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

I probably don't have time to review this, so let me redirect to @hans.




Comment at: clang/lib/CodeGen/MicrosoftCXXABI.cpp:2400
 
+static llvm::GlobalValue *getTlsGuardVar(CodeGenModule ) {
+  llvm::Type *VTy = llvm::Type::getInt8Ty(CGM.getLLVMContext());

My understanding is that every DLL has exactly one `__tls_guard` variable. All 
TLS variables in a DLL are initialized as soon as a thread accesses one TLS 
variable for a DLL. Is that accurate? Can you add comments about the way this 
is intended to work here?


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

https://reviews.llvm.org/D115456

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

[clang] ceb8055 - [OpenCL] Add pure attribute to vload builtins

[Stuart Brady via cfe-commits]

Author: Stuart Brady
Date: 2021-12-16T18:30:58Z
New Revision: ceb80557e523f1894799ebadd5d985e11ee80461

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

LOG: [OpenCL] Add pure attribute to vload builtins

Use the "pure" attribute (or "readonly") for the vload, vload_half and
vloada_half builtins.

Includes test changes to SemaOpenCL/fdeclare-opencl-builtins.cl to avoid
triggering unused-result warnings.

Reviewed By: svenvh

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

Added: 


Modified: 
clang/lib/Headers/opencl-c.h
clang/lib/Sema/OpenCLBuiltins.td
clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl

Removed: 




diff  --git a/clang/lib/Headers/opencl-c.h b/clang/lib/Headers/opencl-c.h
index 32af848a94c4f..77a7a8b9bb3a1 100644
--- a/clang/lib/Headers/opencl-c.h
+++ b/clang/lib/Headers/opencl-c.h
@@ -11190,305 +11190,305 @@ half16 __ovld __cnfn select(half16 a, half16 b, 
ushort16 c);
  * 64-bit aligned if gentype is long, ulong, double.
  */
 
-char2 __ovld vload2(size_t offset, const __constant char *p);
-uchar2 __ovld vload2(size_t offset, const __constant uchar *p);
-short2 __ovld vload2(size_t offset, const __constant short *p);
-ushort2 __ovld vload2(size_t offset, const __constant ushort *p);
-int2 __ovld vload2(size_t offset, const __constant int *p);
-uint2 __ovld vload2(size_t offset, const __constant uint *p);
-long2 __ovld vload2(size_t offset, const __constant long *p);
-ulong2 __ovld vload2(size_t offset, const __constant ulong *p);
-float2 __ovld vload2(size_t offset, const __constant float *p);
-char3 __ovld vload3(size_t offset, const __constant char *p);
-uchar3 __ovld vload3(size_t offset, const __constant uchar *p);
-short3 __ovld vload3(size_t offset, const __constant short *p);
-ushort3 __ovld vload3(size_t offset, const __constant ushort *p);
-int3 __ovld vload3(size_t offset, const __constant int *p);
-uint3 __ovld vload3(size_t offset, const __constant uint *p);
-long3 __ovld vload3(size_t offset, const __constant long *p);
-ulong3 __ovld vload3(size_t offset, const __constant ulong *p);
-float3 __ovld vload3(size_t offset, const __constant float *p);
-char4 __ovld vload4(size_t offset, const __constant char *p);
-uchar4 __ovld vload4(size_t offset, const __constant uchar *p);
-short4 __ovld vload4(size_t offset, const __constant short *p);
-ushort4 __ovld vload4(size_t offset, const __constant ushort *p);
-int4 __ovld vload4(size_t offset, const __constant int *p);
-uint4 __ovld vload4(size_t offset, const __constant uint *p);
-long4 __ovld vload4(size_t offset, const __constant long *p);
-ulong4 __ovld vload4(size_t offset, const __constant ulong *p);
-float4 __ovld vload4(size_t offset, const __constant float *p);
-char8 __ovld vload8(size_t offset, const __constant char *p);
-uchar8 __ovld vload8(size_t offset, const __constant uchar *p);
-short8 __ovld vload8(size_t offset, const __constant short *p);
-ushort8 __ovld vload8(size_t offset, const __constant ushort *p);
-int8 __ovld vload8(size_t offset, const __constant int *p);
-uint8 __ovld vload8(size_t offset, const __constant uint *p);
-long8 __ovld vload8(size_t offset, const __constant long *p);
-ulong8 __ovld vload8(size_t offset, const __constant ulong *p);
-float8 __ovld vload8(size_t offset, const __constant float *p);
-char16 __ovld vload16(size_t offset, const __constant char *p);
-uchar16 __ovld vload16(size_t offset, const __constant uchar *p);
-short16 __ovld vload16(size_t offset, const __constant short *p);
-ushort16 __ovld vload16(size_t offset, const __constant ushort *p);
-int16 __ovld vload16(size_t offset, const __constant int *p);
-uint16 __ovld vload16(size_t offset, const __constant uint *p);
-long16 __ovld vload16(size_t offset, const __constant long *p);
-ulong16 __ovld vload16(size_t offset, const __constant ulong *p);
-float16 __ovld vload16(size_t offset, const __constant float *p);
+char2 __ovld __purefn vload2(size_t offset, const __constant char *p);
+uchar2 __ovld __purefn vload2(size_t offset, const __constant uchar *p);
+short2 __ovld __purefn vload2(size_t offset, const __constant short *p);
+ushort2 __ovld __purefn vload2(size_t offset, const __constant ushort *p);
+int2 __ovld __purefn vload2(size_t offset, const __constant int *p);
+uint2 __ovld __purefn vload2(size_t offset, const __constant uint *p);
+long2 __ovld __purefn vload2(size_t offset, const __constant long *p);
+ulong2 __ovld __purefn vload2(size_t offset, const __constant ulong *p);
+float2 __ovld __purefn vload2(size_t offset, const __constant float *p);
+char3 __ovld __purefn vload3(size_t offset, const __constant char *p);
+uchar3 __ovld __purefn vload3(size_t offset, const __constant uchar *p);
+short3 __ovld __purefn vload3(size_t offset, const __constant short *p);

[PATCH] D110742: [OpenCL] Add pure attributes to vload builtins

[Stuart Brady 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 rGceb80557e523: [OpenCL] Add pure attribute to vload builtins 
(authored by stuart).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110742

Files:
  clang/lib/Headers/opencl-c.h
  clang/lib/Sema/OpenCLBuiltins.td
  clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl

Index: clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
===
--- clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
+++ clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
@@ -46,12 +46,19 @@
 typedef char char2 __attribute__((ext_vector_type(2)));
 typedef char char4 __attribute__((ext_vector_type(4)));
 typedef uchar uchar4 __attribute__((ext_vector_type(4)));
+typedef uchar uchar16 __attribute__((ext_vector_type(16)));
 typedef float float4 __attribute__((ext_vector_type(4)));
+typedef float float16 __attribute__((ext_vector_type(16)));
 typedef half half4 __attribute__((ext_vector_type(4)));
 typedef int int2 __attribute__((ext_vector_type(2)));
 typedef int int4 __attribute__((ext_vector_type(4)));
+typedef uint uint2 __attribute__((ext_vector_type(2)));
 typedef uint uint4 __attribute__((ext_vector_type(4)));
 typedef long long2 __attribute__((ext_vector_type(2)));
+typedef long long8 __attribute__((ext_vector_type(8)));
+typedef ulong ulong4 __attribute__((ext_vector_type(4)));
+typedef short short16 __attribute__((ext_vector_type(16)));
+typedef ushort ushort3 __attribute__((ext_vector_type(3)));
 
 typedef int clk_profiling_info;
 #define CLK_PROFILING_COMMAND_EXEC_TIME 0x1
@@ -284,18 +291,27 @@
   global void *global_p;
   private void *private_p;
   size_t s;
+  ulong4 ul4;
+  short16 s16;
+#if __OPENCL_C_VERSION__ >= CL_VERSION_2_0
+  ushort3 us3;
+  uchar16 uc16;
+#endif
+  long8 l8;
+  uint2 ui2;
+  float16 f16;
 
-  vload4(s, (const __constant ulong *) constant_p);
-  vload16(s, (const __constant short *) constant_p);
+  ul4 = vload4(s, (const __constant ulong *) constant_p);
+  s16 = vload16(s, (const __constant short *) constant_p);
 
 #if __OPENCL_C_VERSION__ >= CL_VERSION_2_0
-  vload3(s, (const __generic ushort *) generic_p);
-  vload16(s, (const __generic uchar *) generic_p);
+  us3 = vload3(s, (const __generic ushort *) generic_p);
+  uc16 = vload16(s, (const __generic uchar *) generic_p);
 #endif
 
-  vload8(s, (const __global long *) global_p);
-  vload2(s, (const __local uint *) local_p);
-  vload16(s, (const __private float *) private_p);
+  l8 = vload8(s, (const __global long *) global_p);
+  ui2 = vload2(s, (const __local uint *) local_p);
+  f16 = vload16(s, (const __private float *) private_p);
 }
 
 kernel void basic_work_item() {
Index: clang/lib/Sema/OpenCLBuiltins.td
===
--- clang/lib/Sema/OpenCLBuiltins.td
+++ clang/lib/Sema/OpenCLBuiltins.td
@@ -806,17 +806,17 @@
   foreach AS = addrspaces in {
 foreach VSize = [2, 3, 4, 8, 16] in {
   foreach name = ["vload" # VSize] in {
-def : Builtin, Size, PointerType, AS>]>;
-def : Builtin, Size, PointerType, AS>]>;
-def : Builtin, Size, PointerType, AS>]>;
-def : Builtin, Size, PointerType, AS>]>;
-def : Builtin, Size, PointerType, AS>]>;
-def : Builtin, Size, PointerType, AS>]>;
-def : Builtin, Size, PointerType, AS>]>;
-def : Builtin, Size, PointerType, AS>]>;
-def : Builtin, Size, PointerType, AS>]>;
-def : Builtin, Size, PointerType, AS>]>;
-def : Builtin, Size, PointerType, AS>]>;
+def : Builtin, Size, PointerType, AS>], Attr.Pure>;
+def : Builtin, Size, PointerType, AS>], Attr.Pure>;
+def : Builtin, Size, PointerType, AS>], Attr.Pure>;
+def : Builtin, Size, PointerType, AS>], Attr.Pure>;
+def : Builtin, Size, PointerType, AS>], Attr.Pure>;
+def : Builtin, Size, PointerType, AS>], Attr.Pure>;
+def : Builtin, Size, PointerType, AS>], Attr.Pure>;
+def : Builtin, Size, PointerType, AS>], Attr.Pure>;
+def : Builtin, Size, PointerType, AS>], Attr.Pure>;
+def : Builtin, Size, PointerType, AS>], Attr.Pure>;
+def : Builtin, Size, PointerType, AS>], Attr.Pure>;
   }
   if defStores then {
 foreach name = ["vstore" # VSize] in {
@@ -848,10 +848,10 @@
 
 multiclass VloadVstoreHalf addrspaces, bit defStores> {
   foreach AS = addrspaces in {
-def : Builtin<"vload_half", [Float, Size, PointerType, AS>]>;
+def : Builtin<"vload_half", [Float, Size, PointerType, AS>], Attr.Pure>;
 foreach VSize = [2, 3, 4, 8, 16] in {
   foreach name = ["vload_half" # VSize, "vloada_half" # VSize] in {
-def : Builtin, Size, PointerType, AS>]>;
+def : Builtin, Size, PointerType, AS>], Attr.Pure>;
   }
 }
 

[PATCH] D115670: Implement some constexpr vector unary operators, fix boolean-ops

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Note: I think I've done everything requested, so I'm hoping to commit this 
tomorrow 1st thing in order to have it in time for everyone's vacations (and so 
my downstream can get it before the new year), unless I hear objections from 
the other reviewers.


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

https://reviews.llvm.org/D115670

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

[PATCH] D115441: [X86][MS] Add 80bit long double support for Windows

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

I see, thanks for the info.  Can you please add a targeted LLVM test for long 
double arguments? From what I can tell, the auto-generated 
update_llc_test_checks.py style tests are not a good fit for testing parameter 
passing because they pattern-match away the stack offsets which are relevant to 
the test.

I think it's also worth breaking this into LLVM and clang-side patches:

1. make clang emit x86_fp80 with -mlong-double-80
2. update LLVM datalayout (must affect clang via clang/lib/Basic/Targets/X86.h) 
to align fp80 to 16 bytes in the MSVC environment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115441

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

[PATCH] D114995: clang-tidy: improve the 'modernize-use-default-member-init'

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D114995#3183240 , @malcolm.parsons 
wrote:

> In D114995#3180475 , @aaron.ballman 
> wrote:
>
>> was there a reason we didn't cover that case originally or was it an 
>> oversight/left for future work?
>
> It was left for future work - by only considering the initializer list of the 
> default constructor, clang-tidy did not have to work out what to do when the 
> constructors don't agree on what value the member init should have.

Thank you for verifying! @oleg.smolsky -- this would be a very useful test case 
to add, btw.

In D114995#3181486 , @oleg.smolsky 
wrote:

> Sure, adding an option is easy, if that's the consensus. What would you call 
> it?

Since we left this for future work, I don't think we need to add a 
configuration option unless a user finds a need for one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114995

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

[PATCH] D112903: [C++20] [Module] Fix bug47116 and implement [module.interface]/p6

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/AST/DeclBase.h:616
 
+  bool isInExportDeclContext() const;
+

I think it would be good to add some comments to document the function (as done 
with surrounding code).



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7793-7794
   "because namespace %1 does not enclose namespace %2">;
+def err_export_non_namespace_scope_name : Error<"cannot export %0 as it is "
+  "not at namespace scope.">;
+def err_redeclaration_non_exported : Error <"cannot export redeclaration %0 
here "





Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7795-7796
+  "not at namespace scope.">;
+def err_redeclaration_non_exported : Error <"cannot export redeclaration %0 
here "
+  "since the previous declaration is not exported.">;
 def err_invalid_declarator_global_scope : Error<

Both of these tweak the formatting somewhat, but the important thing is 
dropping the trailing punctuation from the diagnostic text.



Comment at: clang/lib/AST/DeclBase.cpp:999
+bool Decl::isInExportDeclContext() const {
+  auto *DC = getLexicalDeclContext();
+





Comment at: clang/lib/Sema/SemaDecl.cpp:1622
 
+// [module.interface]p6:
+// A redeclaration of an entity X is implicitly exported if X was introduced by





Comment at: clang/lib/Sema/SemaDecl.cpp:1643-1645
+// A simple wrapper function to ease the call for
+// CheckRedeclarationModuleOwnership and CheckRedeclarationExported and any
+// other thing needed.





Comment at: clang/lib/Sema/SemaDecl.cpp:5789
+  if (!isa(DC))
+Diag(Loc, diag::err_export_non_namespace_scope_name) << Name;
+  else




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

https://reviews.llvm.org/D112903

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

[PATCH] D115883: [analyzer][NFC] Change return value of StoreManager::attemptDownCast function from SVal to Optional

[Gábor Horváth via Phabricator via cfe-commits]

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

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115883

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

[PATCH] D115883: [analyzer][NFC] Change return value of StoreManager::attemptDownCast function from SVal to Optional

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov created this revision.
ASDenysPetrov added reviewers: xazax.hun, steakhal, NoQ, martong, 
baloghadamsoftware.
ASDenysPetrov added a project: clang.
Herald added subscribers: manas, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet.
ASDenysPetrov requested review of this revision.
Herald added a subscriber: cfe-commits.

Refactor return value of `StoreManager::attemptDownCast` function by removing 
the last parameter `bool ` and replace the return value `SVal` with 
`Optional`.  Make the function consistent with the family of 
`evalDerivedToBase` by renaming it to `evalBaseToDerived`. Aligned the code on 
the call side with these changes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115883

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  clang/lib/StaticAnalyzer/Core/Store.cpp

Index: clang/lib/StaticAnalyzer/Core/Store.cpp
===
--- clang/lib/StaticAnalyzer/Core/Store.cpp
+++ clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -314,10 +314,7 @@
   return nullptr;
 }
 
-SVal StoreManager::attemptDownCast(SVal Base, QualType TargetType,
-   bool ) {
-  Failed = false;
-
+Optional StoreManager::evalBaseToDerived(SVal Base, QualType TargetType) {
   const MemRegion *MR = Base.getAsRegion();
   if (!MR)
 return UnknownVal();
@@ -392,7 +389,9 @@
   }
 
   // We failed if the region we ended up with has perfect type info.
-  Failed = isa(MR);
+  if (isa(MR))
+return None;
+
   return UnknownVal();
 }
 
Index: clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -439,14 +439,15 @@
 if (CastE->isGLValue())
   resultType = getContext().getPointerType(resultType);
 
-bool Failed = false;
-
-// Check if the value being cast evaluates to 0.
-if (val.isZeroConstant())
-  Failed = true;
-// Else, evaluate the cast.
-else
-  val = getStoreManager().attemptDownCast(val, T, Failed);
+bool Failed = true;
+
+// Check if the value being cast does not evaluates to 0.
+if (!val.isZeroConstant())
+  if (Optional V =
+  StateMgr.getStoreManager().evalBaseToDerived(val, T)) {
+val = *V;
+Failed = false;
+  }
 
 if (Failed) {
   if (T->isReferenceType()) {
@@ -478,14 +479,13 @@
 if (CastE->isGLValue())
   resultType = getContext().getPointerType(resultType);
 
-bool Failed = false;
-
 if (!val.isConstant()) {
-  val = getStoreManager().attemptDownCast(val, T, Failed);
+  Optional V = getStoreManager().evalBaseToDerived(val, T);
+  val = V ? *V : UnknownVal();
 }
 
 // Failed to cast or the result is unknown, fall back to conservative.
-if (Failed || val.isUnknown()) {
+if (val.isUnknown()) {
   val =
 svalBuilder.conjureSymbolVal(nullptr, CastE, LCtx, resultType,
  currBldrCtx->blockCount());
Index: clang/lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- clang/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ clang/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -762,9 +762,9 @@
   QualType Ty = Ctx.getPointerType(Ctx.getRecordType(Class));
 
   // FIXME: CallEvent maybe shouldn't be directly accessing StoreManager.
-  bool Failed;
-  ThisVal = StateMgr.getStoreManager().attemptDownCast(ThisVal, Ty, Failed);
-  if (Failed) {
+  Optional V =
+  StateMgr.getStoreManager().evalBaseToDerived(ThisVal, Ty);
+  if (!V.hasValue()) {
 // We might have suffered some sort of placement new earlier, so
 // we're constructing in a completely unexpected storage.
 // Fall back to a generic pointer cast for this-value.
@@ -772,7 +772,8 @@
 const CXXRecordDecl *StaticClass = StaticMD->getParent();
 QualType StaticTy = Ctx.getPointerType(Ctx.getRecordType(StaticClass));
 ThisVal = SVB.evalCast(ThisVal, Ty, StaticTy);
-  }
+  } else
+ThisVal = *V;
 }
 
 if (!ThisVal.isUnknown())
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
@@ -172,9 +172,9 @@
   ///dynamic_cast.
   ///  - We don't know (base is a symbolic region and we don't have
   ///enough info to determine if the cast will succeed at run time).
-  /// The function returns an SVal 

[PATCH] D112221: Mark ATOMIC_VAR_INIT and ATOMIC_FLAG_INIT as deprecated

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

In D112221#3196740 , @tambre wrote:

> In D112221#3195955 , @ldionne wrote:
>
>> @tambre Any appetite for a libc++ patch? :)
>>
>> Otherwise, let me know and I'll put it in my list of things to fix up.
>
> Sure, I can take it up during the weekend.

Thanks a lot! Ping me and I'll review it quickly.


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

https://reviews.llvm.org/D112221

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

[PATCH] D114317: [clang-tidy][WIP] Do not run perfect alias checks

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D114317#3197815 , @carlosgalvezp 
wrote:

>> me pretty far behind on reviews
>
> Wow, that's a lot to review! I will keep it in mind and hopefully only ping 
> you when I have a nice final design ready to go. Really appreciate your early 
> feedback!

Happy to help and I really appreciate your patience!

>> I'd like to make sure I'm on the same page as you.
>
> Yes, exactly, from the RFC we agreed that the desired outcome of this patch 
> would be that the same diagnostics and fixits are emitted, regardless of the 
> alias checks actually running. I.e. this patch should not have any visible 
> effects towards the user (other than faster runtime). The patch, as it is 
> right now _will_ eliminate those diagnostics (since the alias checks are 
> removed from the list), which to my understanding are not desirable. In order 
> to implement this, the primary check needs to emit copies of the same 
> diagnostic as if it were the alias check.

Okay, thank you for verifying! I keep reading "copies of the same diagnostic" 
as though you wanted:

  error: forgot to frobble the quux [bugprone-frobble-quux]
  error: forgot to frobble the quux [cert-quux45-cpp]

and going "no, I don't think that's quite what we want." :-)

If it turns out that producing output like `error: forgot to frobble the quux 
[bugprone-frobble-quux, cert-quux45-cpp]` is really difficult, I don't see it 
as being a deal breaker to only emit the primary check name. (Same goes for 
things like `LINT` comments naming the perfect alias vs naming the primary.)

The idea I had for supporting this was: we already register which check is the 
primary and all the names of the aliases, and so we don't need to really run 
the alias check to get its diagnostic output, we could thread some way to go 
from the diagnostics engine back to our registered lists of aliases, and then 
print the alias names manually when emitting the diagnostic for the primary. 
However, I've not tried this idea out and who knows what details I'm missing.




Comment at: clang-tools-extra/clang-tidy/ClangTidyModule.h:25-26
 
+#define CLANG_TIDY_REGISTER_CHECK(Factory, CheckType, CheckName)   
\
+  Factory.registerCheck(CheckName, #CheckType)
+

carlosgalvezp wrote:
> aaron.ballman wrote:
> > I'm not 100% sold that this macro carries its weight. I almost think the 
> > registration code is easier to read without it, but not enough to suggest a 
> > change yet. Curious if others have feelings here.
> I'm not loving it either, to be honest, but I couldn't think of anything 
> better that wouldn't cause developer friction. I've looked into RTTI (banned 
> in LLVM) and hand-made LLVM RRTI, which I think would be quite a burden for 
> check developers since they'd need to add a new enum to the list, implement a 
> getClass() method, etc (IIUC).
> 
> Previous attempts as this problem created a different register function like:
> 
> ```
> Factory.registerCheck("foo-check", misc::PrimaryCheck, "misc-check")
> ```
> 
> Which I don't like much because the developer needs to spend time digging 
> through the code and docs to see what is the name of the check it's aliasing, 
> which is error-prone and can easily get out of sync in the future. 
> 
> With this macro, all of this is automated and always in sync. But again, if 
> someone has some cleverer magic that would work here I'd love to bring it in 
> :) 
> 
> Since replacing existing lines with the macro is a pretty big change, I'll 
> wait until I get OK from reviewers to avoid spending time if in the end we go 
> for another solution.
> With this macro, all of this is automated and always in sync.

Yup, and this is the primary reason I'm not suggesting a change yet. This 
solves the problem we needed to solve, it's just a bit... crufty, maybe? I 
dunno. I would not spend time finding a replacement yet; if someone has a great 
idea, we can consider it then.


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

https://reviews.llvm.org/D114317

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

[PATCH] D102669: [analyzer][ctu] Fix wrong 'multiple definitions' errors caused by space characters in lookup names when parsing the ctu index file

[Balázs Benics 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 rG333d66b09494: [analyzer][ctu] Fix wrong multiple 
definitions errors caused by space… (authored by OikawaKirie, committed 
by steakhal).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102669

Files:
  clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
  clang/include/clang/Basic/DiagnosticCrossTUKinds.td
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  clang/test/Analysis/Inputs/ctu-import.c.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-lookup-name-with-space.cpp
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
  
clang/test/Analysis/Inputs/plist-macros-with-expansion-ctu.c.externalDefMap.txt
  clang/test/Analysis/ctu-inherited-default-ctor.cpp
  clang/test/Analysis/ctu-lookup-name-with-space.cpp
  clang/test/Analysis/func-mapping-test.cpp
  clang/unittests/CrossTU/CrossTranslationUnitTest.cpp

Index: clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
===
--- clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
+++ clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -61,7 +61,7 @@
 ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("index", "txt", IndexFD,
 IndexFileName));
 llvm::ToolOutputFile IndexFile(IndexFileName, IndexFD);
-IndexFile.os() << "c:@F@f#I# " << ASTFileName << "\n";
+IndexFile.os() << "9:c:@F@f#I# " << ASTFileName << "\n";
 IndexFile.os().flush();
 EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
 
Index: clang/test/Analysis/func-mapping-test.cpp
===
--- clang/test/Analysis/func-mapping-test.cpp
+++ clang/test/Analysis/func-mapping-test.cpp
@@ -3,10 +3,10 @@
 int f(int) {
   return 0;
 }
-// CHECK-DAG: c:@F@f#I#
+// CHECK-DAG: 9:c:@F@f#I#
 
 extern const int x = 5;
-// CHECK-DAG: c:@x
+// CHECK-DAG: 4:c:@x
 
 // Non-const variables should not be collected.
 int y = 5;
@@ -18,29 +18,29 @@
   int a;
 };
 extern S const s = {.a = 2};
-// CHECK-DAG: c:@s
+// CHECK-DAG: 4:c:@s
 
 struct SF {
   const int a;
 };
 SF sf = {.a = 2};
-// CHECK-DAG: c:@sf
+// CHECK-DAG: 5:c:@sf
 
 struct SStatic {
   static const int a = 4;
 };
 const int SStatic::a;
-// CHECK-DAG: c:@S@SStatic@a
+// CHECK-DAG: 14:c:@S@SStatic@a
 
 extern int const arr[5] = { 0, 1 };
-// CHECK-DAG: c:@arr
+// CHECK-DAG: 6:c:@arr
 
 union U {
   const int a;
   const unsigned int b;
 };
 U u = {.a = 6};
-// CHECK-DAG: c:@u
+// CHECK-DAG: 4:c:@u
 
 // No USR can be generated for this.
 // Check for no crash in this case.
@@ -48,3 +48,15 @@
   float uf;
   const int ui;
 };
+
+void f(int (*)(char));
+void f(bool (*)(char));
+
+struct G {
+  G() {
+f([](char) -> int { return 42; });
+// CHECK-DAG: 41:c:@S@G@F@G#@Sa@F@operator int (*)(char)#1
+f([](char) -> bool { return true; });
+// CHECK-DAG: 42:c:@S@G@F@G#@Sa@F@operator bool (*)(char)#1
+  }
+};
Index: clang/test/Analysis/ctu-lookup-name-with-space.cpp
===
--- /dev/null
+++ clang/test/Analysis/ctu-lookup-name-with-space.cpp
@@ -0,0 +1,21 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+
+// RUN: echo '"%/S/Inputs/ctu-lookup-name-with-space.cpp" : ["g++", "-c", "%/S/Inputs/ctu-lookup-name-with-space.cpp"]' > %t/invocations.yaml
+// RUN: %clang_extdef_map %S/Inputs/ctu-lookup-name-with-space.cpp -- > %t/externalDefMap.txt
+
+// RUN: cd %t && %clang_cc1 -fsyntax-only -analyze \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=. \
+// RUN:   -analyzer-config ctu-invocation-list=invocations.yaml \
+// RUN:   -verify %s
+
+void importee();
+
+void trigger() {
+  // Call an external function to trigger the parsing process of CTU index.
+  // Refer to file Inputs/ctu-lookup-name-with-space.cpp for more details.
+
+  importee(); // expected-no-diagnostics
+}
Index: clang/test/Analysis/ctu-inherited-default-ctor.cpp
===
--- clang/test/Analysis/ctu-inherited-default-ctor.cpp
+++ clang/test/Analysis/ctu-inherited-default-ctor.cpp
@@ -4,7 +4,7 @@
 // RUN: %clang_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
 // RUN:   -emit-pch -o %t/ctudir/ctu-inherited-default-ctor-other.cpp.ast \
 // RUN:%S/Inputs/ctu-inherited-default-ctor-other.cpp
-// RUN: echo "c:@N@clang@S@DeclContextLookupResult@SingleElementDummyList ctu-inherited-default-ctor-other.cpp.ast" \
+// RUN: echo "59:c:@N@clang@S@DeclContextLookupResult@SingleElementDummyList ctu-inherited-default-ctor-other.cpp.ast" \
 // RUN:   > 

[clang] 333d66b - [analyzer][ctu] Fix wrong 'multiple definitions' errors caused by space characters in lookup names when parsing the ctu index file

[Balazs Benics via cfe-commits]

Author: Ella Ma
Date: 2021-12-16T17:47:59+01:00
New Revision: 333d66b09494b7ebc1a89f2befa79128a56f77e3

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

LOG: [analyzer][ctu] Fix wrong 'multiple definitions' errors caused by space 
characters in lookup names when parsing the ctu index file

This error was found when analyzing MySQL with CTU enabled.

When there are space characters in the lookup name, the current
delimiter searching strategy will make the file path wrongly parsed.
And when two lookup names have the same prefix before their first space
characters, a 'multiple definitions' error will be wrongly reported.

e.g. The lookup names for the two lambda exprs in the test case are
`c:@S@G@F@G#@Sa@F@operator int (*)(char)#1` and
`c:@S@G@F@G#@Sa@F@operator bool (*)(char)#1` respectively. And their
prefixes are both `c:@S@G@F@G#@Sa@F@operator` when using the first space
character as the delimiter.

Solving the problem by adding a length for the lookup name, making the
index items in the format of `USR-Length:USR File-Path`.

Reviewed By: steakhal

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

Added: 
clang/test/Analysis/Inputs/ctu-lookup-name-with-space.cpp
clang/test/Analysis/ctu-lookup-name-with-space.cpp

Modified: 
clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
clang/include/clang/Basic/DiagnosticCrossTUKinds.td
clang/lib/CrossTU/CrossTranslationUnit.cpp
clang/test/Analysis/Inputs/ctu-import.c.externalDefMap.ast-dump.txt
clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.ast-dump.txt
clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt

clang/test/Analysis/Inputs/plist-macros-with-expansion-ctu.c.externalDefMap.txt
clang/test/Analysis/ctu-inherited-default-ctor.cpp
clang/test/Analysis/func-mapping-test.cpp
clang/unittests/CrossTU/CrossTranslationUnitTest.cpp

Removed: 




diff  --git a/clang/docs/analyzer/user-docs/CrossTranslationUnit.rst 
b/clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
index 0e0691b95d5d..0976c9a5b67a 100644
--- a/clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
+++ b/clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
@@ -81,12 +81,12 @@ of `foo.cpp`:
   $
 
 The next step is to create a CTU index file which holds the `USR` name and 
location of external definitions in the
-source files:
+source files in format `: `:
 
 .. code-block:: bash
 
   $ clang-extdef-mapping -p . foo.cpp
-  c:@F@foo# /path/to/your/project/foo.cpp
+  9:c:@F@foo# /path/to/your/project/foo.cpp
   $ clang-extdef-mapping -p . foo.cpp > externalDefMap.txt
 
 We have to modify `externalDefMap.txt` to contain the name of the `.ast` files 
instead of the source files:
@@ -278,12 +278,12 @@ The `invocation list`:
 
 We'd like to analyze `main.cpp` and discover the division by zero bug.
 As we are using On-demand mode, we only need to create a CTU index file which 
holds the `USR` name and location of
-external definitions in the source files:
+external definitions in the source files in format `: 
`:
 
 .. code-block:: bash
 
   $ clang-extdef-mapping -p . foo.cpp
-  c:@F@foo# /path/to/your/project/foo.cpp
+  9:c:@F@foo# /path/to/your/project/foo.cpp
   $ clang-extdef-mapping -p . foo.cpp > externalDefMap.txt
 
 Now everything is available for the CTU analysis.

diff  --git a/clang/include/clang/Basic/DiagnosticCrossTUKinds.td 
b/clang/include/clang/Basic/DiagnosticCrossTUKinds.td
index 4277a3173203..e6ea1956f98a 100644
--- a/clang/include/clang/Basic/DiagnosticCrossTUKinds.td
+++ b/clang/include/clang/Basic/DiagnosticCrossTUKinds.td
@@ -12,8 +12,8 @@ def err_ctu_error_opening : Error<
   "error opening '%0': required by the CrossTU functionality">;
 
 def err_extdefmap_parsing : Error<
-  "error parsing index file: '%0' line: %1 'UniqueID filename' format "
-  "expected">;
+  "error parsing index file: '%0' line: %1 ': ' "
+  "format expected">;
 
 def err_multiple_def_index : Error<
   "multiple definitions are found for the same key in index ">;

diff  --git a/clang/lib/CrossTU/CrossTranslationUnit.cpp 
b/clang/lib/CrossTU/CrossTranslationUnit.cpp
index 0aecad491ecc..cbe07acb76fb 100644
--- a/clang/lib/CrossTU/CrossTranslationUnit.cpp
+++ b/clang/lib/CrossTU/CrossTranslationUnit.cpp
@@ -149,6 +149,35 @@ std::error_code IndexError::convertToErrorCode() const {
   return std::error_code(static_cast(Code), *Category);
 }
 
+/// Parse one line of the input CTU index file.
+///
+/// @param[in]  LineRef The input CTU index item in format
+/// ": ".
+/// @param[out] LookupName  The lookup name in format ":".
+/// @param[out] FilePathThe file path "".
+static bool parseCrossTUIndexItem(StringRef LineRef, StringRef ,
+  StringRef ) 

[PATCH] D114966: [clang][deps] Split stat and file content caches

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added inline comments.



Comment at: 
clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp:200-210
+  // FIXME: This read can fail.
+  // In that case, we should probably update `CacheEntry::MaybeStat`.
+  // However, that is concurrently read at the start of this function
+  // (`needsUpdate` -> `isReadable`), leading to a data race. If we try
+  // to avoid the data race by returning an `error_code` here (without
+  // updating the entry stat value, we can end up attempting to read 
the
+  // file again in the future, which breaks the consistency guarantees

dexonsmith wrote:
> dexonsmith wrote:
> > I'm not quite following this logic. I think it's safe (and important!) to 
> > modify MaybeStat if `read()` fails.
> > 
> > We're in a critical section that either creates and partially initializes 
> > the entry, or incrementally updates it.
> > 
> > In the "creates and partially initializes" case:
> > - All other workers will get nullptr for `Cache.getEntry()` and try to 
> > enter the critical section.
> > - We have just seen a successful MaybeStat value.
> > - `needsRead()` will be `true` since we have not read contents before. We 
> > will immediately try to read.
> > - `read()` should open the original contents and can safely:
> > - on success, update the value for MaybeStat to match the observed size
> > - on failure, drop the value and set the error for MaybeStat to the 
> > observed error
> > - When we leave the critical section, either:
> > - MaybeStat stores an error; no thread will enter the critical section 
> > again
> > - OriginalContents are initialized and `needsRead()` returns false
> > 
> > In the "incrementally updates" case:
> > - `needsRead()` returns false so `read()` will not be called
> > 
> The key property being the last bullet of the first case: that the "create 
> and partially initializes" case guarantees that either `MaybeStat` stores an 
> error (`isReadable()` is false) or `OriginalContents` is initialized 
> (`needsRead()` returns false).
> 
> 
> 
> I think I've found the part I was missing: that this critical section is for 
> a SharedCacheFileEntry (associated with a filename), but the 
> `OriginalContents` is a field on a CachedFileContents which could apply to 
> other UIDs (and `SharedCache.getFileContents()` is actually "get or create"). 
> Since this commit creates the UID map, seems like maybe the race gets worse 
> in this commit? (Not sure)
> 
> 
> 
> Another problem: the UID can change between the first stat above (call to 
> `getUnderlyingFS().status(Filename)`) and the one inside `read()` if another 
> process is writing at the same time. We can't trust the UID mapping from the 
> first `status()` call unless content already exists for that UID.
> 
> I think to avoid this race you need to delay creating "UID to content" map 
> entry until there is the result of a successful `read()` to store.
> 
> I'll describe an algorithm that I think is fairly clean that handles this. 
> I'm using different data structure names to avoid confusion since I've broken 
> it down a little differently:
> - ReadResult: stat (for directories) OR error and uid (failed read) OR stat 
> and content (and optional minimized content and pp ranges, and a way to 
> update them atomically))
> - FilenameMap: map from Filename to `ReadResult*` (shared and sharded; 
> mirrored locally in each worker)
> - UIDMap: map from UID to `ReadResult*` (shared and sharded; probably no 
> local mirror)
> 
> And here's the algorithm:
> ```
> lang=c++
> // Top-level API: get the entry/result for some filename.
> ErrorOr getOrCreateResult(StringRef Filename) {
>   if (ReadResult *Result = lookupEntryForFilename(Filename))
> return minimizeIfNecessary(*Result, ShouldMinimize);
>   if (ErrorOr Result = computeAndStoreResult(Filename))
> return minimizeIfNecessary(*Result, ShouldMinimize);
>   else
> return Result.getError();
> }
> // Compute and store an entry/result for some filename. Returned result
> // has in-sync stat+read info (assuming read was successful).
> ErrorOr computeAndStoreResult(StringRef Filename) {
>   ErrorOr Stat = UnderlyingFS->status(Filename);
>   if (!Stat)
> return Stat.getError(); // Can't cache missing files.
>   if (ReadResult *Result = lookupEntryForUID(Stat->UID))
> return storeFilenameEntry(Filename, *Result); // UID already known.
>   // UID not known. Compute a ReadResult.
>   //
>   // Unless this is a directory (where we don't need to go back to the FS),
>   // ignore existing 'Stat' because without an open file descriptor the UID
>   // could change.
>   Optional Result;
>   if (Stat->isDirectory())
> Result = ReadResult(*Stat);
>   else if (ErrorOr MaybeResult = computeReadResult(Filename))
> Result = std::move(*MaybeResult);
>   else
> return MaybeResult.getError(); // File disappeared...
>   // Store 

[PATCH] D115817: [clang] Cleanup unneeded Function nullptr checks [NFC]

[Mike Rice 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 rG2d0bf1439727: [clang] Cleanup unneeded Function nullptr 
checks [NFC] (authored by mikerice).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115817

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp


Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -1298,47 +1298,44 @@
 
 void CodeGenFunction::GenerateCode(GlobalDecl GD, llvm::Function *Fn,
const CGFunctionInfo ) {
+  assert(Fn && "generating code for null Function");
   const FunctionDecl *FD = cast(GD.getDecl());
   CurGD = GD;
 
   FunctionArgList Args;
   QualType ResTy = BuildFunctionArgList(GD, Args);
 
-  // When generating code for a builtin with an inline declaration, use a
-  // mangled name to hold the actual body, while keeping an external definition
-  // in case the function pointer is referenced somewhere.
-  if (Fn) {
-if (FD->isInlineBuiltinDeclaration()) {
-  std::string FDInlineName = (Fn->getName() + ".inline").str();
-  llvm::Module *M = Fn->getParent();
-  llvm::Function *Clone = M->getFunction(FDInlineName);
-  if (!Clone) {
-Clone = llvm::Function::Create(Fn->getFunctionType(),
-   llvm::GlobalValue::InternalLinkage,
-   Fn->getAddressSpace(), FDInlineName, M);
-Clone->addFnAttr(llvm::Attribute::AlwaysInline);
-  }
-  Fn->setLinkage(llvm::GlobalValue::ExternalLinkage);
-  Fn = Clone;
+  if (FD->isInlineBuiltinDeclaration()) {
+// When generating code for a builtin with an inline declaration, use a
+// mangled name to hold the actual body, while keeping an external
+// definition in case the function pointer is referenced somewhere.
+std::string FDInlineName = (Fn->getName() + ".inline").str();
+llvm::Module *M = Fn->getParent();
+llvm::Function *Clone = M->getFunction(FDInlineName);
+if (!Clone) {
+  Clone = llvm::Function::Create(Fn->getFunctionType(),
+ llvm::GlobalValue::InternalLinkage,
+ Fn->getAddressSpace(), FDInlineName, M);
+  Clone->addFnAttr(llvm::Attribute::AlwaysInline);
 }
-
+Fn->setLinkage(llvm::GlobalValue::ExternalLinkage);
+Fn = Clone;
+  } else {
 // Detect the unusual situation where an inline version is shadowed by a
 // non-inline version. In that case we should pick the external one
 // everywhere. That's GCC behavior too. Unfortunately, I cannot find a way
 // to detect that situation before we reach codegen, so do some late
 // replacement.
-else {
-  for (const FunctionDecl *PD = FD->getPreviousDecl(); PD;
-   PD = PD->getPreviousDecl()) {
-if (LLVM_UNLIKELY(PD->isInlineBuiltinDeclaration())) {
-  std::string FDInlineName = (Fn->getName() + ".inline").str();
-  llvm::Module *M = Fn->getParent();
-  if (llvm::Function *Clone = M->getFunction(FDInlineName)) {
-Clone->replaceAllUsesWith(Fn);
-Clone->eraseFromParent();
-  }
-  break;
+for (const FunctionDecl *PD = FD->getPreviousDecl(); PD;
+ PD = PD->getPreviousDecl()) {
+  if (LLVM_UNLIKELY(PD->isInlineBuiltinDeclaration())) {
+std::string FDInlineName = (Fn->getName() + ".inline").str();
+llvm::Module *M = Fn->getParent();
+if (llvm::Function *Clone = M->getFunction(FDInlineName)) {
+  Clone->replaceAllUsesWith(Fn);
+  Clone->eraseFromParent();
 }
+break;
   }
 }
   }
@@ -1347,8 +1344,7 @@
   if (FD->hasAttr()) {
 // Clear non-distinct debug info that was possibly attached to the function
 // due to an earlier declaration without the nodebug attribute
-if (Fn)
-  Fn->setSubprogram(nullptr);
+Fn->setSubprogram(nullptr);
 // Disable debug info indefinitely for this function
 DebugInfo = nullptr;
   }


Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -1298,47 +1298,44 @@
 
 void CodeGenFunction::GenerateCode(GlobalDecl GD, llvm::Function *Fn,
const CGFunctionInfo ) {
+  assert(Fn && "generating code for null Function");
   const FunctionDecl *FD = cast(GD.getDecl());
   CurGD = GD;
 
   FunctionArgList Args;
   QualType ResTy = BuildFunctionArgList(GD, Args);
 
-  // When generating code for a builtin with an inline declaration, use a
-  // 

[clang] 2d0bf14 - [clang] Cleanup unneeded Function nullptr checks [NFC]

[Mike Rice via cfe-commits]

Author: Mike Rice
Date: 2021-12-16T08:28:10-08:00
New Revision: 2d0bf14397276d1ece9db1eb1858a644aad77ff2

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

LOG: [clang] Cleanup unneeded Function nullptr checks [NFC]

Add an assert and avoid unneeded checks of Fn in
CodeGenFunction::GenerateCode.

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

Added: 


Modified: 
clang/lib/CodeGen/CodeGenFunction.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CodeGenFunction.cpp 
b/clang/lib/CodeGen/CodeGenFunction.cpp
index ed43adfab954..f14e4c33e91a 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -1298,47 +1298,44 @@ QualType 
CodeGenFunction::BuildFunctionArgList(GlobalDecl GD,
 
 void CodeGenFunction::GenerateCode(GlobalDecl GD, llvm::Function *Fn,
const CGFunctionInfo ) {
+  assert(Fn && "generating code for null Function");
   const FunctionDecl *FD = cast(GD.getDecl());
   CurGD = GD;
 
   FunctionArgList Args;
   QualType ResTy = BuildFunctionArgList(GD, Args);
 
-  // When generating code for a builtin with an inline declaration, use a
-  // mangled name to hold the actual body, while keeping an external definition
-  // in case the function pointer is referenced somewhere.
-  if (Fn) {
-if (FD->isInlineBuiltinDeclaration()) {
-  std::string FDInlineName = (Fn->getName() + ".inline").str();
-  llvm::Module *M = Fn->getParent();
-  llvm::Function *Clone = M->getFunction(FDInlineName);
-  if (!Clone) {
-Clone = llvm::Function::Create(Fn->getFunctionType(),
-   llvm::GlobalValue::InternalLinkage,
-   Fn->getAddressSpace(), FDInlineName, M);
-Clone->addFnAttr(llvm::Attribute::AlwaysInline);
-  }
-  Fn->setLinkage(llvm::GlobalValue::ExternalLinkage);
-  Fn = Clone;
+  if (FD->isInlineBuiltinDeclaration()) {
+// When generating code for a builtin with an inline declaration, use a
+// mangled name to hold the actual body, while keeping an external
+// definition in case the function pointer is referenced somewhere.
+std::string FDInlineName = (Fn->getName() + ".inline").str();
+llvm::Module *M = Fn->getParent();
+llvm::Function *Clone = M->getFunction(FDInlineName);
+if (!Clone) {
+  Clone = llvm::Function::Create(Fn->getFunctionType(),
+ llvm::GlobalValue::InternalLinkage,
+ Fn->getAddressSpace(), FDInlineName, M);
+  Clone->addFnAttr(llvm::Attribute::AlwaysInline);
 }
-
+Fn->setLinkage(llvm::GlobalValue::ExternalLinkage);
+Fn = Clone;
+  } else {
 // Detect the unusual situation where an inline version is shadowed by a
 // non-inline version. In that case we should pick the external one
 // everywhere. That's GCC behavior too. Unfortunately, I cannot find a way
 // to detect that situation before we reach codegen, so do some late
 // replacement.
-else {
-  for (const FunctionDecl *PD = FD->getPreviousDecl(); PD;
-   PD = PD->getPreviousDecl()) {
-if (LLVM_UNLIKELY(PD->isInlineBuiltinDeclaration())) {
-  std::string FDInlineName = (Fn->getName() + ".inline").str();
-  llvm::Module *M = Fn->getParent();
-  if (llvm::Function *Clone = M->getFunction(FDInlineName)) {
-Clone->replaceAllUsesWith(Fn);
-Clone->eraseFromParent();
-  }
-  break;
+for (const FunctionDecl *PD = FD->getPreviousDecl(); PD;
+ PD = PD->getPreviousDecl()) {
+  if (LLVM_UNLIKELY(PD->isInlineBuiltinDeclaration())) {
+std::string FDInlineName = (Fn->getName() + ".inline").str();
+llvm::Module *M = Fn->getParent();
+if (llvm::Function *Clone = M->getFunction(FDInlineName)) {
+  Clone->replaceAllUsesWith(Fn);
+  Clone->eraseFromParent();
 }
+break;
   }
 }
   }
@@ -1347,8 +1344,7 @@ void CodeGenFunction::GenerateCode(GlobalDecl GD, 
llvm::Function *Fn,
   if (FD->hasAttr()) {
 // Clear non-distinct debug info that was possibly attached to the function
 // due to an earlier declaration without the nodebug attribute
-if (Fn)
-  Fn->setSubprogram(nullptr);
+Fn->setSubprogram(nullptr);
 // Disable debug info indefinitely for this function
 DebugInfo = nullptr;
   }



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

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thanks for this new check!

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

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

We should also circle back with the CERT authors for updates on missing entries 
in their tables once we've figured out what is missing, and on the terminology 
concerns of referring to these as "deprecated or obsolescent" functions.




Comment at: clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp:33
+  "::wcsrtombs", "::wcstok", "::wcstombs", "::wctomb", "::wmemcpy",
+  "::wmemmove", "::wprintf", "::wscanf", "::strlen"));
+  Finder->addMatcher(

steakhal wrote:
> Why is `strlen` an //obsolescent// function? I don't feel it justified, even 
> if there was a [[ 
> https://wiki.sei.cmu.edu/confluence/display/c/MSC24-C.+Do+not+use+deprecated+or+obsolescent+functions?focusedCommentId=88019519#comment-88019519
>  | comment ]] about it on the cert page.
I think its exclusion from the CERT page was an oversight. `strlen_s()` is a 
secure replacement for `strlen()` and it would be weird to leave it off the 
list.



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

This comment is not accurate. `gets_s()` is a secure replacement for `gets()`.



Comment at: 
clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp:49-59
+  "::asctime", "::bsearch", "::ctime", "::fopen", "::fprintf", "::freopen",
+  "::fscanf", "::fwprintf", "::fwscanf", "::getenv", "::gmtime",
+  "::localtime", "::mbsrtowcs", "::mbstowcs", "::memcpy", "::memmove",
+  "::printf", "::qsort", "::snprintf", "::sprintf", "::sscanf", "::strcat",
+  "::strcpy", "::strerror", "::strncat", "::strncpy", "::strtok",
+  "::swprintf", "::swscanf", "::vfprintf", "::vfscanf", "::vfwprintf",
+  "::vfwscanf", "::vprintf", "::vscanf", "::vsnprintf", "::vsprintf",

This list appears to be missing quite a few functions with secure replacements 
in Annex K. For example: `tmpfile_s`, `tmpnam_s`, `strerrorlen_s`, 
`strlen_s`... can you check the list against the actual Annex K, as it seems 
the CERT recommendation is still out of date.



Comment at: 
clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp:84-86
+diag(Range.getBegin(),
+ "function '%0' is deprecated as of C99, removed from C11.")
+<< Deprecated->getName() << Range;

Fixed a few nits with the code, but `gets()` was never deprecated, so the 
message is not correct (it was present in C99 and removed in C11 with no 
deprecation period). I think it may be better to say "function %0 was removed 
in C11".



Comment at: clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp:103
+
+  diag(Range.getBegin(), "function '%0' %1; '%2' should be used instead.")
+  << FunctionName << getRationale(FunctionName)





Comment at: clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.h:18
+
+/// Checks for deprecated and obsolescent function listed in
+/// CERT C Coding Standard Recommendation MSC24 - C. For the listed functions,

The terminology used in the CERT recommendation is pretty unfortunate and I 
don't think we should replicate it. Many of these are *not* deprecated or 
obsolescent functions and calling them that will confuse users. The crux of the 
CERT recommendation is that these functions have better replacements in more 
modern versions of C. So I would probably try to focus our diagnostics and 
documentation around modernization rather than deprecation.

FWIW, this is feedback that should also go onto the CERT recommendation itself. 
I noticed someone already observed the same thing: 
https://wiki.sei.cmu.edu/confluence/display/c/MSC24-C.+Do+not+use+deprecated+or+obsolescent+functions?focusedCommentId=215482395#comment-215482395


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

https://reviews.llvm.org/D91000

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

[PATCH] D99134: Lambdas are not necessarily locals. This resolves DR48250.

[David Stone via Phabricator via cfe-commits]

davidstone added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99134

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

[PATCH] D114317: [clang-tidy][WIP] Do not run perfect alias checks

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added inline comments.



Comment at: clang-tools-extra/clang-tidy/ClangTidyModule.h:25-26
 
+#define CLANG_TIDY_REGISTER_CHECK(Factory, CheckType, CheckName)   
\
+  Factory.registerCheck(CheckName, #CheckType)
+

aaron.ballman wrote:
> I'm not 100% sold that this macro carries its weight. I almost think the 
> registration code is easier to read without it, but not enough to suggest a 
> change yet. Curious if others have feelings here.
I'm not loving it either, to be honest, but I couldn't think of anything better 
that wouldn't cause developer friction. I've looked into RTTI (banned in LLVM) 
and hand-made LLVM RRTI, which I think would be quite a burden for check 
developers since they'd need to add a new enum to the list, implement a 
getClass() method, etc (IIUC).

Previous attempts as this problem created a different register function like:

```
Factory.registerCheck("foo-check", misc::PrimaryCheck, "misc-check")
```

Which I don't like much because the developer needs to spend time digging 
through the code and docs to see what is the name of the check it's aliasing, 
which is error-prone and can easily get out of sync in the future. 

With this macro, all of this is automated and always in sync. But again, if 
someone has some cleverer magic that would work here I'd love to bring it in :) 

Since replacing existing lines with the macro is a pretty big change, I'll wait 
until I get OK from reviewers to avoid spending time if in the end we go for 
another solution.


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

https://reviews.llvm.org/D114317

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

[PATCH] D114317: [clang-tidy][WIP] Do not run perfect alias checks

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

> me pretty far behind on reviews

Wow, that's a lot to review! I will keep it in mind and hopefully only ping you 
when I have a nice final design ready to go. Really appreciate your early 
feedback!

> I'd like to make sure I'm on the same page as you.

Yes, exactly, from the RFC we agreed that the desired outcome of this patch 
would be that the same diagnostics and fixits are emitted, regardless of the 
alias checks actually running. I.e. this patch should not have any visible 
effects towards the user (other than faster runtime). The patch, as it is right 
now _will_ eliminate those diagnostics (since the alias checks are removed from 
the list), which to my understanding are not desirable. In order to implement 
this, the primary check needs to emit copies of the same diagnostic as if it 
were the alias check.


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

https://reviews.llvm.org/D114317

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

[PATCH] D110742: [OpenCL] Add pure attributes to vload builtins

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Thanks! Bots look happy again :)


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

https://reviews.llvm.org/D110742

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

[clang] 2d89382 - [CodeGen] Avoid more pointer element type accesses

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2021-12-16T16:34:09+01:00
New Revision: 2d89382b5a21423180c7860163bb0e4cd3082e4b

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

LOG: [CodeGen] Avoid more pointer element type accesses

This is enough to build sqlite3 with opaque pointers.

Added: 


Modified: 
clang/lib/CodeGen/CGCall.cpp
clang/lib/CodeGen/CGDecl.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 8ecef6ab9782..28fc75ba466e 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -2686,8 +2686,8 @@ void CodeGenFunction::EmitFunctionProlog(const 
CGFunctionInfo ,
 case ABIArgInfo::Indirect:
 case ABIArgInfo::IndirectAliased: {
   assert(NumIRArgs == 1);
-  Address ParamAddr =
-  Address(Fn->getArg(FirstIRArg), ArgI.getIndirectAlign());
+  Address ParamAddr = Address(Fn->getArg(FirstIRArg), 
ConvertTypeForMem(Ty),
+  ArgI.getIndirectAlign());
 
   if (!hasScalarEvaluationKind(Ty)) {
 // Aggregates and complex variables are accessed by reference. All we
@@ -4869,7 +4869,8 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo 
,
   I->copyInto(*this, AI);
 } else {
   // Skip the extra memcpy call.
-  auto *T = V->getType()->getPointerElementType()->getPointerTo(
+  auto *T = llvm::PointerType::getWithSamePointeeType(
+  cast(V->getType()),
   CGM.getDataLayout().getAllocaAddrSpace());
   IRCallArgs[FirstIRArg] = getTargetHooks().performAddrSpaceCast(
   *this, V, LangAS::Default, CGM.getASTAllocaAddressSpace(), T,

diff  --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp
index 2d1f229108d6..e09279c1d455 100644
--- a/clang/lib/CodeGen/CGDecl.cpp
+++ b/clang/lib/CodeGen/CGDecl.cpp
@@ -1193,7 +1193,7 @@ static void emitStoresForConstant(CodeGenModule , 
const VarDecl ,
 bool valueAlreadyCorrect =
 constant->isNullValue() || isa(constant);
 if (!valueAlreadyCorrect) {
-  Loc = Builder.CreateBitCast(Loc, 
Ty->getPointerTo(Loc.getAddressSpace()));
+  Loc = Builder.CreateElementBitCast(Loc, Ty);
   emitStoresForInitAfterBZero(CGM, constant, Loc, isVolatile, Builder,
   IsAutoInit);
 }



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

[PATCH] D110742: [OpenCL] Add pure attributes to vload builtins

[Stuart Brady via Phabricator via cfe-commits]

stuart updated this revision to Diff 394878.
stuart added a comment.

I've updated the review to include test changes that are required for 
check-clang-semaopencl to pass.


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

https://reviews.llvm.org/D110742

Files:
  clang/lib/Headers/opencl-c.h
  clang/lib/Sema/OpenCLBuiltins.td
  clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl

Index: clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
===
--- clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
+++ clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
@@ -46,12 +46,19 @@
 typedef char char2 __attribute__((ext_vector_type(2)));
 typedef char char4 __attribute__((ext_vector_type(4)));
 typedef uchar uchar4 __attribute__((ext_vector_type(4)));
+typedef uchar uchar16 __attribute__((ext_vector_type(16)));
 typedef float float4 __attribute__((ext_vector_type(4)));
+typedef float float16 __attribute__((ext_vector_type(16)));
 typedef half half4 __attribute__((ext_vector_type(4)));
 typedef int int2 __attribute__((ext_vector_type(2)));
 typedef int int4 __attribute__((ext_vector_type(4)));
+typedef uint uint2 __attribute__((ext_vector_type(2)));
 typedef uint uint4 __attribute__((ext_vector_type(4)));
 typedef long long2 __attribute__((ext_vector_type(2)));
+typedef long long8 __attribute__((ext_vector_type(8)));
+typedef ulong ulong4 __attribute__((ext_vector_type(4)));
+typedef short short16 __attribute__((ext_vector_type(16)));
+typedef ushort ushort3 __attribute__((ext_vector_type(3)));
 
 typedef int clk_profiling_info;
 #define CLK_PROFILING_COMMAND_EXEC_TIME 0x1
@@ -284,18 +291,27 @@
   global void *global_p;
   private void *private_p;
   size_t s;
+  ulong4 ul4;
+  short16 s16;
+#if __OPENCL_C_VERSION__ >= CL_VERSION_2_0
+  ushort3 us3;
+  uchar16 uc16;
+#endif
+  long8 l8;
+  uint2 ui2;
+  float16 f16;
 
-  vload4(s, (const __constant ulong *) constant_p);
-  vload16(s, (const __constant short *) constant_p);
+  ul4 = vload4(s, (const __constant ulong *) constant_p);
+  s16 = vload16(s, (const __constant short *) constant_p);
 
 #if __OPENCL_C_VERSION__ >= CL_VERSION_2_0
-  vload3(s, (const __generic ushort *) generic_p);
-  vload16(s, (const __generic uchar *) generic_p);
+  us3 = vload3(s, (const __generic ushort *) generic_p);
+  uc16 = vload16(s, (const __generic uchar *) generic_p);
 #endif
 
-  vload8(s, (const __global long *) global_p);
-  vload2(s, (const __local uint *) local_p);
-  vload16(s, (const __private float *) private_p);
+  l8 = vload8(s, (const __global long *) global_p);
+  ui2 = vload2(s, (const __local uint *) local_p);
+  f16 = vload16(s, (const __private float *) private_p);
 }
 
 kernel void basic_work_item() {
Index: clang/lib/Sema/OpenCLBuiltins.td
===
--- clang/lib/Sema/OpenCLBuiltins.td
+++ clang/lib/Sema/OpenCLBuiltins.td
@@ -806,17 +806,17 @@
   foreach AS = addrspaces in {
 foreach VSize = [2, 3, 4, 8, 16] in {
   foreach name = ["vload" # VSize] in {
-def : Builtin, Size, PointerType, AS>]>;
-def : Builtin, Size, PointerType, AS>]>;
-def : Builtin, Size, PointerType, AS>]>;
-def : Builtin, Size, PointerType, AS>]>;
-def : Builtin, Size, PointerType, AS>]>;
-def : Builtin, Size, PointerType, AS>]>;
-def : Builtin, Size, PointerType, AS>]>;
-def : Builtin, Size, PointerType, AS>]>;
-def : Builtin, Size, PointerType, AS>]>;
-def : Builtin, Size, PointerType, AS>]>;
-def : Builtin, Size, PointerType, AS>]>;
+def : Builtin, Size, PointerType, AS>], Attr.Pure>;
+def : Builtin, Size, PointerType, AS>], Attr.Pure>;
+def : Builtin, Size, PointerType, AS>], Attr.Pure>;
+def : Builtin, Size, PointerType, AS>], Attr.Pure>;
+def : Builtin, Size, PointerType, AS>], Attr.Pure>;
+def : Builtin, Size, PointerType, AS>], Attr.Pure>;
+def : Builtin, Size, PointerType, AS>], Attr.Pure>;
+def : Builtin, Size, PointerType, AS>], Attr.Pure>;
+def : Builtin, Size, PointerType, AS>], Attr.Pure>;
+def : Builtin, Size, PointerType, AS>], Attr.Pure>;
+def : Builtin, Size, PointerType, AS>], Attr.Pure>;
   }
   if defStores then {
 foreach name = ["vstore" # VSize] in {
@@ -848,10 +848,10 @@
 
 multiclass VloadVstoreHalf addrspaces, bit defStores> {
   foreach AS = addrspaces in {
-def : Builtin<"vload_half", [Float, Size, PointerType, AS>]>;
+def : Builtin<"vload_half", [Float, Size, PointerType, AS>], Attr.Pure>;
 foreach VSize = [2, 3, 4, 8, 16] in {
   foreach name = ["vload_half" # VSize, "vloada_half" # VSize] in {
-def : Builtin, Size, PointerType, AS>]>;
+def : Builtin, Size, PointerType, AS>], Attr.Pure>;
   }
 }
 if defStores then {
@@ -877,7 +877,7 @@
 let MinVersion = CL20 in {
   defm : 

[PATCH] D112081: Define __STDC_NO_THREADS__ when targeting windows-msvc (PR48704)

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

Thanks for the reminder! Landed now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112081

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

[PATCH] D112081: Define __STDC_NO_THREADS__ when targeting windows-msvc (PR48704)

[Hans Wennborg via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbbc690c57213: Define __STDC_NO_THREADS__ when targeting 
windows-msvc (PR48704) (authored by hans).

Changed prior to commit:
  https://reviews.llvm.org/D112081?vs=380704=394877#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112081

Files:
  clang/lib/Basic/Targets/OSTargets.cpp
  clang/test/Preprocessor/init.c


Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -195,6 +195,7 @@
 // MSEXT-NOT:#define _NATIVE_WCHAR_T_DEFINED 1
 // MSEXT-NOT:#define _WCHAR_T_DEFINED 1
 // MSEXT:#define _MSVC_EXECUTION_CHARACTER_SET 65001
+// MSEXT:#define __STDC_NO_THREADS__ 1
 //
 //
 // RUN: %clang_cc1 -x c++ -fms-extensions -triple i686-pc-win32 -E -dM < 
/dev/null | FileCheck -match-full-lines -check-prefix MSEXT-CXX %s
Index: clang/lib/Basic/Targets/OSTargets.cpp
===
--- clang/lib/Basic/Targets/OSTargets.cpp
+++ clang/lib/Basic/Targets/OSTargets.cpp
@@ -203,6 +203,7 @@
   }
 
   Builder.defineMacro("_INTEGRAL_MAX_BITS", "64");
+  Builder.defineMacro("__STDC_NO_THREADS__");
 
   // Starting with VS 2022 17.1, MSVC predefines the below macro to inform
   // users of the execution character set defined at compile time.


Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -195,6 +195,7 @@
 // MSEXT-NOT:#define _NATIVE_WCHAR_T_DEFINED 1
 // MSEXT-NOT:#define _WCHAR_T_DEFINED 1
 // MSEXT:#define _MSVC_EXECUTION_CHARACTER_SET 65001
+// MSEXT:#define __STDC_NO_THREADS__ 1
 //
 //
 // RUN: %clang_cc1 -x c++ -fms-extensions -triple i686-pc-win32 -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix MSEXT-CXX %s
Index: clang/lib/Basic/Targets/OSTargets.cpp
===
--- clang/lib/Basic/Targets/OSTargets.cpp
+++ clang/lib/Basic/Targets/OSTargets.cpp
@@ -203,6 +203,7 @@
   }
 
   Builder.defineMacro("_INTEGRAL_MAX_BITS", "64");
+  Builder.defineMacro("__STDC_NO_THREADS__");
 
   // Starting with VS 2022 17.1, MSVC predefines the below macro to inform
   // users of the execution character set defined at compile time.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] bbc690c - Define __STDC_NO_THREADS__ when targeting windows-msvc (PR48704)

[Hans Wennborg via cfe-commits]

Author: Hans Wennborg
Date: 2021-12-16T16:30:06+01:00
New Revision: bbc690c57213054907284d8964dc0487d38fc57a

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

LOG: Define __STDC_NO_THREADS__ when targeting windows-msvc (PR48704)

MSVC's libc doesn't provide thread.h, so we should set the macro to
indicate that.

We could just set it in C mode, but I noticed that Darwin sets it
unconditionally, so perhaps we should do the same here.

Differential revision: https://reviews.llvm.org/D112081

Added: 


Modified: 
clang/lib/Basic/Targets/OSTargets.cpp
clang/test/Preprocessor/init.c

Removed: 




diff  --git a/clang/lib/Basic/Targets/OSTargets.cpp 
b/clang/lib/Basic/Targets/OSTargets.cpp
index 695cbafe36557..f8f12daaa0722 100644
--- a/clang/lib/Basic/Targets/OSTargets.cpp
+++ b/clang/lib/Basic/Targets/OSTargets.cpp
@@ -203,6 +203,7 @@ static void addVisualCDefines(const LangOptions , 
MacroBuilder ) {
   }
 
   Builder.defineMacro("_INTEGRAL_MAX_BITS", "64");
+  Builder.defineMacro("__STDC_NO_THREADS__");
 
   // Starting with VS 2022 17.1, MSVC predefines the below macro to inform
   // users of the execution character set defined at compile time.

diff  --git a/clang/test/Preprocessor/init.c b/clang/test/Preprocessor/init.c
index 52c33d9b8b7a5..7d838413b8513 100644
--- a/clang/test/Preprocessor/init.c
+++ b/clang/test/Preprocessor/init.c
@@ -195,6 +195,7 @@
 // MSEXT-NOT:#define _NATIVE_WCHAR_T_DEFINED 1
 // MSEXT-NOT:#define _WCHAR_T_DEFINED 1
 // MSEXT:#define _MSVC_EXECUTION_CHARACTER_SET 65001
+// MSEXT:#define __STDC_NO_THREADS__ 1
 //
 //
 // RUN: %clang_cc1 -x c++ -fms-extensions -triple i686-pc-win32 -E -dM < 
/dev/null | FileCheck -match-full-lines -check-prefix MSEXT-CXX %s



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

[clang] 8285522 - [CodeGen] Always update map entry after adding initializer

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2021-12-16T16:29:35+01:00
New Revision: 828552201420720ff52c9a27ea4e8f3697f1abc1

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

LOG: [CodeGen] Always update map entry after adding initializer

With opaque pointers the pointer cast may be a no-op, such that
var and castedAddr are the same. However, we still need to update
the map entry as the underlying global changed. We could explicitly
check whether the global was replaced, but we may as well just
always update the entry.

Added: 


Modified: 
clang/lib/CodeGen/CGDecl.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp
index f6b758ba30a0..2d1f229108d6 100644
--- a/clang/lib/CodeGen/CGDecl.cpp
+++ b/clang/lib/CodeGen/CGDecl.cpp
@@ -405,8 +405,8 @@ void CodeGenFunction::EmitStaticVarDecl(const VarDecl ,
 
   // Store into LocalDeclMap before generating initializer to handle
   // circular references.
-  setAddrOfLocalVar(
-  , Address(addr, ConvertTypeForMem(D.getType()), alignment));
+  llvm::Type *elemTy = ConvertTypeForMem(D.getType());
+  setAddrOfLocalVar(, Address(addr, elemTy, alignment));
 
   // We can't have a VLA here, but we can have a pointer to a VLA,
   // even though that doesn't really make any sense.
@@ -459,8 +459,7 @@ void CodeGenFunction::EmitStaticVarDecl(const VarDecl ,
   // RAUW's the GV uses of this constant will be invalid.
   llvm::Constant *castedAddr =
 llvm::ConstantExpr::getPointerBitCastOrAddrSpaceCast(var, expectedType);
-  if (var != castedAddr)
-LocalDeclMap.find()->second = Address(castedAddr, alignment);
+  LocalDeclMap.find()->second = Address(castedAddr, elemTy, alignment);
   CGM.setStaticLocalDeclAddress(, castedAddr);
 
   CGM.getSanitizerMetadata()->reportGlobalToASan(var, D);



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

[PATCH] D115879: [clang-format] extern with new line brace without indentation

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: HazardyKnusperkeks, curdeius, owenpan, 
MarcusJohnson91.
MyDeveloperDay added projects: clang, clang-format.
MyDeveloperDay requested review of this revision.

https://github.com/llvm/llvm-project/issues/49804

Interaction between IndentExternBlock and AfterExternBlock means you cannot 
have AfterExternBlock = true and IndentExternBlock = NoIndent/Indent

This patch resolves that

  BraceWrapping:
AfterExternBlock: true
  IndentExternBlock: AfterExternBlock

Fixes: #49804


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115879

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3811,6 +3811,26 @@
"int foo16();\n"
"}",
Style);
+
+  Style.IndentExternBlock = FormatStyle::IEBS_AfterExternBlock;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterExternBlock = true;
+  Style.IndentExternBlock = FormatStyle::IEBS_NoIndent;
+  verifyFormat("extern \"C\"\n{ /*13*/\n}", Style);
+  verifyFormat("extern \"C\"\n{\n"
+   "int foo14();\n"
+   "}",
+   Style);
+
+  Style.IndentExternBlock = FormatStyle::IEBS_AfterExternBlock;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterExternBlock = true;
+  Style.IndentExternBlock = FormatStyle::IEBS_Indent;
+  verifyFormat("extern \"C\"\n{ /*13*/\n}", Style);
+  verifyFormat("extern \"C\"\n{\n"
+   "  int foo14();\n"
+   "}",
+   Style);
 }
 
 TEST_F(FormatTest, FormatsInlineASM) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1282,10 +1282,10 @@
 if (FormatTok->Tok.is(tok::string_literal)) {
   nextToken();
   if (FormatTok->Tok.is(tok::l_brace)) {
+if (Style.BraceWrapping.AfterExternBlock) {
+  addUnwrappedLine();
+}
 if (!Style.IndentExternBlock) {
-  if (Style.BraceWrapping.AfterExternBlock) {
-addUnwrappedLine();
-  }
   unsigned AddLevels = Style.BraceWrapping.AfterExternBlock ? 1u : 0u;
   parseBlock(/*MustBeDeclaration=*/true, AddLevels);
 } else {


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3811,6 +3811,26 @@
"int foo16();\n"
"}",
Style);
+
+  Style.IndentExternBlock = FormatStyle::IEBS_AfterExternBlock;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterExternBlock = true;
+  Style.IndentExternBlock = FormatStyle::IEBS_NoIndent;
+  verifyFormat("extern \"C\"\n{ /*13*/\n}", Style);
+  verifyFormat("extern \"C\"\n{\n"
+   "int foo14();\n"
+   "}",
+   Style);
+
+  Style.IndentExternBlock = FormatStyle::IEBS_AfterExternBlock;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterExternBlock = true;
+  Style.IndentExternBlock = FormatStyle::IEBS_Indent;
+  verifyFormat("extern \"C\"\n{ /*13*/\n}", Style);
+  verifyFormat("extern \"C\"\n{\n"
+   "  int foo14();\n"
+   "}",
+   Style);
 }
 
 TEST_F(FormatTest, FormatsInlineASM) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1282,10 +1282,10 @@
 if (FormatTok->Tok.is(tok::string_literal)) {
   nextToken();
   if (FormatTok->Tok.is(tok::l_brace)) {
+if (Style.BraceWrapping.AfterExternBlock) {
+  addUnwrappedLine();
+}
 if (!Style.IndentExternBlock) {
-  if (Style.BraceWrapping.AfterExternBlock) {
-addUnwrappedLine();
-  }
   unsigned AddLevels = Style.BraceWrapping.AfterExternBlock ? 1u : 0u;
   parseBlock(/*MustBeDeclaration=*/true, AddLevels);
 } else {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104478: [clang] p2085 out-of-class comparison operator defaulting

[Nathan Sidwell via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5fbe21a7748f: [clang] p2085 out-of-class comparison operator 
defaulting (authored by urnathan).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D104478?vs=387283=394873#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104478

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
  clang/test/CodeGenCXX/p2085.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1006,7 +1006,7 @@
   
   
 https://wg21.link/p2085r0;>P2085R0
-No
+Clang 14
   
 
   Access checking on specializations
Index: clang/test/CodeGenCXX/p2085.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/p2085.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 --std=c++20 %s -emit-llvm -o - -triple x86_64-linux | FileCheck %s
+
+namespace std {
+struct strong_ordering {
+  int n;
+  constexpr operator int() const { return n; }
+  static const strong_ordering equal, greater, less;
+};
+constexpr inline strong_ordering strong_ordering::equal = {0};
+constexpr inline strong_ordering strong_ordering::greater = {1};
+constexpr inline strong_ordering strong_ordering::less = {-1};
+} // namespace std
+
+struct Space {
+  int i, j;
+
+  std::strong_ordering operator<=>(Space const ) const;
+  bool operator==(Space const ) const;
+};
+
+// Make sure these cause emission
+std::strong_ordering Space::operator<=>(Space const ) const = default;
+// CHECK-LABEL: define{{.*}} @_ZNK5SpacessERKS_
+bool Space::operator==(Space const &) const = default;
+// CHECK-LABEL: define{{.*}} @_ZNK5SpaceeqERKS_
+
+struct Water {
+  int i, j;
+
+  std::strong_ordering operator<=>(Water const ) const;
+  bool operator==(Water const ) const;
+};
+
+// Make sure these do not cause emission
+inline std::strong_ordering Water::operator<=>(Water const ) const = default;
+// CHECK-NOT: define{{.*}} @_ZNK5WaterssERKS_
+inline bool Water::operator==(Water const &) const = default;
+// CHECK-NOT: define{{.*}} @_ZNK5WatereqERKS_
Index: clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
===
--- clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
+++ clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
@@ -1,15 +1,13 @@
 // RUN: %clang_cc1 -std=c++2a -verify %s
 
 struct B {};
-bool operator==(const B&, const B&) = default; // expected-error {{equality comparison operator can only be defaulted in a class definition}} expected-note {{candidate}}
-bool operator<=>(const B&, const B&) = default; // expected-error {{three-way comparison operator can only be defaulted in a class definition}}
 
 template
   bool operator<(const B&, const B&) = default; // expected-error {{comparison operator template cannot be defaulted}}
 
 struct A {
   friend bool operator==(const A&, const A&) = default;
-  friend bool operator!=(const A&, const B&) = default; // expected-error {{invalid parameter type for defaulted equality comparison operator; found 'const B &', expected 'A' or 'const A &'}}
+  friend bool operator!=(const A&, const B&) = default; // expected-error {{parameters for defaulted equality comparison operator must have the same type (found 'const A &' vs 'const B &')}}
   friend bool operator!=(const B&, const B&) = default; // expected-error {{invalid parameter type for defaulted equality comparison}}
   friend bool operator<(const A&, const A&);
   friend bool operator<(const B&, const B&) = default; // expected-error {{invalid parameter type for defaulted relational comparison}}
@@ -29,11 +27,6 @@
 bool operator==(const A&) const = default; // expected-error {{comparison operator template cannot be defaulted}}
 };
 
-// FIXME: The wording is not clear as to whether these are valid, but the
-// intention is that they are not.
-bool operator<(const A&, const A&) = default; // expected-error {{relational comparison operator can only be defaulted in a class definition}}
-bool A::operator<(const A&) const = default; // expected-error {{can only be defaulted in a class definition}}
-
 template struct Dependent {
   using U = typename T::type;
   bool operator==(U) const = default; // expected-error {{found 'Dependent::U'}}
@@ -132,3 +125,41 @@
 friend bool operator==(const B&, const B&) = default; // expected-warning {{deleted}}
   };
 }
+
+namespace p2085 {
+// out-of-class defaulting
+
+struct S1 {
+  bool operator==(S1 const &) const;
+};
+
+bool S1::operator==(S1 const &) const = default;
+
+bool 

[clang] 5fbe21a - [clang] p2085 out-of-class comparison operator defaulting

[Nathan Sidwell via cfe-commits]

Author: Nathan Sidwell
Date: 2021-12-16T07:22:46-08:00
New Revision: 5fbe21a7748f91adbd1b16c95bbfe180642320a3

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

LOG: [clang] p2085 out-of-class comparison operator defaulting

This implements p2085, allowing out-of-class defaulting of comparison
operators, primarily so they need not be inline, IIUC intent. this was
mostly straigh forward, but required reimplementing
Sema::CheckExplicitlyDefaultedComparison, as now there's a case where
we have no a priori clue as to what class a defaulted comparison may
be for. We have to inspect the parameter types to find out. Eg:

class X { ... };
bool operator==(X, X) = default;

Thus reimplemented the parameter type checking, and added 'is this a
friend' functionality for the above case.

Reviewed By: mizvekov

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

Added: 
clang/test/CodeGenCXX/p2085.cpp

Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
clang/www/cxx_status.html

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 038067521559c..29e7c5b447140 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9151,15 +9151,22 @@ def warn_cxx17_compat_defaulted_comparison : Warning<
   "before C++20">, InGroup, DefaultIgnore;
 def err_defaulted_comparison_template : Error<
   "comparison operator template cannot be defaulted">;
-def err_defaulted_comparison_out_of_class : Error<
-  "%sub{select_defaulted_comparison_kind}0 can only be defaulted in a class "
-  "definition">;
+def err_defaulted_comparison_num_args : Error<
+  "%select{non-member|member}0 %sub{select_defaulted_comparison_kind}1"
+  " comparison operator must have %select{2|1}0 parameters">;
 def err_defaulted_comparison_param : Error<
   "invalid parameter type for defaulted 
%sub{select_defaulted_comparison_kind}0"
   "; found %1, expected %2%select{| or %4}3">;
+def err_defaulted_comparison_param_unknown : Error<
+  "invalid parameter type for non-member defaulted"
+  " %sub{select_defaulted_comparison_kind}0"
+  "; found %1, expected class or reference to a constant class">;
 def err_defaulted_comparison_param_mismatch : Error<
   "parameters for defaulted %sub{select_defaulted_comparison_kind}0 "
   "must have the same type%
diff { (found $ vs $)|}1,2">;
+def err_defaulted_comparison_not_friend : Error<
+  "%sub{select_defaulted_comparison_kind}0 is not a friend of"
+  " %select{|incomplete class }1%2">;
 def err_defaulted_comparison_non_const : Error<
   "defaulted member %sub{select_defaulted_comparison_kind}0 must be "
   "const-qualified">;

diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 3f6bde1b9ed6a..119cdf2a3d3c0 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -8435,9 +8435,6 @@ bool Sema::CheckExplicitlyDefaultedComparison(Scope *S, 
FunctionDecl *FD,
   DefaultedComparisonKind DCK) {
   assert(DCK != DefaultedComparisonKind::None && "not a defaulted comparison");
 
-  CXXRecordDecl *RD = dyn_cast(FD->getLexicalDeclContext());
-  assert(RD && "defaulted comparison is not defaulted in a class");
-
   // Perform any unqualified lookups we're going to need to default this
   // function.
   if (S) {
@@ -8455,43 +8452,17 @@ bool Sema::CheckExplicitlyDefaultedComparison(Scope *S, 
FunctionDecl *FD,
   //   const C&, or
   //-- a friend of C having two parameters of type const C& or two
   //   parameters of type C.
-  QualType ExpectedParmType1 = Context.getRecordType(RD);
-  QualType ExpectedParmType2 =
-  Context.getLValueReferenceType(ExpectedParmType1.withConst());
-  if (isa(FD))
-ExpectedParmType1 = ExpectedParmType2;
-  for (const ParmVarDecl *Param : FD->parameters()) {
-if (!Param->getType()->isDependentType() &&
-!Context.hasSameType(Param->getType(), ExpectedParmType1) &&
-!Context.hasSameType(Param->getType(), ExpectedParmType2)) {
-  // Don't diagnose an implicit 'operator=='; we will have diagnosed the
-  // corresponding defaulted 'operator<=>' already.
-  if (!FD->isImplicit()) {
-Diag(FD->getLocation(), diag::err_defaulted_comparison_param)
-<< (int)DCK << Param->getType() << ExpectedParmType1
-<< !isa(FD)
-<< ExpectedParmType2 << Param->getSourceRange();
-  }
-  return true;
-}
-  }
-  if (FD->getNumParams() == 2 &&
-  !Context.hasSameType(FD->getParamDecl(0)->getType(),
-   

[PATCH] D114251: [AST] Add a sugar type for types found via UsingDecl

[David Rector via Phabricator via cfe-commits]

davrec added a comment.

`throughUsingDecl` seems like a good solution, though I defer to regular 
ASTMatchers users.

One other thought: can we add diagnostic notes using this new information, e.g.

  struct A1 {
  protected:
using IntType = char;
  };
  
  struct A2 {
  protected:
using IntType = unsigned long;
  };
  
  struct B : A1, A2 {
using A1::IntType;
  };
  
  struct C : B {
IntType foo;
void setFoo(IntType V) {
  foo = V;
}
  };
  
  
  int main() {
D d;
d.setFoo((unsigned long)-1); // warning: implicit conversion loses info 
  }

It would be nice to add a note pointing to `using A1::IntType` saying 
"introduced here".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114251

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

[clang] 5aefb1d - Revert "[OpenCL] Add pure attribute to vload builtins"

[Stuart Brady via cfe-commits]

Author: Stuart Brady
Date: 2021-12-16T15:16:41Z
New Revision: 5aefb1dc1eabd9286da9511b5227aa83443091b9

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

LOG: Revert "[OpenCL] Add pure attribute to vload builtins"

This reverts commit 1a376bc285358037a5edc48b0d125f91bf5a69ca.

This broke clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl

Added: 


Modified: 
clang/lib/Headers/opencl-c.h
clang/lib/Sema/OpenCLBuiltins.td

Removed: 




diff  --git a/clang/lib/Headers/opencl-c.h b/clang/lib/Headers/opencl-c.h
index 77a7a8b9bb3a1..32af848a94c4f 100644
--- a/clang/lib/Headers/opencl-c.h
+++ b/clang/lib/Headers/opencl-c.h
@@ -11190,305 +11190,305 @@ half16 __ovld __cnfn select(half16 a, half16 b, 
ushort16 c);
  * 64-bit aligned if gentype is long, ulong, double.
  */
 
-char2 __ovld __purefn vload2(size_t offset, const __constant char *p);
-uchar2 __ovld __purefn vload2(size_t offset, const __constant uchar *p);
-short2 __ovld __purefn vload2(size_t offset, const __constant short *p);
-ushort2 __ovld __purefn vload2(size_t offset, const __constant ushort *p);
-int2 __ovld __purefn vload2(size_t offset, const __constant int *p);
-uint2 __ovld __purefn vload2(size_t offset, const __constant uint *p);
-long2 __ovld __purefn vload2(size_t offset, const __constant long *p);
-ulong2 __ovld __purefn vload2(size_t offset, const __constant ulong *p);
-float2 __ovld __purefn vload2(size_t offset, const __constant float *p);
-char3 __ovld __purefn vload3(size_t offset, const __constant char *p);
-uchar3 __ovld __purefn vload3(size_t offset, const __constant uchar *p);
-short3 __ovld __purefn vload3(size_t offset, const __constant short *p);
-ushort3 __ovld __purefn vload3(size_t offset, const __constant ushort *p);
-int3 __ovld __purefn vload3(size_t offset, const __constant int *p);
-uint3 __ovld __purefn vload3(size_t offset, const __constant uint *p);
-long3 __ovld __purefn vload3(size_t offset, const __constant long *p);
-ulong3 __ovld __purefn vload3(size_t offset, const __constant ulong *p);
-float3 __ovld __purefn vload3(size_t offset, const __constant float *p);
-char4 __ovld __purefn vload4(size_t offset, const __constant char *p);
-uchar4 __ovld __purefn vload4(size_t offset, const __constant uchar *p);
-short4 __ovld __purefn vload4(size_t offset, const __constant short *p);
-ushort4 __ovld __purefn vload4(size_t offset, const __constant ushort *p);
-int4 __ovld __purefn vload4(size_t offset, const __constant int *p);
-uint4 __ovld __purefn vload4(size_t offset, const __constant uint *p);
-long4 __ovld __purefn vload4(size_t offset, const __constant long *p);
-ulong4 __ovld __purefn vload4(size_t offset, const __constant ulong *p);
-float4 __ovld __purefn vload4(size_t offset, const __constant float *p);
-char8 __ovld __purefn vload8(size_t offset, const __constant char *p);
-uchar8 __ovld __purefn vload8(size_t offset, const __constant uchar *p);
-short8 __ovld __purefn vload8(size_t offset, const __constant short *p);
-ushort8 __ovld __purefn vload8(size_t offset, const __constant ushort *p);
-int8 __ovld __purefn vload8(size_t offset, const __constant int *p);
-uint8 __ovld __purefn vload8(size_t offset, const __constant uint *p);
-long8 __ovld __purefn vload8(size_t offset, const __constant long *p);
-ulong8 __ovld __purefn vload8(size_t offset, const __constant ulong *p);
-float8 __ovld __purefn vload8(size_t offset, const __constant float *p);
-char16 __ovld __purefn vload16(size_t offset, const __constant char *p);
-uchar16 __ovld __purefn vload16(size_t offset, const __constant uchar *p);
-short16 __ovld __purefn vload16(size_t offset, const __constant short *p);
-ushort16 __ovld __purefn vload16(size_t offset, const __constant ushort *p);
-int16 __ovld __purefn vload16(size_t offset, const __constant int *p);
-uint16 __ovld __purefn vload16(size_t offset, const __constant uint *p);
-long16 __ovld __purefn vload16(size_t offset, const __constant long *p);
-ulong16 __ovld __purefn vload16(size_t offset, const __constant ulong *p);
-float16 __ovld __purefn vload16(size_t offset, const __constant float *p);
+char2 __ovld vload2(size_t offset, const __constant char *p);
+uchar2 __ovld vload2(size_t offset, const __constant uchar *p);
+short2 __ovld vload2(size_t offset, const __constant short *p);
+ushort2 __ovld vload2(size_t offset, const __constant ushort *p);
+int2 __ovld vload2(size_t offset, const __constant int *p);
+uint2 __ovld vload2(size_t offset, const __constant uint *p);
+long2 __ovld vload2(size_t offset, const __constant long *p);
+ulong2 __ovld vload2(size_t offset, const __constant ulong *p);
+float2 __ovld vload2(size_t offset, const __constant float *p);
+char3 __ovld vload3(size_t offset, const __constant char *p);
+uchar3 __ovld vload3(size_t offset, 

[PATCH] D110742: [OpenCL] Add pure attributes to vload builtins

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

looks like this broke tests: http://45.33.8.238/linux/63308/step_7.txt

Please take a look, and revert for now if it takes a while to fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110742

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

[PATCH] D110742: [OpenCL] Add pure attributes to vload builtins

[Stuart Brady 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 rG1a376bc28535: [OpenCL] Add pure attribute to vload builtins 
(authored by stuart).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110742

Files:
  clang/lib/Headers/opencl-c.h
  clang/lib/Sema/OpenCLBuiltins.td

Index: clang/lib/Sema/OpenCLBuiltins.td
===
--- clang/lib/Sema/OpenCLBuiltins.td
+++ clang/lib/Sema/OpenCLBuiltins.td
@@ -806,17 +806,17 @@
   foreach AS = addrspaces in {
 foreach VSize = [2, 3, 4, 8, 16] in {
   foreach name = ["vload" # VSize] in {
-def : Builtin, Size, PointerType, AS>]>;
-def : Builtin, Size, PointerType, AS>]>;
-def : Builtin, Size, PointerType, AS>]>;
-def : Builtin, Size, PointerType, AS>]>;
-def : Builtin, Size, PointerType, AS>]>;
-def : Builtin, Size, PointerType, AS>]>;
-def : Builtin, Size, PointerType, AS>]>;
-def : Builtin, Size, PointerType, AS>]>;
-def : Builtin, Size, PointerType, AS>]>;
-def : Builtin, Size, PointerType, AS>]>;
-def : Builtin, Size, PointerType, AS>]>;
+def : Builtin, Size, PointerType, AS>], Attr.Pure>;
+def : Builtin, Size, PointerType, AS>], Attr.Pure>;
+def : Builtin, Size, PointerType, AS>], Attr.Pure>;
+def : Builtin, Size, PointerType, AS>], Attr.Pure>;
+def : Builtin, Size, PointerType, AS>], Attr.Pure>;
+def : Builtin, Size, PointerType, AS>], Attr.Pure>;
+def : Builtin, Size, PointerType, AS>], Attr.Pure>;
+def : Builtin, Size, PointerType, AS>], Attr.Pure>;
+def : Builtin, Size, PointerType, AS>], Attr.Pure>;
+def : Builtin, Size, PointerType, AS>], Attr.Pure>;
+def : Builtin, Size, PointerType, AS>], Attr.Pure>;
   }
   if defStores then {
 foreach name = ["vstore" # VSize] in {
@@ -848,10 +848,10 @@
 
 multiclass VloadVstoreHalf addrspaces, bit defStores> {
   foreach AS = addrspaces in {
-def : Builtin<"vload_half", [Float, Size, PointerType, AS>]>;
+def : Builtin<"vload_half", [Float, Size, PointerType, AS>], Attr.Pure>;
 foreach VSize = [2, 3, 4, 8, 16] in {
   foreach name = ["vload_half" # VSize, "vloada_half" # VSize] in {
-def : Builtin, Size, PointerType, AS>]>;
+def : Builtin, Size, PointerType, AS>], Attr.Pure>;
   }
 }
 if defStores then {
@@ -877,7 +877,7 @@
 let MinVersion = CL20 in {
   defm : VloadVstoreHalf<[GenericAS], 1>;
 }
-// vload with constant address space is available regardless of version.
+// vload_half and vloada_half with constant address space are available regardless of version.
 defm : VloadVstoreHalf<[ConstantAS], 0>;
 
 // OpenCL v3.0 s6.15.8 - Synchronization Functions.
Index: clang/lib/Headers/opencl-c.h
===
--- clang/lib/Headers/opencl-c.h
+++ clang/lib/Headers/opencl-c.h
@@ -11190,305 +11190,305 @@
  * 64-bit aligned if gentype is long, ulong, double.
  */
 
-char2 __ovld vload2(size_t offset, const __constant char *p);
-uchar2 __ovld vload2(size_t offset, const __constant uchar *p);
-short2 __ovld vload2(size_t offset, const __constant short *p);
-ushort2 __ovld vload2(size_t offset, const __constant ushort *p);
-int2 __ovld vload2(size_t offset, const __constant int *p);
-uint2 __ovld vload2(size_t offset, const __constant uint *p);
-long2 __ovld vload2(size_t offset, const __constant long *p);
-ulong2 __ovld vload2(size_t offset, const __constant ulong *p);
-float2 __ovld vload2(size_t offset, const __constant float *p);
-char3 __ovld vload3(size_t offset, const __constant char *p);
-uchar3 __ovld vload3(size_t offset, const __constant uchar *p);
-short3 __ovld vload3(size_t offset, const __constant short *p);
-ushort3 __ovld vload3(size_t offset, const __constant ushort *p);
-int3 __ovld vload3(size_t offset, const __constant int *p);
-uint3 __ovld vload3(size_t offset, const __constant uint *p);
-long3 __ovld vload3(size_t offset, const __constant long *p);
-ulong3 __ovld vload3(size_t offset, const __constant ulong *p);
-float3 __ovld vload3(size_t offset, const __constant float *p);
-char4 __ovld vload4(size_t offset, const __constant char *p);
-uchar4 __ovld vload4(size_t offset, const __constant uchar *p);
-short4 __ovld vload4(size_t offset, const __constant short *p);
-ushort4 __ovld vload4(size_t offset, const __constant ushort *p);
-int4 __ovld vload4(size_t offset, const __constant int *p);
-uint4 __ovld vload4(size_t offset, const __constant uint *p);
-long4 __ovld vload4(size_t offset, const __constant long *p);
-ulong4 __ovld vload4(size_t offset, const __constant ulong *p);
-float4 __ovld vload4(size_t offset, const __constant float *p);
-char8 __ovld vload8(size_t offset, const 

[clang] 1a376bc - [OpenCL] Add pure attribute to vload builtins

[Stuart Brady via cfe-commits]

Author: Stuart Brady
Date: 2021-12-16T14:55:31Z
New Revision: 1a376bc285358037a5edc48b0d125f91bf5a69ca

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

LOG: [OpenCL] Add pure attribute to vload builtins

Use the "pure" attribute (or "readonly") for the vload, vload_half and
vloada_half builtins.

Reviewed By: svenvh

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

Added: 


Modified: 
clang/lib/Headers/opencl-c.h
clang/lib/Sema/OpenCLBuiltins.td

Removed: 




diff  --git a/clang/lib/Headers/opencl-c.h b/clang/lib/Headers/opencl-c.h
index 32af848a94c4f..77a7a8b9bb3a1 100644
--- a/clang/lib/Headers/opencl-c.h
+++ b/clang/lib/Headers/opencl-c.h
@@ -11190,305 +11190,305 @@ half16 __ovld __cnfn select(half16 a, half16 b, 
ushort16 c);
  * 64-bit aligned if gentype is long, ulong, double.
  */
 
-char2 __ovld vload2(size_t offset, const __constant char *p);
-uchar2 __ovld vload2(size_t offset, const __constant uchar *p);
-short2 __ovld vload2(size_t offset, const __constant short *p);
-ushort2 __ovld vload2(size_t offset, const __constant ushort *p);
-int2 __ovld vload2(size_t offset, const __constant int *p);
-uint2 __ovld vload2(size_t offset, const __constant uint *p);
-long2 __ovld vload2(size_t offset, const __constant long *p);
-ulong2 __ovld vload2(size_t offset, const __constant ulong *p);
-float2 __ovld vload2(size_t offset, const __constant float *p);
-char3 __ovld vload3(size_t offset, const __constant char *p);
-uchar3 __ovld vload3(size_t offset, const __constant uchar *p);
-short3 __ovld vload3(size_t offset, const __constant short *p);
-ushort3 __ovld vload3(size_t offset, const __constant ushort *p);
-int3 __ovld vload3(size_t offset, const __constant int *p);
-uint3 __ovld vload3(size_t offset, const __constant uint *p);
-long3 __ovld vload3(size_t offset, const __constant long *p);
-ulong3 __ovld vload3(size_t offset, const __constant ulong *p);
-float3 __ovld vload3(size_t offset, const __constant float *p);
-char4 __ovld vload4(size_t offset, const __constant char *p);
-uchar4 __ovld vload4(size_t offset, const __constant uchar *p);
-short4 __ovld vload4(size_t offset, const __constant short *p);
-ushort4 __ovld vload4(size_t offset, const __constant ushort *p);
-int4 __ovld vload4(size_t offset, const __constant int *p);
-uint4 __ovld vload4(size_t offset, const __constant uint *p);
-long4 __ovld vload4(size_t offset, const __constant long *p);
-ulong4 __ovld vload4(size_t offset, const __constant ulong *p);
-float4 __ovld vload4(size_t offset, const __constant float *p);
-char8 __ovld vload8(size_t offset, const __constant char *p);
-uchar8 __ovld vload8(size_t offset, const __constant uchar *p);
-short8 __ovld vload8(size_t offset, const __constant short *p);
-ushort8 __ovld vload8(size_t offset, const __constant ushort *p);
-int8 __ovld vload8(size_t offset, const __constant int *p);
-uint8 __ovld vload8(size_t offset, const __constant uint *p);
-long8 __ovld vload8(size_t offset, const __constant long *p);
-ulong8 __ovld vload8(size_t offset, const __constant ulong *p);
-float8 __ovld vload8(size_t offset, const __constant float *p);
-char16 __ovld vload16(size_t offset, const __constant char *p);
-uchar16 __ovld vload16(size_t offset, const __constant uchar *p);
-short16 __ovld vload16(size_t offset, const __constant short *p);
-ushort16 __ovld vload16(size_t offset, const __constant ushort *p);
-int16 __ovld vload16(size_t offset, const __constant int *p);
-uint16 __ovld vload16(size_t offset, const __constant uint *p);
-long16 __ovld vload16(size_t offset, const __constant long *p);
-ulong16 __ovld vload16(size_t offset, const __constant ulong *p);
-float16 __ovld vload16(size_t offset, const __constant float *p);
+char2 __ovld __purefn vload2(size_t offset, const __constant char *p);
+uchar2 __ovld __purefn vload2(size_t offset, const __constant uchar *p);
+short2 __ovld __purefn vload2(size_t offset, const __constant short *p);
+ushort2 __ovld __purefn vload2(size_t offset, const __constant ushort *p);
+int2 __ovld __purefn vload2(size_t offset, const __constant int *p);
+uint2 __ovld __purefn vload2(size_t offset, const __constant uint *p);
+long2 __ovld __purefn vload2(size_t offset, const __constant long *p);
+ulong2 __ovld __purefn vload2(size_t offset, const __constant ulong *p);
+float2 __ovld __purefn vload2(size_t offset, const __constant float *p);
+char3 __ovld __purefn vload3(size_t offset, const __constant char *p);
+uchar3 __ovld __purefn vload3(size_t offset, const __constant uchar *p);
+short3 __ovld __purefn vload3(size_t offset, const __constant short *p);
+ushort3 __ovld __purefn vload3(size_t offset, const __constant ushort *p);
+int3 __ovld __purefn vload3(size_t offset, const __constant int *p);
+uint3 __ovld __purefn 

[clang] a0cf066 - [CodeGen] Store element type in ParamValue

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2021-12-16T15:31:55+01:00
New Revision: a0cf066eac8aa55782a6dd2deb90adc89a707a8b

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

LOG: [CodeGen] Store element type in ParamValue

ParamValue is basically a union between an Address and a Value*.
To be able to reconstruct the Address, we now need to store the
pointer element type.

Added: 


Modified: 
clang/lib/CodeGen/CodeGenFunction.h

Removed: 




diff  --git a/clang/lib/CodeGen/CodeGenFunction.h 
b/clang/lib/CodeGen/CodeGenFunction.h
index 411a36a26332..679c720f7bd7 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -3130,15 +3130,18 @@ class CodeGenFunction : public CodeGenTypeCache {
 
   class ParamValue {
 llvm::Value *Value;
+llvm::Type *ElementType;
 unsigned Alignment;
-ParamValue(llvm::Value *V, unsigned A) : Value(V), Alignment(A) {}
+ParamValue(llvm::Value *V, llvm::Type *T, unsigned A)
+: Value(V), ElementType(T), Alignment(A) {}
   public:
 static ParamValue forDirect(llvm::Value *value) {
-  return ParamValue(value, 0);
+  return ParamValue(value, nullptr, 0);
 }
 static ParamValue forIndirect(Address addr) {
   assert(!addr.getAlignment().isZero());
-  return ParamValue(addr.getPointer(), addr.getAlignment().getQuantity());
+  return ParamValue(addr.getPointer(), addr.getElementType(),
+addr.getAlignment().getQuantity());
 }
 
 bool isIndirect() const { return Alignment != 0; }
@@ -3151,7 +3154,7 @@ class CodeGenFunction : public CodeGenTypeCache {
 
 Address getIndirectAddress() const {
   assert(isIndirect());
-  return Address(Value, CharUnits::fromQuantity(Alignment));
+  return Address(Value, ElementType, CharUnits::fromQuantity(Alignment));
 }
   };
 



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

[clang] 58c8c53 - [CodeGen] Avoid more pointer element type accesses

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2021-12-16T15:26:21+01:00
New Revision: 58c8c5326329bb55df6b9d4f7f8f43a2c82e67c1

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

LOG: [CodeGen] Avoid more pointer element type accesses

Added: 


Modified: 
clang/lib/CodeGen/CGCall.cpp
clang/lib/CodeGen/CGDecl.cpp
clang/lib/CodeGen/CGExpr.cpp
clang/lib/CodeGen/CGExprScalar.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/lib/CodeGen/TargetInfo.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 79442d7f51c4..8ecef6ab9782 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -4964,8 +4964,7 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo 
,
   Builder.CreateMemCpy(TempAlloca, Src, SrcSize);
   Src = TempAlloca;
 } else {
-  Src = Builder.CreateBitCast(Src,
-  
STy->getPointerTo(Src.getAddressSpace()));
+  Src = Builder.CreateElementBitCast(Src, STy);
 }
 
 assert(NumIRArgs == STy->getNumElements());

diff  --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp
index 97ca3a1e1db1..f6b758ba30a0 100644
--- a/clang/lib/CodeGen/CGDecl.cpp
+++ b/clang/lib/CodeGen/CGDecl.cpp
@@ -405,7 +405,8 @@ void CodeGenFunction::EmitStaticVarDecl(const VarDecl ,
 
   // Store into LocalDeclMap before generating initializer to handle
   // circular references.
-  setAddrOfLocalVar(, Address(addr, alignment));
+  setAddrOfLocalVar(
+  , Address(addr, ConvertTypeForMem(D.getType()), alignment));
 
   // We can't have a VLA here, but we can have a pointer to a VLA,
   // even though that doesn't really make any sense.

diff  --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index f22c09a09e3e..635ae8a70334 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -1099,7 +1099,7 @@ Address CodeGenFunction::EmitPointerWithAlignment(const 
Expr *E,
   if (InnerBaseInfo.getAlignmentSource() != AlignmentSource::Decl) {
 if (BaseInfo)
   BaseInfo->mergeForCast(TargetTypeBaseInfo);
-Addr = Address(Addr.getPointer(), Align);
+Addr = Address(Addr.getPointer(), Addr.getElementType(), Align);
   }
 }
 
@@ -,10 +,12 @@ Address CodeGenFunction::EmitPointerWithAlignment(const 
Expr *E,
   CodeGenFunction::CFITCK_UnrelatedCast,
   CE->getBeginLoc());
 }
-return CE->getCastKind() != CK_AddressSpaceConversion
-   ? Builder.CreateBitCast(Addr, ConvertType(E->getType()))
-   : Builder.CreateAddrSpaceCast(Addr,
- ConvertType(E->getType()));
+
+if (CE->getCastKind() == CK_AddressSpaceConversion)
+ return Builder.CreateAddrSpaceCast(Addr, ConvertType(E->getType()));
+
+llvm::Type *ElemTy = ConvertTypeForMem(E->getType()->getPointeeType());
+return Builder.CreateElementBitCast(Addr, ElemTy);
   }
   break;
 
@@ -2527,7 +2529,7 @@ static LValue EmitGlobalVarDeclLValue(CodeGenFunction 
,
   llvm::Type *RealVarTy = CGF.getTypes().ConvertTypeForMem(VD->getType());
   V = EmitBitCastOfLValueToProperType(CGF, V, RealVarTy);
   CharUnits Alignment = CGF.getContext().getDeclAlign(VD);
-  Address Addr(V, Alignment);
+  Address Addr(V, RealVarTy, Alignment);
   // Emit reference to the private copy of the variable if it is an OpenMP
   // threadprivate variable.
   if (CGF.getLangOpts().OpenMP && !CGF.getLangOpts().OpenMPSimd &&
@@ -2705,7 +2707,7 @@ LValue CodeGenFunction::EmitDeclRefLValue(const 
DeclRefExpr *E) {
 /* BaseInfo= */ nullptr,
 /* TBAAInfo= */ nullptr,
 /* forPointeeType= */ true);
-Addr = Address(Val, Alignment);
+Addr = Address(Val, ConvertTypeForMem(E->getType()), Alignment);
   }
   return MakeAddrLValue(Addr, T, AlignmentSource::Decl);
 }
@@ -2782,9 +2784,10 @@ LValue CodeGenFunction::EmitDeclRefLValue(const 
DeclRefExpr *E) {
 // Otherwise, it might be static local we haven't emitted yet for
 // some reason; most likely, because it's in an outer function.
 } else if (VD->isStaticLocal()) {
-  addr = Address(CGM.getOrCreateStaticVarDecl(
-  *VD, CGM.getLLVMLinkageVarDefinition(VD, /*IsConstant=*/false)),
- getContext().getDeclAlign(VD));
+  llvm::Constant *var = CGM.getOrCreateStaticVarDecl(
+  *VD, CGM.getLLVMLinkageVarDefinition(VD, /*IsConstant=*/false));
+  addr = Address(
+  var, 

[PATCH] D115804: [CodeGen] use saturating FP casts when compiling with "no-strict-float-cast-overflow"

[Sanjay Patel 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 rG8c7f2a4f8719: [CodeGen] use saturating FP casts when 
compiling with no-strict-float-cast… (authored by spatel).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D115804?vs=394571=394850#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115804

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGen/no-junk-ftrunc.c


Index: clang/test/CodeGen/no-junk-ftrunc.c
===
--- clang/test/CodeGen/no-junk-ftrunc.c
+++ clang/test/CodeGen/no-junk-ftrunc.c
@@ -1,14 +1,22 @@
 // RUN: %clang_cc1 -S -fno-strict-float-cast-overflow %s -emit-llvm -o - | 
FileCheck %s --check-prefix=NOSTRICT
+
+// When compiling with non-standard semantics, use intrinsics to inhibit the 
optimizer.
+
 // NOSTRICT-LABEL: main
+// NOSTRICT: call i32 @llvm.fptosi.sat.i32.f64
+// NOSTRICT: call i32 @llvm.fptoui.sat.i32.f64
 // NOSTRICT: attributes #0 = {{.*}}"strict-float-cast-overflow"="false"{{.*}}
 
 // The workaround attribute is not applied by default.
 
 // RUN: %clang_cc1 -S %s -emit-llvm -o - | FileCheck %s --check-prefix=STRICT
 // STRICT-LABEL: main
+// STRICT: = fptosi
+// STRICT: = fptoui
 // STRICT-NOT: strict-float-cast-overflow
 
+
 int main() {
-  return 0;
+  double d = 1e20;
+  return (int)d != 1e20 && (unsigned)d != 1e20;
 }
-
Index: clang/lib/CodeGen/CGExprScalar.cpp
===
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -1240,7 +1240,18 @@
 
   if (isa(DstElementTy)) {
 assert(SrcElementTy->isFloatingPointTy() && "Unknown real conversion");
-if (DstElementType->isSignedIntegerOrEnumerationType())
+bool IsSigned = DstElementType->isSignedIntegerOrEnumerationType();
+
+// If we can't recognize overflow as undefined behavior, assume that
+// overflow saturates. This protects against normal optimizations if we are
+// compiling with non-standard FP semantics.
+if (!CGF.CGM.getCodeGenOpts().StrictFloatCastOverflow) {
+  llvm::Intrinsic::ID IID =
+  IsSigned ? llvm::Intrinsic::fptosi_sat : llvm::Intrinsic::fptoui_sat;
+  return Builder.CreateCall(CGF.CGM.getIntrinsic(IID, {DstTy, SrcTy}), 
Src);
+}
+
+if (IsSigned)
   return Builder.CreateFPToSI(Src, DstTy, "conv");
 return Builder.CreateFPToUI(Src, DstTy, "conv");
   }


Index: clang/test/CodeGen/no-junk-ftrunc.c
===
--- clang/test/CodeGen/no-junk-ftrunc.c
+++ clang/test/CodeGen/no-junk-ftrunc.c
@@ -1,14 +1,22 @@
 // RUN: %clang_cc1 -S -fno-strict-float-cast-overflow %s -emit-llvm -o - | FileCheck %s --check-prefix=NOSTRICT
+
+// When compiling with non-standard semantics, use intrinsics to inhibit the optimizer.
+
 // NOSTRICT-LABEL: main
+// NOSTRICT: call i32 @llvm.fptosi.sat.i32.f64
+// NOSTRICT: call i32 @llvm.fptoui.sat.i32.f64
 // NOSTRICT: attributes #0 = {{.*}}"strict-float-cast-overflow"="false"{{.*}}
 
 // The workaround attribute is not applied by default.
 
 // RUN: %clang_cc1 -S %s -emit-llvm -o - | FileCheck %s --check-prefix=STRICT
 // STRICT-LABEL: main
+// STRICT: = fptosi
+// STRICT: = fptoui
 // STRICT-NOT: strict-float-cast-overflow
 
+
 int main() {
-  return 0;
+  double d = 1e20;
+  return (int)d != 1e20 && (unsigned)d != 1e20;
 }
-
Index: clang/lib/CodeGen/CGExprScalar.cpp
===
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -1240,7 +1240,18 @@
 
   if (isa(DstElementTy)) {
 assert(SrcElementTy->isFloatingPointTy() && "Unknown real conversion");
-if (DstElementType->isSignedIntegerOrEnumerationType())
+bool IsSigned = DstElementType->isSignedIntegerOrEnumerationType();
+
+// If we can't recognize overflow as undefined behavior, assume that
+// overflow saturates. This protects against normal optimizations if we are
+// compiling with non-standard FP semantics.
+if (!CGF.CGM.getCodeGenOpts().StrictFloatCastOverflow) {
+  llvm::Intrinsic::ID IID =
+  IsSigned ? llvm::Intrinsic::fptosi_sat : llvm::Intrinsic::fptoui_sat;
+  return Builder.CreateCall(CGF.CGM.getIntrinsic(IID, {DstTy, SrcTy}), Src);
+}
+
+if (IsSigned)
   return Builder.CreateFPToSI(Src, DstTy, "conv");
 return Builder.CreateFPToUI(Src, DstTy, "conv");
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 8c7f2a4 - [CodeGen] use saturating FP casts when compiling with "no-strict-float-cast-overflow"

[Sanjay Patel via cfe-commits]

Author: Sanjay Patel
Date: 2021-12-16T09:10:12-05:00
New Revision: 8c7f2a4f871928d8734ee3f03e67d09086850b60

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

LOG: [CodeGen] use saturating FP casts when compiling with 
"no-strict-float-cast-overflow"

We got an unintended consequence of the optimizer getting smarter when
compiling in a non-standard mode, and there's no good way to inhibit
those optimizations at a later stage. The test is based on an example
linked from D92270.

We allow the "no-strict-float-cast-overflow" exception to normal C
cast rules to preserve legacy code that does not expect overflowing
casts from FP to int to produce UB. See D46236 for details.

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

Added: 


Modified: 
clang/lib/CodeGen/CGExprScalar.cpp
clang/test/CodeGen/no-junk-ftrunc.c

Removed: 




diff  --git a/clang/lib/CodeGen/CGExprScalar.cpp 
b/clang/lib/CodeGen/CGExprScalar.cpp
index 4998d343e5f5e..d34cf7f36f678 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -1240,7 +1240,18 @@ Value *ScalarExprEmitter::EmitScalarCast(Value *Src, 
QualType SrcType,
 
   if (isa(DstElementTy)) {
 assert(SrcElementTy->isFloatingPointTy() && "Unknown real conversion");
-if (DstElementType->isSignedIntegerOrEnumerationType())
+bool IsSigned = DstElementType->isSignedIntegerOrEnumerationType();
+
+// If we can't recognize overflow as undefined behavior, assume that
+// overflow saturates. This protects against normal optimizations if we are
+// compiling with non-standard FP semantics.
+if (!CGF.CGM.getCodeGenOpts().StrictFloatCastOverflow) {
+  llvm::Intrinsic::ID IID =
+  IsSigned ? llvm::Intrinsic::fptosi_sat : llvm::Intrinsic::fptoui_sat;
+  return Builder.CreateCall(CGF.CGM.getIntrinsic(IID, {DstTy, SrcTy}), 
Src);
+}
+
+if (IsSigned)
   return Builder.CreateFPToSI(Src, DstTy, "conv");
 return Builder.CreateFPToUI(Src, DstTy, "conv");
   }

diff  --git a/clang/test/CodeGen/no-junk-ftrunc.c 
b/clang/test/CodeGen/no-junk-ftrunc.c
index 2ab4d8c25a39c..6ae6d30fca434 100644
--- a/clang/test/CodeGen/no-junk-ftrunc.c
+++ b/clang/test/CodeGen/no-junk-ftrunc.c
@@ -1,14 +1,22 @@
 // RUN: %clang_cc1 -S -fno-strict-float-cast-overflow %s -emit-llvm -o - | 
FileCheck %s --check-prefix=NOSTRICT
+
+// When compiling with non-standard semantics, use intrinsics to inhibit the 
optimizer.
+
 // NOSTRICT-LABEL: main
+// NOSTRICT: call i32 @llvm.fptosi.sat.i32.f64
+// NOSTRICT: call i32 @llvm.fptoui.sat.i32.f64
 // NOSTRICT: attributes #0 = {{.*}}"strict-float-cast-overflow"="false"{{.*}}
 
 // The workaround attribute is not applied by default.
 
 // RUN: %clang_cc1 -S %s -emit-llvm -o - | FileCheck %s --check-prefix=STRICT
 // STRICT-LABEL: main
+// STRICT: = fptosi
+// STRICT: = fptoui
 // STRICT-NOT: strict-float-cast-overflow
 
+
 int main() {
-  return 0;
+  double d = 1e20;
+  return (int)d != 1e20 && (unsigned)d != 1e20;
 }
-



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

[PATCH] D115694: [ARM] Introduce an empty "armv8.8-a" architecture.

[Tomas Matheson via Phabricator via cfe-commits]

tmatheson accepted this revision.
tmatheson 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/D115694/new/

https://reviews.llvm.org/D115694

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

[PATCH] D114317: [clang-tidy][WIP] Do not run perfect alias checks

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: alexfh.
aaron.ballman added a subscriber: alexfh.
aaron.ballman added a comment.

In D114317#3169067 , @carlosgalvezp 
wrote:

> Kind ping to reviewers, I mostly want to know if you agree with the general 
> direction so I can go ahead and implement the rest of the patch. It's a big 
> change (replace all check registrations with the macro) so I'd like to avoid 
> extra work if you are strongly against it.
>
> @aaron.ballman I understand you are super busy and don't want to take any 
> more time than needed from you. I'd just love to hear your thoughts about the 
> two missing points that we discussed in the RFC. Nobody else seems to have 
> raised similar concerns about it.
>
> Really appreciate your time!

Sorry about the lack of response! Taking (effectively) two weeks off put me 
pretty far behind on reviews (I'm down to "only" ~30 reviews in my queue as of 
this morning), and I head out for another vacation starting tomorrow until the 
end of the year. So please anticipate further delays from me, sorry!

> It would be desirable to still keep the diagnostic/fixit for the alias 
> checks. I don't know how to implement this in practice. It would require each 
> check to call the diag() function in a loop for all alias checks. This would 
> be very disruptive for check developers. I cannot implement this in the 
> ClangTidyCheck class because the diag() function returns an object, so I 
> can't put a loop there.

I'd like to make sure I'm on the same page as you. What you mean here is that 
you want the diagnostic to be something like: `error: forgot to frobble the 
quux [bugprone-frobble-quux, cert-quux45-cpp]` instead of error: forgot to 
frobble the quux [bugprone-frobble-quux]`? Or do you mean something else?

> Ideally, it would be nice to show "CACHED - 0 seconds" or "SKIPPED - 0 
> seconds" in the profiling output (currently, the alias check just doesn't 
> show up). Again, I don't know how to implement this feature in a 
> non-intrusive way that makes sense.

That would be handy, but I think is totally reasonable for a follow-up.

> I see, thanks for the input! I took configuration into account due to the 
> concerns raised in previous patch attempts and in the RFC. I believe both use 
> cases are valid so it makes sense to me to support both as independent opt-in 
> flags. What do you think, @aaron.ballman ?

Eh, I'm less convinced of this, but I'm also not certain I'm strongly opposed 
(mostly weakly opposed at this point). Aliases with different configuration 
options are positioned to users as being separate checks. For example, we could 
have one check that decides to convert all punctuators it can to be alternate 
tokens (`and` instead of `&&`, etc) and there could be an alias check that 
converts all alternative tokens back into punctuators but is implemented as an 
alias. The fact that it's an alias is an implementation detail that should not 
matter to the user, and being able to disable *all* checks regardless of their 
configuration strikes me as making these kinds of situations confusing to users 
and I think it closes a development door we don't really want to close. I'd 
want to have buy-in from @alexfh as code owner for that option. However, I 
think it'd be good to ensure that the design can support such an option in the 
future (at least, support as much as reasonable) so we can do it as follow-up 
work without extra effort.




Comment at: clang-tools-extra/clang-tidy/ClangTidyModule.h:25-26
 
+#define CLANG_TIDY_REGISTER_CHECK(Factory, CheckType, CheckName)   
\
+  Factory.registerCheck(CheckName, #CheckType)
+

I'm not 100% sold that this macro carries its weight. I almost think the 
registration code is easier to read without it, but not enough to suggest a 
change yet. Curious if others have feelings here.


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

https://reviews.llvm.org/D114317

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