[PATCH] D153835: [Sema] Clone VisibilityAttr for functions in template instantiations

2023-06-26 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 534842.
MaskRay edited the summary of this revision.
MaskRay added a comment.

remove a misleading comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153835

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/CodeGenCXX/visibility.cpp
  clang/utils/TableGen/ClangAttrEmitter.cpp

Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -3627,8 +3627,7 @@
 if (!R.getValueAsBit("ASTNode"))
   continue;
 OS << "case attr::" << R.getName() << ": {\n";
-bool ShouldClone = R.getValueAsBit("Clone") &&
-   (!AppliesToDecl ||
+bool ShouldClone = (!AppliesToDecl ||
 R.getValueAsBit("MeaningfulToClassTemplateDefinition"));
 
 if (!ShouldClone) {
Index: clang/test/CodeGenCXX/visibility.cpp
===
--- clang/test/CodeGenCXX/visibility.cpp
+++ clang/test/CodeGenCXX/visibility.cpp
@@ -951,10 +951,6 @@
 }
 
 namespace test51 {
-  // Test that we use the visibility of struct foo when instantiating the
-  // template. Note that is a case where we disagree with gcc, it produces
-  // a default symbol.
-
   struct HIDDEN foo {
   };
   DEFAULT foo x, y;
@@ -962,8 +958,8 @@
   void DEFAULT zed() {
   }
   template void zed<>();
-  // CHECK-LABEL: define weak_odr hidden void @_ZN6test513zedIXadL_ZNS_1xEvv
-  // CHECK-HIDDEN-LABEL: define weak_odr hidden void @_ZN6test513zedIXadL_ZNS_1xEvv
+  // CHECK-LABEL: define weak_odr void @_ZN6test513zedIXadL_ZNS_1xEvv
+  // CHECK-HIDDEN-LABEL: define weak_odr void @_ZN6test513zedIXadL_ZNS_1xEvv
 
   template void HIDDEN zed<>();
   // CHECK-LABEL: define weak_odr hidden void @_ZN6test513zedIXadL_ZNS_1yEvv(
@@ -1349,15 +1345,15 @@
   template  template 
   U foo::bar() { return {}; }
 
+  /// foo::{zed,bar} instantiate the HIDDEN attributes.
   extern template struct DEFAULT foo;
 
   int use() {
 foo o;
 return o.zed() + o.bar();
   }
-  /// FIXME: foo::bar is hidden in GCC w/ or w/o -fvisibility=hidden.
   // CHECK-LABEL: declare hidden noundef i32 @_ZN6test713fooIiE3zedEv(
-  // CHECK-LABEL: define linkonce_odr noundef i32 @_ZN6test713fooIiE3barIiEET_v(
+  // CHECK-LABEL: define linkonce_odr hidden noundef i32 @_ZN6test713fooIiE3barIiEET_v(
   // CHECK-HIDDEN-LABEL: declare hidden noundef i32 @_ZN6test713fooIiE3zedEv(
-  // CHECK-HIDDEN-LABEL: define linkonce_odr noundef i32 @_ZN6test713fooIiE3barIiEET_v(
+  // CHECK-HIDDEN-LABEL: define linkonce_odr hidden noundef i32 @_ZN6test713fooIiE3barIiEET_v(
 }
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -815,6 +815,18 @@
   continue;
 }
 
+if (auto *A = dyn_cast(TmplAttr)) {
+  if (isa(Tmpl) && !New->hasAttr()) {
+auto *NewA = A->clone(Context);
+NewA->setImplicit(true);
+New->addAttr(NewA);
+  }
+  continue;
+}
+
+if (auto *A = dyn_cast(TmplAttr))
+  continue;
+
 assert(!TmplAttr->isPackExpansion());
 if (TmplAttr->isLateParsed() && LateAttrs) {
   // Late parsed attributes must be instantiated and attached after the
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -2870,9 +2870,11 @@
 typename T::VisibilityType existingValue = existingAttr->getVisibility();
 if (existingValue == value)
   return nullptr;
-S.Diag(existingAttr->getLocation(), diag::err_mismatched_visibility);
-S.Diag(CI.getLoc(), diag::note_previous_attribute);
 D->dropAttr();
+if (!existingAttr->isImplicit()) {
+  S.Diag(existingAttr->getLocation(), diag::err_mismatched_visibility);
+  S.Diag(CI.getLoc(), diag::note_previous_attribute);
+}
   }
   return ::new (S.Context) T(S.Context, CI, value);
 }
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -569,9 +569,6 @@
   list Accessors = [];
   // Set to true for attributes with arguments which require delayed parsing.
   bit LateParsed = 0;
-  // Set to false to prevent an attribute from being propagated from a template
-  // to the instantiation.
-  bit Clone = 1;
   // Set to true for attributes which must be instantiated within templates
   bit TemplateDependent = 0;
   // Set to true for attributes that have a corresponding AST node.
@@ -2993,7 

[PATCH] D153835: [Sema] Clone VisibilityAttr for functions in template instantiations

2023-06-26 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision.
MaskRay added reviewers: clang, aaron.ballman, efriedma, rjmccall, smeenai.
Herald added a project: All.
MaskRay requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fix https://github.com/llvm/llvm-project/issues/31462

Modify the behavior from 7f90b7d4c29d560b57f6026f886adbf4d7ab4382 (2012) to
clone VisibilityAttr for functions. I think that commit fixed a behavior
but made us more difficult to align with GCC.

The new behavior aligns better with GCC. Specifically, an instantiated
FunctionTemplateDecl/CXXMethodDecl now inherits the original visibility
attribute (test71 in visibility.cpp).

The cloned VisibilityAttr is set to implicit. This is important for
`template void HIDDEN zed<>();` to override the original VisibilityAttr 
(test51).

For now, it's important not to clone VisibilityAttr for non-FunctionDecl as that
would break test38 (and change test35 in another way to be incompatible
with GCC).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153835

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/CodeGenCXX/visibility.cpp
  clang/utils/TableGen/ClangAttrEmitter.cpp

Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -3627,8 +3627,7 @@
 if (!R.getValueAsBit("ASTNode"))
   continue;
 OS << "case attr::" << R.getName() << ": {\n";
-bool ShouldClone = R.getValueAsBit("Clone") &&
-   (!AppliesToDecl ||
+bool ShouldClone = (!AppliesToDecl ||
 R.getValueAsBit("MeaningfulToClassTemplateDefinition"));
 
 if (!ShouldClone) {
Index: clang/test/CodeGenCXX/visibility.cpp
===
--- clang/test/CodeGenCXX/visibility.cpp
+++ clang/test/CodeGenCXX/visibility.cpp
@@ -951,10 +951,6 @@
 }
 
 namespace test51 {
-  // Test that we use the visibility of struct foo when instantiating the
-  // template. Note that is a case where we disagree with gcc, it produces
-  // a default symbol.
-
   struct HIDDEN foo {
   };
   DEFAULT foo x, y;
@@ -962,8 +958,9 @@
   void DEFAULT zed() {
   }
   template void zed<>();
-  // CHECK-LABEL: define weak_odr hidden void @_ZN6test513zedIXadL_ZNS_1xEvv
-  // CHECK-HIDDEN-LABEL: define weak_odr hidden void @_ZN6test513zedIXadL_ZNS_1xEvv
+  /// GCC emits hidden zed<> with -fvisibility=hidden.
+  // CHECK-LABEL: define weak_odr void @_ZN6test513zedIXadL_ZNS_1xEvv
+  // CHECK-HIDDEN-LABEL: define weak_odr void @_ZN6test513zedIXadL_ZNS_1xEvv
 
   template void HIDDEN zed<>();
   // CHECK-LABEL: define weak_odr hidden void @_ZN6test513zedIXadL_ZNS_1yEvv(
@@ -1349,15 +1346,15 @@
   template  template 
   U foo::bar() { return {}; }
 
+  /// foo::{zed,bar} instantiate the HIDDEN attributes.
   extern template struct DEFAULT foo;
 
   int use() {
 foo o;
 return o.zed() + o.bar();
   }
-  /// FIXME: foo::bar is hidden in GCC w/ or w/o -fvisibility=hidden.
   // CHECK-LABEL: declare hidden noundef i32 @_ZN6test713fooIiE3zedEv(
-  // CHECK-LABEL: define linkonce_odr noundef i32 @_ZN6test713fooIiE3barIiEET_v(
+  // CHECK-LABEL: define linkonce_odr hidden noundef i32 @_ZN6test713fooIiE3barIiEET_v(
   // CHECK-HIDDEN-LABEL: declare hidden noundef i32 @_ZN6test713fooIiE3zedEv(
-  // CHECK-HIDDEN-LABEL: define linkonce_odr noundef i32 @_ZN6test713fooIiE3barIiEET_v(
+  // CHECK-HIDDEN-LABEL: define linkonce_odr hidden noundef i32 @_ZN6test713fooIiE3barIiEET_v(
 }
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -815,6 +815,18 @@
   continue;
 }
 
+if (auto *A = dyn_cast(TmplAttr)) {
+  if (isa(Tmpl) && !New->hasAttr()) {
+auto *NewA = A->clone(Context);
+NewA->setImplicit(true);
+New->addAttr(NewA);
+  }
+  continue;
+}
+
+if (auto *A = dyn_cast(TmplAttr))
+  continue;
+
 assert(!TmplAttr->isPackExpansion());
 if (TmplAttr->isLateParsed() && LateAttrs) {
   // Late parsed attributes must be instantiated and attached after the
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -2870,9 +2870,11 @@
 typename T::VisibilityType existingValue = existingAttr->getVisibility();
 if (existingValue == value)
   return nullptr;
-S.Diag(existingAttr->getLocation(), diag::err_mismatched_visibility);
-S.Diag(CI.getLoc(), diag::note_previous_attribute);
 D->dropAttr();
+if (!existingAttr->isImplicit()) {
+  

[clang] 32c7efd - [test] Improve symbol visibility test

2023-06-26 Thread Fangrui Song via cfe-commits

Author: Fangrui Song
Date: 2023-06-26T22:20:02-07:00
New Revision: 32c7efddfde9e59d88fcd9254b0433f6646d5b6d

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

LOG: [test] Improve symbol visibility test

Added: 


Modified: 
clang/test/CodeGenCXX/visibility.cpp

Removed: 




diff  --git a/clang/test/CodeGenCXX/visibility.cpp 
b/clang/test/CodeGenCXX/visibility.cpp
index 841813009cd4f..d3e3fb341c572 100644
--- a/clang/test/CodeGenCXX/visibility.cpp
+++ b/clang/test/CodeGenCXX/visibility.cpp
@@ -957,13 +957,17 @@ namespace test51 {
 
   struct HIDDEN foo {
   };
-  DEFAULT foo x;
+  DEFAULT foo x, y;
   template
   void DEFAULT zed() {
   }
   template void zed<>();
   // CHECK-LABEL: define weak_odr hidden void 
@_ZN6test513zedIXadL_ZNS_1xEvv
   // CHECK-HIDDEN-LABEL: define weak_odr hidden void 
@_ZN6test513zedIXadL_ZNS_1xEvv
+
+  template void HIDDEN zed<>();
+  // CHECK-LABEL: define weak_odr hidden void 
@_ZN6test513zedIXadL_ZNS_1yEvv(
+  // CHECK-HIDDEN-LABEL: define weak_odr hidden void 
@_ZN6test513zedIXadL_ZNS_1yEvv(
 }
 
 namespace test52 {
@@ -1332,3 +1336,28 @@ namespace test70 {
   B::~B() {}
   // Check lines at top of file.
 }
+
+// https://github.com/llvm/llvm-project/issues/31462
+namespace test71 {
+  template 
+  struct foo {
+static HIDDEN T zed();
+template  HIDDEN U bar();
+  };
+  template 
+  T foo::zed() { return {}; }
+  template  template 
+  U foo::bar() { return {}; }
+
+  extern template struct DEFAULT foo;
+
+  int use() {
+foo o;
+return o.zed() + o.bar();
+  }
+  /// FIXME: foo::bar is hidden in GCC w/ or w/o -fvisibility=hidden.
+  // CHECK-LABEL: declare hidden noundef i32 @_ZN6test713fooIiE3zedEv(
+  // CHECK-LABEL: define linkonce_odr noundef i32 
@_ZN6test713fooIiE3barIiEET_v(
+  // CHECK-HIDDEN-LABEL: declare hidden noundef i32 @_ZN6test713fooIiE3zedEv(
+  // CHECK-HIDDEN-LABEL: define linkonce_odr noundef i32 
@_ZN6test713fooIiE3barIiEET_v(
+}



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


[PATCH] D129635: [OpenMP] Update the default version of OpenMP to 5.1

2023-06-26 Thread Saiyedul Islam via Phabricator via cfe-commits
saiislam added a comment.

@jdoerfert @RaviNarayanaswamy @ABataev

Are there any features in this table which have been already implemented but 
have not been tagged?
https://clang.llvm.org/docs/OpenMPSupport.html#openmp-5-1-implementation-details


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129635

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


[PATCH] D153006: [clang][dataflow] Perform deep copies in copy and move operations.

2023-06-26 Thread Martin Böhme via Phabricator via cfe-commits
mboehme marked an inline comment as done.
mboehme added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/RecordOps.cpp:20
+
+void copyRecord(AggregateStorageLocation , AggregateStorageLocation ,
+Environment ) {

MaskRay wrote:
> See 
> https://llvm.org/docs/CodingStandards.html#use-namespace-qualifiers-to-implement-previously-declared-functions
> 
> I'll change this.
Thanks for pointing this out!

https://reviews.llvm.org/D153833


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153006

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


[PATCH] D153833: [clang][dataflow] Use namespace qualifiers when defining functions.

2023-06-26 Thread Martin Böhme via Phabricator via cfe-commits
mboehme created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
mboehme requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

See

https://llvm.org/docs/CodingStandards.html#use-namespace-qualifiers-to-implement-previously-declared-functions

Thank you to MaskRay for pointing this out on

https://reviews.llvm.org/D153006


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153833

Files:
  clang/lib/Analysis/FlowSensitive/RecordOps.cpp


Index: clang/lib/Analysis/FlowSensitive/RecordOps.cpp
===
--- clang/lib/Analysis/FlowSensitive/RecordOps.cpp
+++ clang/lib/Analysis/FlowSensitive/RecordOps.cpp
@@ -14,11 +14,9 @@
 
 #define DEBUG_TYPE "dataflow"
 
-namespace clang {
-namespace dataflow {
-
-void copyRecord(AggregateStorageLocation , AggregateStorageLocation ,
-Environment ) {
+void clang::dataflow::copyRecord(AggregateStorageLocation ,
+ AggregateStorageLocation ,
+ Environment ) {
   LLVM_DEBUG({
 if (Dst.getType().getCanonicalType().getUnqualifiedType() !=
 Src.getType().getCanonicalType().getUnqualifiedType()) {
@@ -62,9 +60,10 @@
   }
 }
 
-bool recordsEqual(const AggregateStorageLocation , const Environment 
,
-  const AggregateStorageLocation ,
-  const Environment ) {
+bool clang::dataflow::recordsEqual(const AggregateStorageLocation ,
+   const Environment ,
+   const AggregateStorageLocation ,
+   const Environment ) {
   LLVM_DEBUG({
 if (Loc2.getType().getCanonicalType().getUnqualifiedType() !=
 Loc1.getType().getCanonicalType().getUnqualifiedType()) {
@@ -124,6 +123,3 @@
 
   return true;
 }
-
-} // namespace dataflow
-} // namespace clang


Index: clang/lib/Analysis/FlowSensitive/RecordOps.cpp
===
--- clang/lib/Analysis/FlowSensitive/RecordOps.cpp
+++ clang/lib/Analysis/FlowSensitive/RecordOps.cpp
@@ -14,11 +14,9 @@
 
 #define DEBUG_TYPE "dataflow"
 
-namespace clang {
-namespace dataflow {
-
-void copyRecord(AggregateStorageLocation , AggregateStorageLocation ,
-Environment ) {
+void clang::dataflow::copyRecord(AggregateStorageLocation ,
+ AggregateStorageLocation ,
+ Environment ) {
   LLVM_DEBUG({
 if (Dst.getType().getCanonicalType().getUnqualifiedType() !=
 Src.getType().getCanonicalType().getUnqualifiedType()) {
@@ -62,9 +60,10 @@
   }
 }
 
-bool recordsEqual(const AggregateStorageLocation , const Environment ,
-  const AggregateStorageLocation ,
-  const Environment ) {
+bool clang::dataflow::recordsEqual(const AggregateStorageLocation ,
+   const Environment ,
+   const AggregateStorageLocation ,
+   const Environment ) {
   LLVM_DEBUG({
 if (Loc2.getType().getCanonicalType().getUnqualifiedType() !=
 Loc1.getType().getCanonicalType().getUnqualifiedType()) {
@@ -124,6 +123,3 @@
 
   return true;
 }
-
-} // namespace dataflow
-} // namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105759: Implement P2361 Unevaluated string literals

2023-06-26 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments.



Comment at: clang/lib/Lex/Pragma.cpp:807
   if (Tok.is(tok::string_literal) && !Tok.hasUDSuffix()) {
 StringLiteralParser Literal(Tok, PP);
 if (Literal.hadError)

aaron.ballman wrote:
> aaron.ballman wrote:
> > cor3ntin wrote:
> > > aaron.ballman wrote:
> > > > Should this also be modified?
> > > Probably but because I'm not super familiar with module map things I 
> > > preferred being conservative
> > Paging @rsmith for opinions.
> > 
> > Lacking those opinions, I think being conservative here is fine.
> Pinging @ChuanqiXu for opinions.
I think the both options (to modify it or not) are acceptable. Because the 
input here  should be the output of the clang itself. See 
https://github.com/llvm/llvm-project/blob/ebd0b8a0472b865b7eb6e1a32af97ae31d829033/clang/lib/Basic/Module.cpp#L229-L231
 and 
https://github.com/llvm/llvm-project/blob/ebd0b8a0472b865b7eb6e1a32af97ae31d829033/clang/lib/Frontend/Rewrite/FrontendActions.cpp#L238-L240.

We can see there is no deprecated prefix. So while it is acceptable to modify 
this since its pattern matches the paper, it doesn't matter really since we can 
control the input completely.

Personally, I prefer to not touch it. Since I feel like this use case doesn't 
have been used a lot. So the effort here may not be worthy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105759

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


[PATCH] D153542: [C++20][Modules] Implement P2615R1 exported specialization diagnostics.

2023-06-26 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments.



Comment at: clang/lib/Sema/SemaModule.cpp:848-849
+if (auto *FD = dyn_cast(D)) {
+  if (FD->getTemplateSpecializationKind() == TSK_ExplicitSpecialization)
+BadExport = true;
+} else if (auto *VD = dyn_cast(D)) {

ChuanqiXu wrote:
> iains wrote:
> > ChuanqiXu wrote:
> > > Given P2615R1 doesn't allow explicit-instantiation in export block too.
> > I think I must be missing something - I do not see that in the paper - 
> > please could you expand your comment?
> See the proposed change in #[module.interface]p1 in the paper, it changed:
> 
> ```
> export declaration
> ```
> 
> to 
> 
> 
> ```
> export named-declaration
> ```
> 
> And the change between the old declaration and the new named declaration is 
> in the proposed change in #[dcl.pre]p1. So it shows the special declarations 
> shouldn't be allowed after `export`.
> 
> Also I noted it is allowed to write `export { declaration-seq_opt }` where 
> the `declaration-seq` may contain special declarations. It looks weird to me. 
> I'll try to check it and ask WG21 when necessary.
I checked the record. And it shows that the current wording reflects the 
intention. I mean

```
export template<> class A;
```

is not allowed. But,

```
export { template<> class A };
```

is allowed.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153542

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


[PATCH] D153542: [C++20][Modules] Implement P2615R1 exported specialization diagnostics.

2023-06-26 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments.



Comment at: clang/lib/Sema/SemaModule.cpp:848-849
+if (auto *FD = dyn_cast(D)) {
+  if (FD->getTemplateSpecializationKind() == TSK_ExplicitSpecialization)
+BadExport = true;
+} else if (auto *VD = dyn_cast(D)) {

iains wrote:
> ChuanqiXu wrote:
> > Given P2615R1 doesn't allow explicit-instantiation in export block too.
> I think I must be missing something - I do not see that in the paper - please 
> could you expand your comment?
See the proposed change in #[module.interface]p1 in the paper, it changed:

```
export declaration
```

to 


```
export named-declaration
```

And the change between the old declaration and the new named declaration is in 
the proposed change in #[dcl.pre]p1. So it shows the special declarations 
shouldn't be allowed after `export`.

Also I noted it is allowed to write `export { declaration-seq_opt }` where the 
`declaration-seq` may contain special declarations. It looks weird to me. I'll 
try to check it and ask WG21 when necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153542

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


[PATCH] D134334: [Clang] Fix crash in isCXXDeclarationSpecifier when attempting to annotate template name

2023-06-26 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added a comment.

ping @rsmith


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

https://reviews.llvm.org/D134334

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


[PATCH] D153580: [SystemZ][z/OS] Add support for z/OS link step (executable and shared libs)

2023-06-26 Thread Sean via Phabricator via cfe-commits
SeanP updated this revision to Diff 534816.
SeanP added a comment.

Apply comments from D153582  for similar 
changes.


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

https://reviews.llvm.org/D153580

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/ZOS.cpp
  clang/lib/Driver/ToolChains/ZOS.h
  clang/test/Driver/zos-ld.c

Index: clang/test/Driver/zos-ld.c
===
--- /dev/null
+++ clang/test/Driver/zos-ld.c
@@ -0,0 +1,123 @@
+// General tests that ld invocations for z/OS are valid.
+
+// 1. General C link for executable
+// RUN: %clang -### --target=s390x-ibm-zos %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=C-LD %s
+
+// C-LD: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// C-LD: "AMODE=64,LIST,DYNAM=DLL,MSGLEVEL=4,CASE=MIXED,REUS=RENT"
+// C-LD-SAME: "-e" "CELQSTRT"
+// C-LD-SAME: "-O" "CELQSTRT"
+// C-LD-SAME: "-u" "CELQMAIN"
+// C-LD-SAME: "-x" "/dev/null"
+// C-LD-SAME: "-S" "//'CEE.SCEEBND2'"
+// C-LD-SAME: "-S" "//'SYS1.CSSLIB'"
+// C-LD-SAME: "//'CEE.SCEELIB(CELQS001)'"
+// C-LD-SAME: "//'CEE.SCEELIB(CELQS003)'"
+// C-LD-SAME: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}zos{{/|}}libclang_rt.builtins-s390x.a"
+
+// 2. General C link for dll
+// RUN: %clang -### --shared --target=s390x-ibm-zos %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=C-LD-DLL %s
+
+// C-LD-DLL: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// C-LD-DLL: "AMODE=64,LIST,DYNAM=DLL,MSGLEVEL=4,CASE=MIXED,REUS=RENT"
+// C-LD-DLL-NOT: "-e" "CELQSTRT"
+// C-LD-DLL-NOT: "-O" "CELQSTRT"
+// C-LD-DLL-NOT: "-u" "CELQMAIN"
+// C-LD-DLL-SAME: "-x" "{{.*}}.x"
+// C-LD-DLL-SAME: "-S" "//'CEE.SCEEBND2'"
+// C-LD-DLL-SAME: "-S" "//'SYS1.CSSLIB'"
+// C-LD-DLL-SAME: "//'CEE.SCEELIB(CELQS001)'"
+// C-LD-DLL-SAME: "//'CEE.SCEELIB(CELQS003)'"
+// C-LD-DLL-SAME: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}zos{{/|}}libclang_rt.builtins-s390x.a"
+
+// 3. General C++ link for executable
+// RUN: %clangxx -### --target=s390x-ibm-zos %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CXX-LD %s
+
+// CXX-LD: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CXX-LD: "AMODE=64,LIST,DYNAM=DLL,MSGLEVEL=4,CASE=MIXED,REUS=RENT"
+// CXX-LD-SAME: "-e" "CELQSTRT"
+// CXX-LD-SAME: "-O" "CELQSTRT"
+// CXX-LD-SAME: "-u" "CELQMAIN"
+// CXX-LD-SAME: "-x" "/dev/null"
+// CXX-LD-SAME: "-S" "//'CEE.SCEEBND2'"
+// CXX-LD-SAME: "-S" "//'SYS1.CSSLIB'"
+// CXX-LD-SAME: "//'CEE.SCEELIB(CELQS001)'"
+// CXX-LD-SAME: "//'CEE.SCEELIB(CELQS003)'"
+// CXX-LD-SAME: "//'CEE.SCEELIB(CRTDQCXE)'"
+// CXX-LD-SAME: "//'CEE.SCEELIB(CRTDQCXS)'"
+// CXX-LD-SAME: "//'CEE.SCEELIB(CRTDQCXP)'"
+// CXX-LD-SAME: "//'CEE.SCEELIB(CRTDQCXA)'"
+// CXX-LD-SAME: "//'CEE.SCEELIB(CRTDQXLA)'"
+// CXX-LD-SAME: "//'CEE.SCEELIB(CRTDQUNW)'"
+// CXX-LD-SAME: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}zos{{/|}}libclang_rt.builtins-s390x.a"
+
+// 4. General C++ link for dll
+// RUN: %clangxx -### --shared --target=s390x-ibm-zos %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CXX-LD-DLL %s
+
+// CXX-LD-DLL: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CXX-LD-DLL: "AMODE=64,LIST,DYNAM=DLL,MSGLEVEL=4,CASE=MIXED,REUS=RENT"
+// CXX-LD-DLL-NOT: "-e" "CELQSTRT"
+// CXX-LD-DLL-NOT: "-O" "CELQSTRT"
+// CXX-LD-DLL-NOT: "-u" "CELQMAIN"
+// CXX-LD-DLL-SAME: "-x" "{{.*}}.x"
+// CXX-LD-DLL-SAME: "-S" "//'CEE.SCEEBND2'"
+// CXX-LD-DLL-SAME: "-S" "//'SYS1.CSSLIB'"
+// CXX-LD-DLL-SAME: "//'CEE.SCEELIB(CELQS001)'"
+// CXX-LD-DLL-SAME: "//'CEE.SCEELIB(CELQS003)'"
+// CXX-LD-DLL-SAME: "//'CEE.SCEELIB(CRTDQCXE)'"
+// CXX-LD-DLL-SAME: "//'CEE.SCEELIB(CRTDQCXS)'"
+// CXX-LD-DLL-SAME: "//'CEE.SCEELIB(CRTDQCXP)'"
+// CXX-LD-DLL-SAME: "//'CEE.SCEELIB(CRTDQCXA)'"
+// CXX-LD-DLL-SAME: "//'CEE.SCEELIB(CRTDQXLA)'"
+// CXX-LD-DLL-SAME: "//'CEE.SCEELIB(CRTDQUNW)'"
+// CXX-LD-DLL-SAME: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}zos{{/|}}libclang_rt.builtins-s390x.a"
+
+// 5. C++ link for executable w/ -mzos-hlq-le=, -mzos-hlq-csslib=
+// RUN: %clangxx -### --target=s390x-ibm-zos %s 2>&1 \
+// RUN:   -mzos-hlq-le= -mzos-hlq-csslib= \
+// RUN:   | FileCheck --check-prefix=CXX-LD5 %s
+
+// CXX-LD5: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CXX-LD5: "AMODE=64,LIST,DYNAM=DLL,MSGLEVEL=4,CASE=MIXED,REUS=RENT"
+// CXX-LD5-SAME: "-e" "CELQSTRT"
+// CXX-LD5-SAME: "-O" "CELQSTRT"
+// CXX-LD5-SAME: "-u" "CELQMAIN"
+// CXX-LD5-SAME: "-x" "/dev/null"
+// CXX-LD5-SAME: "-S" "//'.SCEEBND2'"
+// CXX-LD5-SAME: "-S" "//'.CSSLIB'"
+// CXX-LD5-SAME: "//'.SCEELIB(CELQS001)'"
+// CXX-LD5-SAME: "//'.SCEELIB(CELQS003)'"
+// CXX-LD5-SAME: "//'.SCEELIB(CRTDQCXE)'"
+// CXX-LD5-SAME: "//'.SCEELIB(CRTDQCXS)'"
+// CXX-LD5-SAME: "//'.SCEELIB(CRTDQCXP)'"
+// CXX-LD5-SAME: "//'.SCEELIB(CRTDQCXA)'"
+// CXX-LD5-SAME: "//'.SCEELIB(CRTDQXLA)'"
+// CXX-LD5-SAME: "//'.SCEELIB(CRTDQUNW)'"
+// CXX-LD5-SAME: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}zos{{/|}}libclang_rt.builtins-s390x.a"
+
+// 6. C++ link 

[PATCH] D150646: [clang][X86] Add __cpuidex function to cpuid.h

2023-06-26 Thread Aiden Grossman via Phabricator via cfe-commits
aidengrossman added a comment.

In D150646#4450551 , @glandium wrote:

> Did you find something?

Not yet. I just finished finals a week ago so I haven't had as much time as I 
would've liked to get through this. I'll contact some people tonight in regards 
to the Windows flag situation and ask for their thoughts on this and hopefully 
have some sort of solution at least by next Monday.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150646

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


[PATCH] D144829: [WIP][BPF] Add a few new insns under cpu=v4

2023-06-26 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song updated this revision to Diff 534801.
yonghong-song added a comment.

- avoid changing conditions during JMP -> JMPL conversion. Otherwise, 
verification may fail in some cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144829

Files:
  clang/lib/Basic/Targets/BPF.cpp
  clang/lib/Basic/Targets/BPF.h
  clang/test/Misc/target-invalid-cpu-note.c
  llvm/lib/Target/BPF/AsmParser/BPFAsmParser.cpp
  llvm/lib/Target/BPF/BPF.td
  llvm/lib/Target/BPF/BPFISelDAGToDAG.cpp
  llvm/lib/Target/BPF/BPFISelLowering.cpp
  llvm/lib/Target/BPF/BPFInstrFormats.td
  llvm/lib/Target/BPF/BPFInstrInfo.td
  llvm/lib/Target/BPF/BPFMIPeephole.cpp
  llvm/lib/Target/BPF/BPFMISimplifyPatchable.cpp
  llvm/lib/Target/BPF/BPFSubtarget.cpp
  llvm/lib/Target/BPF/BPFSubtarget.h
  llvm/lib/Target/BPF/Disassembler/BPFDisassembler.cpp
  llvm/lib/Target/BPF/MCTargetDesc/BPFAsmBackend.cpp
  llvm/lib/Target/BPF/MCTargetDesc/BPFInstPrinter.cpp
  llvm/lib/Target/BPF/MCTargetDesc/BPFMCCodeEmitter.cpp
  llvm/lib/Target/BPF/MCTargetDesc/BPFMCFixups.h
  llvm/lib/Target/BPF/MCTargetDesc/BPFMCTargetDesc.cpp
  llvm/test/CodeGen/BPF/bswap.ll
  llvm/test/CodeGen/BPF/sdiv_smod.ll
  llvm/test/CodeGen/BPF/sext_ld.ll
  llvm/test/CodeGen/BPF/sext_mov.ll

Index: llvm/test/CodeGen/BPF/sext_mov.ll
===
--- /dev/null
+++ llvm/test/CodeGen/BPF/sext_mov.ll
@@ -0,0 +1,109 @@
+; RUN: llc -march=bpfel -mcpu=v4 -verify-machineinstrs -show-mc-encoding < %s | FileCheck %s
+; Source:
+;  short f1(int a) {
+;return (char)a;
+;  }
+;  int f2(int a) {
+;return (char)a;
+;  }
+;  long f3(int a) {
+;return (char)a;
+;  }
+;  int f4(int a) {
+;return (short)a;
+;  }
+;  long f5(int a) {
+;return (short)a;
+;  }
+;  long f6(long a) {
+;return (int)a;
+;  }
+; Compilation flags:
+;   clang -target bpf -O2 -S -emit-llvm -Xclang -disable-llvm-passes t.c
+
+; Function Attrs: nounwind
+define dso_local i16 @f1(i32 noundef %a) #0 {
+entry:
+  %a.addr = alloca i32, align 4
+  store i32 %a, ptr %a.addr, align 4, !tbaa !3
+  %0 = load i32, ptr %a.addr, align 4, !tbaa !3
+  %conv = trunc i32 %0 to i8
+  %conv1 = sext i8 %conv to i16
+  ret i16 %conv1
+}
+; CHECK: w0 = (s8)w1  # encoding: [0xbc,0x10,0x08,0x00,0x00,0x00,0x00,0x00]
+
+; Function Attrs: nounwind
+define dso_local i32 @f2(i32 noundef %a) #0 {
+entry:
+  %a.addr = alloca i32, align 4
+  store i32 %a, ptr %a.addr, align 4, !tbaa !3
+  %0 = load i32, ptr %a.addr, align 4, !tbaa !3
+  %conv = trunc i32 %0 to i8
+  %conv1 = sext i8 %conv to i32
+  ret i32 %conv1
+}
+; CHECK: w0 = (s8)w1  # encoding: [0xbc,0x10,0x08,0x00,0x00,0x00,0x00,0x00]
+
+; Function Attrs: nounwind
+define dso_local i64 @f3(i32 noundef %a) #0 {
+entry:
+  %a.addr = alloca i32, align 4
+  store i32 %a, ptr %a.addr, align 4, !tbaa !3
+  %0 = load i32, ptr %a.addr, align 4, !tbaa !3
+  %conv = trunc i32 %0 to i8
+  %conv1 = sext i8 %conv to i64
+  ret i64 %conv1
+}
+; CHECK: r0 = (s8)r1  # encoding: [0xbf,0x10,0x08,0x00,0x00,0x00,0x00,0x00]
+
+; Function Attrs: nounwind
+define dso_local i32 @f4(i32 noundef %a) #0 {
+entry:
+  %a.addr = alloca i32, align 4
+  store i32 %a, ptr %a.addr, align 4, !tbaa !3
+  %0 = load i32, ptr %a.addr, align 4, !tbaa !3
+  %conv = trunc i32 %0 to i16
+  %conv1 = sext i16 %conv to i32
+  ret i32 %conv1
+}
+; CHECK: w0 = (s16)w1  # encoding: [0xbc,0x10,0x10,0x00,0x00,0x00,0x00,0x00]
+
+; Function Attrs: nounwind
+define dso_local i64 @f5(i32 noundef %a) #0 {
+entry:
+  %a.addr = alloca i32, align 4
+  store i32 %a, ptr %a.addr, align 4, !tbaa !3
+  %0 = load i32, ptr %a.addr, align 4, !tbaa !3
+  %conv = trunc i32 %0 to i16
+  %conv1 = sext i16 %conv to i64
+  ret i64 %conv1
+}
+; CHECK: r0 = (s16)r1  # encoding: [0xbf,0x10,0x10,0x00,0x00,0x00,0x00,0x00]
+
+; Function Attrs: nounwind
+define dso_local i64 @f6(i64 noundef %a) #0 {
+entry:
+  %a.addr = alloca i64, align 8
+  store i64 %a, ptr %a.addr, align 8, !tbaa !7
+  %0 = load i64, ptr %a.addr, align 8, !tbaa !7
+  %conv = trunc i64 %0 to i32
+  %conv1 = sext i32 %conv to i64
+  ret i64 %conv1
+}
+; CHECK: r0 = (s32)r1  # encoding: [0xbf,0x10,0x20,0x00,0x00,0x00,0x00,0x00]
+
+attributes #0 = { nounwind "frame-pointer"="all" "no-trapping-math"="true" "stack-protector-buffer-size"="8" }
+
+!llvm.module.flags = !{!0, !1}
+!llvm.ident = !{!2}
+
+!0 = !{i32 1, !"wchar_size", i32 4}
+!1 = !{i32 7, !"frame-pointer", i32 2}
+!2 = !{!"clang version 17.0.0 (https://github.com/llvm/llvm-project.git 569bd3b841e3167ddd7c6ceeddb282d3c280e761)"}
+!3 = !{!4, !4, i64 0}
+!4 = !{!"int", !5, i64 0}
+!5 = !{!"omnipotent char", !6, i64 0}
+!6 = !{!"Simple C/C++ TBAA"}
+!7 = !{!8, !8, i64 0}
+!8 = !{!"long", !5, i64 0}
Index: llvm/test/CodeGen/BPF/sext_ld.ll
===
--- /dev/null
+++ llvm/test/CodeGen/BPF/sext_ld.ll
@@ -0,0 +1,104 @@
+; RUN: llc -march=bpfel 

[PATCH] D153579: [clang-format] Fix RAS reference alignment when PAS is left or middle

2023-06-26 Thread Owen Pan via Phabricator via cfe-commits
owenpan added inline comments.



Comment at: clang/lib/Format/WhitespaceManager.cpp:459
 Changes[i].Spaces != 0) {
   for (int Previous = i - 1;
Previous >= 0 &&

See below.



Comment at: clang/lib/Format/WhitespaceManager.cpp:463-466
+if (Changes[Previous].Tok->is(tok::amp) ||
+Changes[Previous].Tok->is(tok::ampamp)) {
+  if (Style.ReferenceAlignment != FormatStyle::RAS_Right &&
+  Style.ReferenceAlignment != FormatStyle::RAS_Pointer) {

Shorter and IMO easier to follow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153579

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


[PATCH] D153585: [clang-format] Fix align consecutive declarations over function pointers

2023-06-26 Thread Owen Pan via Phabricator via cfe-commits
owenpan added inline comments.



Comment at: clang/lib/Format/WhitespaceManager.cpp:869-871
 if (C.Tok->is(TT_FunctionDeclarationName))
   return true;
+if (C.Tok->is(TT_FunctionTypeLParen))

We should merge the two `if` statements.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153585

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


[PATCH] D152628: [RISCV] Add __builtin_riscv_zip/unzip for Zbkb to match gcc.

2023-06-26 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added a comment.
Herald added a subscriber: wangpc.

In D152628#4416794 , @asb wrote:

> I'm not super familiar with these builtins so this might be a silly question 
> why are the new builtins added in this patch LiLi (long int) rather than ZiZi 
> (int32_t) like the old `_32` suffixed builtins?

In my head I was thinking this would be the name we would use for xlen for 
i32/i64 too so I was matching clmul. But maybe that's not a good idea. I think 
we just need to figure out the header file definition for these intrinsics and 
get both compilers to agree on that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152628

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


[PATCH] D148869: [Driver][BareMetal] Support --emit-static-lib in BareMetal driver

2023-06-26 Thread Petr Hosek via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
phosek marked an inline comment as done.
Closed by commit rG5614d1a0a10d: [Driver][BareMetal] Support --emit-static-lib 
in BareMetal driver (authored by phosek).

Changed prior to commit:
  https://reviews.llvm.org/D148869?vs=515561=534780#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148869

Files:
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/BareMetal.h
  clang/test/Driver/baremetal.cpp

Index: clang/test/Driver/baremetal.cpp
===
--- clang/test/Driver/baremetal.cpp
+++ clang/test/Driver/baremetal.cpp
@@ -1,5 +1,9 @@
 // UNSUPPORTED: system-windows
 
+// RUN: %clang -### %s --target=armv6-none-eabi --emit-static-lib 2>&1 \
+// RUN:   | FileCheck -check-prefixes=CHECK-STATIC-LIB %s
+// CHECK-STATIC-LIB: {{.*}}llvm-ar{{.*}}" "rcsD"
+
 // RUN: %clang %s -### --target=armv6m-none-eabi -o %t.out 2>&1 \
 // RUN: -T semihosted.lds \
 // RUN: -L some/directory/user/asked/for \
Index: clang/lib/Driver/ToolChains/BareMetal.h
===
--- clang/lib/Driver/ToolChains/BareMetal.h
+++ clang/lib/Driver/ToolChains/BareMetal.h
@@ -32,6 +32,7 @@
 
 protected:
   Tool *buildLinker() const override;
+  Tool *buildStaticLibTool() const override;
 
 public:
   bool useIntegratedAs() const override { return true; }
@@ -83,6 +84,20 @@
 namespace tools {
 namespace baremetal {
 
+class LLVM_LIBRARY_VISIBILITY StaticLibTool : public Tool {
+public:
+  StaticLibTool(const ToolChain )
+  : Tool("baremetal::StaticLibTool", "llvm-ar", TC) {}
+
+  bool hasIntegratedCPP() const override { return false; }
+  bool isLinkJob() const override { return true; }
+
+  void ConstructJob(Compilation , const JobAction ,
+const InputInfo , const InputInfoList ,
+const llvm::opt::ArgList ,
+const char *LinkingOutput) const override;
+};
+
 class LLVM_LIBRARY_VISIBILITY Linker : public Tool {
 public:
   Linker(const ToolChain ) : Tool("baremetal::Linker", "ld.lld", TC) {}
Index: clang/lib/Driver/ToolChains/BareMetal.cpp
===
--- clang/lib/Driver/ToolChains/BareMetal.cpp
+++ clang/lib/Driver/ToolChains/BareMetal.cpp
@@ -232,6 +232,10 @@
   return new tools::baremetal::Linker(*this);
 }
 
+Tool *BareMetal::buildStaticLibTool() const {
+  return new tools::baremetal::StaticLibTool(*this);
+}
+
 std::string BareMetal::computeSysRoot() const {
   return computeBaseSysRoot(getDriver(), getTriple());
 }
@@ -368,6 +372,51 @@
   llvm_unreachable("Unhandled RuntimeLibType.");
 }
 
+void baremetal::StaticLibTool::ConstructJob(Compilation , const JobAction ,
+const InputInfo ,
+const InputInfoList ,
+const ArgList ,
+const char *LinkingOutput) const {
+  const Driver  = getToolChain().getDriver();
+
+  // Silence warning for "clang -g foo.o -o foo"
+  Args.ClaimAllArgs(options::OPT_g_Group);
+  // and "clang -emit-llvm foo.o -o foo"
+  Args.ClaimAllArgs(options::OPT_emit_llvm);
+  // and for "clang -w foo.o -o foo". Other warning options are already
+  // handled somewhere else.
+  Args.ClaimAllArgs(options::OPT_w);
+  // Silence warnings when linking C code with a C++ '-stdlib' argument.
+  Args.ClaimAllArgs(options::OPT_stdlib_EQ);
+
+  // ar tool command "llvm-ar   ".
+  ArgStringList CmdArgs;
+  // Create and insert file members with a deterministic index.
+  CmdArgs.push_back("rcsD");
+  CmdArgs.push_back(Output.getFilename());
+
+  for (const auto  : Inputs) {
+if (II.isFilename()) {
+  CmdArgs.push_back(II.getFilename());
+}
+  }
+
+  // Delete old output archive file if it already exists before generating a new
+  // archive file.
+  const char *OutputFileName = Output.getFilename();
+  if (Output.isFilename() && llvm::sys::fs::exists(OutputFileName)) {
+if (std::error_code EC = llvm::sys::fs::remove(OutputFileName)) {
+  D.Diag(diag::err_drv_unable_to_remove_file) << EC.message();
+  return;
+}
+  }
+
+  const char *Exec = Args.MakeArgString(getToolChain().GetStaticLibToolPath());
+  C.addCommand(std::make_unique(JA, *this,
+ ResponseFileSupport::AtFileCurCP(),
+ Exec, CmdArgs, Inputs, Output));
+}
+
 void baremetal::Linker::ConstructJob(Compilation , const JobAction ,
  const InputInfo ,
  const InputInfoList ,
@@ -417,8 +466,7 @@
   CmdArgs.push_back("-o");
   CmdArgs.push_back(Output.getFilename());
 

[PATCH] D148869: [Driver][BareMetal] Support --emit-static-lib in BareMetal driver

2023-06-26 Thread Petr Hosek via Phabricator via cfe-commits
phosek marked an inline comment as done.
phosek added inline comments.



Comment at: clang/lib/Driver/ToolChains/BareMetal.cpp:170
 bool BareMetal::handlesTarget(const llvm::Triple ) {
   return isARMBareMetal(Triple) || isAArch64BareMetal(Triple) ||
  isRISCVBareMetal(Triple);

abrachet wrote:
> Looks like clang-format is complaining about this file
Addressed in the commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148869

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


[clang] 5614d1a - [Driver][BareMetal] Support --emit-static-lib in BareMetal driver

2023-06-26 Thread Petr Hosek via cfe-commits

Author: Petr Hosek
Date: 2023-06-26T23:19:02Z
New Revision: 5614d1a0a10de06887a9a021c104e601eb441413

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

LOG: [Driver][BareMetal] Support --emit-static-lib in BareMetal driver

This allows building static libraries with Clang driver.

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/BareMetal.cpp
clang/lib/Driver/ToolChains/BareMetal.h
clang/test/Driver/baremetal.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/BareMetal.cpp 
b/clang/lib/Driver/ToolChains/BareMetal.cpp
index 09cc72f03363d..47a533501ec73 100644
--- a/clang/lib/Driver/ToolChains/BareMetal.cpp
+++ b/clang/lib/Driver/ToolChains/BareMetal.cpp
@@ -232,6 +232,10 @@ Tool *BareMetal::buildLinker() const {
   return new tools::baremetal::Linker(*this);
 }
 
+Tool *BareMetal::buildStaticLibTool() const {
+  return new tools::baremetal::StaticLibTool(*this);
+}
+
 std::string BareMetal::computeSysRoot() const {
   return computeBaseSysRoot(getDriver(), getTriple());
 }
@@ -368,6 +372,51 @@ void BareMetal::AddLinkRuntimeLib(const ArgList ,
   llvm_unreachable("Unhandled RuntimeLibType.");
 }
 
+void baremetal::StaticLibTool::ConstructJob(Compilation , const JobAction 
,
+const InputInfo ,
+const InputInfoList ,
+const ArgList ,
+const char *LinkingOutput) const {
+  const Driver  = getToolChain().getDriver();
+
+  // Silence warning for "clang -g foo.o -o foo"
+  Args.ClaimAllArgs(options::OPT_g_Group);
+  // and "clang -emit-llvm foo.o -o foo"
+  Args.ClaimAllArgs(options::OPT_emit_llvm);
+  // and for "clang -w foo.o -o foo". Other warning options are already
+  // handled somewhere else.
+  Args.ClaimAllArgs(options::OPT_w);
+  // Silence warnings when linking C code with a C++ '-stdlib' argument.
+  Args.ClaimAllArgs(options::OPT_stdlib_EQ);
+
+  // ar tool command "llvm-ar   ".
+  ArgStringList CmdArgs;
+  // Create and insert file members with a deterministic index.
+  CmdArgs.push_back("rcsD");
+  CmdArgs.push_back(Output.getFilename());
+
+  for (const auto  : Inputs) {
+if (II.isFilename()) {
+  CmdArgs.push_back(II.getFilename());
+}
+  }
+
+  // Delete old output archive file if it already exists before generating a 
new
+  // archive file.
+  const char *OutputFileName = Output.getFilename();
+  if (Output.isFilename() && llvm::sys::fs::exists(OutputFileName)) {
+if (std::error_code EC = llvm::sys::fs::remove(OutputFileName)) {
+  D.Diag(diag::err_drv_unable_to_remove_file) << EC.message();
+  return;
+}
+  }
+
+  const char *Exec = Args.MakeArgString(getToolChain().GetStaticLibToolPath());
+  C.addCommand(std::make_unique(JA, *this,
+ ResponseFileSupport::AtFileCurCP(),
+ Exec, CmdArgs, Inputs, Output));
+}
+
 void baremetal::Linker::ConstructJob(Compilation , const JobAction ,
  const InputInfo ,
  const InputInfoList ,
@@ -417,8 +466,7 @@ void baremetal::Linker::ConstructJob(Compilation , const 
JobAction ,
   CmdArgs.push_back("-o");
   CmdArgs.push_back(Output.getFilename());
 
-  C.addCommand(std::make_unique(JA, *this,
- ResponseFileSupport::AtFileCurCP(),
- 
Args.MakeArgString(TC.GetLinkerPath()),
- CmdArgs, Inputs, Output));
+  C.addCommand(std::make_unique(
+  JA, *this, ResponseFileSupport::AtFileCurCP(),
+  Args.MakeArgString(TC.GetLinkerPath()), CmdArgs, Inputs, Output));
 }

diff  --git a/clang/lib/Driver/ToolChains/BareMetal.h 
b/clang/lib/Driver/ToolChains/BareMetal.h
index e594b12a7c924..fc39a2a10e019 100644
--- a/clang/lib/Driver/ToolChains/BareMetal.h
+++ b/clang/lib/Driver/ToolChains/BareMetal.h
@@ -32,6 +32,7 @@ class LLVM_LIBRARY_VISIBILITY BareMetal : public ToolChain {
 
 protected:
   Tool *buildLinker() const override;
+  Tool *buildStaticLibTool() const override;
 
 public:
   bool useIntegratedAs() const override { return true; }
@@ -83,6 +84,20 @@ class LLVM_LIBRARY_VISIBILITY BareMetal : public ToolChain {
 namespace tools {
 namespace baremetal {
 
+class LLVM_LIBRARY_VISIBILITY StaticLibTool : public Tool {
+public:
+  StaticLibTool(const ToolChain )
+  : Tool("baremetal::StaticLibTool", "llvm-ar", TC) {}
+
+  bool hasIntegratedCPP() const override { return false; }
+  bool isLinkJob() const override { return true; }
+
+  void ConstructJob(Compilation , const JobAction ,
+ 

[PATCH] D139837: [Clang] Implements CTAD for aggregates P1816R0 and P2082R1

2023-06-26 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen updated this revision to Diff 534778.
ychen added a comment.

- add a DeductionCandidate parameter


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139837

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/TemplateDeduction.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/SemaTemplate/aggregate-deduction-candidate.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1246,7 +1246,7 @@
 
   Class template argument deduction for aggregates
   https://wg21.link/p1816r0;>P1816R0
-  No
+  Clang 17
 

 https://wg21.link/p2082r1;>P2082R1
Index: clang/test/SemaTemplate/aggregate-deduction-candidate.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/aggregate-deduction-candidate.cpp
@@ -0,0 +1,364 @@
+// RUN: %clang_cc1 -std=c++20 -verify -ast-dump -ast-dump-decl-types -ast-dump-filter "deduction guide" %s | FileCheck %s --strict-whitespace
+
+namespace Basic {
+  template struct A {
+T x;
+T y;
+  };
+
+  A a1 = {3.0, 4.0};
+  A a2 = {.x = 3.0, .y = 4.0};
+
+  A a3(3.0, 4.0);
+
+  // CHECK-LABEL: Dumping Basic:::
+  // CHECK: FunctionTemplateDecl {{.*}} implicit 
+  // CHECK: |-TemplateTypeParmDecl {{.*}} referenced class depth 0 index 0 T
+  // CHECK: |-CXXDeductionGuideDecl {{.*}} implicit  'auto (T, T) -> A'
+  // CHECK: | |-ParmVarDecl {{.*}} 'T'
+  // CHECK: | `-ParmVarDecl {{.*}} 'T'
+  // CHECK: `-CXXDeductionGuideDecl {{.*}} implicit used  'auto (double, double) -> Basic::A'
+  // CHECK:   |-TemplateArgument type 'double'
+  // CHECK:   | `-BuiltinType {{.*}} 'double'
+  // CHECK:   |-ParmVarDecl {{.*}} 'double':'double'
+  // CHECK:   `-ParmVarDecl {{.*}} 'double':'double'
+  // CHECK: FunctionProtoType {{.*}} 'auto (T, T) -> A' dependent trailing_return cdecl
+  // CHECK: |-InjectedClassNameType {{.*}} 'A' dependent
+  // CHECK: | `-CXXRecord {{.*}} 'A'
+  // CHECK: |-TemplateTypeParmType {{.*}} 'T' dependent depth 0 index 0
+  // CHECK: | `-TemplateTypeParm {{.*}} 'T'
+  // CHECK: `-TemplateTypeParmType {{.*}} 'T' dependent depth 0 index 0
+  // CHECK:   `-TemplateTypeParm {{.*}} 'T'
+
+  template  struct S { // expected-note 2 {{candidate}}
+T x;
+T y;
+  };
+
+  template  struct C { // expected-note 10 {{candidate}}
+S s;
+T t;
+  };
+
+  template  struct D { // expected-note 6 {{candidate}}
+S s;
+T t;
+  };
+
+  C c1 = {1, 2}; // expected-error {{no viable}}
+  C c2 = {1, 2, 3}; // expected-error {{no viable}}
+  C c3 = {{1u, 2u}, 3};
+
+  C c4(1, 2);// expected-error {{no viable}}
+  C c5(1, 2, 3); // expected-error {{no viable}}
+  C c6({1u, 2u}, 3);
+
+  D d1 = {1, 2}; // expected-error {{no viable}}
+  D d2 = {1, 2, 3};
+
+  D d3(1, 2); // expected-error {{no viable}}
+  // CTAD succeed but brace elision is not allowed for parenthesized aggregate init. 
+  D d4(1, 2, 3); // expected-error {{no viable}}
+
+  // CHECK-LABEL: Dumping Basic:::
+  // CHECK: FunctionTemplateDecl {{.*}} implicit 
+  // CHECK: |-TemplateTypeParmDecl {{.*}} referenced typename depth 0 index 0 T
+  // CHECK: |-CXXDeductionGuideDecl {{.*}} implicit  'auto (S, T) -> C'
+  // CHECK: | |-ParmVarDecl {{.*}} 'S':'S'
+  // CHECK: | `-ParmVarDecl {{.*}} 'T'
+  // CHECK: `-CXXDeductionGuideDecl {{.*}} implicit used  'auto (S, int) -> Basic::C'
+  // CHECK:   |-TemplateArgument type 'int'
+  // CHECK:   | `-BuiltinType {{.*}} 'int'
+  // CHECK:   |-ParmVarDecl {{.*}} 'S':'Basic::S'
+  // CHECK:   `-ParmVarDecl {{.*}} 'int':'int'
+  // CHECK: FunctionProtoType {{.*}} 'auto (S, T) -> C' dependent trailing_return cdecl
+  // CHECK: |-InjectedClassNameType {{.*}} 'C' dependent
+  // CHECK: | `-CXXRecord {{.*}} 'C'
+  // CHECK: |-ElaboratedType {{.*}} 'S' sugar dependent
+  // CHECK: | `-TemplateSpecializationType {{.*}} 'S' dependent S
+  // CHECK: |   `-TemplateArgument type 'T'
+  // CHECK: | `-TemplateTypeParmType {{.*}} 'T' dependent depth 0 index 0
+  // CHECK: |   `-TemplateTypeParm {{.*}} 'T'
+  // CHECK: `-TemplateTypeParmType {{.*}} 'T' dependent depth 0 index 0
+  // CHECK:   `-TemplateTypeParm {{.*}} 'T'
+
+  // CHECK-LABEL: Dumping Basic:::
+  // CHECK: FunctionTemplateDecl {{.*}} implicit 
+  

[PATCH] D153667: [HIP]: Add gpu-link-output to control link job creation

2023-06-26 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added a comment.

In D153667#4450705 , @jrbyrnes wrote:

> In D153667#4450517 , @jhuber6 wrote:
>
>> What's the difference here between this and the existing `--hip-link`?
>
> Hi @jhuber6
>
> The commit is poorly named, the main purpose is to introduce 
> `-no-gpu-link-output.`
>
> We want a way to produce relocatable from source. In terms of the Driver, 
> this means building actions and jobs for phases up to `phases::Assemble`. 
> `-no- gpu-link-output` does this by overriding BuildActions to stop after 
> `phases::Assemble` (similar to `-no-gpu-bundle-output`). `-gpu-link-output` 
> is NFCI. COMGR would be the client of this, and it would be up to COMGR to 
> handle linking of the relocatable.
>
> AFAICT, `-hip-link` allows for linking of offload-bundles, so it is 
> conceptually different. We can get (somewhat) close to what we with 
> `-emit-llvm -hip-link`, but that is probably more due to `-emit-llvm`. 
> `-hip-link` by itself produces linker actions / jobs which what we are trying 
> to avoid here.

So, you run the backend and obtain a relocatable ELF, but do not link it via 
`lld`? If I'm understanding this correctly, that would be the difference 
between `-flto` and `-fno-lto`, or `-foffload-lto` and `-fno-offload-lto`, 
AMDGPU always having `-flto` on currently. Also I recall AMDGPU / HIP 
completely disabling the backend step at some point, so it only emits LLVM-IR.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153667

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


[PATCH] D152996: [RISCV][POC] Model frm control for vfadd

2023-06-26 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVInsertReadWriteCSR.cpp:105
+.addImm(FRMImm);
+MI.addOperand(MachineOperand::CreateReg(RISCV::FRM, /*IsDef*/ false,
+/*IsImp*/ true));

We still need to add the RISCV::FRM operand for FRMImm == 7. But we should 
probably do it in `RISCVTargetLowering::AdjustInstrPostInstrSelection`



Comment at: llvm/lib/Target/RISCV/RISCVInstrFormats.td:226
+  // to the correct CSR.
+  bit IsRVVFixedPoint = 0;
+  let TSFlags{20} =  IsRVVFixedPoint;

Something like UsesVXRM is probably better? As we established before, not 
everything the vector spec calls fixed point uses VXRM. Or make HasRoundOp a 
2-bit field that encodings what type of rounding mode?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152996

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


[PATCH] D153667: [HIP]: Add gpu-link-output to control link job creation

2023-06-26 Thread Jeffrey Byrnes via Phabricator via cfe-commits
jrbyrnes added a comment.

In D153667#4450517 , @jhuber6 wrote:

> What's the difference here between this and the existing `--hip-link`?

Hi @jhuber6

The commit is poorly named, the main purpose is to introduce 
`-no-gpu-link-output.`

We want a way to produce relocatable from source. In terms of the Driver, this 
means building actions and jobs for phases up to `phases::Assemble`. `-no- 
gpu-link-output` does this by overriding BuildActions to stop after 
`phases::Assemble` (similar to `-no-gpu-bundle-output`). `-gpu-link-output` is 
NFCI. COMGR would be the client of this, and it would be up to COMGR to handle 
linking of the relocatable.

AFAICT, `-hip-link` allows for linking of offload-bundles, so it is 
conceptually different. We can get (somewhat) close to what we with `-emit-llvm 
-hip-link`, but that is probably more due to `-emit-llvm`. `-hip-link` by 
itself produces linker actions / jobs which what we are trying to avoid here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153667

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


[PATCH] D153652: [Support] Don't set "all_exe" mode by default for file written by llvm::writeToOutput

2023-06-26 Thread Alexey Lapshin via Phabricator via cfe-commits
avl added inline comments.



Comment at: 
llvm/test/tools/llvm-dwarfutil/ELF/X86/mirror-permissions-unix.test:2
+## The Unix version of this test must use umask(1) because
+## llvm-darfutil respects the umask in setting output permissions.
+## Setting the umask to 0 ensures deterministic permissions across





Comment at: llvm/unittests/Support/raw_ostream_test.cpp:499
+  ErrorOr Perms = llvm::sys::fs::getPermissions(Path);
+  ASSERT_TRUE(!!Perms);
+  EXPECT_EQ(0, *Perms & llvm::sys::fs::all_exe);

!!  looks a bit unclear. Probably check it in more explicit way?

EXPECT_TRUE(Perms && !(*Perms & llvm::sys::fs::all_exe)); 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153652

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


[PATCH] D152986: [clang] Allow 'nomerge' attribute for function pointers

2023-06-26 Thread Eduard Zingerman 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 rG06eee734c1ea: [clang] Allow nomerge attribute 
for function pointers (authored by eddyz87).

Changed prior to commit:
  https://reviews.llvm.org/D152986?vs=534585=534762#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152986

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/attr-nomerge.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Parser/stmt-attributes.c
  clang/test/Parser/stmt-attributes.cpp
  clang/test/Parser/stmt-attributes.m
  clang/test/Sema/attr-nomerge-ast.cpp
  clang/test/Sema/attr-nomerge.cpp

Index: clang/test/Sema/attr-nomerge.cpp
===
--- clang/test/Sema/attr-nomerge.cpp
+++ clang/test/Sema/attr-nomerge.cpp
@@ -8,10 +8,14 @@
   int x;
   [[clang::nomerge]] x = 10; // expected-warning {{'nomerge' attribute is ignored because there exists no call expression inside the statement}}
 
-  [[clang::nomerge]] label: bar(); // expected-error {{'nomerge' attribute only applies to functions and statements}}
+  [[clang::nomerge]] label: bar(); // expected-error {{'nomerge' attribute only applies to functions, statements and variables}}
 
 }
 
 [[clang::nomerge]] int f();
 
-[[clang::nomerge]] static int i = f(); // expected-error {{'nomerge' attribute only applies to functions and statements}}
+[[clang::nomerge]] static int i = f(); // expected-warning {{'nomerge' attribute is ignored because 'i' is not a function pointer}}
+
+[[clang::nomerge]] void (*j)(void);
+
+struct [[clang::nomerge]] buz {}; // expected-error {{'nomerge' attribute only applies to functions, statements and variables}}
Index: clang/test/Sema/attr-nomerge-ast.cpp
===
--- clang/test/Sema/attr-nomerge-ast.cpp
+++ clang/test/Sema/attr-nomerge-ast.cpp
@@ -4,6 +4,7 @@
 [[clang::nomerge]] void func();
 void func();
 [[clang::nomerge]] void func() {}
+[[clang::nomerge]] void (*var)(void);
 
 // CHECK: FunctionDecl {{.*}} func 'void ()'
 // CHECK-NEXT: NoMergeAttr
@@ -14,3 +15,6 @@
 // CHECK-NEXT: FunctionDecl {{.*}} func 'void ()'
 // CHECK-NEXT: CompoundStmt
 // CHECK-NEXT: NoMergeAttr
+
+// CHECK-NEXT: VarDecl {{.*}} var 'void (*)()'
+// CHECK-NEXT: NoMergeAttr
Index: clang/test/Parser/stmt-attributes.m
===
--- clang/test/Parser/stmt-attributes.m
+++ clang/test/Parser/stmt-attributes.m
@@ -19,7 +19,7 @@
 
 @implementation Test
 - (void)foo __attribute__((nomerge)) {
-  // expected-error@-1 {{'nomerge' attribute only applies to functions and statements}}
+  // expected-error@-1 {{'nomerge' attribute only applies to functions, statements and variables}}
 }
 
 - (void)bar {
Index: clang/test/Parser/stmt-attributes.cpp
===
--- clang/test/Parser/stmt-attributes.cpp
+++ clang/test/Parser/stmt-attributes.cpp
@@ -6,7 +6,7 @@
 
 template 
 class __attribute__((nomerge)) A {
-  // expected-error@-1 {{'nomerge' attribute only applies to functions and statements}}
+  // expected-error@-1 {{'nomerge' attribute only applies to functions, statements and variables}}
 };
 
 class B : public A<> {
Index: clang/test/Parser/stmt-attributes.c
===
--- clang/test/Parser/stmt-attributes.c
+++ clang/test/Parser/stmt-attributes.c
@@ -82,9 +82,11 @@
   int x;
   __attribute__((nomerge)) x = 10; // expected-warning {{'nomerge' attribute is ignored because there exists no call expression inside the statement}}
 
-  __attribute__((nomerge)) label : bar(); // expected-error {{'nomerge' attribute only applies to functions and statements}}
+  __attribute__((nomerge)) label : bar(); // expected-error {{'nomerge' attribute only applies to functions, statements and variables}}
 }
 
 int f(void);
 
-__attribute__((nomerge)) static int i; // expected-error {{'nomerge' attribute only applies to functions and statements}}
+__attribute__((nomerge)) static int (*fptr)(void);
+__attribute__((nomerge)) static int i; // expected-warning {{'nomerge' attribute is ignored because 'i' is not a function pointer}}
+struct buz {} __attribute__((nomerge)); // expected-error {{'nomerge' attribute only applies to functions, statements and variables}}
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -105,7 +105,7 @@

[clang] 06eee73 - [clang] Allow 'nomerge' attribute for function pointers

2023-06-26 Thread Eduard Zingerman via cfe-commits

Author: Eduard Zingerman
Date: 2023-06-27T01:15:45+03:00
New Revision: 06eee734c1ea9de754c1e7e8c79c0897b9e01350

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

LOG: [clang] Allow 'nomerge' attribute for function pointers

Allow specifying 'nomerge' attribute for function pointers,
e.g. like in the following C code:

extern void (*foo)(void) __attribute__((nomerge));
void bar(long i) {
  if (i)
foo();
  else
foo();
}

With the goal to attach 'nomerge' to both calls done through 'foo':

@foo = external local_unnamed_addr global ptr, align 8
define dso_local void @bar(i64 noundef %i) local_unnamed_addr #0 {
  ; ...
  %0 = load ptr, ptr @foo, align 8, !tbaa !5
  ; ...
if.then:
  tail call void %0() #1
  br label %if.end
if.else:
  tail call void %0() #1
  br label %if.end
if.end:
  ret void
}
; ...
attributes #1 = { nomerge ... }

Report a warning in case if 'nomerge' is specified for a variable that
is not a function pointer, e.g.:

t.c:2:22: warning: 'nomerge' attribute is ignored because 'j' is not a 
function pointer [-Wignored-attributes]
2 | int j __attribute__((nomerge));
  |  ^

The intended use-case is for BPF backend.

BPF provides a sort of "standard library" functions that are called
helpers. BPF also verifies usage of these helpers before program
execution. Because of limitations of verification / runtime model it
is important to keep calls to some of such helpers from merging.

An example could be found by the link [1], there input C code:

 if (data_end - data > 1024) {
 bpf_for_each_map_elem(, cb, _data, 0);
 } else {
 bpf_for_each_map_elem(, cb, _data, 0);
 }

Is converted to bytecode equivalent to:

 if (data_end - data > 1024)
   tmp = 
 else
   tmp = 
 bpf_for_each_map_elem(tmp, cb, _data, 0);

However, BPF verification/runtime requires to use the same map address
for each particular `bpf_for_each_map_elem()` call.

The 'nomerge' attribute is a perfect match for this situation, but
unfortunately BPF helpers are declared as pointers to functions:

static long (*bpf_for_each_map_elem)(void *map, ...) = (void *) 164;

Hence, this commit, allowing to use 'nomerge' for function pointers.

[1] https://lore.kernel.org/bpf/03bdf90f-f374-1e67-69d6-76dd9c831...@meta.com/

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

Added: 


Modified: 
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/AttrDocs.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/CodeGen/CGCall.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/CodeGen/attr-nomerge.cpp
clang/test/Misc/pragma-attribute-supported-attributes-list.test
clang/test/Parser/stmt-attributes.c
clang/test/Parser/stmt-attributes.cpp
clang/test/Parser/stmt-attributes.m
clang/test/Sema/attr-nomerge-ast.cpp
clang/test/Sema/attr-nomerge.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/Attr.td 
b/clang/include/clang/Basic/Attr.td
index b9e701641fdf1..d5204b2869667 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -1482,9 +1482,8 @@ def : MutualExclusions<[Likely, Unlikely]>;
 def NoMerge : DeclOrStmtAttr {
   let Spellings = [Clang<"nomerge">];
   let Documentation = [NoMergeDocs];
-  let Subjects = SubjectList<[Function, Stmt], ErrorDiag,
- "functions and statements">;
-  let SimpleHandler = 1;
+  let Subjects = SubjectList<[Function, Stmt, Var], ErrorDiag,
+ "functions, statements and variables">;
 }
 
 def MustTail : StmtAttr {

diff  --git a/clang/include/clang/Basic/AttrDocs.td 
b/clang/include/clang/Basic/AttrDocs.td
index 5178ca43b9710..2c950231255d7 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -549,7 +549,31 @@ over the tradeoff between code size and debug information 
precision.
 
 ``nomerge`` attribute can also be used as function attribute to prevent all
 calls to the specified function from merging. It has no effect on indirect
-calls.
+calls to such functions. For example:
+
+.. code-block:: c++
+
+  [[clang::nomerge]] void foo(int) {}
+
+  void bar(int x) {
+auto *ptr = foo;
+if (x) foo(1); else foo(2); // will not be merged
+if (x) ptr(1); else ptr(2); // indirect call, can be merged
+  }
+
+``nomerge`` attribute can also be used for pointers to functions to
+prevent calls through such pointer from merging. In such case the
+effect applies only to a specific function pointer. For example:
+
+.. code-block:: c++
+
+  [[clang::nomerge]] void (*foo)(int);
+
+  void bar(int x) {
+auto *ptr = 

[PATCH] D153702: [Clang] Implement P2738R1 - constexpr cast from void*

2023-06-26 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added inline comments.



Comment at: clang/test/CXX/expr/expr.const/p5-26.cpp:13
+(void)static_cast(a); //cxx23-note {{cast from 'void *' is not allowed 
in a constant expression in C++ standards before C++2c}}
+(void)static_cast(a);
+(void)static_cast(a);

Why no diagnostic on this line and the next?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153702

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


[clang] 214e7ce - [NFC] Bump up DIAG_SIZE_AST as we have hit the limit as of f27afed.

2023-06-26 Thread Douglas Yung via cfe-commits

Author: Douglas Yung
Date: 2023-06-26T14:35:03-07:00
New Revision: 214e7ce4e4b08b77fa5878bf1147419d66854167

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

LOG: [NFC] Bump up DIAG_SIZE_AST as we have hit the limit as of f27afed.

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticIDs.h

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticIDs.h 
b/clang/include/clang/Basic/DiagnosticIDs.h
index 6dd78bf93eaab..a59ff25fe20eb 100644
--- a/clang/include/clang/Basic/DiagnosticIDs.h
+++ b/clang/include/clang/Basic/DiagnosticIDs.h
@@ -36,7 +36,7 @@ namespace clang {
   DIAG_SIZE_SERIALIZATION =  120,
   DIAG_SIZE_LEX   =  400,
   DIAG_SIZE_PARSE =  700,
-  DIAG_SIZE_AST   =  250,
+  DIAG_SIZE_AST   =  300,
   DIAG_SIZE_COMMENT   =  100,
   DIAG_SIZE_CROSSTU   =  100,
   DIAG_SIZE_SEMA  = 4500,



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


[PATCH] D150646: [clang][X86] Add __cpuidex function to cpuid.h

2023-06-26 Thread Mike Hommey via Phabricator via cfe-commits
glandium added a comment.

In D150646#4382763 , @aidengrossman 
wrote:

> Interesting. I would've thought `-fms-extensions` by itself would've declared 
> that macro (which is what I gathered reading what caused the MSVC compatible 
> builtins to get included in clang), but apparently not. For whatever reason, 
> `LangOpts.MicrosoftExt` only seems to be set to true if 
> `-fms-compaitilibty-version` (maybe on top of `-fms-compatibility`) are also 
> passed (not 100% on this though). I'll have to do some more digging soon.

Did you find something?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150646

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


[PATCH] D152741: [WPD] implement -funknown-vtable-visibility-filepaths

2023-06-26 Thread Di Mo via Phabricator via cfe-commits
modimo added a comment.

In D152741#4445807 , @wenlei wrote:

>> For concrete data are you talking about between the different solutions e.g. 
>> --lto-whole-program-visibility, -funknown-vtable-visibility-filepaths, RTTI 
>> based, FatLTO based etc or something else?
>
> Right, between the different solutions. RTTI based solution doesn't exist 
> yet, so maybe just compare using `-fwhole-program-vtables` on a known safe 
> set of files vs using `-funknown-vtable-visibility-filepaths` on a known 
> unsafe set of files first.

On a large Meta service with manually opting in an internal source folder with 
`-fwhole-program-vtables` there's 2,933 single implementation methods that get 
devirtualized. Using `-funknown-vtable-visibility-filepath` on the same service 
for the `third-party` directory there's 32,800 single implementation method 
devirts.

>> The ordering for conflicts is embedded in the logic for 
>> CodeGenModule::GetVCallVisibilityLevel which has priority order of
>
> I was thinking about different source of visibility instead of absolute order 
> of visibility itself - i.e. what is the rule if 
> `__attribute__((visibility("...")))` conflicts with 
> `-funknown-vtable-visibility-filepaths` setting for a specific type? This may 
> not be an immediately important question, but just as example of the knock on 
> effect of added complexity, which may or may not be justified depending on 
> the benefit, which goes back to data from experiments.

That complexity already exists with `-fvisibility=hidden` interacting with 
`__attribute__((visibility("...")))` where the most conservative annotation 
wins out. Having a type annotated with `unknown` visibility is just adding a 
more conservative option than `public`.

> We have `-wholeprogramdevirt-skip`; with this patch, we will have 
> `-funknown-vtable-visibility-filepaths`; later on, we will have another RTTI 
> based solution, then there's FatObj solution. It feels like a lot of stuff 
> trying to solve one problem, so wondering if this addition here is going to 
> provide enough value in the end state.

My current prototype RTTI implementation doesn't really need an `unknown` 
visibility because it's generating and passing a blocklist at symbol resolution 
time. For FatObj, the input into WPD is identical to when everything is built 
with ThinLTO so `unknown` isn't that valuable either. The original intent was 
to use this to roll out WPD to select services but performance-wise opting in 
folders with `-fwhole-program-vtables` proves just as effective without having 
to modify LLVM. With that use case gone, there's no longer a need on my side 
for this change. Others may find value for this in the interim to 
on-board/evaluate WPD but that's not very concrete value.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152741

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


[PATCH] D153667: [HIP]: Add gpu-link-output to control link job creation

2023-06-26 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added a comment.

What's the difference here between this and the existing `--hip-link`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153667

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


[clang] d54bd7a - [dataflow] Fix -Wunused-variable in -DLLVM_ENABLE_ASSERTIONS=off builds after D153006

2023-06-26 Thread Fangrui Song via cfe-commits

Author: Fangrui Song
Date: 2023-06-26T14:00:59-07:00
New Revision: d54bd7aced8b75e04a2925ce99c816f55c350428

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

LOG: [dataflow] Fix -Wunused-variable in -DLLVM_ENABLE_ASSERTIONS=off builds 
after D153006

Added: 


Modified: 
clang/lib/Analysis/FlowSensitive/RecordOps.cpp

Removed: 




diff  --git a/clang/lib/Analysis/FlowSensitive/RecordOps.cpp 
b/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
index b32bf35aa28fd..d7145b0e2b312 100644
--- a/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
+++ b/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
@@ -19,17 +19,15 @@ namespace dataflow {
 
 void copyRecord(AggregateStorageLocation , AggregateStorageLocation ,
 Environment ) {
-  QualType Type = Src.getType();
-
   LLVM_DEBUG({
 if (Dst.getType().getCanonicalType().getUnqualifiedType() !=
-Type.getCanonicalType().getUnqualifiedType()) {
-  llvm::dbgs() << "Source type " << Type << "\n";
+Src.getType().getCanonicalType().getUnqualifiedType()) {
+  llvm::dbgs() << "Source type " << Src.getType() << "\n";
   llvm::dbgs() << "Destination type " << Dst.getType() << "\n";
 }
   });
   assert(Dst.getType().getCanonicalType().getUnqualifiedType() ==
- Type.getCanonicalType().getUnqualifiedType());
+ Src.getType().getCanonicalType().getUnqualifiedType());
 
   for (auto [Field, SrcFieldLoc] : Src.children()) {
 assert(SrcFieldLoc != nullptr);



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


[PATCH] D153006: [clang][dataflow] Perform deep copies in copy and move operations.

2023-06-26 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/RecordOps.cpp:20
+
+void copyRecord(AggregateStorageLocation , AggregateStorageLocation ,
+Environment ) {

See 
https://llvm.org/docs/CodingStandards.html#use-namespace-qualifiers-to-implement-previously-declared-functions

I'll change this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153006

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


[PATCH] D149280: [clang-tidy] Add modernize-printf-to-std-print check

2023-06-26 Thread Piotr Zegar via Phabricator via cfe-commits
PiotrZSL added a comment.

In D149280#4450418 , @mikecrowe wrote:

> Despite the fact that I've worked on what became this check for about two 
> years, now that it's landed I've suddenly spotted a significant flaw: 
> `printf` returns the number of characters printed, whereas `std::print` 
> doesn't return anything. None of my test cases made use of the return value. 
> I think this means that I need to only match on calls that don't make use of 
> the return value. I shall try to do that.

You can provide warning, but without fix. But that would need to be put into 
documentation.
If we lucky, and there will be no issues with current version, then you can put 
this in separate patch.
Anyway I do not think that many project use `printf`, most probably they use 
`std::cout`, `fmt::format` or some other custom wrapper around something.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149280

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


[PATCH] D153493: [dataflow] avoid more accidental copies of Environment

2023-06-26 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment.

In D153493#4450358 , @sammccall wrote:

>> I realize the complexity is frustrating, but I don't see how that's related 
>> to the issue here. The complexity of the `JoinedStateBuilder` is caused by 
>> the desire to minimize copies. The multiple cases come directly from the 
>> algorithm itself, most particularly that we may or may not be in an initial 
>> state, and the previous block may or may not have an environment we care 
>> about. At least, that is the specific issue I'm concerned with.
>
> We have 9 state transitions (3 states x 3 operations). Each would be less 
> than half as complicated if it didn't have to deal with Lattice's join being 
> mutating and Environment's being non-mutating.
>
> I believe further simplifications are possible (essentially: back at the 
> callsite track `vector All` plus `deque Owned` for 
> owned copies, special case `All.size() == 0` and `All.size() == 1` and 
> otherwise `joinVec(All)`.
> However with Lattice's mutating join involved this no longer works, and after 
> addressing that it's no longer simpler.

I'm pretty sure I agree on all of this, please document it somewhere (file an 
Issue in the github tracker?). FWIW, I think mutating join was a case of 
premature optimization gone bad, and ultimately the builtin lattice should not 
be built in at all -- it should exist outside the core and be just another 
client.

>> "join with InitEnv" should reduce to the identity function, so if its not, 
>> that feels like a problem we're sweeping under the rug.
>
> Only in a mathematical sense - in reality humans need to read these SAT 
> systems, and we're using a solver that can't perform that reduction.

I think we're talking past each other (and actually agreeing), sorry. To be 
clear, I don't mean that as a criticism of this change -- I think you've hit an 
important point and fixing it might make this simpler. I totally agree about 
SAT systems, and the solver, etc. But, I'm saying that it shouldn't come to 
that. I think the Environment join operation is broken if  `join(InitEnv, E) != 
E`. I'm asking whether we can reduce (some) complexity by fixing that.  From 
your earlier point, there are more problems, but I want to focus on this 
particular one in case solving it makes other things easier.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153493

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


[PATCH] D149280: [clang-tidy] Add modernize-printf-to-std-print check

2023-06-26 Thread Mike Crowe via Phabricator via cfe-commits
mikecrowe added a comment.

Despite the fact that I've worked on what became this check for about two 
years, now that it's landed I've suddenly spotted a significant flaw: `printf` 
returns the number of characters printed, whereas `std::print` doesn't return 
anything. None of my test cases made use of the return value. I think this 
means that I need to only match on calls that don't make use of the return 
value. I shall try to do that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149280

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


[PATCH] D153589: [NFC] Initialize pointer fields and remove a needless null check.

2023-06-26 Thread Sindhu Chittireddy via Phabricator via cfe-commits
schittir added inline comments.



Comment at: clang/lib/Analysis/CFG.cpp:4156
   // CFG generation for unevaluated operands.
-  if (S && !S->isTypeDependent() && S->isPotentiallyEvaluated())
+  if (!S->isTypeDependent() && S->isPotentiallyEvaluated())
 return VisitChildren(S);

schittir wrote:
> erichkeane wrote:
> > schittir wrote:
> > > erichkeane wrote:
> > > > schittir wrote:
> > > > > erichkeane wrote:
> > > > > > I get that we're counting on the dereference on 4145 to have made 
> > > > > > this check irrelevant, but are we sure that we KNOW that "S" is 
> > > > > > non-null here otherwise?  That is, is the bug actually 4145 doing 
> > > > > > 'alwaysAdd' without checking vs the unnecessary check here?
> > > > > VisitCXXTypeidExpr is used only in one place - here 
> > > > > https://github.com/llvm/llvm-project/blob/a89c9b35be08b665cc1a01d840bc20349ba1308f/clang/lib/Analysis/CFG.cpp#L2288
> > > > >  where S is not null. Null check for S already happens at the 
> > > > > beginning of the method where VisitCXXTypeidExpr is called. 
> > > > SG, an assert is perhaps not a bad idea, but LGTM.  I don't know very 
> > > > much about the CFG stuff, so aaron might wish to take a final look.
> > > What is SG? 
> > "Sounds Good"
> Haha! 
> Thanks for the review. 
> SG, an assert is perhaps not a bad idea, but LGTM.  I don't know very much 
> about the CFG stuff, so aaron might wish to take a final look.

Hi @aaron.ballman, could you please comment on this? Thank you!


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

https://reviews.llvm.org/D153589

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


[PATCH] D153493: [dataflow] avoid more accidental copies of Environment

2023-06-26 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

In D153493#4450191 , @ymandel wrote:

> I don't want to block the progress you're making elsewhere and I think the 
> concerns here are sufficiently localized to revisit another time. So, feel 
> free to reify any suggestions/disagreements in FIXMEs and submit.
>
> In D153493#4449993 , @sammccall 
> wrote:
>
>>> Regarding the design. This looks like an optimal solution in terms of 
>>> copying but introduces lifetime risks and complexity that I'm uncomfortable 
>>> with.
>>
>> FWIW I agree with the complexity (not the lifetime risks).
>> I think it ultimately stems from having too many interacting types & 
>> functions with complicated and inconsistent signatures & constraints 
>> (TypeErasedDataflowAnalysisState, Environment, TypeErasedLattice, Lattice, 
>> the various join functions, etc) so we resort to case-bashing.
>> However this system is difficult to make changes to, and getting consensus 
>> as to what changes are good also doesn't seem easy. So I think we need to be 
>> able to fix bugs within the existing design (where copies need to be avoided 
>> ad-hoc).
>
> I realize the complexity is frustrating, but I don't see how that's related 
> to the issue here. The complexity of the `JoinedStateBuilder` is caused by 
> the desire to minimize copies. The multiple cases come directly from the 
> algorithm itself, most particularly that we may or may not be in an initial 
> state, and the previous block may or may not have an environment we care 
> about. At least, that is the specific issue I'm concerned with.

We have 9 state transitions (3 states x 3 operations). Each would be less than 
half as complicated if it didn't have to deal with Lattice's join being 
mutating and Environment's being non-mutating.

I believe further simplifications are possible (essentially: back at the 
callsite track `vector All` plus `deque Owned` for 
owned copies, special case `All.size() == 0` and `All.size() == 1` and 
otherwise `joinVec(All)`.
However with Lattice's mutating join involved this no longer works, and after 
addressing that it's no longer simpler.

>>> There's a lot of flexibility here and I wonder if we can reduce that 
>>> without (substantially?) impacting the amount of copying. Specifically, I 
>>> wonder if we can have the builder *always* own an environment, which 
>>> simplifies the handling of CurrentState and, generally, the number of cases 
>>> to consider. It costs more in principle, but maybe negligible in practice?
>>
>> I don't understand the specific suggestion:
>>
>> - if we create the builder lazily, we're just moving handling this extra 
>> state into the callsites
>> - if the builder owns a copy of initenv, its FC token will end up in the 
>> result FC
>> - if the builder owns a copy of initenv and detects when it should discard 
>> it, that just sounds like nullptr with extra steps
>>
>> can you clarify?
>
> I suppose I'm thinking along the lines of:
>
>> - if the builder owns a copy of initenv, its FC token will end up in the 
>> result FC
>
> Can you expand on why this is a problem? That would help motivate the 
> complexity.

The purpose of this change is to stop introducing meaningless variables into 
the FC, because they impede debugging and slow down the SAT solver.
An extra join with a copy of InitEnv introduces up to 3 extra variables: one 
for InitEnv, one for the copy, one for the join.

> "join with InitEnv" should reduce to the identity function, so if its not, 
> that feels like a problem we're sweeping under the rug.

Only in a mathematical sense - in reality humans need to read these SAT 
systems, and we're using a solver that can't perform that reduction.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153493

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


[PATCH] D153582: [SystemZ][z/OS] Add required options/macro/etc for z/os compilation step

2023-06-26 Thread Sean via Phabricator via cfe-commits
SeanP marked 3 inline comments as done.
SeanP added a comment.

Thanks.  made these changes.


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

https://reviews.llvm.org/D153582

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


[PATCH] D153582: [SystemZ][z/OS] Add required options/macro/etc for z/os compilation step

2023-06-26 Thread Sean via Phabricator via cfe-commits
SeanP updated this revision to Diff 534729.
SeanP added a comment.

apply feedback


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

https://reviews.llvm.org/D153582

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Basic/Targets/SystemZ.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/ZOS.cpp
  clang/lib/Driver/ToolChains/ZOS.h
  clang/test/Driver/zos-comp-cxx.cpp
  clang/test/Driver/zos-comp.c
  clang/test/Driver/zos-driver-defaults.c
  clang/test/Preprocessor/init-s390x.c

Index: clang/test/Preprocessor/init-s390x.c
===
--- clang/test/Preprocessor/init-s390x.c
+++ clang/test/Preprocessor/init-s390x.c
@@ -183,17 +183,12 @@
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=s390x-none-zos -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix S390X-ZOS %s
 // RUN: %clang_cc1 -x c++ -std=gnu++14 -E -dM -ffreestanding -triple=s390x-none-zos -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix S390X-ZOS -check-prefix S390X-ZOS-GNUXX %s
 
-// S390X-ZOS-GNUXX: #define _EXT 1
 // S390X-ZOS:   #define _LONG_LONG 1
-// S390X-ZOS-GNUXX: #define _MI_BUILTIN 1
-// S390X-ZOS:   #define _OPEN_DEFAULT 1
-// S390X-ZOS:   #define _UNIX03_WITHDRAWN 1
-// S390X-ZOS-GNUXX: #define _XOPEN_SOURCE 600
 // S390X-ZOS:   #define __370__ 1
 // S390X-ZOS:   #define __64BIT__ 1
 // S390X-ZOS:   #define __BFP__ 1
 // S390X-ZOS:   #define __BOOL__ 1
-// S390X-ZOS-GNUXX: #define __DLL__ 1
+// S390X-ZOS:   #define __COMPILER_VER__ 0x5000
 // S390X-ZOS:   #define __LONGNAME__ 1
 // S390X-ZOS:   #define __MVS__ 1
 // S390X-ZOS:   #define __THW_370__ 1
Index: clang/test/Driver/zos-driver-defaults.c
===
--- clang/test/Driver/zos-driver-defaults.c
+++ clang/test/Driver/zos-driver-defaults.c
@@ -1,8 +1,21 @@
-// RUN: %clang -### --target=s390x-none-zos -fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-SHORT-ENUMS %s
+// RUN: %clang -### --target=s390x-none-zos -fsyntax-only %s 2>&1 | FileCheck --check-prefixes=CHECK-C-MACRO,CHECK-SHORT-ENUMS %s
+// RUN: %clang -### --target=s390x-none-zos -fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-ZOS-INCLUDES %s
 // RUN: %clang -### --target=s390x-none-zos -fno-short-enums -fsyntax-only %s 2>&1 | FileCheck %s
+// RUN: %clangxx -### --target=s390x-none-zos -fsyntax-only %s 2>&1 | FileCheck --check-prefixes=CHECK-C-MACRO,CHECK-CXX-MACRO %s
+// RUN: %clang -### --target=s390x-none-zos -x c++ -fsyntax-only %s 2>&1 | FileCheck --check-prefixes=CHECK-C-MACRO,CHECK-CXX-MACRO %s
+
+//CHECK-C-MACRO: -D_UNIX03_WITHDRAWN
+//CHECK-C-MACRO: -D_OPEN_DEFAULT
+
+//CHECK-CXX-MACRO: -D_XOPEN_SOURCE=600
+//CHECK-USER-CXX-MACRO-NOT: -D_XOPEN_SOURCE=600
+//CHECK-USER-CXX-MACRO: "-D" "_XOPEN_SOURCE=700"
 
 //CHECK-SHORT-ENUMS: -fshort-enums
 //CHECK-SHORT-ENUMS: -fno-signed-char
 
+//CHECK-ZOS-INCLUDES: clang{{.*}} "-cc1" "-triple" "s390x-none-zos"
+//CHECK-ZOS-INCLUDES-SAME: "-internal-isystem" {{".+/lib/clang/.+/include/zos_wrappers"}} "-internal-isystem" {{".+/lib/clang/.+/include"}}
+
 //CHECK-NOT: -fshort-enums
 //CHECK: -fno-signed-char
Index: clang/test/Driver/zos-comp.c
===
--- /dev/null
+++ clang/test/Driver/zos-comp.c
@@ -0,0 +1,75 @@
+// Tests that the z/OS toolchain adds system includes to its search path.
+
+// RUN: %clang -c -### %s --target=s390x-ibm-zos 2>&1 \
+// RUN:   | FileCheck %s
+
+// CHECK: "-D_UNIX03_WITHDRAWN"
+// CHECK-SAME: "-D_OPEN_DEFAULT"
+// CHECK-SAME: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-SAME: "-internal-isystem" "[[RESOURCE_DIR]]{{(/|)}}include{{(/|)}}zos_wrappers"
+// CHECK-SAME: "-internal-isystem" "[[RESOURCE_DIR]]{{(/|)}}include"
+// CHECK-SAME: "-internal-isystem" "/usr/include"
+// CHECK-SAME: "-fshort-enums"
+// CHECK-SAME: "-fno-signed-char"
+// CHECK-SAME: "-fno-signed-wchar"
+
+// RUN: %clang -c -### -mzos-sys-include=/ABC/DEF %s 2>&1 \
+// RUN:		--target=s390x-ibm-zos \
+// RUN:   | FileCheck --check-prefixes=CHECK2 %s
+
+// CHECK2: "-D_UNIX03_WITHDRAWN"
+// CHECK2-SAME: "-D_OPEN_DEFAULT"
+// CHECK2-SAME: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK2-SAME: "-internal-isystem" "[[RESOURCE_DIR]]{{(/|)}}include{{(/|)}}zos_wrappers"
+// CHECK2-SAME: "-internal-isystem" "[[RESOURCE_DIR]]{{(/|)}}include"
+// CHECK2-SAME: "-internal-isystem" "/ABC/DEF"
+// CHECK2-NOT: "-internal-isystem" "/usr/include"
+// CHECK2-SAME: "-fshort-enums"
+// CHECK2-SAME: "-fno-signed-char"
+// CHECK2-SAME: "-fno-signed-wchar"
+
+// RUN: %clang -c -### -mzos-sys-include=/ABC/DEF:/ghi/jkl %s 2>&1 \
+// RUN:		--target=s390x-ibm-zos \
+// RUN:   | FileCheck --check-prefixes=CHECK3 %s
+
+// CHECK3: "-D_UNIX03_WITHDRAWN"
+// CHECK3-SAME: "-D_OPEN_DEFAULT"
+// CHECK3-SAME: "-resource-dir" 

[PATCH] D153667: [HIP]: Add gpu-link-output to control link job creation

2023-06-26 Thread Jeffrey Byrnes via Phabricator via cfe-commits
jrbyrnes updated this revision to Diff 534725.
jrbyrnes marked an inline comment as done.
jrbyrnes added a comment.

Fix tests + add tests. Add phase test for -fgpu-rdc --no-gpu-link-output (these 
are not intended to be used together)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153667

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/hip-device-compile.hip
  clang/test/Driver/hip-phases.hip
  clang/test/Driver/hip-rdc-device-only.hip

Index: clang/test/Driver/hip-rdc-device-only.hip
===
--- clang/test/Driver/hip-rdc-device-only.hip
+++ clang/test/Driver/hip-rdc-device-only.hip
@@ -5,7 +5,7 @@
 // RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 \
 // RUN:   -c -nogpuinc -nogpulib --cuda-device-only -fgpu-rdc \
 // RUN:   %S/Inputs/hip_multiple_inputs/a.cu \
-// RUN:   %S/Inputs/hip_multiple_inputs/b.hip --gpu-bundle-output \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip --gpu-bundle-output --gpu-link-output \
 // RUN: 2>&1 | FileCheck -check-prefixes=COMMON,EMITBC %s
 
 // With `-emit-llvm`, the output should be the same as the aforementioned line
@@ -15,14 +15,14 @@
 // RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 \
 // RUN:   -c -emit-llvm -nogpuinc -nogpulib --cuda-device-only -fgpu-rdc \
 // RUN:   %S/Inputs/hip_multiple_inputs/a.cu \
-// RUN:   %S/Inputs/hip_multiple_inputs/b.hip --gpu-bundle-output \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip --gpu-bundle-output --gpu-link-output \
 // RUN: 2>&1 | FileCheck -check-prefixes=COMMON,EMITBC %s
 
 // RUN: %clang -### --target=x86_64-linux-gnu \
 // RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 \
 // RUN:   -S -nogpuinc -nogpulib --cuda-device-only -fgpu-rdc \
 // RUN:   %S/Inputs/hip_multiple_inputs/a.cu \
-// RUN:   %S/Inputs/hip_multiple_inputs/b.hip --gpu-bundle-output \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip --gpu-bundle-output --gpu-link-output \
 // RUN: 2>&1 | FileCheck -check-prefixes=COMMON,EMITLL %s
 
 // With `-emit-llvm`, the output should be the same as the aforementioned line
@@ -32,7 +32,7 @@
 // RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 \
 // RUN:   -S -emit-llvm -nogpuinc -nogpulib --cuda-device-only -fgpu-rdc \
 // RUN:   %S/Inputs/hip_multiple_inputs/a.cu \
-// RUN:   %S/Inputs/hip_multiple_inputs/b.hip --gpu-bundle-output \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip --gpu-bundle-output --gpu-link-output \
 // RUN: 2>&1 | FileCheck -check-prefixes=COMMON,EMITLL %s
 
 // With `-save-temps`, commane lines for each steps are dumped. For assembly
@@ -43,7 +43,7 @@
 // RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 \
 // RUN:   -S -nogpuinc -nogpulib --cuda-device-only -fgpu-rdc \
 // RUN:   %S/Inputs/hip_multiple_inputs/a.cu \
-// RUN:   %S/Inputs/hip_multiple_inputs/b.hip --gpu-bundle-output \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip --gpu-bundle-output --gpu-link-output \
 // RUN: 2>&1 | FileCheck -check-prefix=SAVETEMP %s
 
 // Check output one file without bundling cause error.
@@ -54,6 +54,12 @@
 // RUN:   %S/Inputs/hip_multiple_inputs/a.cu -o %t.s --no-gpu-bundle-output \
 // RUN: 2>&1 | FileCheck -check-prefix=FAIL %s
 
+// RUN: %clang -### --target=x86_64-linux-gnu \
+// RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 \
+// RUN:   -S -nogpuinc -nogpulib --cuda-device-only -fgpu-rdc \
+// RUN:   %S/Inputs/hip_multiple_inputs/a.cu -o %t.s --no-gpu-link-output \
+// RUN: 2>&1 | FileCheck -check-prefix=FAIL %s
+
 // COMMON: [[CLANG:".*clang.*"]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
 // COMMON-SAME: "-aux-triple" "x86_64-unknown-linux-gnu"
 // EMITBC-SAME: "-emit-llvm-bc"
Index: clang/test/Driver/hip-phases.hip
===
--- clang/test/Driver/hip-phases.hip
+++ clang/test/Driver/hip-phases.hip
@@ -244,6 +244,53 @@
 // DASM-NOT: clang-offload-bundler
 // DASM-NOT: host
 
+//
+// Test single gpu architecture with compile to relocatable in device-only
+// compilation mode.
+//
+// RUN: %clang -x hip --target=x86_64-unknown-linux-gnu -ccc-print-phases \
+// RUN: --cuda-gpu-arch=gfx803 %s --cuda-device-only --no-gpu-link-output 2>&1 \
+// RUN: | FileCheck -check-prefixes=RELOC %s
+// RELOC-DAG: [[P0:[0-9]+]]: input, "{{.*}}hip-phases.hip", [[T:hip]], (device-[[T]], [[ARCH:gfx803]])
+// RELOC-DAG: [[P1:[0-9]+]]: preprocessor, {[[P0]]}, [[T]]-cpp-output, (device-[[T]], [[ARCH]])
+// RELOC-DAG: [[P2:[0-9]+]]: compiler, {[[P1]]}, ir, (device-[[T]], [[ARCH]])
+// RELOC-DAG: [[P3:[0-9]+]]: backend, {[[P2]]}, assembler, (device-[[T]], [[ARCH]])
+// RELOC-DAG: [[P4:[0-9]+]]: assembler, {[[P3]]}, object, (device-[[T]], [[ARCH]])
+// RELOC-NOT: linker
+// RELOC-DAG: [[P5:[0-9]+]]: offload, "device-[[T]] (amdgcn-amd-amdhsa:[[ARCH]])" {[[P4]]}, object
+
+// RUN: %clang -x hip 

[PATCH] D153728: [llvm] Move AttributeMask to a separate header

2023-06-26 Thread Nikita Popov via Phabricator via cfe-commits
nikic accepted this revision.
nikic 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/D153728/new/

https://reviews.llvm.org/D153728

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


[PATCH] D153418: Adding iconv support to CharSetConverter class

2023-06-26 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

> I think this is reasonable since gcc's fexec-charset option also says the 
> name can be any encoding supported by the iconv library (copy pasted below 
> from https://gcc.gnu.org/onlinedocs/gcc-13.1.0/gcc.pdf) so this would match 
> that behaviour

"gcc did it" doesn't mean the issues raised don't exist; it just means the gcc 
developers care less about those issues (and they use GNU iconv on Windows, 
which we can't do).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153418

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


[clang] 5c8d247 - [dataflow] Fix -Wunused-variable in -DLLVM_ENABLE_ASSERTIONS=off builds after D153006

2023-06-26 Thread Fangrui Song via cfe-commits

Author: Fangrui Song
Date: 2023-06-26T12:48:20-07:00
New Revision: 5c8d24732b75002df475e2af91220b2056a06979

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

LOG: [dataflow] Fix -Wunused-variable in -DLLVM_ENABLE_ASSERTIONS=off builds 
after D153006

Added: 


Modified: 
clang/lib/Analysis/FlowSensitive/RecordOps.cpp

Removed: 




diff  --git a/clang/lib/Analysis/FlowSensitive/RecordOps.cpp 
b/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
index 92d9cef8ea858..b32bf35aa28fd 100644
--- a/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
+++ b/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
@@ -67,17 +67,15 @@ void copyRecord(AggregateStorageLocation , 
AggregateStorageLocation ,
 bool recordsEqual(const AggregateStorageLocation , const Environment 
,
   const AggregateStorageLocation ,
   const Environment ) {
-  QualType Type = Loc1.getType();
-
   LLVM_DEBUG({
 if (Loc2.getType().getCanonicalType().getUnqualifiedType() !=
-Type.getCanonicalType().getUnqualifiedType()) {
-  llvm::dbgs() << "Loc1 type " << Type << "\n";
+Loc1.getType().getCanonicalType().getUnqualifiedType()) {
+  llvm::dbgs() << "Loc1 type " << Loc1.getType() << "\n";
   llvm::dbgs() << "Loc2 type " << Loc2.getType() << "\n";
 }
   });
   assert(Loc2.getType().getCanonicalType().getUnqualifiedType() ==
- Type.getCanonicalType().getUnqualifiedType());
+ Loc1.getType().getCanonicalType().getUnqualifiedType());
 
   for (auto [Field, FieldLoc1] : Loc1.children()) {
 assert(FieldLoc1 != nullptr);



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


[PATCH] D150978: [clang][Sema] Add CodeCompletionContext::CCC_ObjCClassForwardDecl

2023-06-26 Thread David Goldman via Phabricator via cfe-commits
dgoldman updated this revision to Diff 534714.
dgoldman added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150978

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang/include/clang/Sema/CodeCompleteConsumer.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Parse/ParseObjc.cpp
  clang/lib/Sema/CodeCompleteConsumer.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/tools/libclang/CIndexCodeCompletion.cpp

Index: clang/tools/libclang/CIndexCodeCompletion.cpp
===
--- clang/tools/libclang/CIndexCodeCompletion.cpp
+++ clang/tools/libclang/CIndexCodeCompletion.cpp
@@ -537,6 +537,7 @@
 case CodeCompletionContext::CCC_Other:
 case CodeCompletionContext::CCC_ObjCInterface:
 case CodeCompletionContext::CCC_ObjCImplementation:
+case CodeCompletionContext::CCC_ObjCClassForwardDecl:
 case CodeCompletionContext::CCC_NewName:
 case CodeCompletionContext::CCC_MacroName:
 case CodeCompletionContext::CCC_PreprocessorExpression:
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -8460,6 +8460,24 @@
 Results.data(), Results.size());
 }
 
+void Sema::CodeCompleteObjCClassForwardDecl(Scope *S) {
+  ResultBuilder Results(*this, CodeCompleter->getAllocator(),
+CodeCompleter->getCodeCompletionTUInfo(),
+CodeCompletionContext::CCC_ObjCClassForwardDecl);
+  Results.EnterNewScope();
+
+  if (CodeCompleter->includeGlobals()) {
+// Add all classes.
+AddInterfaceResults(Context.getTranslationUnitDecl(), CurContext, false,
+false, Results);
+  }
+
+  Results.ExitScope();
+
+  HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
+}
+
 void Sema::CodeCompleteObjCSuperclass(Scope *S, IdentifierInfo *ClassName,
   SourceLocation ClassNameLoc) {
   ResultBuilder Results(*this, CodeCompleter->getAllocator(),
Index: clang/lib/Sema/CodeCompleteConsumer.cpp
===
--- clang/lib/Sema/CodeCompleteConsumer.cpp
+++ clang/lib/Sema/CodeCompleteConsumer.cpp
@@ -83,6 +83,7 @@
   case CCC_ObjCCategoryName:
   case CCC_IncludedFile:
   case CCC_Attribute:
+  case CCC_ObjCClassForwardDecl:
 return false;
   }
 
@@ -166,6 +167,8 @@
 return "Attribute";
   case CCKind::CCC_Recovery:
 return "Recovery";
+  case CCKind::CCC_ObjCClassForwardDecl:
+return "ObjCClassForwardDecl";
   }
   llvm_unreachable("Invalid CodeCompletionContext::Kind!");
 }
Index: clang/lib/Parse/ParseObjc.cpp
===
--- clang/lib/Parse/ParseObjc.cpp
+++ clang/lib/Parse/ParseObjc.cpp
@@ -153,6 +153,11 @@
 
   while (true) {
 MaybeSkipAttributes(tok::objc_class);
+if (Tok.is(tok::code_completion)) {
+  cutOffParsing();
+  Actions.CodeCompleteObjCClassForwardDecl(getCurScope());
+  return Actions.ConvertDeclToDeclGroup(nullptr);
+}
 if (expectIdentifier()) {
   SkipUntil(tok::semi);
   return Actions.ConvertDeclToDeclGroup(nullptr);
Index: clang/lib/Frontend/ASTUnit.cpp
===
--- clang/lib/Frontend/ASTUnit.cpp
+++ clang/lib/Frontend/ASTUnit.cpp
@@ -322,6 +322,7 @@
   if (ID->getDefinition())
 Contexts |= (1LL << CodeCompletionContext::CCC_Expression);
   Contexts |= (1LL << CodeCompletionContext::CCC_ObjCInterfaceName);
+  Contexts |= (1LL << CodeCompletionContext::CCC_ObjCClassForwardDecl);
 }
 
 // Deal with tag names.
@@ -2028,6 +2029,7 @@
   case CodeCompletionContext::CCC_IncludedFile:
   case CodeCompletionContext::CCC_Attribute:
   case CodeCompletionContext::CCC_NewName:
+  case CodeCompletionContext::CCC_ObjCClassForwardDecl:
 // We're looking for nothing, or we're looking for names that cannot
 // be hidden.
 return;
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -13429,6 +13429,7 @@
  ArrayRef Protocols);
   void CodeCompleteObjCProtocolDecl(Scope *S);
   void CodeCompleteObjCInterfaceDecl(Scope *S);
+  void CodeCompleteObjCClassForwardDecl(Scope *S);
   void CodeCompleteObjCSuperclass(Scope *S,
   IdentifierInfo *ClassName,
   SourceLocation ClassNameLoc);
Index: clang/include/clang/Sema/CodeCompleteConsumer.h

[PATCH] D153493: [dataflow] avoid more accidental copies of Environment

2023-06-26 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel accepted this revision.
ymandel added a comment.
This revision is now accepted and ready to land.

I don't want to block the progress you're making elsewhere and I think the 
concerns here are sufficiently localized to revisit another time. So, feel free 
to reify any suggestions/disagreements in FIXMEs and submit.

In D153493#4449993 , @sammccall wrote:

>> Regarding the design. This looks like an optimal solution in terms of 
>> copying but introduces lifetime risks and complexity that I'm uncomfortable 
>> with.
>
> FWIW I agree with the complexity (not the lifetime risks).
> I think it ultimately stems from having too many interacting types & 
> functions with complicated and inconsistent signatures & constraints 
> (TypeErasedDataflowAnalysisState, Environment, TypeErasedLattice, Lattice, 
> the various join functions, etc) so we resort to case-bashing.
> However this system is difficult to make changes to, and getting consensus as 
> to what changes are good also doesn't seem easy. So I think we need to be 
> able to fix bugs within the existing design (where copies need to be avoided 
> ad-hoc).

I realize the complexity is frustrating, but I don't see how that's related to 
the issue here. The complexity of the `JoinedStateBuilder` is caused by the 
desire to minimize copies. The multiple cases come directly from the algorithm 
itself, most particularly that we may or may not be in an initial state, and 
the previous block may or may not have an environment we care about. At least, 
that is the specific issue I'm concerned with.

>> There's a lot of flexibility here and I wonder if we can reduce that without 
>> (substantially?) impacting the amount of copying. Specifically, I wonder if 
>> we can have the builder *always* own an environment, which simplifies the 
>> handling of CurrentState and, generally, the number of cases to consider. It 
>> costs more in principle, but maybe negligible in practice?
>
> I don't understand the specific suggestion:
>
> - if we create the builder lazily, we're just moving handling this extra 
> state into the callsites
> - if the builder owns a copy of initenv, its FC token will end up in the 
> result FC
> - if the builder owns a copy of initenv and detects when it should discard 
> it, that just sounds like nullptr with extra steps
>
> can you clarify?

I suppose I'm thinking along the lines of:

> - if the builder owns a copy of initenv, its FC token will end up in the 
> result FC

Can you expand on why this is a problem? That would help motivate the 
complexity. "join with InitEnv" should reduce to the identity function, so if 
its not, that feels like a problem we're sweeping under the rug.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153493

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


[PATCH] D153417: New CharSetConverter wrapper class for ConverterEBCDIC

2023-06-26 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

In D153417#4449812 , 
@abhina.sreeskantharajan wrote:

> In D153417#4438764 , @efriedma 
> wrote:
>
>> As I mentioned at 
>> https://discourse.llvm.org/t/rfc-enabling-fexec-charset-support-to-llvm-and-clang-reposting/71512
>>  , I think SimplifyLibCalls needs to be aware of encodings.  To make that 
>> work, this probably needs to be somewhere in llvm/ , not clang/ .
>
> There was previous resistance to putting this in LLVM 
> https://discourse.llvm.org/t/rfc-adding-a-charset-converter-to-the-llvm-support-library/69795/17,
>  I'm not sure if that sentiment has changed. I think it would be best to have 
> some sort of implementation plan to tackle the SimplifyLibCalls issue before 
> shifting this code

I don't think anyone is particularly against the concept of adding a charset 
conversion API; most of the discussion is around the choice to implement the 
API as a thin wrapper around POSIX iconv().  And that concern applies equally 
no matter where the code is located in the source tree.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153417

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


[PATCH] D153805: Expose DataflowAnalysisContext.querySolver().

2023-06-26 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel accepted this revision.
ymandel added inline comments.
This revision is now accepted and ready to land.



Comment at: 
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:178-181
+  /// Possible outcomes are:
+  /// - `Satisfiable`: A satisfying assignment exists and is returned.
+  /// - `Unsatisfiable`: A satisfying assignment does not exist.
+  /// - `TimedOut`: The search for a satisfying assignment was not completed.

nit: Given that these are options of `Solver::Result`, I think you can just 
delete these lines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153805

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


[PATCH] D153691: [Driver][ARM] Warn about -mabi= for assembler input

2023-06-26 Thread Fangrui Song 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 rGfe5bab537270: [Driver][ARM] Warn about -mabi= for assembler 
input (authored by MaskRay).

Changed prior to commit:
  https://reviews.llvm.org/D153691?vs=534668=534705#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153691

Files:
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/test/Driver/arm-abi.c


Index: clang/test/Driver/arm-abi.c
===
--- clang/test/Driver/arm-abi.c
+++ clang/test/Driver/arm-abi.c
@@ -61,3 +61,9 @@
 // CHECK-APCS-GNU: "-target-abi" "apcs-gnu"
 // CHECK-AAPCS: "-target-abi" "aapcs"
 // CHECK-AAPCS-LINUX: "-target-abi" "aapcs-linux"
+
+// RUN: %clang --target=arm---gnueabi -mabi=aapcs -x assembler %s -### -o 
/dev/null 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-ASM %s
+
+/// The combination -x assember & -mabi is not implemented, but for GCC 
compatibility we accept with a warning.
+// CHECK-ASM: warning: argument unused during compilation: '-mabi={{.*}}'
Index: clang/lib/Driver/ToolChains/Arch/ARM.cpp
===
--- clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -524,6 +524,12 @@
 }
   }
 }
+
+// The integrated assembler doesn't implement e_flags setting behavior for
+// -meabi=gnu (gcc -mabi={apcs-gnu,atpcs} passes -meabi=gnu to gas). For
+// compatibility we accept but warn.
+if (Arg *A = Args.getLastArgNoClaim(options::OPT_mabi_EQ))
+  A->ignoreTargetSpecific();
   }
 
   if (getReadTPMode(D, Args, Triple, ForAS) == ReadTPMode::TPIDRURW)


Index: clang/test/Driver/arm-abi.c
===
--- clang/test/Driver/arm-abi.c
+++ clang/test/Driver/arm-abi.c
@@ -61,3 +61,9 @@
 // CHECK-APCS-GNU: "-target-abi" "apcs-gnu"
 // CHECK-AAPCS: "-target-abi" "aapcs"
 // CHECK-AAPCS-LINUX: "-target-abi" "aapcs-linux"
+
+// RUN: %clang --target=arm---gnueabi -mabi=aapcs -x assembler %s -### -o /dev/null 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-ASM %s
+
+/// The combination -x assember & -mabi is not implemented, but for GCC compatibility we accept with a warning.
+// CHECK-ASM: warning: argument unused during compilation: '-mabi={{.*}}'
Index: clang/lib/Driver/ToolChains/Arch/ARM.cpp
===
--- clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -524,6 +524,12 @@
 }
   }
 }
+
+// The integrated assembler doesn't implement e_flags setting behavior for
+// -meabi=gnu (gcc -mabi={apcs-gnu,atpcs} passes -meabi=gnu to gas). For
+// compatibility we accept but warn.
+if (Arg *A = Args.getLastArgNoClaim(options::OPT_mabi_EQ))
+  A->ignoreTargetSpecific();
   }
 
   if (getReadTPMode(D, Args, Triple, ForAS) == ReadTPMode::TPIDRURW)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] fe5bab5 - [Driver][ARM] Warn about -mabi= for assembler input

2023-06-26 Thread Fangrui Song via cfe-commits

Author: Fangrui Song
Date: 2023-06-26T12:28:02-07:00
New Revision: fe5bab537270e21469b8dfe09aae2ec11f1aca7f

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

LOG: [Driver][ARM] Warn about -mabi= for assembler input

Previously, Clang Driver reported a warning when assembler input was assembled
with the -mabi= option. D152856 added TargetSpecific to -mabi= option and
reported an error for such a case. This change restores the previous behavior by
reporting a warning.

GCC translates -mabi={apcs-gnu,atpcs} to gas -meabi=gnu and other -mabi= values
to -meabi=5. We don't support setting e_flags to any value other than
EF_ARM_EABI_VER5.

Close https://github.com/ClangBuiltLinux/linux/issues/1878

Reviewed By: michaelplatings

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/Arch/ARM.cpp
clang/test/Driver/arm-abi.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Arch/ARM.cpp 
b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
index 4f2475efa18b8..15b370fa7d014 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -524,6 +524,12 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver 
,
 }
   }
 }
+
+// The integrated assembler doesn't implement e_flags setting behavior for
+// -meabi=gnu (gcc -mabi={apcs-gnu,atpcs} passes -meabi=gnu to gas). For
+// compatibility we accept but warn.
+if (Arg *A = Args.getLastArgNoClaim(options::OPT_mabi_EQ))
+  A->ignoreTargetSpecific();
   }
 
   if (getReadTPMode(D, Args, Triple, ForAS) == ReadTPMode::TPIDRURW)

diff  --git a/clang/test/Driver/arm-abi.c b/clang/test/Driver/arm-abi.c
index 548704965b247..38076f092c8e6 100644
--- a/clang/test/Driver/arm-abi.c
+++ b/clang/test/Driver/arm-abi.c
@@ -61,3 +61,9 @@
 // CHECK-APCS-GNU: "-target-abi" "apcs-gnu"
 // CHECK-AAPCS: "-target-abi" "aapcs"
 // CHECK-AAPCS-LINUX: "-target-abi" "aapcs-linux"
+
+// RUN: %clang --target=arm---gnueabi -mabi=aapcs -x assembler %s -### -o 
/dev/null 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-ASM %s
+
+/// The combination -x assember & -mabi is not implemented, but for GCC 
compatibility we accept with a warning.
+// CHECK-ASM: warning: argument unused during compilation: '-mabi={{.*}}'



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


[PATCH] D153805: Expose DataflowAnalysisContext.querySolver().

2023-06-26 Thread Samira Bazuzi via Phabricator via cfe-commits
bazuzi created this revision.
bazuzi added reviewers: ymandel, gribozavr2, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
bazuzi requested review of this revision.
Herald added a project: clang.

This allows for use of the same solver used by the DAC for additional solving 
post-analysis and thus shared use of MaxIterations in WatchedLiteralsSolver.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153805

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h


Index: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
===
--- clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
@@ -174,6 +174,13 @@
 
   Arena () { return *A; }
 
+  /// Returns the outcome of satisfiability checking on `Constraints`.
+  /// Possible outcomes are:
+  /// - `Satisfiable`: A satisfying assignment exists and is returned.
+  /// - `Unsatisfiable`: A satisfying assignment does not exist.
+  /// - `TimedOut`: The search for a satisfying assignment was not completed.
+  Solver::Result querySolver(llvm::DenseSet Constraints);
+
 private:
   friend class Environment;
 
@@ -203,13 +210,6 @@
   AtomicBoolValue , llvm::DenseSet ,
   llvm::DenseSet );
 
-  /// Returns the outcome of satisfiability checking on `Constraints`.
-  /// Possible outcomes are:
-  /// - `Satisfiable`: A satisfying assignment exists and is returned.
-  /// - `Unsatisfiable`: A satisfying assignment does not exist.
-  /// - `TimedOut`: The search for a satisfying assignment was not completed.
-  Solver::Result querySolver(llvm::DenseSet Constraints);
-
   /// Returns true if the solver is able to prove that there is no satisfying
   /// assignment for `Constraints`
   bool isUnsatisfiable(llvm::DenseSet Constraints) {


Index: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
===
--- clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
@@ -174,6 +174,13 @@
 
   Arena () { return *A; }
 
+  /// Returns the outcome of satisfiability checking on `Constraints`.
+  /// Possible outcomes are:
+  /// - `Satisfiable`: A satisfying assignment exists and is returned.
+  /// - `Unsatisfiable`: A satisfying assignment does not exist.
+  /// - `TimedOut`: The search for a satisfying assignment was not completed.
+  Solver::Result querySolver(llvm::DenseSet Constraints);
+
 private:
   friend class Environment;
 
@@ -203,13 +210,6 @@
   AtomicBoolValue , llvm::DenseSet ,
   llvm::DenseSet );
 
-  /// Returns the outcome of satisfiability checking on `Constraints`.
-  /// Possible outcomes are:
-  /// - `Satisfiable`: A satisfying assignment exists and is returned.
-  /// - `Unsatisfiable`: A satisfying assignment does not exist.
-  /// - `TimedOut`: The search for a satisfying assignment was not completed.
-  Solver::Result querySolver(llvm::DenseSet Constraints);
-
   /// Returns true if the solver is able to prove that there is no satisfying
   /// assignment for `Constraints`
   bool isUnsatisfiable(llvm::DenseSet Constraints) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D153584: [dataflow] Make SAT solver deterministic

2023-06-26 Thread Sam McCall via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd85c233e4b05: [dataflow] Make SAT solver deterministic 
(authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D153584?vs=533740=534699#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153584

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
  clang/include/clang/Analysis/FlowSensitive/DebugSupport.h
  clang/include/clang/Analysis/FlowSensitive/Solver.h
  clang/include/clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h
  clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
  clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
  clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp
  clang/unittests/Analysis/FlowSensitive/DebugSupportTest.cpp
  clang/unittests/Analysis/FlowSensitive/SolverTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/SolverTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/SolverTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/SolverTest.cpp
@@ -30,8 +30,8 @@
 
 // Checks if the conjunction of `Vals` is satisfiable and returns the
 // corresponding result.
-Solver::Result solve(llvm::DenseSet Vals) {
-  return WatchedLiteralsSolver().solve(std::move(Vals));
+Solver::Result solve(llvm::ArrayRef Vals) {
+  return WatchedLiteralsSolver().solve(Vals);
 }
 
 void expectUnsatisfiable(Solver::Result Result) {
Index: clang/unittests/Analysis/FlowSensitive/DebugSupportTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/DebugSupportTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/DebugSupportTest.cpp
@@ -187,34 +187,32 @@
 }
 
 TEST(ConstraintSetDebugStringTest, Empty) {
-  llvm::DenseSet Constraints;
+  llvm::ArrayRef Constraints;
   EXPECT_THAT(debugString(Constraints), StrEq(""));
 }
 
 TEST(ConstraintSetDebugStringTest, Simple) {
   ConstraintContext Ctx;
-  llvm::DenseSet Constraints;
+  std::vector Constraints;
   auto X = cast(Ctx.atom());
   auto Y = cast(Ctx.atom());
-  Constraints.insert(Ctx.disj(X, Y));
-  Constraints.insert(Ctx.disj(X, Ctx.neg(Y)));
+  Constraints.push_back(Ctx.disj(X, Y));
+  Constraints.push_back(Ctx.disj(X, Ctx.neg(Y)));
 
   auto Expected = R"((or
 X
-(not
-Y))
+Y)
 (or
 X
-Y)
+(not
+Y))
 )";
   EXPECT_THAT(debugString(Constraints, {{X, "X"}, {Y, "Y"}}),
   StrEq(Expected));
 }
 
 Solver::Result CheckSAT(std::vector Constraints) {
-  llvm::DenseSet ConstraintsSet(Constraints.begin(),
- Constraints.end());
-  return WatchedLiteralsSolver().solve(std::move(ConstraintsSet));
+  return WatchedLiteralsSolver().solve(std::move(Constraints));
 }
 
 TEST(SATCheckDebugStringTest, AtomicBoolean) {
Index: clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp
===
--- clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp
+++ clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp
@@ -20,6 +20,7 @@
 #include "clang/Analysis/FlowSensitive/Solver.h"
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/STLExtras.h"
@@ -177,7 +178,7 @@
 
 /// Converts the conjunction of `Vals` into a formula in conjunctive normal
 /// form where each clause has at least one and at most three literals.
-BooleanFormula buildBooleanFormula(const llvm::DenseSet ) {
+BooleanFormula buildBooleanFormula(const llvm::ArrayRef ) {
   // The general strategy of the algorithm implemented below is to map each
   // of the sub-values in `Vals` to a unique variable and use these variables in
   // the resulting CNF expression to avoid exponential blow up. The number of
@@ -438,7 +439,7 @@
   std::vector ActiveVars;
 
 public:
-  explicit WatchedLiteralsSolverImpl(const llvm::DenseSet )
+  explicit WatchedLiteralsSolverImpl(const llvm::ArrayRef )
   : Formula(buildBooleanFormula(Vals)), LevelVars(Formula.LargestVar + 1),
 LevelStates(Formula.LargestVar + 1) {
 assert(!Vals.empty());
@@ -718,7 +719,7 @@
   }
 };
 
-Solver::Result WatchedLiteralsSolver::solve(llvm::DenseSet Vals) {
+Solver::Result WatchedLiteralsSolver::solve(llvm::ArrayRef Vals) {
   if (Vals.empty())
 return Solver::Result::Satisfiable({{}});
   auto [Res, Iterations] =
Index: clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
===
--- clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
+++ clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
@@ -150,17 +150,10 @@
 return 

[clang] d85c233 - [dataflow] Make SAT solver deterministic

2023-06-26 Thread Sam McCall via cfe-commits

Author: Sam McCall
Date: 2023-06-26T21:16:25+02:00
New Revision: d85c233e4b05497a6d8c6e3f2a5fd63d9eeb200d

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

LOG: [dataflow] Make SAT solver deterministic

The SAT solver imported its constraints by iterating over an unordered DenseSet.
The path taken, and ultimately the runtime, the specific solution found, and
whether it would time out or complete could depend on the iteration order.

Instead, have the caller specify an ordered collection of constraints.
If this is built in a deterministic way, the system can have deterministic
behavior.
(The main alternative is to sort the constraints by value, but this option
is simpler today).

A lot of nondeterminism still appears to be remain in the framework, so today
the solver's inputs themselves are not deterministic yet.

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

Added: 


Modified: 
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
clang/include/clang/Analysis/FlowSensitive/DebugSupport.h
clang/include/clang/Analysis/FlowSensitive/Solver.h
clang/include/clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h
clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp
clang/unittests/Analysis/FlowSensitive/DebugSupportTest.cpp
clang/unittests/Analysis/FlowSensitive/SolverTest.cpp

Removed: 




diff  --git 
a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h 
b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
index 3a149b5ff397f..eba42fc3418f6 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
@@ -25,6 +25,7 @@
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/SetVector.h"
 #include "llvm/Support/Compiler.h"
 #include 
 #include 
@@ -200,7 +201,7 @@ class DataflowAnalysisContext {
   /// to track tokens of flow conditions that were already visited by recursive
   /// calls.
   void addTransitiveFlowConditionConstraints(
-  AtomicBoolValue , llvm::DenseSet ,
+  AtomicBoolValue , llvm::SetVector ,
   llvm::DenseSet );
 
   /// Returns the outcome of satisfiability checking on `Constraints`.
@@ -208,11 +209,11 @@ class DataflowAnalysisContext {
   /// - `Satisfiable`: A satisfying assignment exists and is returned.
   /// - `Unsatisfiable`: A satisfying assignment does not exist.
   /// - `TimedOut`: The search for a satisfying assignment was not completed.
-  Solver::Result querySolver(llvm::DenseSet Constraints);
+  Solver::Result querySolver(llvm::SetVector Constraints);
 
   /// Returns true if the solver is able to prove that there is no satisfying
   /// assignment for `Constraints`
-  bool isUnsatisfiable(llvm::DenseSet Constraints) {
+  bool isUnsatisfiable(llvm::SetVector Constraints) {
 return querySolver(std::move(Constraints)).getStatus() ==
Solver::Result::Status::Unsatisfiable;
   }

diff  --git a/clang/include/clang/Analysis/FlowSensitive/DebugSupport.h 
b/clang/include/clang/Analysis/FlowSensitive/DebugSupport.h
index ca50ffc5f5c8a..360786b02729f 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DebugSupport.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DebugSupport.h
@@ -57,7 +57,7 @@ std::string debugString(
 ///
 ///   Names assigned to atoms should not be repeated in `AtomNames`.
 std::string debugString(
-const llvm::DenseSet ,
+const llvm::ArrayRef Constraints,
 llvm::DenseMap AtomNames = {{}});
 
 /// Returns a string representation for `Constraints` - a collection of boolean
@@ -73,14 +73,6 @@ std::string debugString(
 std::string debugString(
 ArrayRef Constraints, const Solver::Result ,
 llvm::DenseMap AtomNames = {{}});
-inline std::string debugString(
-const llvm::DenseSet ,
-const Solver::Result ,
-llvm::DenseMap AtomNames = {{}}) {
-  std::vector ConstraintsVec(Constraints.begin(),
-  Constraints.end());
-  return debugString(ConstraintsVec, Result, std::move(AtomNames));
-}
 
 } // namespace dataflow
 } // namespace clang

diff  --git a/clang/include/clang/Analysis/FlowSensitive/Solver.h 
b/clang/include/clang/Analysis/FlowSensitive/Solver.h
index e4d450c8d12bb..10964eab8c34c 100644
--- a/clang/include/clang/Analysis/FlowSensitive/Solver.h
+++ b/clang/include/clang/Analysis/FlowSensitive/Solver.h
@@ -15,9 +15,13 @@
 #define LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_SOLVER_H
 
 #include "clang/Analysis/FlowSensitive/Value.h"
+#include "clang/Basic/LLVM.h"
+#include 

[PATCH] D153691: [Driver][ARM] Warn about -mabi= for assembler input

2023-06-26 Thread Michael Platings via Phabricator via cfe-commits
michaelplatings accepted this revision.
michaelplatings 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/D153691/new/

https://reviews.llvm.org/D153691

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


[PATCH] D153493: [dataflow] avoid more accidental copies of Environment

2023-06-26 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 534691.
sammccall added a comment.

address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153493

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp

Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -221,6 +221,59 @@
   const char *Message;
 };
 
+// Builds a joined TypeErasedDataflowAnalysisState from 0 or more sources,
+// each of which may be owned (built as part of the join) or external (a
+// reference to an Environment that will outlive the builder).
+// Avoids unneccesary copies of the environment.
+class JoinedStateBuilder {
+  AnalysisContext 
+  std::optional OwnedState;
+  // Points either to OwnedState, an external Environment, or nothing.
+  const TypeErasedDataflowAnalysisState *CurrentState = nullptr;
+
+public:
+  JoinedStateBuilder(AnalysisContext ) : AC(AC) {}
+
+  void addOwned(TypeErasedDataflowAnalysisState State) {
+if (!CurrentState) {
+  OwnedState = std::move(State);
+  CurrentState = &*OwnedState;
+} else if (!OwnedState) {
+  OwnedState.emplace(std::move(CurrentState->Lattice),
+ CurrentState->Env.join(State.Env, AC.Analysis));
+  AC.Analysis.joinTypeErased(OwnedState->Lattice, State.Lattice);
+} else {
+  OwnedState->Env = CurrentState->Env.join(State.Env, AC.Analysis);
+  AC.Analysis.joinTypeErased(OwnedState->Lattice, State.Lattice);
+}
+  }
+  void addUnowned(const TypeErasedDataflowAnalysisState ) {
+if (!CurrentState) {
+  CurrentState = 
+} else if (!OwnedState) {
+  OwnedState.emplace(CurrentState->Lattice,
+ CurrentState->Env.join(State.Env, AC.Analysis));
+  AC.Analysis.joinTypeErased(OwnedState->Lattice, State.Lattice);
+} else {
+  OwnedState->Env = CurrentState->Env.join(State.Env, AC.Analysis);
+  AC.Analysis.joinTypeErased(OwnedState->Lattice, State.Lattice);
+}
+  }
+  TypeErasedDataflowAnalysisState take() && {
+if (!OwnedState) {
+  if (CurrentState)
+OwnedState.emplace(CurrentState->Lattice, CurrentState->Env.fork());
+  else
+// FIXME: Consider passing `Block` to Analysis.typeErasedInitialElement
+// to enable building analyses like computation of dominators that
+// initialize the state of each basic block differently.
+OwnedState.emplace(AC.Analysis.typeErasedInitialElement(),
+   AC.InitEnv.fork());
+}
+return std::move(*OwnedState);
+  }
+};
+
 } // namespace
 
 /// Computes the input state for a given basic block by joining the output
@@ -267,9 +320,7 @@
 }
   }
 
-  std::optional MaybeState;
-
-  auto  = AC.Analysis;
+  JoinedStateBuilder Builder(AC);
   for (const CFGBlock *Pred : Preds) {
 // Skip if the `Block` is unreachable or control flow cannot get past it.
 if (!Pred || Pred->hasNoReturnElement())
@@ -282,36 +333,30 @@
 if (!MaybePredState)
   continue;
 
-TypeErasedDataflowAnalysisState PredState = MaybePredState->fork();
-if (Analysis.builtinOptions()) {
+if (AC.Analysis.builtinOptions()) {
   if (const Stmt *PredTerminatorStmt = Pred->getTerminatorStmt()) {
+// We have a terminator: we need to mutate an environment to describe
+// when the terminator is taken. Copy now.
+TypeErasedDataflowAnalysisState Copy = MaybePredState->fork();
+
 const StmtToEnvMap StmtToEnv(AC.CFCtx, AC.BlockStates);
 auto [Cond, CondValue] =
-TerminatorVisitor(StmtToEnv, PredState.Env,
+TerminatorVisitor(StmtToEnv, Copy.Env,
   blockIndexInPredecessor(*Pred, Block))
 .Visit(PredTerminatorStmt);
 if (Cond != nullptr)
   // FIXME: Call transferBranchTypeErased even if BuiltinTransferOpts
   // are not set.
-  Analysis.transferBranchTypeErased(CondValue, Cond, PredState.Lattice,
-PredState.Env);
+  AC.Analysis.transferBranchTypeErased(CondValue, Cond, Copy.Lattice,
+   Copy.Env);
+Builder.addOwned(std::move(Copy));
+continue;
   }
 }
-
-if (MaybeState) {
-  Analysis.joinTypeErased(MaybeState->Lattice, PredState.Lattice);
-  MaybeState->Env.join(PredState.Env, Analysis);
-} else {
-  MaybeState = std::move(PredState);
-}
-  }
-  if (!MaybeState) {
-// FIXME: Consider passing `Block` to `Analysis.typeErasedInitialElement()`
-// to enable 

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

2023-06-26 Thread Sedenion via Phabricator via cfe-commits
Sedeniono updated this revision to Diff 534688.
Sedeniono added a comment.

As suggested by @owenpan, split the commits differently. My previous statement, 
that the whole fix consists of a revert of D129064 
 was not entirely correct: The fix 
reintroduces the `resize()` that got removed in D129064 
, but it additionally surrounds it by the `if 
(!Line.InPPDirective)`. Otherwise, the formatting is wrong if some PP 
directives are encountered. (The FormatMacroRegardlessOfPreviousIndent tests 
check this.)
Also, as @owenpan suggested, I replaced the `!Line.First->is(tok::comment)` 
with `Line.First->isNot(tok::comment)` in `adjustToUnmodifiedLine()`.
I also rebased the patch to the current main branch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151047

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/unittests/Format/FormatTestSelective.cpp

Index: clang/unittests/Format/FormatTestSelective.cpp
===
--- clang/unittests/Format/FormatTestSelective.cpp
+++ clang/unittests/Format/FormatTestSelective.cpp
@@ -528,6 +528,26 @@
 format("  int a;\n"
"void ff() {}",
11, 0));
+
+  // https://github.com/llvm/llvm-project/issues/59178
+  Style = getMozillaStyle();
+  EXPECT_EQ("int a()\n"
+"{\n"
+"return 0;\n"
+"}\n"
+"int b()\n"
+"{\n"
+"  return 42;\n"
+"}",
+format("int a()\n"
+   "{\n"
+   "return 0;\n"
+   "}\n"
+   "int b()\n"
+   "{\n"
+   "return 42;\n" // Format this line only
+   "}",
+   32, 0));
 }
 
 TEST_F(FormatTestSelective, UnderstandsTabs) {
@@ -617,6 +637,70 @@
  "namespace ns1 { namespace ns2 {\n"
  "}}";
   EXPECT_EQ(Code, format(Code, 0, 0));
+
+  Style = getLLVMStyle();
+  Style.FixNamespaceComments = false;
+  Code = "namespace ns {\n"
+ "#define REF(alias) alias alias_var;\n"
+ "}";
+  EXPECT_EQ(Code, format(Code, 51, 1));
+}
+
+TEST_F(FormatTestSelective, FormatMacroRegardlessOfPreviousIndent) {
+  // clang-format currently does not (or should not) take into account the
+  // indent of previous unformatted lines when formatting a PP directive.
+  // Technically speaking, LevelIndentTracker::IndentForLevel is only for non-PP
+  // lines. So these tests here check that the indent of previous non-PP lines
+  // do not affect the formatting. If this requirement changes, the tests here
+  // need to be adapted.
+  Style = getLLVMStyle();
+
+  Style.IndentPPDirectives = FormatStyle::PPDirectiveIndentStyle::PPDIS_None;
+  EXPECT_EQ("  class Foo {\n"
+"void test() {\n"
+"#ifdef 1\n"
+"#define some\n" // Formatted line
+"#endif\n"   // That this line is also formatted might be a bug.
+"}};", // Dito: Bug?
+format("  class Foo {\n"
+   "void test() {\n"
+   "#ifdef 1\n"
+   "#define some\n" // format this line
+   " #endif\n"
+   "}};",
+   75, 0));
+
+  Style.IndentPPDirectives =
+  FormatStyle::PPDirectiveIndentStyle::PPDIS_BeforeHash;
+  EXPECT_EQ("  class Foo {\n"
+"void test() {\n"
+"#ifdef 1\n"
+"  #define some\n" // Formatted line
+" #endif\n"
+"}};",
+format("  class Foo {\n"
+   "void test() {\n"
+   "#ifdef 1\n"
+   "#define some\n" // format this line
+   " #endif\n"
+   "}};",
+   75, 0));
+
+  Style.IndentPPDirectives =
+  FormatStyle::PPDirectiveIndentStyle::PPDIS_AfterHash;
+  EXPECT_EQ("  class Foo {\n"
+"void test() {\n"
+"#ifdef 1\n"
+"#  define some\n" // Formatted line
+"#endif\n" // That this line is also formatted might be a bug.
+"}};",
+format("  class Foo {\n"
+   "void test() {\n"
+   "#ifdef 1\n"
+   "#define some\n" // format this line
+   " #endif\n"
+   "}};",
+   75, 0));
 }
 
 } // end namespace
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ 

[PATCH] D153493: [dataflow] avoid more accidental copies of Environment

2023-06-26 Thread Sam McCall via Phabricator via cfe-commits
sammccall marked an inline comment as done.
sammccall added a comment.

> Regarding the design. This looks like an optimal solution in terms of copying 
> but introduces lifetime risks and complexity that I'm uncomfortable with.

FWIW I agree with the complexity (not the lifetime risks).
I think it ultimately stems from having too many interacting types & functions 
with complicated and inconsistent signatures & constraints 
(TypeErasedDataflowAnalysisState, Environment, TypeErasedLattice, Lattice, the 
various join functions, etc) so we resort to case-bashing.
However this system is difficult to make changes to, and getting consensus as 
to what changes are good also doesn't seem easy. So I think we need to be able 
to fix bugs within the existing design (where copies need to be avoided ad-hoc).

> There's a lot of flexibility here and I wonder if we can reduce that without 
> (substantially?) impacting the amount of copying. Specifically, I wonder if 
> we can have the builder *always* own an environment, which simplifies the 
> handling of CurrentState and, generally, the number of cases to consider. It 
> costs more in principle, but maybe negligible in practice?

I don't understand the specific suggestion:

- if we create the builder lazily, we're just moving handling this extra state 
into the callsites
- if the builder owns a copy of initenv, its FC token will end up in the result 
FC
- if the builder owns a copy of initenv and detects when it should discard it, 
that just sounds like nullptr with extra steps

can you clarify?




Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:208
+if (!CurrentState) {
+  CurrentState = 
+} else if (!OwnedState) {

ymandel wrote:
> Is there some invariant that guarantees the safety here? Because `const &` 
> can bind to a temporary, so this feels like a risky API.
The invariant is that an external/unowned environment will outlive the joiner, 
which is just a temporary helper object.

Added comments to the class & CurrentState.

> const & can bind to a temporary

given that the method name is **addUnowned** I think "i shouldn't pass null" is 
more important to communicate than "I shouldn't pass a temporary", though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153493

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


[PATCH] D153798: [clang-format] Correctly annotate operator free function call

2023-06-26 Thread Emilia Kond via Phabricator via cfe-commits
rymiel created this revision.
rymiel added reviewers: HazardyKnusperkeks, owenpan, MyDeveloperDay.
Herald added projects: All, clang, clang-format.
Herald added a subscriber: cfe-commits.
rymiel requested review of this revision.

The annotator correctly annotates an overloaded operator call when
called as a member function, like `x.operator+(y)`, however, when called
as a free function, like `operator+(x, y)`, the annotator assumed it was
an overloaded operator function *declaration*, instead of a call.

This patch allows for a free function call to correctly be annotated as
a call, but only if the current like cannot be a declaration, usually
within the bodies of a function.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153798

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


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -657,6 +657,33 @@
   EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_OverloadedOperator);
   EXPECT_TOKEN(Tokens[3], tok::r_paren, TT_OverloadedOperator);
   EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_OverloadedOperatorLParen);
+
+  Tokens = annotate("class Foo {\n"
+"  int operator+(a* b);\n"
+"}");
+  ASSERT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::kw_operator, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[5], tok::plus, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[6], tok::l_paren, TT_OverloadedOperatorLParen);
+  EXPECT_TOKEN(Tokens[8], tok::star, TT_PointerOrReference);
+
+  Tokens = annotate("void foo() {\n"
+"  operator+(a * b);\n"
+"}");
+  ASSERT_EQ(Tokens.size(), 15u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::plus, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[7], tok::l_paren, TT_OverloadedOperatorLParen);
+  EXPECT_TOKEN(Tokens[9], tok::star, TT_BinaryOperator);
+
+  Tokens = annotate("class Foo {\n"
+"  void foo() {\n"
+"operator+(a * b);\n"
+"  }\n"
+"}");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::plus, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[10], tok::l_paren, TT_OverloadedOperatorLParen);
+  EXPECT_TOKEN(Tokens[12], tok::star, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, OverloadedOperatorInTemplate) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -310,8 +310,13 @@
   // If faced with "a.operator*(argument)" or "a->operator*(argument)",
   // i.e. the operator is called as a member function,
   // then the argument must be an expression.
+  // If faced with "operator+(argument)", i.e. the operator is called as
+  // a free function, then the argument is an expression only if the 
current
+  // line can't be a declaration.
   bool OperatorCalledAsMemberFunction =
-  Prev->Previous && Prev->Previous->isOneOf(tok::period, tok::arrow);
+  (Prev->Previous &&
+   Prev->Previous->isOneOf(tok::period, tok::arrow)) ||
+  (!Line.MustBeDeclaration && !Line.InMacroBody);
   Contexts.back().IsExpression = OperatorCalledAsMemberFunction;
 } else if (OpeningParen.is(TT_VerilogInstancePortLParen)) {
   Contexts.back().IsExpression = true;


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -657,6 +657,33 @@
   EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_OverloadedOperator);
   EXPECT_TOKEN(Tokens[3], tok::r_paren, TT_OverloadedOperator);
   EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_OverloadedOperatorLParen);
+
+  Tokens = annotate("class Foo {\n"
+"  int operator+(a* b);\n"
+"}");
+  ASSERT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::kw_operator, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[5], tok::plus, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[6], tok::l_paren, TT_OverloadedOperatorLParen);
+  EXPECT_TOKEN(Tokens[8], tok::star, TT_PointerOrReference);
+
+  Tokens = annotate("void foo() {\n"
+"  operator+(a * b);\n"
+"}");
+  ASSERT_EQ(Tokens.size(), 15u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::plus, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[7], tok::l_paren, TT_OverloadedOperatorLParen);
+  EXPECT_TOKEN(Tokens[9], tok::star, TT_BinaryOperator);
+
+  Tokens = annotate("class Foo {\n"
+"  void foo() {\n"
+"

[clang] afa1fc4 - OpenMP: Add fma math test

2023-06-26 Thread Matt Arsenault via cfe-commits

Author: Matt Arsenault
Date: 2023-06-26T14:47:56-04:00
New Revision: afa1fc47b669d8cbd480cb50a0d10aea8e1e4007

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

LOG: OpenMP: Add fma math test

Added: 


Modified: 
clang/test/Headers/amdgcn_openmp_device_math.c

Removed: 




diff  --git a/clang/test/Headers/amdgcn_openmp_device_math.c 
b/clang/test/Headers/amdgcn_openmp_device_math.c
index b508afb49362d..74093538550bd 100644
--- a/clang/test/Headers/amdgcn_openmp_device_math.c
+++ b/clang/test/Headers/amdgcn_openmp_device_math.c
@@ -14,21 +14,33 @@
 
 // CHECK-C-LABEL: @test_math_f64(
 // CHECK-C-NEXT:  entry:
-// CHECK-C-NEXT:[[RETVAL_I8:%.*]] = alloca double, align 8, addrspace(5)
-// CHECK-C-NEXT:[[__X_ADDR_I9:%.*]] = alloca double, align 8, addrspace(5)
-// CHECK-C-NEXT:[[RETVAL_I3:%.*]] = alloca double, align 8, addrspace(5)
-// CHECK-C-NEXT:[[__X_ADDR_I4:%.*]] = alloca double, align 8, addrspace(5)
+// CHECK-C-NEXT:[[RETVAL_I13:%.*]] = alloca double, align 8, addrspace(5)
+// CHECK-C-NEXT:[[__X_ADDR_I14:%.*]] = alloca double, align 8, addrspace(5)
+// CHECK-C-NEXT:[[__Y_ADDR_I:%.*]] = alloca double, align 8, addrspace(5)
+// CHECK-C-NEXT:[[__Z_ADDR_I:%.*]] = alloca double, align 8, addrspace(5)
+// CHECK-C-NEXT:[[RETVAL_I9:%.*]] = alloca double, align 8, addrspace(5)
+// CHECK-C-NEXT:[[__X_ADDR_I10:%.*]] = alloca double, align 8, addrspace(5)
+// CHECK-C-NEXT:[[RETVAL_I4:%.*]] = alloca double, align 8, addrspace(5)
+// CHECK-C-NEXT:[[__X_ADDR_I5:%.*]] = alloca double, align 8, addrspace(5)
 // CHECK-C-NEXT:[[RETVAL_I:%.*]] = alloca double, align 8, addrspace(5)
 // CHECK-C-NEXT:[[__X_ADDR_I:%.*]] = alloca double, align 8, addrspace(5)
 // CHECK-C-NEXT:[[X_ADDR:%.*]] = alloca double, align 8, addrspace(5)
+// CHECK-C-NEXT:[[Y_ADDR:%.*]] = alloca double, align 8, addrspace(5)
+// CHECK-C-NEXT:[[Z_ADDR:%.*]] = alloca double, align 8, addrspace(5)
 // CHECK-C-NEXT:[[L1:%.*]] = alloca double, align 8, addrspace(5)
 // CHECK-C-NEXT:[[L2:%.*]] = alloca double, align 8, addrspace(5)
 // CHECK-C-NEXT:[[L3:%.*]] = alloca double, align 8, addrspace(5)
+// CHECK-C-NEXT:[[L4:%.*]] = alloca double, align 8, addrspace(5)
 // CHECK-C-NEXT:[[X_ADDR_ASCAST:%.*]] = addrspacecast ptr addrspace(5) 
[[X_ADDR]] to ptr
+// CHECK-C-NEXT:[[Y_ADDR_ASCAST:%.*]] = addrspacecast ptr addrspace(5) 
[[Y_ADDR]] to ptr
+// CHECK-C-NEXT:[[Z_ADDR_ASCAST:%.*]] = addrspacecast ptr addrspace(5) 
[[Z_ADDR]] to ptr
 // CHECK-C-NEXT:[[L1_ASCAST:%.*]] = addrspacecast ptr addrspace(5) [[L1]] 
to ptr
 // CHECK-C-NEXT:[[L2_ASCAST:%.*]] = addrspacecast ptr addrspace(5) [[L2]] 
to ptr
 // CHECK-C-NEXT:[[L3_ASCAST:%.*]] = addrspacecast ptr addrspace(5) [[L3]] 
to ptr
+// CHECK-C-NEXT:[[L4_ASCAST:%.*]] = addrspacecast ptr addrspace(5) [[L4]] 
to ptr
 // CHECK-C-NEXT:store double [[X:%.*]], ptr [[X_ADDR_ASCAST]], align 8
+// CHECK-C-NEXT:store double [[Y:%.*]], ptr [[Y_ADDR_ASCAST]], align 8
+// CHECK-C-NEXT:store double [[Z:%.*]], ptr [[Z_ADDR_ASCAST]], align 8
 // CHECK-C-NEXT:[[TMP0:%.*]] = load double, ptr [[X_ADDR_ASCAST]], align 8
 // CHECK-C-NEXT:[[RETVAL_ASCAST_I:%.*]] = addrspacecast ptr addrspace(5) 
[[RETVAL_I]] to ptr
 // CHECK-C-NEXT:[[__X_ADDR_ASCAST_I:%.*]] = addrspacecast ptr addrspace(5) 
[[__X_ADDR_I]] to ptr
@@ -37,38 +49,65 @@
 // CHECK-C-NEXT:[[CALL_I:%.*]] = call double @__ocml_sin_f64(double 
noundef [[TMP1]]) #[[ATTR3:[0-9]+]]
 // CHECK-C-NEXT:store double [[CALL_I]], ptr [[L1_ASCAST]], align 8
 // CHECK-C-NEXT:[[TMP2:%.*]] = load double, ptr [[X_ADDR_ASCAST]], align 8
-// CHECK-C-NEXT:[[RETVAL_ASCAST_I5:%.*]] = addrspacecast ptr addrspace(5) 
[[RETVAL_I3]] to ptr
-// CHECK-C-NEXT:[[__X_ADDR_ASCAST_I6:%.*]] = addrspacecast ptr 
addrspace(5) [[__X_ADDR_I4]] to ptr
-// CHECK-C-NEXT:store double [[TMP2]], ptr [[__X_ADDR_ASCAST_I6]], align 8
-// CHECK-C-NEXT:[[TMP3:%.*]] = load double, ptr [[__X_ADDR_ASCAST_I6]], 
align 8
-// CHECK-C-NEXT:[[CALL_I7:%.*]] = call double @__ocml_cos_f64(double 
noundef [[TMP3]]) #[[ATTR3]]
-// CHECK-C-NEXT:store double [[CALL_I7]], ptr [[L2_ASCAST]], align 8
+// CHECK-C-NEXT:[[RETVAL_ASCAST_I6:%.*]] = addrspacecast ptr addrspace(5) 
[[RETVAL_I4]] to ptr
+// CHECK-C-NEXT:[[__X_ADDR_ASCAST_I7:%.*]] = addrspacecast ptr 
addrspace(5) [[__X_ADDR_I5]] to ptr
+// CHECK-C-NEXT:store double [[TMP2]], ptr [[__X_ADDR_ASCAST_I7]], align 8
+// CHECK-C-NEXT:[[TMP3:%.*]] = load double, ptr [[__X_ADDR_ASCAST_I7]], 
align 8
+// CHECK-C-NEXT:[[CALL_I8:%.*]] = call double @__ocml_cos_f64(double 
noundef [[TMP3]]) #[[ATTR3]]
+// CHECK-C-NEXT:store double [[CALL_I8]], ptr [[L2_ASCAST]], align 8
 // 

[PATCH] D153226: OpenMP: Don't include stdbool.h in builtin headers

2023-06-26 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm closed this revision.
arsenm added a comment.

dee1f5b32c3a6a5694c3bb2fbf68d162447a5970 



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

https://reviews.llvm.org/D153226

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


[PATCH] D152858: OpenMP: Use generated checks and pragma declare target

2023-06-26 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm closed this revision.
arsenm added a comment.

53d28b2a71063b974ec058ced614d1c95d5584fe 



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

https://reviews.llvm.org/D152858

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


[clang] dee1f5b - OpenMP: Don't include stdbool.h in builtin headers

2023-06-26 Thread Matt Arsenault via cfe-commits

Author: Matt Arsenault
Date: 2023-06-26T14:45:09-04:00
New Revision: dee1f5b32c3a6a5694c3bb2fbf68d162447a5970

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

LOG: OpenMP: Don't include stdbool.h in builtin headers

Pre-C99 didn't include bool, and C99 allows you to redefine true/false
apparently.

Added: 
clang/test/Headers/openmp-device-functions-bool.c

Modified: 
clang/lib/Headers/__clang_hip_libdevice_declares.h
clang/lib/Headers/openmp_wrappers/__clang_openmp_device_functions.h

Removed: 




diff  --git a/clang/lib/Headers/__clang_hip_libdevice_declares.h 
b/clang/lib/Headers/__clang_hip_libdevice_declares.h
index 2fc5e17d2f1ea..ec55d1a171eaa 100644
--- a/clang/lib/Headers/__clang_hip_libdevice_declares.h
+++ b/clang/lib/Headers/__clang_hip_libdevice_declares.h
@@ -289,8 +289,15 @@ __device__ __attribute__((pure)) _Float16 
__ocml_pown_f16(_Float16, int);
 typedef _Float16 __2f16 __attribute__((ext_vector_type(2)));
 typedef short __2i16 __attribute__((ext_vector_type(2)));
 
+// We need to match C99's bool and get an i1 in the IR.
+#ifdef __cplusplus
+typedef bool __ockl_bool;
+#else
+typedef _Bool __ockl_bool;
+#endif
+
 __device__ __attribute__((const)) float __ockl_fdot2(__2f16 a, __2f16 b,
- float c, bool s);
+ float c, __ockl_bool s);
 __device__ __attribute__((const)) __2f16 __ocml_ceil_2f16(__2f16);
 __device__ __attribute__((const)) __2f16 __ocml_fabs_2f16(__2f16);
 __device__ __2f16 __ocml_cos_2f16(__2f16);

diff  --git 
a/clang/lib/Headers/openmp_wrappers/__clang_openmp_device_functions.h 
b/clang/lib/Headers/openmp_wrappers/__clang_openmp_device_functions.h
index 279fb26fbaf78..d5b6846b03488 100644
--- a/clang/lib/Headers/openmp_wrappers/__clang_openmp_device_functions.h
+++ b/clang/lib/Headers/openmp_wrappers/__clang_openmp_device_functions.h
@@ -40,7 +40,6 @@ extern "C" {
 
 // Import types which will be used by __clang_hip_libdevice_declares.h
 #ifndef __cplusplus
-#include 
 #include 
 #endif
 

diff  --git a/clang/test/Headers/openmp-device-functions-bool.c 
b/clang/test/Headers/openmp-device-functions-bool.c
new file mode 100644
index 0..81e5d3c0d57fc
--- /dev/null
+++ b/clang/test/Headers/openmp-device-functions-bool.c
@@ -0,0 +1,89 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py 
UTC_ARGS: --version 2
+// RUN: %clang_cc1 -x c -fopenmp -triple amdgcn-amd-amdhsa -aux-triple 
x86_64-unknown-unknown -fopenmp-targets=amdgcn-amd-amdhsa -internal-isystem 
%S/../../lib/Headers/openmp_wrappers -include __clang_openmp_device_functions.h 
-internal-isystem %S/../../lib/Headers/openmp_wrappers -internal-isystem 
%S/Inputs/include -emit-llvm %s -fopenmp-is-device  -o - | FileCheck %s 
--check-prefixes=CHECK,CHECK-C
+// RUN: %clang_cc1 -x c++ -fopenmp -triple amdgcn-amd-amdhsa -aux-triple 
x86_64-unknown-unknown -fopenmp-targets=amdgcn-amd-amdhsa -internal-isystem 
%S/../../lib/Headers/openmp_wrappers -include __clang_openmp_device_functions.h 
-internal-isystem %S/../../lib/Headers/openmp_wrappers -internal-isystem 
%S/Inputs/include -emit-llvm %s -fopenmp-is-device  -o - | FileCheck %s 
--check-prefixes=CHECK,CHECK-CPP
+
+// Test that we did not include  in C, and OCKL functions using bool
+// produce an i1
+
+#ifdef __cplusplus
+typedef bool ockl_bool;
+#define EXTERN_C extern "C"
+#else
+typedef _Bool ockl_bool;
+#define EXTERN_C
+#endif
+
+#pragma omp begin declare target
+
+// CHECK-LABEL: define hidden float @test_fdot2
+// CHECK-SAME: (<2 x half> noundef [[A:%.*]], <2 x half> noundef [[B:%.*]], 
float noundef [[C:%.*]], i1 noundef zeroext [[S:%.*]]) #[[ATTR0:[0-9]+]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[RETVAL:%.*]] = alloca float, align 4, addrspace(5)
+// CHECK-NEXT:[[A_ADDR:%.*]] = alloca <2 x half>, align 4, addrspace(5)
+// CHECK-NEXT:[[B_ADDR:%.*]] = alloca <2 x half>, align 4, addrspace(5)
+// CHECK-NEXT:[[C_ADDR:%.*]] = alloca float, align 4, addrspace(5)
+// CHECK-NEXT:[[S_ADDR:%.*]] = alloca i8, align 1, addrspace(5)
+// CHECK-NEXT:[[RETVAL_ASCAST:%.*]] = addrspacecast ptr addrspace(5) 
[[RETVAL]] to ptr
+// CHECK-NEXT:[[A_ADDR_ASCAST:%.*]] = addrspacecast ptr addrspace(5) 
[[A_ADDR]] to ptr
+// CHECK-NEXT:[[B_ADDR_ASCAST:%.*]] = addrspacecast ptr addrspace(5) 
[[B_ADDR]] to ptr
+// CHECK-NEXT:[[C_ADDR_ASCAST:%.*]] = addrspacecast ptr addrspace(5) 
[[C_ADDR]] to ptr
+// CHECK-NEXT:[[S_ADDR_ASCAST:%.*]] = addrspacecast ptr addrspace(5) 
[[S_ADDR]] to ptr
+// CHECK-NEXT:store <2 x half> [[A]], ptr [[A_ADDR_ASCAST]], align 4
+// CHECK-NEXT:store <2 x half> [[B]], ptr [[B_ADDR_ASCAST]], align 4
+// CHECK-NEXT:store float [[C]], ptr 

[clang] 53d28b2 - OpenMP: Use generated checks and pragma declare target

2023-06-26 Thread Matt Arsenault via cfe-commits

Author: Matt Arsenault
Date: 2023-06-26T14:45:09-04:00
New Revision: 53d28b2a71063b974ec058ced614d1c95d5584fe

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

LOG: OpenMP: Use generated checks and pragma declare target

Added: 


Modified: 
clang/test/Headers/amdgcn_openmp_device_math.c

Removed: 




diff  --git a/clang/test/Headers/amdgcn_openmp_device_math.c 
b/clang/test/Headers/amdgcn_openmp_device_math.c
index 1f3eac82c038a..b508afb49362d 100644
--- a/clang/test/Headers/amdgcn_openmp_device_math.c
+++ b/clang/test/Headers/amdgcn_openmp_device_math.c
@@ -1,7 +1,8 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
 // RUN: %clang_cc1 -internal-isystem %S/Inputs/include -x c -fopenmp -triple 
x86_64-unknown-unknown -fopenmp-targets=amdgcn-amd-amdhsa -emit-llvm-bc %s -o 
%t-host.bc
-// RUN: %clang_cc1 -internal-isystem %S/../../lib/Headers/openmp_wrappers 
-include __clang_openmp_device_functions.h -internal-isystem 
%S/../../lib/Headers/openmp_wrappers -internal-isystem %S/Inputs/include -x c 
-fopenmp -triple amdgcn-amd-amdhsa -aux-triple x86_64-unknown-unknown 
-fopenmp-targets=amdgcn-amd-amdhsa -emit-llvm %s -fopenmp-is-device 
-fopenmp-host-ir-file-path %t-host.bc -o - | FileCheck %s 
--check-prefixes=CHECK-C,CHECK
+// RUN: %clang_cc1 -internal-isystem %S/../../lib/Headers/openmp_wrappers 
-include __clang_openmp_device_functions.h -internal-isystem 
%S/../../lib/Headers/openmp_wrappers -internal-isystem %S/Inputs/include -x c 
-fopenmp -triple amdgcn-amd-amdhsa -aux-triple x86_64-unknown-unknown 
-fopenmp-targets=amdgcn-amd-amdhsa -emit-llvm %s -fopenmp-is-device 
-fopenmp-host-ir-file-path %t-host.bc -o - | FileCheck %s 
--check-prefixes=CHECK,CHECK-C
 // RUN: %clang_cc1 -internal-isystem %S/Inputs/include -x c++ -fopenmp -triple 
x86_64-unknown-unknown -fopenmp-targets=amdgcn-amd-amdhsa -emit-llvm-bc %s -o 
%t-host.bc
-// RUN: %clang_cc1 -internal-isystem %S/../../lib/Headers/openmp_wrappers 
-include __clang_openmp_device_functions.h -internal-isystem 
%S/../../lib/Headers/openmp_wrappers -internal-isystem %S/Inputs/include -x c++ 
-fopenmp -triple amdgcn-amd-amdhsa -aux-triple x86_64-unknown-unknown 
-fopenmp-targets=amdgcn-amd-amdhsa -emit-llvm %s -fopenmp-is-device 
-fopenmp-host-ir-file-path %t-host.bc -o - | FileCheck %s 
--check-prefixes=CHECK-CPP,CHECK
+// RUN: %clang_cc1 -internal-isystem %S/../../lib/Headers/openmp_wrappers 
-include __clang_openmp_device_functions.h -internal-isystem 
%S/../../lib/Headers/openmp_wrappers -internal-isystem %S/Inputs/include -x c++ 
-fopenmp -triple amdgcn-amd-amdhsa -aux-triple x86_64-unknown-unknown 
-fopenmp-targets=amdgcn-amd-amdhsa -emit-llvm %s -fopenmp-is-device 
-fopenmp-host-ir-file-path %t-host.bc -o - | FileCheck %s 
--check-prefixes=CHECK,CHECK-CPP
 
 #ifdef __cplusplus
 #include 
@@ -9,43 +10,290 @@
 #include 
 #endif
 
+#pragma omp begin declare target
+
+// CHECK-C-LABEL: @test_math_f64(
+// CHECK-C-NEXT:  entry:
+// CHECK-C-NEXT:[[RETVAL_I8:%.*]] = alloca double, align 8, addrspace(5)
+// CHECK-C-NEXT:[[__X_ADDR_I9:%.*]] = alloca double, align 8, addrspace(5)
+// CHECK-C-NEXT:[[RETVAL_I3:%.*]] = alloca double, align 8, addrspace(5)
+// CHECK-C-NEXT:[[__X_ADDR_I4:%.*]] = alloca double, align 8, addrspace(5)
+// CHECK-C-NEXT:[[RETVAL_I:%.*]] = alloca double, align 8, addrspace(5)
+// CHECK-C-NEXT:[[__X_ADDR_I:%.*]] = alloca double, align 8, addrspace(5)
+// CHECK-C-NEXT:[[X_ADDR:%.*]] = alloca double, align 8, addrspace(5)
+// CHECK-C-NEXT:[[L1:%.*]] = alloca double, align 8, addrspace(5)
+// CHECK-C-NEXT:[[L2:%.*]] = alloca double, align 8, addrspace(5)
+// CHECK-C-NEXT:[[L3:%.*]] = alloca double, align 8, addrspace(5)
+// CHECK-C-NEXT:[[X_ADDR_ASCAST:%.*]] = addrspacecast ptr addrspace(5) 
[[X_ADDR]] to ptr
+// CHECK-C-NEXT:[[L1_ASCAST:%.*]] = addrspacecast ptr addrspace(5) [[L1]] 
to ptr
+// CHECK-C-NEXT:[[L2_ASCAST:%.*]] = addrspacecast ptr addrspace(5) [[L2]] 
to ptr
+// CHECK-C-NEXT:[[L3_ASCAST:%.*]] = addrspacecast ptr addrspace(5) [[L3]] 
to ptr
+// CHECK-C-NEXT:store double [[X:%.*]], ptr [[X_ADDR_ASCAST]], align 8
+// CHECK-C-NEXT:[[TMP0:%.*]] = load double, ptr [[X_ADDR_ASCAST]], align 8
+// CHECK-C-NEXT:[[RETVAL_ASCAST_I:%.*]] = addrspacecast ptr addrspace(5) 
[[RETVAL_I]] to ptr
+// CHECK-C-NEXT:[[__X_ADDR_ASCAST_I:%.*]] = addrspacecast ptr addrspace(5) 
[[__X_ADDR_I]] to ptr
+// CHECK-C-NEXT:store double [[TMP0]], ptr [[__X_ADDR_ASCAST_I]], align 8
+// CHECK-C-NEXT:[[TMP1:%.*]] = load double, ptr [[__X_ADDR_ASCAST_I]], 
align 8
+// CHECK-C-NEXT:[[CALL_I:%.*]] = call double @__ocml_sin_f64(double 
noundef [[TMP1]]) #[[ATTR3:[0-9]+]]
+// CHECK-C-NEXT:store double [[CALL_I]], ptr 

[PATCH] D152720: [clangd][ObjC] Support ObjC class rename from implementation decls

2023-06-26 Thread David Goldman via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1b66840f7103: [clangd][ObjC] Support ObjC class rename from 
implementation decls (authored by dgoldman).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152720

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp
  clang-tools-extra/clangd/unittests/RenameTests.cpp

Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -840,6 +840,20 @@
   foo('x');
 }
   )cpp",
+
+  // ObjC class with a category.
+  R"cpp(
+@interface [[Fo^o]]
+@end
+@implementation [[F^oo]]
+@end
+@interface [[Fo^o]] (Category)
+@end
+@implementation [[F^oo]] (Category)
+@end
+
+void func([[Fo^o]] *f) {}
+  )cpp",
   };
   llvm::StringRef NewName = "NewName";
   for (llvm::StringRef T : Tests) {
@@ -890,6 +904,15 @@
   )cpp",
"not a supported kind", HeaderFile},
 
+  {R"cpp(// disallow - category rename.
+@interface Foo
+@end
+@interface Foo (Cate^gory)
+@end
+  )cpp",
+   "Cannot rename symbol: there is no symbol at the given location",
+   HeaderFile},
+
   {
   R"cpp(
  #define MACRO 1
@@ -1468,7 +1491,7 @@
 
 TEST(CrossFileRenameTests, WithUpToDateIndex) {
   MockCompilationDatabase CDB;
-  CDB.ExtraClangFlags = {"-xc++"};
+  CDB.ExtraClangFlags = {"-xobjective-c++"};
   // rename is runnning on all "^" points in FooH, and "[[]]" ranges are the
   // expected rename occurrences.
   struct Case {
@@ -1557,13 +1580,12 @@
 }
   )cpp",
   },
-  {
-  // virtual templated method
-  R"cpp(
+  {// virtual templated method
+   R"cpp(
 template  class Foo { virtual void [[m]](); };
 class Bar : Foo { void [[^m]]() override; };
   )cpp",
-  R"cpp(
+   R"cpp(
   #include "foo.h"
 
   template void Foo::[[m]]() {}
@@ -1571,8 +1593,7 @@
   // the canonical Foo::m().
   // https://github.com/clangd/clangd/issues/1325
   class Baz : Foo { void m() override; };
-)cpp"
-  },
+)cpp"},
   {
   // rename on constructor and destructor.
   R"cpp(
@@ -1677,6 +1698,20 @@
 }
   )cpp",
   },
+  {
+  // Objective-C classes.
+  R"cpp(
+@interface [[Fo^o]]
+@end
+  )cpp",
+  R"cpp(
+#include "foo.h"
+@implementation [[Foo]]
+@end
+
+void func([[Foo]] *f) {}
+  )cpp",
+  },
   };
 
   trace::TestTracer Tracer;
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -1127,6 +1127,16 @@
   )cpp";
   EXPECT_DECLS("ObjCCategoryImplDecl", "@interface Foo(Ext)");
 
+  Code = R"cpp(
+@interface Foo
+@end
+@interface Foo (Ext)
+@end
+@implementation Foo ([[Ext]])
+@end
+  )cpp";
+  EXPECT_DECLS("ObjCCategoryImplDecl", "@interface Foo(Ext)");
+
   Code = R"cpp(
 void test(id p);
   )cpp";
@@ -1216,10 +1226,7 @@
 std::string DumpedReferences;
   };
 
-  /// Parses \p Code, finds function or namespace '::foo' and annotates its body
-  /// with results of findExplicitReferences.
-  /// See actual tests for examples of annotation format.
-  AllRefs annotateReferencesInFoo(llvm::StringRef Code) {
+  TestTU newTU(llvm::StringRef Code) {
 TestTU TU;
 TU.Code = std::string(Code);
 
@@ -1228,30 +1235,11 @@
 TU.ExtraArgs.push_back("-std=c++20");
 TU.ExtraArgs.push_back("-xobjective-c++");
 
-auto AST = TU.build();
-auto *TestDecl = (AST, "foo");
-if (auto *T = llvm::dyn_cast(TestDecl))
-  TestDecl = T->getTemplatedDecl();
-
-std::vector Refs;
-if (const auto *Func = llvm::dyn_cast(TestDecl))
-  findExplicitReferences(
-  Func->getBody(),
-  [](ReferenceLoc R) { Refs.push_back(std::move(R)); },
-  AST.getHeuristicResolver());
-else if (const auto *NS = llvm::dyn_cast(TestDecl))
-  findExplicitReferences(
-  NS,
-  [, ](ReferenceLoc R) {
-// Avoid adding the namespace foo decl to the results.
-if (R.Targets.size() == 1 && R.Targets.front() == NS)
-  return;
-Refs.push_back(std::move(R));
-  },
-  AST.getHeuristicResolver());
-else
-  ADD_FAILURE() << "Failed 

[clang-tools-extra] 1b66840 - [clangd][ObjC] Support ObjC class rename from implementation decls

2023-06-26 Thread David Goldman via cfe-commits

Author: David Goldman
Date: 2023-06-26T14:43:37-04:00
New Revision: 1b66840f71030f5d5934e99398a59c3485ba111e

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

LOG: [clangd][ObjC] Support ObjC class rename from implementation decls

Reviewed By: kadircet

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

Added: 


Modified: 
clang-tools-extra/clangd/FindTarget.cpp
clang-tools-extra/clangd/SemanticHighlighting.cpp
clang-tools-extra/clangd/refactor/Rename.cpp
clang-tools-extra/clangd/unittests/FindTargetTests.cpp
clang-tools-extra/clangd/unittests/RenameTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/FindTarget.cpp 
b/clang-tools-extra/clangd/FindTarget.cpp
index eead9e6a3a7a4..630b75059b6ba 100644
--- a/clang-tools-extra/clangd/FindTarget.cpp
+++ b/clang-tools-extra/clangd/FindTarget.cpp
@@ -708,8 +708,23 @@ llvm::SmallVector refInDecl(const Decl *D,
   {OCID->getClassInterface()}});
   Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(),
   OCID->getCategoryNameLoc(),
-  /*IsDecl=*/true,
+  /*IsDecl=*/false,
   {OCID->getCategoryDecl()}});
+  Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(),
+  OCID->getCategoryNameLoc(),
+  /*IsDecl=*/true,
+  {OCID}});
+}
+
+void VisitObjCImplementationDecl(const ObjCImplementationDecl *OIMD) {
+  Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(),
+  OIMD->getLocation(),
+  /*IsDecl=*/false,
+  {OIMD->getClassInterface()}});
+  Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(),
+  OIMD->getLocation(),
+  /*IsDecl=*/true,
+  {OIMD}});
 }
   };
 

diff  --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp 
b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index 3826170892e8c..f6a3f7ac66aa0 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -128,7 +128,7 @@ std::optional kindForDecl(const NamedDecl 
*D,
 return HighlightingKind::Class;
   if (isa(D))
 return HighlightingKind::Interface;
-  if (isa(D))
+  if (isa(D))
 return HighlightingKind::Namespace;
   if (auto *MD = dyn_cast(D))
 return MD->isStatic() ? HighlightingKind::StaticMethod

diff  --git a/clang-tools-extra/clangd/refactor/Rename.cpp 
b/clang-tools-extra/clangd/refactor/Rename.cpp
index b3270534b13b1..97ea5e1836579 100644
--- a/clang-tools-extra/clangd/refactor/Rename.cpp
+++ b/clang-tools-extra/clangd/refactor/Rename.cpp
@@ -19,6 +19,7 @@
 #include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclObjC.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/ParentMapContext.h"
 #include "clang/AST/Stmt.h"
@@ -140,6 +141,18 @@ const NamedDecl *canonicalRenameDecl(const NamedDecl *D) {
   return dyn_cast(D->getCanonicalDecl());
 }
 
+// Some AST nodes can reference multiple declarations. We try to pick the
+// relevant one to rename here.
+const NamedDecl *pickInterestingTarget(const NamedDecl *D) {
+  // We only support renaming the class name, not the category name. This has
+  // to be done outside of canonicalization since we don't want a category name
+  // reference to be canonicalized to the class.
+  if (const auto *CD = dyn_cast(D))
+if (const auto CI = CD->getClassInterface())
+  return CI;
+  return D;
+}
+
 llvm::DenseSet locateDeclAt(ParsedAST ,
SourceLocation TokenStartLoc) {
   unsigned Offset =
@@ -156,6 +169,7 @@ llvm::DenseSet locateDeclAt(ParsedAST 
,
targetDecl(SelectedNode->ASTNode,
   DeclRelation::Alias | DeclRelation::TemplatePattern,
   AST.getHeuristicResolver())) {
+D = pickInterestingTarget(D);
 Result.insert(canonicalRenameDecl(D));
   }
   return Result;

diff  --git a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp 
b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
index 9979628941bfe..64ac524fc5187 100644
--- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -1127,6 +1127,16 @@ TEST_F(TargetDeclTest, ObjC) {
   )cpp";
   EXPECT_DECLS("ObjCCategoryImplDecl", "@interface Foo(Ext)");
 
+  Code = R"cpp(
+@interface Foo
+@end
+@interface Foo (Ext)
+@end
+@implementation Foo 

[PATCH] D153370: [RISCV] Add support for custom instructions for Sifive S76.

2023-06-26 Thread Craig Topper via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4c37d30e22ae: [RISCV] Add support for custom instructions 
for Sifive S76. (authored by garvitgupta08, committed by craig.topper).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153370

Files:
  clang/test/Driver/riscv-cpus.c
  llvm/docs/RISCVUsage.rst
  llvm/docs/ReleaseNotes.rst
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
  llvm/lib/Target/RISCV/RISCVFeatures.td
  llvm/lib/Target/RISCV/RISCVInstrInfoXSf.td
  llvm/lib/Target/RISCV/RISCVProcessors.td
  llvm/test/MC/RISCV/attribute-arch.s
  llvm/test/MC/RISCV/xsfcie-invalid.s
  llvm/test/MC/RISCV/xsfcie-valid.s

Index: llvm/test/MC/RISCV/xsfcie-valid.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/xsfcie-valid.s
@@ -0,0 +1,42 @@
+# SCIE - SiFive Custom Instructions Extension.
+# RUN: llvm-mc %s -triple=riscv32 -mattr=+xsfcie -riscv-no-aliases -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-ENC,CHECK-INST %s
+# RUN: llvm-mc %s -triple=riscv64 -mattr=+xsfcie -riscv-no-aliases -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-ENC,CHECK-INST %s
+# RUN: llvm-mc -filetype=obj -triple riscv32 -mattr=+xsfcie < %s \
+# RUN: | llvm-objdump --mattr=+xsfcie -M no-aliases -d - \
+# RUN: | FileCheck -check-prefix=CHECK-INST %s
+# RUN: llvm-mc -filetype=obj -triple riscv64 -mattr=+xsfcie < %s \
+# RUN: | llvm-objdump --mattr=+xsfcie -M no-aliases -d - \
+# RUN: | FileCheck -check-prefix=CHECK-INST %s
+# RUN: llvm-mc %s -triple=riscv64 -mcpu=sifive-s76 -riscv-no-aliases -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-ENC,CHECK-INST %s
+# RUN: llvm-mc -filetype=obj -triple riscv64 -mcpu=sifive-s76 < %s \
+# RUN: | llvm-objdump --mcpu=sifive-s76 -M no-aliases -d - \
+# RUN: | FileCheck -check-prefix=CHECK-INST %s
+
+# CHECK-INST: cflush.d.l1 zero
+# CHECK-ENC: encoding: [0x73,0x00,0x00,0xfc]
+# CHECK-INST: cflush.d.l1 zero
+# CHECK-ENC: encoding: [0x73,0x00,0x00,0xfc]
+cflush.d.l1 x0
+cflush.d.l1
+
+# CHECK-INST: cflush.d.l1 t2
+# CHECK-ENC: encoding: [0x73,0x80,0x03,0xfc]
+cflush.d.l1 x7
+
+# CHECK-INST: cdiscard.d.l1   zero
+# CHECK-ENC: encoding: [0x73,0x00,0x20,0xfc]
+# CHECK-INST: cdiscard.d.l1 zero
+# CHECK-ENC: encoding: [0x73,0x00,0x20,0xfc]
+cdiscard.d.l1 x0
+cdiscard.d.l1
+
+# CHECK-INST: cdiscard.d.l1   t2
+# CHECK-ENC: encoding: [0x73,0x80,0x23,0xfc]
+cdiscard.d.l1 x7
+
+# CHECK-INST: cease
+# CHECK-ENC: encoding: [0x73,0x00,0x50,0x30]
+cease
Index: llvm/test/MC/RISCV/xsfcie-invalid.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/xsfcie-invalid.s
@@ -0,0 +1,25 @@
+# SCIE - SiFive Custom Instructions Extension.
+# RUN: not llvm-mc -triple riscv32 -mattr=-xsfcie < %s 2>&1 | FileCheck %s
+# RUN: not llvm-mc -triple riscv64 -mattr=-xsfcie < %s 2>&1 | FileCheck %s
+
+cflush.d.l1 0x10 # CHECK: :[[@LINE]]:13: error: invalid operand for instruction
+
+cdiscard.d.l1 0x10 # CHECK: :[[@LINE]]:15: error: invalid operand for instruction
+
+cflush.d.l1 # CHECK: :[[@LINE]]:1: error: instruction requires the following: 'XSfcie' (SiFive Custom Instruction Extension SCIE.)
+
+cdiscard.d.l1 # CHECK: :[[@LINE]]:1: error: instruction requires the following: 'XSfcie' (SiFive Custom Instruction Extension SCIE.)
+
+cflush.d.l1 x0 # CHECK: :[[@LINE]]:1: error: instruction requires the following: 'XSfcie' (SiFive Custom Instruction Extension SCIE.)
+
+cflush.d.l1 x7 # CHECK: :[[@LINE]]:1: error: instruction requires the following: 'XSfcie' (SiFive Custom Instruction Extension SCIE.)
+
+cdiscard.d.l1 x0 # CHECK: :[[@LINE]]:1: error: instruction requires the following: 'XSfcie' (SiFive Custom Instruction Extension SCIE.)
+
+cdiscard.d.l1 x7 # CHECK: :[[@LINE]]:1: error: instruction requires the following: 'XSfcie' (SiFive Custom Instruction Extension SCIE.)
+
+cease x1 # CHECK: :[[@LINE]]:7: error: invalid operand for instruction
+
+cease 0x10 # CHECK: :[[@LINE]]:7: error: invalid operand for instruction
+
+cease # CHECK: :[[@LINE]]:1: error: instruction requires the following: 'XSfcie' (SiFive Custom Instruction Extension SCIE.)
Index: llvm/test/MC/RISCV/attribute-arch.s
===
--- llvm/test/MC/RISCV/attribute-arch.s
+++ llvm/test/MC/RISCV/attribute-arch.s
@@ -278,3 +278,6 @@
 
 .attribute arch, "rv32i_zvfbfwma0p6"
 # CHECK: .attribute 5, "rv32i2p1_f2p2_zicsr2p0_zve32f1p0_zve32x1p0_zvfbfwma0p6_zvl32b1p0"
+
+.attribute arch, "rv64i_xsfcie"
+# CHECK: attribute  5, "rv64i2p1_xsfcie1p0"
Index: llvm/lib/Target/RISCV/RISCVProcessors.td
===
--- llvm/lib/Target/RISCV/RISCVProcessors.td
+++ llvm/lib/Target/RISCV/RISCVProcessors.td
@@ -142,7 

[clang] 4c37d30 - [RISCV] Add support for custom instructions for Sifive S76.

2023-06-26 Thread Craig Topper via cfe-commits

Author: Garvit Gupta
Date: 2023-06-26T11:36:00-07:00
New Revision: 4c37d30e22ae655394c8b3a7e292c06d393b9b44

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

LOG: [RISCV] Add support for custom instructions for Sifive S76.

Support for below instruction is added
1. CFLUSH.D.L1
2. CDISCARD.D.L1
3. CEASE

Additionally, Zihintpause extension is added to sifive s76 for pause
instruction.

Spec - 
https://sifive.cdn.prismic.io/sifive/767804da-53b2-4893-97d5-b7c030ae0a94_s76mc_core_complex_manual_21G3.pdf

Reviewed By: craig.topper

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

Added: 
llvm/test/MC/RISCV/xsfcie-invalid.s
llvm/test/MC/RISCV/xsfcie-valid.s

Modified: 
clang/test/Driver/riscv-cpus.c
llvm/docs/RISCVUsage.rst
llvm/docs/ReleaseNotes.rst
llvm/lib/Support/RISCVISAInfo.cpp
llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
llvm/lib/Target/RISCV/RISCVFeatures.td
llvm/lib/Target/RISCV/RISCVInstrInfoXSf.td
llvm/lib/Target/RISCV/RISCVProcessors.td
llvm/test/MC/RISCV/attribute-arch.s

Removed: 




diff  --git a/clang/test/Driver/riscv-cpus.c b/clang/test/Driver/riscv-cpus.c
index a484b07ce330a..a56dbfc9896a2 100644
--- a/clang/test/Driver/riscv-cpus.c
+++ b/clang/test/Driver/riscv-cpus.c
@@ -112,7 +112,8 @@
 // MCPU-SIFIVE-S76: "-nostdsysteminc" "-target-cpu" "sifive-s76"
 // MCPU-SIFIVE-S76: "-target-feature" "+m" "-target-feature" "+a" 
"-target-feature" "+f" "-target-feature" "+d"
 // MCPU-SIFIVE-S76: "-target-feature" "+c"
-// MCPU-SIFIVE-S76: "-target-feature" "+zicsr" "-target-feature" "+zifencei"
+// MCPU-SIFIVE-S76: "-target-feature" "+zicsr" "-target-feature" "+zifencei" 
"-target-feature" "+zihintpause"
+// MCPU-SIFIVE-S76: "-target-feature" "+xsfcie"
 // MCPU-SIFIVE-S76: "-target-abi" "lp64d"
 
 // mcpu with default march

diff  --git a/llvm/docs/RISCVUsage.rst b/llvm/docs/RISCVUsage.rst
index 1bfa9678427c0..66c4aa61fe65b 100644
--- a/llvm/docs/RISCVUsage.rst
+++ b/llvm/docs/RISCVUsage.rst
@@ -273,3 +273,6 @@ The current vendor extensions supported are:
 
 ``XCVmac``
   LLVM implements `version 1.3.1 of the Core-V Multiply-Accumulate (MAC) 
custom instructions specification 
`_
 by Core-V.  All instructions are prefixed with `cv.mac.` as described in the 
specification. These instructions are only available for riscv32 at this time.
+
+``XSfcie``
+  LLVM implements `version 1.0.0 of the SiFive Custom Instruction Extension 
(CIE) Software Specification 
`_
 by SiFive.  All custom instruction are added as described in the 
specification, and the riscv-toolchain-convention document linked above. These 
instructions are only available for S76 processor at this time.

diff  --git a/llvm/docs/ReleaseNotes.rst b/llvm/docs/ReleaseNotes.rst
index 3e070b2ebdfa1..25e76db089d1a 100644
--- a/llvm/docs/ReleaseNotes.rst
+++ b/llvm/docs/ReleaseNotes.rst
@@ -210,6 +210,8 @@ Changes to the RISC-V Backend
   extension disassembler/assembler.
 * Added support for the vendor-defined Xsfvcp (SiFive VCIX) extension
   disassembler/assembler.
+* Added support for the vendor-defined Xsfcie (SiFive CIE) extension
+  disassembler/assembler.
 * Support for the now-ratified Zawrs extension is no longer experimental.
 * Adds support for the vendor-defined XTHeadCmo (cache management operations) 
extension.
 * Adds support for the vendor-defined XTHeadSync (multi-core synchronization 
instructions) extension.

diff  --git a/llvm/lib/Support/RISCVISAInfo.cpp 
b/llvm/lib/Support/RISCVISAInfo.cpp
index f9a38cf21efed..58a98bc497a89 100644
--- a/llvm/lib/Support/RISCVISAInfo.cpp
+++ b/llvm/lib/Support/RISCVISAInfo.cpp
@@ -66,6 +66,7 @@ static const RISCVSupportedExtension SupportedExtensions[] = {
 {"v", RISCVExtensionVersion{1, 0}},
 
 // vendor-defined ('X') extensions
+{"xsfcie", RISCVExtensionVersion{1, 0}},
 {"xsfvcp", RISCVExtensionVersion{1, 0}},
 {"xtheadba", RISCVExtensionVersion{1, 0}},
 {"xtheadbb", RISCVExtensionVersion{1, 0}},

diff  --git a/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp 
b/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
index cc1996a1c1702..e6ea6baa72ff4 100644
--- a/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
+++ b/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
@@ -558,6 +558,8 @@ DecodeStatus RISCVDisassembler::getInstruction(MCInst , 
uint64_t ,
   "XTHeadVdot custom opcode table");
 TRY_TO_DECODE_FEATURE(RISCV::FeatureVendorXSfvcp, DecoderTableXSfvcp32,
   "SiFive 

[PATCH] D153092: [Clang][CodeGen]`vtable`, `typeinfo` et al. are globals

2023-06-26 Thread Alex Voicu via Phabricator via cfe-commits
AlexVlx added a comment.

In D153092#4448514 , @yaxunl wrote:

> In D153092#4447445 , @AlexVlx wrote:
>
>> Fixed issue found via internal testing (thanks @yaxunl).
>
> Can we add a test to cover the regression found via internal testing? Thanks.

Done, I had forgotten about that.


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

https://reviews.llvm.org/D153092

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


[PATCH] D153092: [Clang][CodeGen]`vtable`, `typeinfo` et al. are globals

2023-06-26 Thread Alex Voicu via Phabricator via cfe-commits
AlexVlx updated this revision to Diff 534670.
AlexVlx added a comment.

Add missing test for `vtable` initializers on the `__device__` side.


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

https://reviews.llvm.org/D153092

Files:
  clang/lib/CodeGen/CGVTT.cpp
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/vtable-align-address-space.cpp
  clang/test/CodeGenCXX/vtable-assume-load-address-space.cpp
  clang/test/CodeGenCXX/vtable-consteval-address-space.cpp
  clang/test/CodeGenCXX/vtable-constexpr-address-space.cpp
  clang/test/CodeGenCXX/vtable-key-function-address-space.cpp
  clang/test/CodeGenCXX/vtable-layout-extreme-address-space.cpp
  clang/test/CodeGenCXX/vtable-linkage-address-space.cpp
  clang/test/CodeGenCXX/vtable-pointer-initialization-address-space.cpp
  clang/test/CodeGenCXX/vtt-address-space.cpp
  clang/test/CodeGenCXX/vtt-layout-address-space.cpp
  clang/test/Headers/hip-header.hip

Index: clang/test/Headers/hip-header.hip
===
--- clang/test/Headers/hip-header.hip
+++ clang/test/Headers/hip-header.hip
@@ -43,6 +43,22 @@
 
 // expected-no-diagnostics
 
+// Check handling of overriden, implicitly __host__ dtor (should emit as a
+// nullptr to global)
+
+struct vbase {
+virtual ~vbase();
+};
+
+template
+struct vderived : public vbase {
+~vderived();
+};
+
+template struct vderived;
+
+// CHECK: @_ZTV8vderivedIvE = weak_odr unnamed_addr addrspace(1) constant { [4 x ptr addrspace(1)] } zeroinitializer, comdat, align 8
+
 // Check support for pure and deleted virtual functions
 struct base {
   __host__
@@ -60,9 +76,8 @@
 __device__ void test_vf() {
 derived d;
 }
-// CHECK: @_ZTV7derived = linkonce_odr unnamed_addr addrspace(1) constant { [4 x ptr] } { [4 x ptr] [ptr null, ptr null, ptr @_ZN7derived2pvEv, ptr @__cxa_deleted_virtual] }, comdat, align 8
-// CHECK: @_ZTV4base = linkonce_odr unnamed_addr addrspace(1) constant { [4 x ptr] } { [4 x ptr] [ptr null, ptr null, ptr @__cxa_pure_virtual, ptr @__cxa_deleted_virtual] }, comdat, align 8
-
+// CHECK: @_ZTV7derived = linkonce_odr unnamed_addr addrspace(1) constant { [4 x ptr addrspace(1)] } { [4 x ptr addrspace(1)] [ptr addrspace(1) null, ptr addrspace(1) null, ptr addrspace(1) addrspacecast (ptr @_ZN7derived2pvEv to ptr addrspace(1)), ptr addrspace(1) addrspacecast (ptr @__cxa_deleted_virtual to ptr addrspace(1))] }, comdat, align 8
+// CHECK: @_ZTV4base = linkonce_odr unnamed_addr addrspace(1) constant { [4 x ptr addrspace(1)] } { [4 x ptr addrspace(1)] [ptr addrspace(1) null, ptr addrspace(1) null, ptr addrspace(1) addrspacecast (ptr @__cxa_pure_virtual to ptr addrspace(1)), ptr addrspace(1) addrspacecast (ptr @__cxa_deleted_virtual to ptr addrspace(1))] }, comdat, align 8
 // CHECK: define{{.*}}void @__cxa_pure_virtual()
 // CHECK: define{{.*}}void @__cxa_deleted_virtual()
 
Index: clang/test/CodeGenCXX/vtt-layout-address-space.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/vtt-layout-address-space.cpp
@@ -0,0 +1,89 @@
+// RUN: %clang_cc1 %s -triple=amdgcn-amd-amdhsa -std=c++11 -emit-llvm -o - | FileCheck %s
+
+// Test1::B should just have a single entry in its VTT, which points to the vtable.
+namespace Test1 {
+struct A { };
+
+struct B : virtual A {
+  virtual void f();
+};
+
+void B::f() { }
+}
+
+// Check that we don't add a secondary virtual pointer for Test2::A, since Test2::A doesn't have any virtual member functions or bases.
+namespace Test2 {
+  struct A { };
+
+  struct B : A { virtual void f(); };
+  struct C : virtual B { };
+
+  C c;
+}
+
+// This is the sample from the C++ Itanium ABI, p2.6.2.
+namespace Test3 {
+  class A1 { int i; };
+  class A2 { int i; virtual void f(); };
+  class V1 : public A1, public A2 { int i; };
+  class B1 { int i; };
+  class B2 { int i; };
+  class V2 : public B1, public B2, public virtual V1 { int i; };
+  class V3 {virtual void g(); };
+  class C1 : public virtual V1 { int i; };
+  class C2 : public virtual V3, virtual V2 { int i; };
+  class X1 { int i; };
+  class C3 : public X1 { int i; };
+  class D : public C1, public C2, public C3 { int i;  };
+
+  D d;
+}
+
+// This is the sample from the C++ Itanium ABI, p2.6.2, with the change suggested
+// (making A2 a virtual base of V1)
+namespace Test4 {
+  class A1 { int i; };
+  class A2 { int i; virtual void f(); };
+  class V1 : public A1, public virtual A2 { int i; };
+  class B1 { int i; };
+  class B2 { int i; };
+  class V2 : public B1, public B2, public virtual V1 { int i; };
+  class V3 {virtual void g(); };
+  class C1 : public virtual V1 { int i; };
+  class C2 : public virtual V3, virtual V2 { int i; };
+  class X1 { int i; };
+  class C3 : public X1 { int i; };
+  class D : public C1, public C2, public C3 { int i;  };
+
+  D d;
+}
+
+namespace Test5 {
+  struct A {
+virtual void f() = 0;

[PATCH] D153691: [Driver][ARM] Warn about -mabi= for assembler input

2023-06-26 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 534668.
MaskRay marked an inline comment as done.
MaskRay added a comment.

Thanks for the comments. Improved comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153691

Files:
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/test/Driver/arm-abi.c


Index: clang/test/Driver/arm-abi.c
===
--- clang/test/Driver/arm-abi.c
+++ clang/test/Driver/arm-abi.c
@@ -61,3 +61,9 @@
 // CHECK-APCS-GNU: "-target-abi" "apcs-gnu"
 // CHECK-AAPCS: "-target-abi" "aapcs"
 // CHECK-AAPCS-LINUX: "-target-abi" "aapcs-linux"
+
+// RUN: %clang --target=arm---gnueabi -mabi=aapcs -x assembler %s -### -o 
/dev/null 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-ASM %s
+
+/// The combination -x assember & -mabi is not implemented, but for GCC 
compatibility we accept with a warning.
+// CHECK-ASM: warning: argument unused during compilation: '-mabi={{.*}}'
Index: clang/lib/Driver/ToolChains/Arch/ARM.cpp
===
--- clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -524,6 +524,12 @@
 }
   }
 }
+
+// The integrated assembler doesn't implement e_flags setting behavior in
+// GNU assembler -meabi=gnu (gcc -mabi={apcs-gnu,atpcs}). For compatibility
+// we accept but warn.
+if (Arg *A = Args.getLastArgNoClaim(options::OPT_mabi_EQ))
+  A->ignoreTargetSpecific();
   }
 
   if (getReadTPMode(D, Args, Triple, ForAS) == ReadTPMode::TPIDRURW)


Index: clang/test/Driver/arm-abi.c
===
--- clang/test/Driver/arm-abi.c
+++ clang/test/Driver/arm-abi.c
@@ -61,3 +61,9 @@
 // CHECK-APCS-GNU: "-target-abi" "apcs-gnu"
 // CHECK-AAPCS: "-target-abi" "aapcs"
 // CHECK-AAPCS-LINUX: "-target-abi" "aapcs-linux"
+
+// RUN: %clang --target=arm---gnueabi -mabi=aapcs -x assembler %s -### -o /dev/null 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-ASM %s
+
+/// The combination -x assember & -mabi is not implemented, but for GCC compatibility we accept with a warning.
+// CHECK-ASM: warning: argument unused during compilation: '-mabi={{.*}}'
Index: clang/lib/Driver/ToolChains/Arch/ARM.cpp
===
--- clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -524,6 +524,12 @@
 }
   }
 }
+
+// The integrated assembler doesn't implement e_flags setting behavior in
+// GNU assembler -meabi=gnu (gcc -mabi={apcs-gnu,atpcs}). For compatibility
+// we accept but warn.
+if (Arg *A = Args.getLastArgNoClaim(options::OPT_mabi_EQ))
+  A->ignoreTargetSpecific();
   }
 
   if (getReadTPMode(D, Args, Triple, ForAS) == ReadTPMode::TPIDRURW)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] 9aaf3cf - [clang-tidy][NFC] Repharse a lite bit documentation for misc-header-include-cycle check

2023-06-26 Thread Piotr Zegar via cfe-commits

Author: Piotr Zegar
Date: 2023-06-26T18:20:39Z
New Revision: 9aaf3cf9bfe803d0a32709aa160951a6675b5926

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

LOG: [clang-tidy][NFC] Repharse a lite bit documentation for 
misc-header-include-cycle check

Change documentation, to avoid some duplication,
and make it sound beter.

Added: 


Modified: 
clang-tools-extra/docs/clang-tidy/checks/misc/header-include-cycle.rst

Removed: 




diff  --git 
a/clang-tools-extra/docs/clang-tidy/checks/misc/header-include-cycle.rst 
b/clang-tools-extra/docs/clang-tidy/checks/misc/header-include-cycle.rst
index 622dc784f3c61..de08f0d0bea07 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/misc/header-include-cycle.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/misc/header-include-cycle.rst
@@ -21,42 +21,44 @@ Check detects cyclic ``#include`` dependencies between 
user-defined headers.
 
 // Include chain: A->B->C->A
 
-Header files are a crucial part of many C++ programs, as they provide a way to
-organize declarations and definitions that are shared across multiple source
-files. However, header files can also create problems when they become 
entangled
+Header files are a crucial part of many C++ programs as they provide a way to
+organize declarations and definitions shared across multiple source files.
+However, header files can also create problems when they become entangled
 in complex dependency cycles. Such cycles can cause issues with compilation
 times, unnecessary rebuilds, and make it harder to understand the overall
 structure of the code.
 
-To address these issues, this check has been developed. This check is designed
-to detect cyclic dependencies between header files, also known as
-"include cycles". An include cycle occurs when a header file `A` includes a
-header file `B`, and header file `B` (or any later included header file in the
-chain) includes back header file `A`, leading to a circular dependency cycle.
-
-This check operates at the preprocessor level and analyzes user-defined headers
-and their dependencies. It focuses specifically on detecting include cycles,
-and ignores other types or function dependencies. This allows it to provide a
-specialized analysis that is focused on identifying and preventing issues
-related to header file organization.
-
-The benefits of using this check are numerous. By detecting include cycles 
early
-in the development process, developers can identify and resolve these issues
-before they become more 
diff icult and time-consuming to fix. This can lead to
-faster compile times, improved code quality, and a more maintainable codebase
-overall. Additionally, by ensuring that header files are organized in a way 
that
-avoids cyclic dependencies, developers can make their code easier to understand
-and modify over time.
-
-It's worth noting that this tool only analyzes user-defined headers and their
-dependencies, excluding system includes such as standard library headers and
-third-party library headers. System includes are usually well-designed and free
-of include cycles, and ignoring them helps to focus on potential issues within
-the project's own codebase. This limitation doesn't diminish the tool's ability
-to detect ``#include`` cycles within the analyzed code. As with any tool,
-developers should use their judgment when evaluating the warnings produced by
-the check and be prepared to make exceptions or modifications to their code as
-needed.
+To address these issues, a check has been developed to detect cyclic
+dependencies between header files, also known as "include cycles".
+An include cycle occurs when a header file `A` includes header file `B`,
+and `B` (or any subsequent included header file) includes back header file `A`,
+resulting in a circular dependency cycle.
+
+This check operates at the preprocessor level and specifically analyzes
+user-defined headers and their dependencies. It focuses solely on detecting
+include cycles while disregarding other types or function dependencies.
+This specialized analysis helps identify and prevent issues related to header
+file organization.
+
+By detecting include cycles early in the development process, developers can
+identify and resolve these issues before they become more 
diff icult and
+time-consuming to fix. This can lead to faster compile times, improved code
+quality, and a more maintainable codebase overall. Additionally, by ensuring
+that header files are organized in a way that avoids cyclic dependencies,
+developers can make their code easier to understand and modify over time.
+
+It's worth noting that only user-defined headers their dependencies are 
analyzed,
+System includes such as standard library headers and third-party library 
headers
+are excluded. 

[PATCH] D153417: New CharSetConverter wrapper class for ConverterEBCDIC

2023-06-26 Thread Abhina Sree via Phabricator via cfe-commits
abhina.sreeskantharajan added a comment.

In D153417#4438764 , @efriedma wrote:

> As I mentioned at 
> https://discourse.llvm.org/t/rfc-enabling-fexec-charset-support-to-llvm-and-clang-reposting/71512
>  , I think SimplifyLibCalls needs to be aware of encodings.  To make that 
> work, this probably needs to be somewhere in llvm/ , not clang/ .

There was previous resistance to putting this in LLVM 
https://discourse.llvm.org/t/rfc-adding-a-charset-converter-to-the-llvm-support-library/69795/17,
 I'm not sure if that sentiment has changed. I think it would be best to have 
some sort of implementation plan to tackle the SimplifyLibCalls issue before 
shifting this code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153417

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


[PATCH] D153695: [clang][Interp] Fix passing parameters of composite type

2023-06-26 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeEmitter.h:71
 
-  /// Parameter indices.
-  llvm::DenseMap Params;
+  /// Parameter indices. >
+  llvm::DenseMap Params;

I don't understand the additional comment,



Comment at: clang/lib/AST/Interp/Context.cpp:30
 bool Context::isPotentialConstantExpr(State , const FunctionDecl *FD) {
+  llvm::errs() << __PRETTY_FUNCTION__ << "\n";
+  FD->dump();

left over debugging code?



Comment at: clang/lib/AST/Interp/Context.cpp:56
 bool Context::evaluateAsRValue(State , const Expr *E, APValue ) {
+  llvm::errs() << __PRETTY_FUNCTION__ << "\n";
+  E->dump();

Left over debugging code?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153695

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


[PATCH] D152829: clang: Add start of header test for __clang_hip_libdevice_declares

2023-06-26 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm added a comment.

ping


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

https://reviews.llvm.org/D152829

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


[PATCH] D153690: [clang][Sema] Remove dead diagnostic for loss of __unaligned qualifier

2023-06-26 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment.

Can we get a snippet of code to demonstrate what isn't being diagnosed anymore, 
please?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153690

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


[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

2023-06-26 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:18217
 
+  NewFD->setIsPlaceholderVar(LangOpts.CPlusPlus && II && II->isPlaceholder());
   if (PrevDecl && !isa(PrevDecl)) {

cor3ntin wrote:
> shafik wrote:
> > Why can't we fold this into `FieldDecl::Create`? This comment applies in 
> > some other places as well.
> By adding a parameter to FieldDecl::Create? We could, I'm not sure it would 
> necessarily be cleaner. Let me know what you prefer.
It seems like having the code consolidated make for better maintainability and 
figuring this out for future folks but I am willing to be convinced otherwise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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


[PATCH] D139837: [Clang] Implements CTAD for aggregates P1816R0 and P2082R1

2023-06-26 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

Thanks for this!  I have no concerns from what I can see, but I see the others 
are doing a very thorough review, so I'll let them do the approvals.




Comment at: clang/include/clang/AST/DeclCXX.h:1911
   setRangeEnd(EndLocation);
-setIsCopyDeductionCandidate(false);
+setDeductionCandidateKind(DeductionCandidateKind::Normal);
   }

ychen wrote:
> cor3ntin wrote:
> > I'm wondering if the constructor should take a `DeductionCandidateKind` 
> > defaulted to normal here. All the places where it's set seem to be 
> > immediately after construction.
> That's true indeed. The awkward aspect is that the 
> `CXXDeductionGuideDecl::Create` call is far from `setDeductionCandidateKind`; 
> making `CXXDeductionGuideDecl::Create` takes a `DeductionCandidateKind` would 
> several other less related functions takes `DeductionCandidateKind` also.
I'm in favor of having this be a part of CXXDeductionGuideDecl::Create and the 
ctor as a parameter instead as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139837

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


[PATCH] D153493: [dataflow] avoid more accidental copies of Environment

2023-06-26 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment.

Just FYI (since you mentioned it in your description): LatticeEffect is 
vestigial and should be removed. It's now only used (properly) for widening.

Regarding the design.  This looks like an optimal solution in terms of copying 
but introduces lifetime risks and complexity that I'm uncomfortable with. 
There's a lot of flexibility here and I wonder if we can reduce that without 
(substantially?) impacting the amount of copying. Specifically, I wonder if we 
can have the builder *always* own an environment, which simplifies the handling 
of `CurrentState` and, generally, the number of cases to consider. It costs 
more in principle, but maybe negligible in practice?




Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:613
 mergeDistinctValues(Loc->getType(), *Val, *this, *It->second, 
Other,
 JoinedEnv, Model)) {
   JoinedEnv.LocToVal.insert({Loc, MergedVal});

nit: i think you can now drop the braces.



Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:208
+if (!CurrentState) {
+  CurrentState = 
+} else if (!OwnedState) {

Is there some invariant that guarantees the safety here? Because `const &` can 
bind to a temporary, so this feels like a risky API.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153493

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


[PATCH] D153179: [clang codegen] Fix ABI for HVA returns on AArch64 MSVC.

2023-06-26 Thread Eli Friedman 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 rGa1540e4852a9: [clang codegen] Fix ABI for HVA returns on 
AArch64 MSVC. (authored by efriedma).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153179

Files:
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/test/CodeGenCXX/homogeneous-aggregates.cpp


Index: clang/test/CodeGenCXX/homogeneous-aggregates.cpp
===
--- clang/test/CodeGenCXX/homogeneous-aggregates.cpp
+++ clang/test/CodeGenCXX/homogeneous-aggregates.cpp
@@ -285,3 +285,20 @@
 double foo(HFA v) { return v.x + v.y; }
 // WOA64: define dso_local noundef double 
@"?foo@non_empty_field@@YANUHFA@1@@Z"([4 x double] %{{.*}})
 }
+
+namespace pr62223 {
+// HVAs don't follow the normal rules for return values. That means they can
+// have base classes, user-defined ctors, and protected/private members.
+// (The same restrictions that apply to HVA arguments still apply.)
+typedef double V __attribute((ext_vector_type(2)));
+struct base { V v; };
+struct test : base { test(double); protected: V v2;};
+test f(test *x) { return *x; }
+// WOA64: define dso_local %"struct.pr62223::test" 
@"?f@pr62223@@YA?AUtest@1@PEAU21@@Z"(ptr noundef %{{.*}})
+
+// The above rule only apples to HVAs, not HFAs.
+struct base2 { double v; };
+struct test2 : base2 { test2(double); protected: double v2;};
+test2 f(test2 *x) { return *x; }
+// WOA64: define dso_local void @"?f@pr62223@@YA?AUtest2@1@PEAU21@@Z"(ptr 
inreg noalias sret(%"struct.pr62223::test2") align 8 %{{.*}}, ptr noundef 
%{{.*}})
+}
Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -13,6 +13,7 @@
 //
 
//===--===//
 
+#include "ABIInfo.h"
 #include "CGCXXABI.h"
 #include "CGCleanup.h"
 #include "CGVTables.h"
@@ -1099,7 +1100,19 @@
   return isDeletingDtor(GD);
 }
 
-static bool isTrivialForMSVC(const CXXRecordDecl *RD) {
+static bool isTrivialForMSVC(const CXXRecordDecl *RD, QualType Ty,
+ CodeGenModule ) {
+  // On AArch64, HVAs that can be passed in registers can also be returned
+  // in registers. (Note this is using the MSVC definition of an HVA; see
+  // isPermittedToBeHomogeneousAggregate().)
+  const Type *Base = nullptr;
+  uint64_t NumElts = 0;
+  if (CGM.getTarget().getTriple().isAArch64() &&
+  CGM.getTypes().getABIInfo().isHomogeneousAggregate(Ty, Base, NumElts) &&
+  isa(Base)) {
+return true;
+  }
+
   // We use the C++14 definition of an aggregate, so we also
   // check for:
   //   No private or protected non static data members.
@@ -1128,7 +1141,8 @@
   if (!RD)
 return false;
 
-  bool isTrivialForABI = RD->canPassInRegisters() && isTrivialForMSVC(RD);
+  bool isTrivialForABI = RD->canPassInRegisters() &&
+ isTrivialForMSVC(RD, FI.getReturnType(), CGM);
 
   // MSVC always returns structs indirectly from C++ instance methods.
   bool isIndirectReturn = !isTrivialForABI || FI.isInstanceMethod();


Index: clang/test/CodeGenCXX/homogeneous-aggregates.cpp
===
--- clang/test/CodeGenCXX/homogeneous-aggregates.cpp
+++ clang/test/CodeGenCXX/homogeneous-aggregates.cpp
@@ -285,3 +285,20 @@
 double foo(HFA v) { return v.x + v.y; }
 // WOA64: define dso_local noundef double @"?foo@non_empty_field@@YANUHFA@1@@Z"([4 x double] %{{.*}})
 }
+
+namespace pr62223 {
+// HVAs don't follow the normal rules for return values. That means they can
+// have base classes, user-defined ctors, and protected/private members.
+// (The same restrictions that apply to HVA arguments still apply.)
+typedef double V __attribute((ext_vector_type(2)));
+struct base { V v; };
+struct test : base { test(double); protected: V v2;};
+test f(test *x) { return *x; }
+// WOA64: define dso_local %"struct.pr62223::test" @"?f@pr62223@@YA?AUtest@1@PEAU21@@Z"(ptr noundef %{{.*}})
+
+// The above rule only apples to HVAs, not HFAs.
+struct base2 { double v; };
+struct test2 : base2 { test2(double); protected: double v2;};
+test2 f(test2 *x) { return *x; }
+// WOA64: define dso_local void @"?f@pr62223@@YA?AUtest2@1@PEAU21@@Z"(ptr inreg noalias sret(%"struct.pr62223::test2") align 8 %{{.*}}, ptr noundef %{{.*}})
+}
Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -13,6 +13,7 @@
 //
 //===--===//
 
+#include "ABIInfo.h"
 #include "CGCXXABI.h"
 #include "CGCleanup.h"
 #include "CGVTables.h"
@@ 

[clang] a1540e4 - [clang codegen] Fix ABI for HVA returns on AArch64 MSVC.

2023-06-26 Thread Eli Friedman via cfe-commits

Author: Eli Friedman
Date: 2023-06-26T10:45:41-07:00
New Revision: a1540e4852a90e7f72c996a3832edfd20968bd9e

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

LOG: [clang codegen] Fix ABI for HVA returns on AArch64 MSVC.

MSVC normally has a bunch of restrictions on returning values directly
which don't apply to passing values directly.  (This roughly corresponds
to the definition of a C++14 aggregate.)  However, these restrictions
don't apply to HVAs; make sure we check for that.

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

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

Added: 


Modified: 
clang/lib/CodeGen/MicrosoftCXXABI.cpp
clang/test/CodeGenCXX/homogeneous-aggregates.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/MicrosoftCXXABI.cpp 
b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
index 19b95e61e2283..5ae455dd29f75 100644
--- a/clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -13,6 +13,7 @@
 //
 
//===--===//
 
+#include "ABIInfo.h"
 #include "CGCXXABI.h"
 #include "CGCleanup.h"
 #include "CGVTables.h"
@@ -1099,7 +1100,19 @@ bool MicrosoftCXXABI::hasMostDerivedReturn(GlobalDecl 
GD) const {
   return isDeletingDtor(GD);
 }
 
-static bool isTrivialForMSVC(const CXXRecordDecl *RD) {
+static bool isTrivialForMSVC(const CXXRecordDecl *RD, QualType Ty,
+ CodeGenModule ) {
+  // On AArch64, HVAs that can be passed in registers can also be returned
+  // in registers. (Note this is using the MSVC definition of an HVA; see
+  // isPermittedToBeHomogeneousAggregate().)
+  const Type *Base = nullptr;
+  uint64_t NumElts = 0;
+  if (CGM.getTarget().getTriple().isAArch64() &&
+  CGM.getTypes().getABIInfo().isHomogeneousAggregate(Ty, Base, NumElts) &&
+  isa(Base)) {
+return true;
+  }
+
   // We use the C++14 definition of an aggregate, so we also
   // check for:
   //   No private or protected non static data members.
@@ -1128,7 +1141,8 @@ bool MicrosoftCXXABI::classifyReturnType(CGFunctionInfo 
) const {
   if (!RD)
 return false;
 
-  bool isTrivialForABI = RD->canPassInRegisters() && isTrivialForMSVC(RD);
+  bool isTrivialForABI = RD->canPassInRegisters() &&
+ isTrivialForMSVC(RD, FI.getReturnType(), CGM);
 
   // MSVC always returns structs indirectly from C++ instance methods.
   bool isIndirectReturn = !isTrivialForABI || FI.isInstanceMethod();

diff  --git a/clang/test/CodeGenCXX/homogeneous-aggregates.cpp 
b/clang/test/CodeGenCXX/homogeneous-aggregates.cpp
index 972b0031a9e38..2fcfcf4390945 100644
--- a/clang/test/CodeGenCXX/homogeneous-aggregates.cpp
+++ b/clang/test/CodeGenCXX/homogeneous-aggregates.cpp
@@ -285,3 +285,20 @@ struct HFA {
 double foo(HFA v) { return v.x + v.y; }
 // WOA64: define dso_local noundef double 
@"?foo@non_empty_field@@YANUHFA@1@@Z"([4 x double] %{{.*}})
 }
+
+namespace pr62223 {
+// HVAs don't follow the normal rules for return values. That means they can
+// have base classes, user-defined ctors, and protected/private members.
+// (The same restrictions that apply to HVA arguments still apply.)
+typedef double V __attribute((ext_vector_type(2)));
+struct base { V v; };
+struct test : base { test(double); protected: V v2;};
+test f(test *x) { return *x; }
+// WOA64: define dso_local %"struct.pr62223::test" 
@"?f@pr62223@@YA?AUtest@1@PEAU21@@Z"(ptr noundef %{{.*}})
+
+// The above rule only apples to HVAs, not HFAs.
+struct base2 { double v; };
+struct test2 : base2 { test2(double); protected: double v2;};
+test2 f(test2 *x) { return *x; }
+// WOA64: define dso_local void @"?f@pr62223@@YA?AUtest2@1@PEAU21@@Z"(ptr 
inreg noalias sret(%"struct.pr62223::test2") align 8 %{{.*}}, ptr noundef 
%{{.*}})
+}



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


[PATCH] D153653: [clang][Interp] Make CXXTemporaryObjectExprs leave a value behind

2023-06-26 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1022
+
+if (DiscardResult)
+  return this->emitPopPtr(E);

Could you just pass `DiscardResult` to `visitLocalInitializer`


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

https://reviews.llvm.org/D153653

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


[PATCH] D105759: Implement P2361 Unevaluated string literals

2023-06-26 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 534640.
cor3ntin marked 2 inline comments as done.
cor3ntin added a comment.

Address some of Aaron's comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105759

Files:
  clang-tools-extra/test/clang-tidy/checkers/modernize/unary-static-assert.cpp
  clang/include/clang/AST/Expr.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Lex/LiteralSupport.h
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Expr.cpp
  clang/lib/Lex/LiteralSupport.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Lex/Pragma.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/test/CXX/dcl.dcl/dcl.link/p2.cpp
  clang/test/CXX/dcl.dcl/p4-0x.cpp
  clang/test/FixIt/fixit-static-assert.cpp
  clang/test/Parser/cxx0x-attributes.cpp
  clang/test/SemaCXX/static-assert.cpp
  clang/test/SemaCXX/warn-thread-safety-parsing.cpp

Index: clang/test/SemaCXX/warn-thread-safety-parsing.cpp
===
--- clang/test/SemaCXX/warn-thread-safety-parsing.cpp
+++ clang/test/SemaCXX/warn-thread-safety-parsing.cpp
@@ -309,7 +309,7 @@
 
 int gb_var_arg GUARDED_BY(mu1);
 
-int gb_non_ascii GUARDED_BY(L"wide"); // expected-warning {{ignoring 'guarded_by' attribute because its argument is invalid}}
+int gb_non_ascii GUARDED_BY(L"wide"); // expected-error {{an unevaluated string literal cannot have an encoding prefix}}
 
 int gb_var_args __attribute__((guarded_by(mu1, mu2))); // \
   // expected-error {{'guarded_by' attribute takes one argument}}
Index: clang/test/SemaCXX/static-assert.cpp
===
--- clang/test/SemaCXX/static-assert.cpp
+++ clang/test/SemaCXX/static-assert.cpp
@@ -29,12 +29,21 @@
 S s1; // expected-note {{in instantiation of template class 'S' requested here}}
 S s2;
 
-static_assert(false, L"\x"); // expected-error {{static assertion failed: L"\x"}}
-static_assert(false, u"\U000317FF"); // expected-error {{static assertion failed: u"\U000317FF"}}
-
-static_assert(false, u8"Ω"); // expected-error {{static assertion failed: u8"\316\251"}}
-static_assert(false, L"\u1234"); // expected-error {{static assertion failed: L"\x1234"}}
-static_assert(false, L"\x1ff" "0\x123" "fx\xf" "goop"); // expected-error {{static assertion failed: L"\x1FF""0\x123""fx\xFgoop"}}
+static_assert(false, L"\x"); // expected-error {{an unevaluated string literal cannot have an encoding prefix}} \
+ // expected-error {{invalid escape sequence '\x' in an unevaluated string literal}}
+static_assert(false, u"\U000317FF"); // expected-error {{an unevaluated string literal cannot have an encoding prefix}}
+// FIXME: render this as u8"\u03A9"
+static_assert(false, u8"Ω"); // expected-error {{an unevaluated string literal cannot have an encoding prefix}}
+static_assert(false, L"\u1234"); // expected-error {{an unevaluated string literal cannot have an encoding prefix}}
+static_assert(false, L"\x1ff"// expected-error {{an unevaluated string literal cannot have an encoding prefix}} \
+ // expected-error {{invalid escape sequence '\x1ff' in an unevaluated string literal}}
+ "0\x123"// expected-error {{invalid escape sequence '\x123' in an unevaluated string literal}}
+ "fx\xf" // expected-error {{invalid escape sequence '\xf' in an unevaluated string literal}}
+ "goop");
+
+static_assert(false, "\'\"\?\\\a\b\f\n\r\t\v"); // expected-error {{'"?\}}
+static_assert(true, "\xFF"); // expected-error {{invalid escape sequence '\xFF' in an unevaluated string literal}}
+static_assert(true, "\123"); // expected-error {{invalid escape sequence '\123' in an unevaluated string literal}}
 
 static_assert(false, R"(a
 \tb
Index: clang/test/Parser/cxx0x-attributes.cpp
===
--- clang/test/Parser/cxx0x-attributes.cpp
+++ clang/test/Parser/cxx0x-attributes.cpp
@@ -445,3 +445,8 @@
   ) {
   }
 };
+
+namespace P2361 {
+[[deprecated(L"abc")]] void a(); // expected-error{{an unevaluated string literal cannot have an encoding prefix}}
+[[nodiscard("\123")]] int b(); // expected-error{{invalid escape sequence '\123' in an unevaluated string literal}}
+}
Index: clang/test/FixIt/fixit-static-assert.cpp
===
--- clang/test/FixIt/fixit-static-assert.cpp
+++ clang/test/FixIt/fixit-static-assert.cpp

[PATCH] D139837: [Clang] Implements CTAD for aggregates P1816R0 and P2082R1

2023-06-26 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment.

In D139837#4449536 , @ychen wrote:

> In D139837#4449327 , @cor3ntin 
> wrote:
>
>> @ychen You probably want to update the description, i think it's updated 
>> (you implemented ctad for parenthesized ctrs)
>
> Thanks for the remainder, Corentin. Hopefully, this will be ready to be 
> landed soon.

I hope so too. I pinged @aaron.ballman :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139837

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


[PATCH] D153649: [clang][Interp] Fix return statements with expresssion in void functions

2023-06-26 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeStmtGen.cpp:312
   return this->emitRet(*ReturnType, RS);
+} else if (RE->getType()->isVoidType()) {
+  if (!this->visit(RE))

You could also guard the cleanup and `emitRet` above w/ this check, so as to 
avoid code repetition. 


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

https://reviews.llvm.org/D153649

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


[PATCH] D153293: [clang][WebAssembly] support wasm32-wasi shared libraries

2023-06-26 Thread Sam Clegg 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 rG55e199a2c9f4: [clang][WebAssembly] Support wasm32-wasi 
shared libraries (authored by dicej, committed by sbc100).

Changed prior to commit:
  https://reviews.llvm.org/D153293?vs=533375=534632#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153293

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Driver/ToolChains/WebAssembly.cpp
  clang/test/Driver/wasm-toolchain.c
  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
@@ -99,13 +99,6 @@
 return Reloc::Static;
   }
 
-  if (!TT.isOSEmscripten()) {
-// Relocation modes other than static are currently implemented in a way
-// that only works for Emscripten, so disable them if we aren't targeting
-// Emscripten.
-return Reloc::Static;
-  }
-
   return *RM;
 }
 
Index: clang/test/Driver/wasm-toolchain.c
===
--- clang/test/Driver/wasm-toolchain.c
+++ clang/test/Driver/wasm-toolchain.c
@@ -33,6 +33,20 @@
 // LINK_KNOWN: "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
+// -shared should be passed through to `wasm-ld` and not include crt1.o with a known OS.
+
+// RUN: %clang -### -shared --target=wasm32-wasi --sysroot=/foo %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_KNOWN_SHARED %s
+// LINK_KNOWN_SHARED: "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
+// LINK_KNOWN_SHARED: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "-shared" "[[temp]]" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
+
+// -shared should be passed through to `wasm-ld` and not include crt1.o with an unknown OS.
+
+// RUN: %clang -### -shared --target=wasm32-unknown-unknown --sysroot=/foo %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_UNKNOWN_SHARED %s
+// LINK_UNKNOWN_SHARED: "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
+// LINK_UNKNOWN_SHARED: wasm-ld{{.*}}" "-shared" "[[temp]]" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
+
 // A basic C link command-line with optimization with known OS.
 
 // RUN: %clang -### -O2 --target=wasm32-wasi --sysroot=/foo %s 2>&1 \
@@ -46,6 +60,18 @@
 // RUN:   | FileCheck -check-prefix=COMPILE %s
 // COMPILE: "-cc1" {{.*}} "-internal-isystem" "/foo/include/wasm32-wasi" "-internal-isystem" "/foo/include"
 
+// -fPIC should work on a known OS
+
+// RUN: %clang -### -fPIC --target=wasm32-wasi --sysroot=/foo %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=COMPILE_KNOWN_PIC %s
+// COMPILE_KNOWN_PIC: "-cc1" {{.*}} "-mrelocation-model" "pic" "-pic-level" "2" {{.*}} "-internal-isystem" "/foo/include/wasm32-wasi" "-internal-isystem" "/foo/include"
+
+// -fPIC should work on an unknown OS
+
+// RUN: %clang -### -fPIC --target=wasm32-unknown-unknown --sysroot=/foo %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=COMPILE_UNKNOWN_PIC %s
+// COMPILE_UNKNOWN_PIC: "-cc1" {{.*}} "-mrelocation-model" "pic" "-pic-level" "2"
+
 // Thread-related command line tests.
 
 // '-pthread' sets +atomics, +bulk-memory, +mutable-globals, +sign-ext, and --shared-memory
Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -101,13 +101,16 @@
   << CM << A->getOption().getName();
 }
   }
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles))
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles, options::OPT_shared))
 CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath(Crt1)));
   if (Entry) {
 CmdArgs.push_back(Args.MakeArgString("--entry"));
 CmdArgs.push_back(Args.MakeArgString(Entry));
   }
 
+  if (Args.hasArg(options::OPT_shared))
+CmdArgs.push_back(Args.MakeArgString("-shared"));
+
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -727,6 +727,12 @@
 
 WebAssembly Support
 ^^^
+- Shared library support (and PIC code generation) for WebAssembly is no longer
+  limited to the Emscripten target OS and now works with other targets such as
+  wasm32-wasi.  Note that the `format
+  `_
+  is not yet stable and may change 

[clang] 55e199a - [clang][WebAssembly] Support wasm32-wasi shared libraries

2023-06-26 Thread Sam Clegg via cfe-commits

Author: Joel Dice
Date: 2023-06-26T10:31:40-07:00
New Revision: 55e199a2c9f4b218499733d60129deffa0a025fe

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

LOG: [clang][WebAssembly] Support wasm32-wasi shared libraries

This adds support for Emscripten-style shared libraries [1] to
non-emscripten targets, such as `wasm32-wasi`.  Previously, only static
linking was supported, and the `-shared` and `-fPIC` flags were simply
ignored.  Now both flags are honored.

Since WASI runtimes do not necessarily include JavaScript support, we
cannot rely on the JS-based Emscripten linker to link shared libraries.
Instead, we link them using the Component Model proposal [2].

We have prototyped shared library support in `wasi-sdk` [3] and put
together a demo [4] which uses a patched version of `wit-component` [5]
to link libraries using the Component Model.  We plan to submit the
required changes upstream to the respective repositories in the next
week or two.

[1] https://github.com/WebAssembly/tool-conventions/blob/main/DynamicLinking.md
[2] 
https://github.com/WebAssembly/component-model/blob/main/design/mvp/examples/SharedEverythingDynamicLinking.md
[3] https://github.com/dicej/wasi-sdk/tree/dynamic-linking
[4] https://github.com/dicej/component-linking-demo
[5] 
https://github.com/bytecodealliance/wasm-tools/tree/main/crates/wit-component

Signed-off-by: Joel Dice 

Reviewed By: sbc100

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Driver/ToolChains/WebAssembly.cpp
clang/test/Driver/wasm-toolchain.c
llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 055aa9008b03b..c022e9ffb6604 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -727,6 +727,12 @@ AIX Support
 
 WebAssembly Support
 ^^^
+- Shared library support (and PIC code generation) for WebAssembly is no longer
+  limited to the Emscripten target OS and now works with other targets such as
+  wasm32-wasi.  Note that the `format
+  
`_
+  is not yet stable and may change between LLVM versions.  Also, WASI does not
+  yet have facilities to load dynamic libraries.
 
 AVR Support
 ^^^

diff  --git a/clang/lib/Driver/ToolChains/WebAssembly.cpp 
b/clang/lib/Driver/ToolChains/WebAssembly.cpp
index 016b70b1c2ede..fb9c8e4910e8b 100644
--- a/clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ b/clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -101,13 +101,16 @@ void wasm::Linker::ConstructJob(Compilation , const 
JobAction ,
   << CM << A->getOption().getName();
 }
   }
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles))
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles, 
options::OPT_shared))
 CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath(Crt1)));
   if (Entry) {
 CmdArgs.push_back(Args.MakeArgString("--entry"));
 CmdArgs.push_back(Args.MakeArgString(Entry));
   }
 
+  if (Args.hasArg(options::OPT_shared))
+CmdArgs.push_back(Args.MakeArgString("-shared"));
+
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {

diff  --git a/clang/test/Driver/wasm-toolchain.c 
b/clang/test/Driver/wasm-toolchain.c
index bfe696d7846e7..909d27f0f2d9c 100644
--- a/clang/test/Driver/wasm-toolchain.c
+++ b/clang/test/Driver/wasm-toolchain.c
@@ -33,6 +33,20 @@
 // LINK_KNOWN: "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" 
"-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
+// -shared should be passed through to `wasm-ld` and not include crt1.o with a 
known OS.
+
+// RUN: %clang -### -shared --target=wasm32-wasi --sysroot=/foo %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_KNOWN_SHARED %s
+// LINK_KNOWN_SHARED: "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
+// LINK_KNOWN_SHARED: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "-shared" 
"[[temp]]" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
+
+// -shared should be passed through to `wasm-ld` and not include crt1.o with 
an unknown OS.
+
+// RUN: %clang -### -shared --target=wasm32-unknown-unknown --sysroot=/foo %s 
2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_UNKNOWN_SHARED %s
+// LINK_UNKNOWN_SHARED: "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
+// LINK_UNKNOWN_SHARED: wasm-ld{{.*}}" "-shared" "[[temp]]" "-lc" 
"{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
+
 // A basic C link command-line with optimization with known OS.
 
 // RUN: %clang -### -O2 --target=wasm32-wasi 

[PATCH] D151587: [clang][ConstantEmitter] have tryEmitPrivate[ForVarInit] try ConstExprEmitter fast-path first

2023-06-26 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments.



Comment at: clang/test/CodeGenOpenCL/amdgpu-nullptr.cl:60
 
-// CHECK: @fold_priv ={{.*}} local_unnamed_addr addrspace(1) global ptr 
addrspace(5) addrspacecast (ptr null to ptr addrspace(5)), align 4
+// CHECK: @fold_priv ={{.*}} local_unnamed_addr addrspace(1) global ptr 
addrspace(5) addrspacecast (ptr addrspace(1) null to ptr addrspace(5)), align 4
 private short *fold_priv = (private short*)(generic int*)(global void*)0;

arsenm wrote:
> This should have stayed a cast from generic, 1->5 cast isn't defined
If it's not supposed to behave this way, it's a bug in LLVM constant folding; 
the following folds the same way if you pass it to `opt -S`.  Given that, I 
don't think it should block this patch.  (I think the relevant code is 
CastInst::isEliminableCastPair.)

```
target datalayout = 
"e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32-p7:160:256:256:32-p8:128:128-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5-G1-ni:7:8"
target triple = "amdgcn"
@fold_priv = local_unnamed_addr addrspace(1) global ptr addrspace(5) 
addrspacecast (ptr addrspacecast(ptr addrspace(1) null to ptr) to ptr 
addrspace(5)), align 4
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151587

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


[PATCH] D105759: Implement P2361 Unevaluated string literals

2023-06-26 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin marked 6 inline comments as done.
cor3ntin added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1411
   let Documentation = [DeprecatedDocs];
+  let ParseArgumentsAsUnevaluated = 1;
 }

aaron.ballman wrote:
> What is the plan for non-standard attributes? Are you planning to handle 
> those in a follow-up, or should we be investigating those right now?
I don't feel I'm qualified to answer that. Ideally, attributes that expect 
string literals that are not evaluated should follow suite.



Comment at: clang/include/clang/Parse/Parser.h:1857-1859
bool FailImmediatelyOnInvalidExpr = false,
-   bool EarlyTypoCorrection = false);
+   bool EarlyTypoCorrection = false,
+   bool AllowEvaluatedString = true);

aaron.ballman wrote:
> Two default `bool` params is a bad thing but three default `bool` params 
> seems like we should fix the interface at this point. WDYT?
> 
> Also, it's not clear what the new parameter will do, the function could use 
> comments unless fixing the interface makes it sufficiently clear.
I'm still not sure that's the best solution. `AllowEvaluatedString` would only 
ever be false for attributes, I consider duplicating the function, except it 
does quite a bit for variadics, which apparently attribute support

Maybe would could have

```
ParseAttributeArgumentList
ParseExpressionList
ParseExpressionListImpl?
```
?





Comment at: clang/lib/AST/Expr.cpp:1165
+unsigned CharByteWidth = mapCharByteWidth(Ctx.getTargetInfo(), Kind);
+unsigned ByteLength = Str.size();
+assert((ByteLength % CharByteWidth == 0) &&

cor3ntin wrote:
> aaron.ballman wrote:
> > shafik wrote:
> > > Isn't this the same as `Length`?
> > It is -- I think we can get rid of `ByteLength`, but it's possible that 
> > this exists because of the optimization comment below. I don't insist, but 
> > it would be nice to know if we can replace the switch with `Length /= 
> > CharByteWidth` these days.
> Only when CharByteWidth == 1
I think we should.



Comment at: clang/lib/Lex/LiteralSupport.cpp:98
+  case 't':
+  case 'r':
+return true;

aaron.ballman wrote:
> We're still missing support for some escape characters from: 
> http://eel.is/c++draft/lex#nt:simple-escape-sequence-char
> 
> Just to verify, UCNs have already been handled by the time we get here, so we 
> don't need to care about those, correct?
> Just to verify, UCNs have already been handled by the time we get here, so we 
> don't need to care about those, correct?

They are dealt with elsewhere yes (and supported)



Comment at: clang/lib/Lex/LiteralSupport.cpp:2080-2082
+  if (!isUnevaluated() && Features.PascalStrings &&
+  ThisTokBuf + 1 != ThisTokEnd && ThisTokBuf[0] == '\\' &&
+  ThisTokBuf[1] == 'p') {

aaron.ballman wrote:
> Is there test coverage that we diagnose this properly?
What sort of test would you like to see?



Comment at: clang/lib/Parse/ParseExpr.cpp:3501-3503
+} else if (!AllowEvaluatedString && tok::isStringLiteral(Tok.getKind())) {
+  Expr = ParseUnevaluatedStringLiteralExpression();
+} else {

aaron.ballman wrote:
> I'm surprised we need special logic in `ParseExpressionList()` for handling 
> unevaluated string literals; I would have expected that to be needed when 
> parsing a string literal. Nothing changed in the grammar for 
> http://eel.is/c++draft/expr.post.general#nt:expression-list (or 
> initializer-list), so these changes seem wrong. Can you explain the changes a 
> bit more?
We use `ParseExpressionList` when parsing attribute arguments, and some 
attributes have unevaluate string as argument - I agree with you that I'd 
rather find a better solution for attributes, but I came up empty. There is no 
further reason for this change, and you are right it does not match the grammar.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:878-879
 
-  if (!isIntOrBool(AL.getArgAsExpr(0))) {
+  Expr *First = AL.getArgAsExpr(0);
+  if (isa(First) || !isIntOrBool(First)) {
 S.Diag(AL.getLoc(), diag::err_attribute_argument_n_type)

aaron.ballman wrote:
> Test coverage for these changes?
There is one somewhere, I don;t remember where, The reason we need to do that 
is that Unevaluated StringLiterals don''t have types



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16470
   StringLiteral *Lit = cast(LangStr);
-  if (!Lit->isOrdinary()) {
-Diag(LangStr->getExprLoc(), diag::err_language_linkage_spec_not_ascii)
-  << LangStr->getSourceRange();
-return nullptr;
-  }
+  assert(Lit->isUnevaluated() && "Unexpected string literal kind");
 

aaron.ballman wrote:
> Test coverage for changes?
There are some in dcl.link/p2.cpp


[PATCH] D139837: [Clang] Implements CTAD for aggregates P1816R0 and P2082R1

2023-06-26 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen added a comment.

In D139837#4449327 , @cor3ntin wrote:

> @ychen You probably want to update the description, i think it's updated (you 
> implemented ctad for parenthesized ctrs)

Thanks for the remainder, Corentin. Hopefully, this will be ready to be landed 
soon.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139837

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


[PATCH] D153670: [clang/HeaderSearch] Make sure `loadSubdirectoryModuleMaps` doesn't cause loading of regular files

2023-06-26 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG03a0f4b61ca5: [clang/HeaderSearch] Make sure 
`loadSubdirectoryModuleMaps` doesnt cause… (authored by akyrtzi).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153670

Files:
  clang/lib/Lex/HeaderSearch.cpp
  clang/unittests/Tooling/DependencyScannerTest.cpp


Index: clang/unittests/Tooling/DependencyScannerTest.cpp
===
--- clang/unittests/Tooling/DependencyScannerTest.cpp
+++ clang/unittests/Tooling/DependencyScannerTest.cpp
@@ -239,3 +239,65 @@
   EXPECT_EQ(convert_to_slash(DepFile),
 "test.cpp.o: /root/test.cpp /root/header.h\n");
 }
+
+TEST(DependencyScanner, ScanDepsWithModuleLookup) {
+  std::vector CommandLine = {
+  "clang",
+  "-target",
+  "x86_64-apple-macosx10.7",
+  "-c",
+  "test.m",
+  "-o"
+  "test.m.o",
+  "-fmodules",
+  "-I/root/SomeSources",
+  };
+  StringRef CWD = "/root";
+
+  auto VFS = new llvm::vfs::InMemoryFileSystem();
+  VFS->setCurrentWorkingDirectory(CWD);
+  auto Sept = llvm::sys::path::get_separator();
+  std::string OtherPath =
+  std::string(llvm::formatv("{0}root{0}SomeSources{0}other.h", Sept));
+  std::string TestPath = std::string(llvm::formatv("{0}root{0}test.m", Sept));
+
+  VFS->addFile(OtherPath, 0, llvm::MemoryBuffer::getMemBuffer("\n"));
+  VFS->addFile(TestPath, 0, llvm::MemoryBuffer::getMemBuffer("@import 
Foo;\n"));
+
+  struct InterceptorFS : llvm::vfs::ProxyFileSystem {
+std::vector StatPaths;
+std::vector ReadFiles;
+
+InterceptorFS(IntrusiveRefCntPtr UnderlyingFS)
+: ProxyFileSystem(UnderlyingFS) {}
+
+llvm::ErrorOr status(const Twine ) override {
+  StatPaths.push_back(Path.str());
+  return ProxyFileSystem::status(Path);
+}
+
+llvm::ErrorOr>
+openFileForRead(const Twine ) override {
+  ReadFiles.push_back(Path.str());
+  return ProxyFileSystem::openFileForRead(Path);
+}
+  };
+
+  auto InterceptFS = llvm::makeIntrusiveRefCnt(VFS);
+
+  DependencyScanningService Service(ScanningMode::DependencyDirectivesScan,
+ScanningOutputFormat::Make);
+  DependencyScanningTool ScanTool(Service, InterceptFS);
+
+  // This will fail with "fatal error: module 'Foo' not found" but it doesn't
+  // matter, the point of the test is to check that files are not read
+  // unnecessarily.
+  std::string DepFile;
+  ASSERT_THAT_ERROR(
+  ScanTool.getDependencyFile(CommandLine, CWD).moveInto(DepFile),
+  llvm::Failed());
+
+  EXPECT_TRUE(llvm::find(InterceptFS->StatPaths, OtherPath) ==
+  InterceptFS->StatPaths.end());
+  EXPECT_EQ(InterceptFS->ReadFiles, std::vector{"test.m"});
+}
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1917,6 +1917,8 @@
   llvm::vfs::FileSystem  = FileMgr.getVirtualFileSystem();
   for (llvm::vfs::directory_iterator Dir = FS.dir_begin(DirNative, EC), DirEnd;
Dir != DirEnd && !EC; Dir.increment(EC)) {
+if (Dir->type() == llvm::sys::fs::file_type::regular_file)
+  continue;
 bool IsFramework = llvm::sys::path::extension(Dir->path()) == ".framework";
 if (IsFramework == SearchDir.isFramework())
   loadModuleMapFile(Dir->path(), SearchDir.isSystemHeaderDirectory(),


Index: clang/unittests/Tooling/DependencyScannerTest.cpp
===
--- clang/unittests/Tooling/DependencyScannerTest.cpp
+++ clang/unittests/Tooling/DependencyScannerTest.cpp
@@ -239,3 +239,65 @@
   EXPECT_EQ(convert_to_slash(DepFile),
 "test.cpp.o: /root/test.cpp /root/header.h\n");
 }
+
+TEST(DependencyScanner, ScanDepsWithModuleLookup) {
+  std::vector CommandLine = {
+  "clang",
+  "-target",
+  "x86_64-apple-macosx10.7",
+  "-c",
+  "test.m",
+  "-o"
+  "test.m.o",
+  "-fmodules",
+  "-I/root/SomeSources",
+  };
+  StringRef CWD = "/root";
+
+  auto VFS = new llvm::vfs::InMemoryFileSystem();
+  VFS->setCurrentWorkingDirectory(CWD);
+  auto Sept = llvm::sys::path::get_separator();
+  std::string OtherPath =
+  std::string(llvm::formatv("{0}root{0}SomeSources{0}other.h", Sept));
+  std::string TestPath = std::string(llvm::formatv("{0}root{0}test.m", Sept));
+
+  VFS->addFile(OtherPath, 0, llvm::MemoryBuffer::getMemBuffer("\n"));
+  VFS->addFile(TestPath, 0, llvm::MemoryBuffer::getMemBuffer("@import Foo;\n"));
+
+  struct InterceptorFS : llvm::vfs::ProxyFileSystem {
+std::vector StatPaths;
+std::vector ReadFiles;
+
+InterceptorFS(IntrusiveRefCntPtr UnderlyingFS)
+: ProxyFileSystem(UnderlyingFS) {}
+
+llvm::ErrorOr status(const Twine ) override {
+  StatPaths.push_back(Path.str());
+  return 

[clang] 03a0f4b - [clang/HeaderSearch] Make sure `loadSubdirectoryModuleMaps` doesn't cause loading of regular files

2023-06-26 Thread Argyrios Kyrtzidis via cfe-commits

Author: Argyrios Kyrtzidis
Date: 2023-06-26T10:18:02-07:00
New Revision: 03a0f4b61ca50a267a405a29ff1986473a55f9d9

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

LOG: [clang/HeaderSearch] Make sure `loadSubdirectoryModuleMaps` doesn't cause 
loading of regular files

`HeaderSearch::loadSubdirectoryModuleMaps` `stat`s all the files in a directory 
which causes the dependency scanning
service to load and cache their contents. This is problematic because a file 
may be in the process of being generated
and could be cached by the dep-scan service while it is still incomplete.

To address this change `loadSubdirectoryModuleMaps` to ignore regular files.

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

Added: 


Modified: 
clang/lib/Lex/HeaderSearch.cpp
clang/unittests/Tooling/DependencyScannerTest.cpp

Removed: 




diff  --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp
index 723479ca4fbbc..6fe655fb24277 100644
--- a/clang/lib/Lex/HeaderSearch.cpp
+++ b/clang/lib/Lex/HeaderSearch.cpp
@@ -1917,6 +1917,8 @@ void 
HeaderSearch::loadSubdirectoryModuleMaps(DirectoryLookup ) {
   llvm::vfs::FileSystem  = FileMgr.getVirtualFileSystem();
   for (llvm::vfs::directory_iterator Dir = FS.dir_begin(DirNative, EC), DirEnd;
Dir != DirEnd && !EC; Dir.increment(EC)) {
+if (Dir->type() == llvm::sys::fs::file_type::regular_file)
+  continue;
 bool IsFramework = llvm::sys::path::extension(Dir->path()) == ".framework";
 if (IsFramework == SearchDir.isFramework())
   loadModuleMapFile(Dir->path(), SearchDir.isSystemHeaderDirectory(),

diff  --git a/clang/unittests/Tooling/DependencyScannerTest.cpp 
b/clang/unittests/Tooling/DependencyScannerTest.cpp
index abcc2c787b0d0..8735fcad20046 100644
--- a/clang/unittests/Tooling/DependencyScannerTest.cpp
+++ b/clang/unittests/Tooling/DependencyScannerTest.cpp
@@ -239,3 +239,65 @@ TEST(DependencyScanner, ScanDepsWithFS) {
   EXPECT_EQ(convert_to_slash(DepFile),
 "test.cpp.o: /root/test.cpp /root/header.h\n");
 }
+
+TEST(DependencyScanner, ScanDepsWithModuleLookup) {
+  std::vector CommandLine = {
+  "clang",
+  "-target",
+  "x86_64-apple-macosx10.7",
+  "-c",
+  "test.m",
+  "-o"
+  "test.m.o",
+  "-fmodules",
+  "-I/root/SomeSources",
+  };
+  StringRef CWD = "/root";
+
+  auto VFS = new llvm::vfs::InMemoryFileSystem();
+  VFS->setCurrentWorkingDirectory(CWD);
+  auto Sept = llvm::sys::path::get_separator();
+  std::string OtherPath =
+  std::string(llvm::formatv("{0}root{0}SomeSources{0}other.h", Sept));
+  std::string TestPath = std::string(llvm::formatv("{0}root{0}test.m", Sept));
+
+  VFS->addFile(OtherPath, 0, llvm::MemoryBuffer::getMemBuffer("\n"));
+  VFS->addFile(TestPath, 0, llvm::MemoryBuffer::getMemBuffer("@import 
Foo;\n"));
+
+  struct InterceptorFS : llvm::vfs::ProxyFileSystem {
+std::vector StatPaths;
+std::vector ReadFiles;
+
+InterceptorFS(IntrusiveRefCntPtr UnderlyingFS)
+: ProxyFileSystem(UnderlyingFS) {}
+
+llvm::ErrorOr status(const Twine ) override {
+  StatPaths.push_back(Path.str());
+  return ProxyFileSystem::status(Path);
+}
+
+llvm::ErrorOr>
+openFileForRead(const Twine ) override {
+  ReadFiles.push_back(Path.str());
+  return ProxyFileSystem::openFileForRead(Path);
+}
+  };
+
+  auto InterceptFS = llvm::makeIntrusiveRefCnt(VFS);
+
+  DependencyScanningService Service(ScanningMode::DependencyDirectivesScan,
+ScanningOutputFormat::Make);
+  DependencyScanningTool ScanTool(Service, InterceptFS);
+
+  // This will fail with "fatal error: module 'Foo' not found" but it doesn't
+  // matter, the point of the test is to check that files are not read
+  // unnecessarily.
+  std::string DepFile;
+  ASSERT_THAT_ERROR(
+  ScanTool.getDependencyFile(CommandLine, CWD).moveInto(DepFile),
+  llvm::Failed());
+
+  EXPECT_TRUE(llvm::find(InterceptFS->StatPaths, OtherPath) ==
+  InterceptFS->StatPaths.end());
+  EXPECT_EQ(InterceptFS->ReadFiles, std::vector{"test.m"});
+}



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


[PATCH] D151587: [clang][ConstantEmitter] have tryEmitPrivate[ForVarInit] try ConstExprEmitter fast-path first

2023-06-26 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm added inline comments.



Comment at: clang/test/CodeGenOpenCL/amdgpu-nullptr.cl:60
 
-// CHECK: @fold_priv ={{.*}} local_unnamed_addr addrspace(1) global ptr 
addrspace(5) addrspacecast (ptr null to ptr addrspace(5)), align 4
+// CHECK: @fold_priv ={{.*}} local_unnamed_addr addrspace(1) global ptr 
addrspace(5) addrspacecast (ptr addrspace(1) null to ptr addrspace(5)), align 4
 private short *fold_priv = (private short*)(generic int*)(global void*)0;

This should have stayed a cast from generic, 1->5 cast isn't defined


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151587

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


[PATCH] D152686: [clangd] Enforce strict unused includes by default

2023-06-26 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment.

This patch seems to have broken clangd for a project of mine. Most of my 
headers are now flagged as not being directly used even though they're the root 
header for the symbol.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152686

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


[PATCH] D151963: [clang][NFC] Remove trailing whitespaces and enforce it in lib, include and docs

2023-06-26 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D151963#4449346 , @aaron.ballman 
wrote:

> In D151963#4390675 , @erichkeane 
> wrote:
>
>> This looks fine to me.  HOWEVER, please make sure to add this commit to 
>> `https://github.com/llvm/llvm-project/blob/main/.git-blame-ignore-revs` in a 
>> follow-up NFC commit.
>
> Please be sure to do this.

This did get handled, thanks! Today I learned that changes to the top level in 
the monorepo don't make it to either of the llvm-commits or cfe-commits mailing 
lists.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151963

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


[PATCH] D147875: [clang][Diagnostics] Show line numbers when printing code snippets

2023-06-26 Thread Jessica Clarke via Phabricator via cfe-commits
jrtc27 added a comment.

In D147875#4448545 , @hans wrote:

> I noticed that at least for some cases of `-Wformat`, the line numbers on the 
> left seem to be off: https://github.com/llvm/llvm-project/issues/63524

That's because DisplayLineNo refers to the line of the diagnostic, not the 
first source line, and then counts up. What's the reason for DisplayLineNo in 
the first place? Given the printed line comes from LineNo, surely we should be 
using LineNo instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147875

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


[PATCH] D105759: Implement P2361 Unevaluated string literals

2023-06-26 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin marked 2 inline comments as done.
cor3ntin added inline comments.



Comment at: clang/lib/AST/Expr.cpp:1140
+  case Unevaluated:
+return sizeof(char); // Host;
+break;

aaron.ballman wrote:
> shafik wrote:
> > Why not grouped w/ `Ordinary` above?
> Specifically because we want the host encoding, not the target encoding.
an unevaluated string is a sequence of 1-byte even on platforms were 
`sizeof(char)` would be 2 or 4. It's never influenced by the target's properties


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105759

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


[PATCH] D105759: Implement P2361 Unevaluated string literals

2023-06-26 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a subscriber: ChuanqiXu.
aaron.ballman added a comment.

This also should update the cxx_status page and have a release note.




Comment at: clang/include/clang/Basic/Attr.td:1411
   let Documentation = [DeprecatedDocs];
+  let ParseArgumentsAsUnevaluated = 1;
 }

What is the plan for non-standard attributes? Are you planning to handle those 
in a follow-up, or should we be investigating those right now?



Comment at: clang/include/clang/Parse/Parser.h:1857-1859
bool FailImmediatelyOnInvalidExpr = false,
-   bool EarlyTypoCorrection = false);
+   bool EarlyTypoCorrection = false,
+   bool AllowEvaluatedString = true);

Two default `bool` params is a bad thing but three default `bool` params seems 
like we should fix the interface at this point. WDYT?

Also, it's not clear what the new parameter will do, the function could use 
comments unless fixing the interface makes it sufficiently clear.



Comment at: clang/lib/AST/Expr.cpp:1140
+  case Unevaluated:
+return sizeof(char); // Host;
+break;

shafik wrote:
> Why not grouped w/ `Ordinary` above?
Specifically because we want the host encoding, not the target encoding.



Comment at: clang/lib/AST/Expr.cpp:1165
+unsigned CharByteWidth = mapCharByteWidth(Ctx.getTargetInfo(), Kind);
+unsigned ByteLength = Str.size();
+assert((ByteLength % CharByteWidth == 0) &&

shafik wrote:
> Isn't this the same as `Length`?
It is -- I think we can get rid of `ByteLength`, but it's possible that this 
exists because of the optimization comment below. I don't insist, but it would 
be nice to know if we can replace the switch with `Length /= CharByteWidth` 
these days.



Comment at: clang/lib/AST/Expr.cpp:1190
+StringLiteralBits.CharByteWidth = 1;
+StringLiteralBits.IsPascal = false;
+  }

Add `assert(!Pascal && "Can't make an unevaluated Pascal string");` ?



Comment at: clang/lib/AST/Expr.cpp:1201
   // Initialize the trailing array of char holding the string data.
-  std::memcpy(getTrailingObjects(), Str.data(), ByteLength);
+  std::memcpy(getTrailingObjects(), Str.data(), Str.size());
 

shafik wrote:
> Isn't `Str.size()` the same as `ByteLength`? 
I think it's more clear to use `Str.size()` because we're copying from 
`Str.data()`.



Comment at: clang/lib/AST/Expr.cpp:1238
   switch (getKind()) {
+  case Unevaluated: // fallthrough. no prefix.
   case Ordinary:





Comment at: clang/lib/Lex/LiteralSupport.cpp:90
 
+static bool EscapeValidInUnevaluatedStringLiteral(char Escape) {
+  switch (Escape) {

shafik wrote:
> Should we use `Is` as a prefix here? Right now it should like we are 
> modifying something.
+1, I think `Is` would be an improvement.



Comment at: clang/lib/Lex/LiteralSupport.cpp:98
+  case 't':
+  case 'r':
+return true;

We're still missing support for some escape characters from: 
http://eel.is/c++draft/lex#nt:simple-escape-sequence-char

Just to verify, UCNs have already been handled by the time we get here, so we 
don't need to care about those, correct?



Comment at: clang/lib/Lex/LiteralSupport.cpp:1912
+  hadError = true;
+  return;
+}

Doesn't returning here leave the object in a partially-initialized state? That 
seems bad.



Comment at: clang/lib/Lex/LiteralSupport.cpp:2080-2082
+  if (!isUnevaluated() && Features.PascalStrings &&
+  ThisTokBuf + 1 != ThisTokEnd && ThisTokBuf[0] == '\\' &&
+  ThisTokBuf[1] == 'p') {

Is there test coverage that we diagnose this properly?



Comment at: clang/lib/Lex/PPMacroExpansion.cpp:1872-1873
 else if (Tok.is(tok::string_literal) && !Tok.hasUDSuffix()) {
-  StringLiteralParser Literal(Tok, *this);
+  StringLiteralParser Literal(Tok, *this,
+  StringLiteralEvalMethod::Unevaluated);
   if (Literal.hadError)

Test coverage for this change?



Comment at: clang/lib/Lex/Pragma.cpp:807
   if (Tok.is(tok::string_literal) && !Tok.hasUDSuffix()) {
 StringLiteralParser Literal(Tok, PP);
 if (Literal.hadError)

aaron.ballman wrote:
> cor3ntin wrote:
> > aaron.ballman wrote:
> > > Should this also be modified?
> > Probably but because I'm not super familiar with module map things I 
> > preferred being conservative
> Paging @rsmith for opinions.
> 
> Lacking those opinions, I think being conservative here is fine.
Pinging @ChuanqiXu for opinions.



Comment at: clang/lib/Parse/ParseExpr.cpp:3501-3503
+} else if (!AllowEvaluatedString && 

[PATCH] D105759: Implement P2361 Unevaluated string literals

2023-06-26 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin marked 43 inline comments as done.
cor3ntin added inline comments.



Comment at: clang/lib/AST/Expr.cpp:1165
+unsigned CharByteWidth = mapCharByteWidth(Ctx.getTargetInfo(), Kind);
+unsigned ByteLength = Str.size();
+assert((ByteLength % CharByteWidth == 0) &&

shafik wrote:
> Isn't this the same as `Length`?
Only when CharByteWidth == 1



Comment at: clang/lib/AST/Expr.cpp:1201
   // Initialize the trailing array of char holding the string data.
-  std::memcpy(getTrailingObjects(), Str.data(), ByteLength);
+  std::memcpy(getTrailingObjects(), Str.data(), Str.size());
 

shafik wrote:
> Isn't `Str.size()` the same as `ByteLength`? 
ByteLength isn't defined in this scope, I guess i could move it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105759

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


[PATCH] D152720: [clangd][ObjC] Support ObjC class rename from implementation decls

2023-06-26 Thread David Goldman via Phabricator via cfe-commits
dgoldman updated this revision to Diff 534618.
dgoldman marked an inline comment as done.
dgoldman added a comment.

Fixes for review + rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152720

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp
  clang-tools-extra/clangd/unittests/RenameTests.cpp

Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -840,6 +840,20 @@
   foo('x');
 }
   )cpp",
+
+  // ObjC class with a category.
+  R"cpp(
+@interface [[Fo^o]]
+@end
+@implementation [[F^oo]]
+@end
+@interface [[Fo^o]] (Category)
+@end
+@implementation [[F^oo]] (Category)
+@end
+
+void func([[Fo^o]] *f) {}
+  )cpp",
   };
   llvm::StringRef NewName = "NewName";
   for (llvm::StringRef T : Tests) {
@@ -890,6 +904,15 @@
   )cpp",
"not a supported kind", HeaderFile},
 
+  {R"cpp(// disallow - category rename.
+@interface Foo
+@end
+@interface Foo (Cate^gory)
+@end
+  )cpp",
+   "Cannot rename symbol: there is no symbol at the given location",
+   HeaderFile},
+
   {
   R"cpp(
  #define MACRO 1
@@ -1468,7 +1491,7 @@
 
 TEST(CrossFileRenameTests, WithUpToDateIndex) {
   MockCompilationDatabase CDB;
-  CDB.ExtraClangFlags = {"-xc++"};
+  CDB.ExtraClangFlags = {"-xobjective-c++"};
   // rename is runnning on all "^" points in FooH, and "[[]]" ranges are the
   // expected rename occurrences.
   struct Case {
@@ -1557,13 +1580,12 @@
 }
   )cpp",
   },
-  {
-  // virtual templated method
-  R"cpp(
+  {// virtual templated method
+   R"cpp(
 template  class Foo { virtual void [[m]](); };
 class Bar : Foo { void [[^m]]() override; };
   )cpp",
-  R"cpp(
+   R"cpp(
   #include "foo.h"
 
   template void Foo::[[m]]() {}
@@ -1571,8 +1593,7 @@
   // the canonical Foo::m().
   // https://github.com/clangd/clangd/issues/1325
   class Baz : Foo { void m() override; };
-)cpp"
-  },
+)cpp"},
   {
   // rename on constructor and destructor.
   R"cpp(
@@ -1677,6 +1698,20 @@
 }
   )cpp",
   },
+  {
+  // Objective-C classes.
+  R"cpp(
+@interface [[Fo^o]]
+@end
+  )cpp",
+  R"cpp(
+#include "foo.h"
+@implementation [[Foo]]
+@end
+
+void func([[Foo]] *f) {}
+  )cpp",
+  },
   };
 
   trace::TestTracer Tracer;
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -1127,6 +1127,16 @@
   )cpp";
   EXPECT_DECLS("ObjCCategoryImplDecl", "@interface Foo(Ext)");
 
+  Code = R"cpp(
+@interface Foo
+@end
+@interface Foo (Ext)
+@end
+@implementation Foo ([[Ext]])
+@end
+  )cpp";
+  EXPECT_DECLS("ObjCCategoryImplDecl", "@interface Foo(Ext)");
+
   Code = R"cpp(
 void test(id p);
   )cpp";
@@ -1216,10 +1226,7 @@
 std::string DumpedReferences;
   };
 
-  /// Parses \p Code, finds function or namespace '::foo' and annotates its body
-  /// with results of findExplicitReferences.
-  /// See actual tests for examples of annotation format.
-  AllRefs annotateReferencesInFoo(llvm::StringRef Code) {
+  TestTU newTU(llvm::StringRef Code) {
 TestTU TU;
 TU.Code = std::string(Code);
 
@@ -1228,30 +1235,11 @@
 TU.ExtraArgs.push_back("-std=c++20");
 TU.ExtraArgs.push_back("-xobjective-c++");
 
-auto AST = TU.build();
-auto *TestDecl = (AST, "foo");
-if (auto *T = llvm::dyn_cast(TestDecl))
-  TestDecl = T->getTemplatedDecl();
-
-std::vector Refs;
-if (const auto *Func = llvm::dyn_cast(TestDecl))
-  findExplicitReferences(
-  Func->getBody(),
-  [](ReferenceLoc R) { Refs.push_back(std::move(R)); },
-  AST.getHeuristicResolver());
-else if (const auto *NS = llvm::dyn_cast(TestDecl))
-  findExplicitReferences(
-  NS,
-  [, ](ReferenceLoc R) {
-// Avoid adding the namespace foo decl to the results.
-if (R.Targets.size() == 1 && R.Targets.front() == NS)
-  return;
-Refs.push_back(std::move(R));
-  },
-  AST.getHeuristicResolver());
-else
-  ADD_FAILURE() << "Failed to find ::foo decl for test";
+return TU;
+  }
 
+ 

[PATCH] D153582: [SystemZ][z/OS] Add required options/macro/etc for z/os compilation step

2023-06-26 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments.



Comment at: clang/test/Driver/zos-comp.c:3
+
+// RUN: %clang -c -### %s 2>&1 \
+// RUN:--target=s390x-ibm-zos \

wrap too quickly.

The whole thing can be placed on one line.



Comment at: clang/test/Driver/zos-comp.c:8
+// CHECK: "-D_UNIX03_WITHDRAWN"
+// CHECK: "-D_OPEN_DEFAULT"
+// CHECK: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"

Use `-SAME:` whenever applicable


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

https://reviews.llvm.org/D153582

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


  1   2   3   >