[PATCH] D126438: [clang-format] Fix an invalid code generation in RemoveBracesLLVM

[Marek Kurdej via Phabricator via cfe-commits]

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

LGTM. Good catch for this bug!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126438

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

[PATCH] D126302: [PowerPC] Diagnose invalid combination with Altivec, VSX and soft-float

[Qiu Chaofan via Phabricator via cfe-commits]

qiucf added inline comments.



Comment at: clang/lib/Basic/Targets/PPC.cpp:450
 
+  // Cannot allow VSX with no Altivec.
+  if (llvm::is_contained(FeaturesVec, "-hard-float") &&

Comments in reverse order?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126302

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

[PATCH] D126365: [git-clang-format] Stop ignoring changes for files with space in path

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

Can you update the diff with context? See 
https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126365

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

[PATCH] D126438: [clang-format] Fix an invalid code generation in RemoveBracesLLVM

[Owen Pan via Phabricator via cfe-commits]

owenpan created this revision.
owenpan added reviewers: curdeius, MyDeveloperDay, HazardyKnusperkeks.
owenpan added a project: clang-format.
Herald added a project: All.
owenpan requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126438

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -25349,6 +25349,30 @@
"}",
Style);
 
+  verifyFormat("if (a)\n"
+   "  if (b)\n"
+   "c;\n"
+   "  else {\n"
+   "if (d)\n"
+   "  e;\n"
+   "  }\n"
+   "else\n"
+   "  f;",
+   Style);
+
+  verifyFormat("if (a)\n"
+   "  if (b)\n"
+   "c;\n"
+   "  else {\n"
+   "if (d)\n"
+   "  e;\n"
+   "else if (f)\n"
+   "  g;\n"
+   "  }\n"
+   "else\n"
+   "  h;",
+   Style);
+
   verifyFormat("if (a)\n"
"  b;\n"
"else if (c)\n"
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2587,10 +2587,13 @@
   FormatTok->setFinalizedType(TT_ElseLBrace);
   ElseLeftBrace = FormatTok;
   CompoundStatementIndenter Indenter(this, Style, Line->Level);
-  if (parseBlock(/*MustBeDeclaration=*/false, /*AddLevels=*/1u,
- /*MunchSemi=*/true,
- KeepElseBraces) == IfStmtKind::IfOnly) {
-Kind = IfStmtKind::IfElseIf;
+  const IfStmtKind ElseBlockKind =
+  parseBlock(/*MustBeDeclaration=*/false, /*AddLevels=*/1u,
+ /*MunchSemi=*/true, KeepElseBraces);
+  if ((ElseBlockKind == IfStmtKind::IfOnly ||
+   ElseBlockKind == IfStmtKind::IfElseIf) &&
+  FormatTok->is(tok::kw_else)) {
+KeepElseBraces = true;
   }
   addUnwrappedLine();
 } else if (FormatTok->is(tok::kw_if)) {


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -25349,6 +25349,30 @@
"}",
Style);
 
+  verifyFormat("if (a)\n"
+   "  if (b)\n"
+   "c;\n"
+   "  else {\n"
+   "if (d)\n"
+   "  e;\n"
+   "  }\n"
+   "else\n"
+   "  f;",
+   Style);
+
+  verifyFormat("if (a)\n"
+   "  if (b)\n"
+   "c;\n"
+   "  else {\n"
+   "if (d)\n"
+   "  e;\n"
+   "else if (f)\n"
+   "  g;\n"
+   "  }\n"
+   "else\n"
+   "  h;",
+   Style);
+
   verifyFormat("if (a)\n"
"  b;\n"
"else if (c)\n"
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2587,10 +2587,13 @@
   FormatTok->setFinalizedType(TT_ElseLBrace);
   ElseLeftBrace = FormatTok;
   CompoundStatementIndenter Indenter(this, Style, Line->Level);
-  if (parseBlock(/*MustBeDeclaration=*/false, /*AddLevels=*/1u,
- /*MunchSemi=*/true,
- KeepElseBraces) == IfStmtKind::IfOnly) {
-Kind = IfStmtKind::IfElseIf;
+  const IfStmtKind ElseBlockKind =
+  parseBlock(/*MustBeDeclaration=*/false, /*AddLevels=*/1u,
+ /*MunchSemi=*/true, KeepElseBraces);
+  if ((ElseBlockKind == IfStmtKind::IfOnly ||
+   ElseBlockKind == IfStmtKind::IfElseIf) &&
+  FormatTok->is(tok::kw_else)) {
+KeepElseBraces = true;
   }
   addUnwrappedLine();
 } else if (FormatTok->is(tok::kw_if)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102122: Support warn_unused_result on typedefs

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:2943
+def WarnUnusedResultClang : InheritableAttr {
+  let Spellings = [CXX11<"clang", "warn_unused_result">];
+  let Subjects = SubjectList<[ObjCMethod, Enum, Record, FunctionLike, 
TypedefName]>;

Should we allow this new behavior for the GNU attribute spelling too? (Not 
`[[gnu::..]]` but `__attribute__((warn_unused_result))`). We don't generally 
avoid extending the meaning of `__attribute__((...))` compared to GCC.



Comment at: clang/lib/AST/Expr.cpp:1525-1527
+  if (const auto *TD = getCallReturnType(Ctx)->getAs())
+if (const auto *A = TD->getDecl()->getAttr())
+  return A;

This should loop over `TypedefType` sugar; the attribute could be in a nested 
typedef.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102122

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

[PATCH] D125788: [flang][driver] Rename `flang-new` as `flang`

[Steve Scalpone via Phabricator via cfe-commits]

sscalpone added a comment.

Recommend that you contact NAG for more info on the test suite.  Here's the 
only reference that I could find: 
https://fortran-lang.discourse.group/t/fortran-compiler-testing-framework/1573/10


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125788

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

[PATCH] D126247: `readability-indentifier-naming` resolution order and examples

[Kazys Stepanas via Phabricator via cfe-commits]

KazNX updated this revision to Diff 432159.
KazNX added a comment.

Updated to address feedback.

- Typo corrections.
- Additional declarations.
- Changed source reference URL.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126247

Files:
  clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst

Index: clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
@@ -2742,3 +2742,468 @@
 double   dValueDouble = 0.0;
 ULONGulValueUlong = 0;
 DWORDdwValueDword = 0;
+
+Resolution order
+
+
+The full set of identifier classifications listed above includes some overlap,
+where an individual identifier can fall into several classifications. Below
+is a list of the classifications supported by ``readability-identifier-naming``
+presented in the order in which the classifications are resolved. Some
+classifications appear multiple times as they can be checked in different
+contexts. The check returns as soon as the first valid classification is
+matched. This occurs when the semantics of the identifier match and there is a
+valid option for the classification present in the active configuration - e.g.,
+``readability-identifier-naming.AbstractClassCase``.
+
+ - Typedef  ``[typedef]``
+ - TypeAlias  ``[using Alias = ...]``
+ - InlineNamespace ``[inline namespace]``
+ - Namespace ``[namespace]``
+ - 
+   - ScopedEnumValue ``[enum class member]``
+   - EnumConstant  ``[enum member]``
+   - Constant
+   - Invalid
+ - 
+   - AbstractClass ``[class, struct, pure-virtual present]``
+   - Struct ``[struct]``
+   - Class ``[class, struct]``
+   - Struct ``[class]``
+   - Union ``[union]``
+   - Enum ``[enum, enum class]``
+   - Invalid
+ -  - does not cover ``[static, constexpr]``
+   - ``[const]``
+ - ConstantMember
+ - Constant
+   - PrivateMember ``[private]``
+   - ProtectedMember ``[protected]``
+   - PublicMember ``[public]``
+   - Member
+   - Invalid
+ - 
+   - ConstexprVariable ``[constexpr]``
+   - ``[const]``
+ - ConstantPointerParameter ``[*]``
+ - ConstantParameter
+ - Constant
+   - ParameterPack ``[...]``
+   - PointerParameter ``[*]``
+   - Parameter
+   - Invalid
+ - 
+   - ConstexprVariable ``[constexpr]``
+   - ``[const]``
+ - ClassConstant ``[const, static]``
+ - 
+   - GlobalConstantPointer ``[const *]``
+   - GlobalConstant ``[const]``
+ - StaticConstant ``[static, const]``
+ - 
+   - LocalConstantPointer ``[const *]``
+   - LocalConstant ``[const]``
+ - Constant ``[const]``
+   - 
+ - ClassMember ``[static]``
+   - 
+ - GlobalPointer ``[*]``
+ - GlobalVariable
+   - 
+ - StaticVariable ``[static]``
+ - LocalPointer ``[*]``
+ - LocalVariable
+   - 
+ - LocalVariable
+   - Variable
+ - 
+   - 
+   - ``[constexpr]``
+ - ConstexprMethod
+ - ConstexprFunction
+   - ClassMethod ``[static]``
+   - VirtualMethod ``[virtual]``
+   - PrivateMethod ``[private]``
+   - ProtectedMethod ``[protected]``
+   - PublicMethod ``[public]``
+   - Method
+   - Function
+   - Invalid
+ - 
+   - 
+   - ConstexprFunction ``[constexpr]``
+   - GlobalFunction ``[static method, static function, in any namespace]``
+   - Function
+   - Invalid
+ - 
+   - 
+ - TypeTemplateParameter
+ - TemplateParameter
+ - Invalid
+   - 
+ - ValueTemplateParameter
+ - TemplateParameter
+ - Invalid
+   - 
+ - TemplateTemplateParameter
+ - TemplateParameter
+ - Invalid
+ - Invalid
+
+Labels listed in ``<>`` brackets are semantic qualifiers and attempt to
+illustrate the semantic context within which Clang-tidy resolves the
+classification. These are not formal semantic labels, rather labels which
+attempt disambiguation within the context of this document. For example,
+ identifiers that clang tidy is currently looking only at
+member variables.
+
+Items in ``[]`` brackets provide C/C++ keywords and/or decorations relevant to
+that particular classification. These must be present in the declaration for
+Clang-tidy to match the classification.
+
+List nesting is used to collate classifications which share some semantic
+identification - either a logical grouping using ``<>`` or ``[]`` qualifiers -
+and are each validated within that same semantic context. That is, all the
+classifications listed under  are each attempted in turn
+against anything identified as a parameter.
+
+The label ``Invalid`` indicates that valid classifications have been exhausted
+for a particular  and no further attempts will be made to
+classify the identifier in any other context. For example, in the 
+ semantic context, clang tidy will abort matching afte

[PATCH] D126289: [Clang][Driver] Fix include paths for `--sysroot /` on Linux

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/include/clang/Driver/ToolChain.h:219
+
+  static std::string concat(const std::string &Path, const Twine &A,
+const Twine &B = "", const Twine &C = "",

egorzhdan wrote:
> MaskRay wrote:
> > This can use `llvm::sys::path::append`
> The implementation of this already uses `llvm::sys::path::append`, do you 
> mean replacing the usages of `concat` with `llvm::sys::path::append`?
> That would produce quite a lot of boilerplate I think, since 
> `llvm::sys::path::append` modifies the path in-place, so every call to 
> `concat` would expand into 3 lines of code. Also it would be very easy to 
> forget to pass `Style::posix` and cause incorrect behavior on Windows 
> (something I've stumbled upon while making this patch).
Ah I missed that. The current `concat` impl looks good to me.



Comment at: clang/lib/Driver/ToolChains/Linux.cpp:277
 
-  addPathIfExists(D, SysRoot + "/lib/" + MultiarchTriple, Paths);
-  addPathIfExists(D, SysRoot + "/lib/../" + OSLibDir, Paths);
+  addPathIfExists(D, concat(SysRoot, "/lib/", MultiarchTriple), Paths);
+  addPathIfExists(D, concat(SysRoot, "/lib/../", OSLibDir), Paths);

Can the trailing `/` in a path component be removed now? Ditto below.



Comment at: clang/test/Driver/linux-header-search.cpp:99
+// CHECK-BASIC-LIBSTDCXX-SYSROOT-SLASH: "-cc1"
+// CHECK-BASIC-LIBSTDCXX-SYSROOT-SLASH: "-isysroot" "[[SYSROOT:[^"]+/]]"
+// CHECK-BASIC-LIBSTDCXX-SYSROOT-SLASH: "-internal-isystem" 
"[[SYSROOT]]usr/lib/gcc/x86_64-unknown-linux-gnu/4.8/../../../../x86_64-unknown-linux-gnu/include"

add `-SAME` whenever applicable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126289

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

[PATCH] D126341: Order implicitly instantiated global variable's initializer by the reverse instantiation order

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

> The underlying problem is basically wg21.link/cwg362 which has no concensus 
> yet.

CWG362 has a clear consensus and has been closed with that consensus for 18 
years. The consensus, per that issue, is:

> We discussed this and agreed that **we really do mean the the order is 
> unspecified**.

which seems clear that our current behavior is a valid choice.

In D126341#3537947 , @rnk wrote:

> First, why should these guarantees be limited to instantiations and not 
> inline variables? Such as:
>
>   int f();
>   inline int gv1 = f();
>   inline int gv2 = gv1 + 1; // rely on previous

Inline variables have initialization order guarantees already. An inline 
variable `B` can rely on a prior inline variable `A` being initialized first if 
`A` is defined before `B` in every TU where `B` is defined. And a non-inline 
variable `C` can rely on a prior inline variable `B` being initialized first if 
`B` is defined before `C`.

There is no comparable guarantee for instantiated variables, in part because an 
intended implementation strategy is to instantiate them separately, and combine 
those instantiation units with the compilation units at link time in an 
arbitrary order, and in part because instantiation order could in general be 
different in different TUs.

> Second, LLVM doesn't guarantee that global_ctors at the same priority execute 
> in order. See the langref: 
> https://llvm.org/docs/LangRef.html#the-llvm-global-ctors-global-variable So, 
> without a guarantee from LLVM, Clang can't rely on this behavior. LLVM relies 
> on this lack of an ordering guarantee to power globalopt.

It doesn't seem reasonable for us to provide a guarantee here, and in fact, we 
//can't// provide a guarantee in general (once there's more than one TU, a 
consistent global total order might not exist). I think the question is, should 
we make some minor effort to make the broken code that's relying on 
initialization order of unordered variables happen to do the right thing most 
of the time, or should we keep the status quo that is likely to result in 
problems happening earlier? I don't think it's clear whether the superficial 
increase in compatibility with GCC is a net positive or negative -- giving the 
surprising initialization order may counter-intuitively help people to find 
ordering bugs faster.




Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:582
 
-AddGlobalCtor(Fn, 65535, COMDATKey);
+AddGlobalCtor(Fn, 65535, COMDATKey, IsInstantiation);
 if (COMDATKey && (getTriple().isOSBinFormatELF() ||

This is effectively initializing instantiated variables in reverse 
instantiation order. That seems like it'll make things worse as much as it 
makes things better. For example, given:

```
#include 
template int a = (std::cout << "hello, ", 0);
template int b = (std::cout << "world", 0);
int main() {
  (void)a;
  (void)b;
}
```

... we currently print `"hello, world"`, but with this change we'll print 
`"worldhello, "`.

If we want a sensible initialization order, I think we need a different 
strategy, that will probably require `Sema` to be a lot more careful about what 
order it instantiates variables in and what order it passes them to the AST 
consumer: if an instantiation A triggers another instantiation B, we should 
defer passing A to the consumer until B has been instantiated and passed to the 
consumer. That's probably not too hard to implement, by adding an entry to the 
pending instantiation list to say "now pass this to the consumer" in the case 
where one instantiation triggers another. But I do wonder whether that level of 
complexity is worthwhile, given that code relying on this behavior is broken.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1562-1563
   // FIXME: Type coercion of void()* types.
-  GlobalCtors.push_back(Structor(Priority, Ctor, AssociatedData));
+  if (InsertFront)
+GlobalCtors.emplace(GlobalCtors.begin(), Priority, Ctor, AssociatedData);
+  else

This is quadratic in the number of instantiated variables. I don't think that's 
acceptable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126341

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

[PATCH] D125904: [Cuda] Use fallback method to mangle externalized decls if no CUID given

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 updated this revision to Diff 432146.
jhuber6 added a comment.

Add test for #line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125904

Files:
  clang/lib/CodeGen/CGCUDANV.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGenCUDA/device-fun-linkage.cu
  clang/test/CodeGenCUDA/static-device-var-rdc.cu

Index: clang/test/CodeGenCUDA/static-device-var-rdc.cu
===
--- clang/test/CodeGenCUDA/static-device-var-rdc.cu
+++ clang/test/CodeGenCUDA/static-device-var-rdc.cu
@@ -2,12 +2,12 @@
 // REQUIRES: amdgpu-registered-target
 
 // RUN: %clang_cc1 -no-opaque-pointers -triple amdgcn-amd-amdhsa -fcuda-is-device \
-// RUN:   -std=c++11 -fgpu-rdc -emit-llvm -o - -x hip %s | FileCheck \
-// RUN:   -check-prefixes=DEV,INT-DEV %s
+// RUN:   -std=c++11 -fgpu-rdc -emit-llvm -o %t.nocuid.dev -x hip %s
+// RUN: cat %t.nocuid.dev | FileCheck -check-prefixes=DEV,INT-DEV %s
 
 // RUN: %clang_cc1 -no-opaque-pointers -triple x86_64-gnu-linux \
-// RUN:   -std=c++11 -fgpu-rdc -emit-llvm -o - -x hip %s | FileCheck \
-// RUN:   -check-prefixes=HOST,INT-HOST %s
+// RUN:   -std=c++11 -fgpu-rdc -emit-llvm -o %t.nocuid.host -x hip %s
+// RUN: cat %t.nocuid.host | FileCheck -check-prefixes=HOST,INT-HOST %s
 
 // RUN: %clang_cc1 -no-opaque-pointers -triple amdgcn-amd-amdhsa -fcuda-is-device -cuid=abc \
 // RUN:   -std=c++11 -fgpu-rdc -emit-llvm -o - -x hip %s > %t.dev
@@ -21,6 +21,7 @@
 // variable names.
 
 // RUN: cat %t.dev %t.host | FileCheck -check-prefix=POSTFIX %s
+// RUN: cat %t.nocuid.dev %t.nocuid.host | FileCheck -check-prefix=POSTFIX-ID %s
 
 // Negative tests.
 
@@ -48,6 +49,9 @@
 
 #include "Inputs/cuda.h"
 
+// Make sure we can still mangle with a line directive.
+#line 0 "-"
+
 // Test function scope static device variable, which should not be externalized.
 // DEV-DAG: @_ZZ6kernelPiPPKiE1w = internal addrspace(4) constant i32 1
 
@@ -56,8 +60,8 @@
 // HOST-DAG: @_ZL1y = internal global i32 undef
 
 // Test normal static device variables
-// INT-DEV-DAG: @_ZL1x = addrspace(1) externally_initialized global i32 0
-// INT-HOST-DAG: @[[DEVNAMEX:[0-9]+]] = {{.*}}c"_ZL1x\00"
+// INT-DEV-DAG: @_ZL1x[[FILEID:.*]] = addrspace(1) externally_initialized global i32 0
+// INT-HOST-DAG: @[[DEVNAMEX:[0-9]+]] = {{.*}}c"_ZL1x[[FILEID:.*]]\00"
 
 // Test externalized static device variables
 // EXT-DEV-DAG: @_ZL1x.static.[[HASH:.*]] = addrspace(1) externally_initialized global i32 0
@@ -66,6 +70,8 @@
 
 // POSTFIX: @_ZL1x.static.[[HASH:.*]] = addrspace(1) externally_initialized global i32 0
 // POSTFIX: @[[DEVNAMEX:[0-9]+]] = {{.*}}c"_ZL1x.static.[[HASH]]\00"
+// POSTFIX-ID: @_ZL1x.static.[[FILEID:.*]] = addrspace(1) externally_initialized global i32 0
+// POSTFIX-ID: @[[DEVNAMEX:[0-9]+]] = {{.*}}c"_ZL1x.static.[[FILEID]]\00"
 
 static __device__ int x;
 
@@ -75,8 +81,8 @@
 static __device__ int x2;
 
 // Test normal static device variables
-// INT-DEV-DAG: @_ZL1y = addrspace(4) externally_initialized global i32 0
-// INT-HOST-DAG: @[[DEVNAMEY:[0-9]+]] = {{.*}}c"_ZL1y\00"
+// INT-DEV-DAG: @_ZL1y[[FILEID:.*]] = addrspace(4) externally_initialized global i32 0
+// INT-HOST-DAG: @[[DEVNAMEY:[0-9]+]] = {{.*}}c"_ZL1y[[FILEID:.*]]\00"
 
 // Test externalized static device variables
 // EXT-DEV-DAG: @_ZL1y.static.[[HASH]] = addrspace(4) externally_initialized global i32 0
Index: clang/test/CodeGenCUDA/device-fun-linkage.cu
===
--- clang/test/CodeGenCUDA/device-fun-linkage.cu
+++ clang/test/CodeGenCUDA/device-fun-linkage.cu
@@ -23,10 +23,10 @@
 // Ensure that unused static device function is eliminated
 static __device__ void static_func() {}
 // NORDC-NEG-NOT: define{{.*}} void @_ZL13static_funcv()
-// RDC-NEG-NOT:   define{{.*}} void @_ZL13static_funcv()
+// RDC-NEG-NOT:   define{{.*}} void @_ZL13static_funcv[[FILEID:.*]]()
 
 // Ensure that kernel function has external or weak_odr
 // linkage regardless static specifier
 static __global__ void static_kernel() {}
 // NORDC: define void @_ZL13static_kernelv()
-// RDC:   define weak_odr void @_ZL13static_kernelv()
+// RDC:   define weak_odr void @_ZL13static_kernelv[[FILEID:.*]]()
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -1467,7 +1467,10 @@
   bool stopAutoInit();
 
   /// Print the postfix for externalized static variable or kernels for single
-  /// source offloading languages CUDA and HIP.
+  /// source offloading languages CUDA and HIP. The unique postfix is created
+  /// using either the CUID argument, or the file's UniqueID and active macros.
+  /// The fallback method without a CUID requires that the offloading toolchain
+  /// does not define separate macros via the -c

[PATCH] D126341: Order implicitly instantiated global variable's initializer by the reverse instantiation order

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

In D126341#3537947 , @rnk wrote:

> I'm somewhat supportive of the goal here, but I think there are still some 
> underlying issues.
>
> First, why should these guarantees be limited to instantiations and not 
> inline variables? Such as:
>
>   int f();
>   inline int gv1 = f();
>   inline int gv2 = gv1 + 1; // rely on previous

I think this is because (as discussed in cwg362, 
https://eel.is/c++draft/lex.phases#1.8) instantiations happen in instantiation 
units where no order is specified. inline variables live in TU, where the order 
is specified.

> Second, LLVM doesn't guarantee that global_ctors at the same priority execute 
> in order. See the langref: 
> https://llvm.org/docs/LangRef.html#the-llvm-global-ctors-global-variable So, 
> without a guarantee from LLVM, Clang can't rely on this behavior. LLVM relies 
> on this lack of an ordering guarantee to power globalopt.

Yeah, I noticed that too. In practice, it looks like we do rely on the order in 
llvm.global_ctors to implement the static init order (for the above inline 
variable case, https://clang.godbolt.org/z/G3YoY5bc9). If globalopt decides to 
reorder the two init functions, the result would be wrong.

> Last, what happens when the same global is implicitly instantiated in some 
> other TU? Won't that disrupt the ordering?
>
> +@alexander-shaposhnikov, who is working on global opt changes.

Hmm, that's a great point. My thoughts are it would not unless the linker is 
weird/inconsistent about which COMDAT to pick. Since for each TU, one globalvar 
has all its dependency globalvars initialized in the correct order in init 
array. Say two TUs, both have T<8>, no matter which T<8> the linker picks, it 
would pick all the T<8>'s dependencies according to the same criteria. Hence 
the init order is kept. I wouldn't say I'm 100% sure. At least it appears to me 
so at the moment. I'll do some experiments locally.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126341

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

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

[Paul Robinson via Phabricator via cfe-commits]

probinson added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5596
+  DefaultedSMFArePOD = false;
+  }
+

dblaikie wrote:
> probinson wrote:
> > Does this mean `-fclang-abi-compat` will override the PS4/Darwin special 
> > case?  I think we don't want to do that.
> I don't think so - this expression will make `DefaultedSMFArePOD` false when 
> it's already false due to the target not being PS4 or Darwin, yeah? So it'll 
> redundantly/harmlessly set it to false.
No, I mean if it *is* PS4, it will turn true into false, and that's bad.  If 
I'm reading this correctly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119051

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

[PATCH] D126289: [Clang][Driver] Fix include paths for `--sysroot /` on Linux

[Egor Zhdan via Phabricator via cfe-commits]

egorzhdan added inline comments.



Comment at: clang/include/clang/Driver/ToolChain.h:219
+
+  static std::string concat(const std::string &Path, const Twine &A,
+const Twine &B = "", const Twine &C = "",

MaskRay wrote:
> This can use `llvm::sys::path::append`
The implementation of this already uses `llvm::sys::path::append`, do you mean 
replacing the usages of `concat` with `llvm::sys::path::append`?
That would produce quite a lot of boilerplate I think, since 
`llvm::sys::path::append` modifies the path in-place, so every call to `concat` 
would expand into 3 lines of code. Also it would be very easy to forget to pass 
`Style::posix` and cause incorrect behavior on Windows (something I've stumbled 
upon while making this patch).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126289

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

[PATCH] D125904: [Cuda] Use fallback method to mangle externalized decls if no CUID given

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D125904#3538742 , @jhuber6 wrote:

> In D125904#3538714 , @tra wrote:
>
>> It would be great to have some compile-time checks for that, if possible. 
>> Otherwise it will only manifest at run-time and the end user will have no 
>> clue what's going on.
>
> Not sure how we could check it at compile-time, if we knew what it was 
> supposed to be we could just set it properly right?

We don't need to know the specific values, just that they match between the 
host and device. The host would need to have all of the expected names used by 
the registration glue matching the corresponding symbol on the GPU side. 
Extracting that symbol from the GPU binary might be tricky. Oh, well, we tried.




Comment at: clang/lib/CodeGen/CodeGenModule.cpp:6855
+  if (auto EC = llvm::sys::fs::getUniqueID(PLoc.getFilename(), ID))
+SM.getDiagnostics().Report(diag::err_cannot_open_file)
+<< PLoc.getFilename() << EC.message();

jhuber6 wrote:
> tra wrote:
> > If we're compiling an already preprocessed file with `#line` directives, I 
> > assume that the source location will be pointing to the file specified by 
> > `#line`, not the input file itself.
> > If that file is not available (and it does not have to. E.g. I someone has 
> > preprocess the original CUDA source and use creduce on the preprocessed 
> > code) then we'll get an error when we should not have.
> > 
> > It also results in a different CUIDs being generated for different 
> > identifiers, which is different from one CUID for everything generated 
> > during the compilation. I can not tell whether it matters in practice. As 
> > long as they are in sync betwee the host and the device, it should be OK 
> > and it may even have the benefit of allowing commoning things that come 
> > from headers, while driver-set common CUID would make all such instances 
> > unique.
> > 
> > 
> This should try to use the line directive first, and the current file second 
> if that's not available. The only thing that could change this for the host 
> to device is macros, but we check those currently so it should always be in 
> sync. The downside is if the user somehow only passes a macro to the device 
> side it'll somewhat silently fail when we try to register it.
> This should try to use the line directive first, and the current file second 
> if that's not available.

SGTM. Please add a test case with a bogus `#line` in it to make sure we don't 
crash on it.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125904

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

[PATCH] D125904: [Cuda] Use fallback method to mangle externalized decls if no CUID given

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

In D125904#3538714 , @tra wrote:

> It would be great to have some compile-time checks for that, if possible. 
> Otherwise it will only manifest at run-time and the end user will have no 
> clue what's going on.

Not sure how we could check it at compile-time, if we knew what it was supposed 
to be we could just set it properly right?




Comment at: clang/lib/CodeGen/CodeGenModule.cpp:6855
+  if (auto EC = llvm::sys::fs::getUniqueID(PLoc.getFilename(), ID))
+SM.getDiagnostics().Report(diag::err_cannot_open_file)
+<< PLoc.getFilename() << EC.message();

tra wrote:
> If we're compiling an already preprocessed file with `#line` directives, I 
> assume that the source location will be pointing to the file specified by 
> `#line`, not the input file itself.
> If that file is not available (and it does not have to. E.g. I someone has 
> preprocess the original CUDA source and use creduce on the preprocessed code) 
> then we'll get an error when we should not have.
> 
> It also results in a different CUIDs being generated for different 
> identifiers, which is different from one CUID for everything generated during 
> the compilation. I can not tell whether it matters in practice. As long as 
> they are in sync betwee the host and the device, it should be OK and it may 
> even have the benefit of allowing commoning things that come from headers, 
> while driver-set common CUID would make all such instances unique.
> 
> 
This should try to use the line directive first, and the current file second if 
that's not available. The only thing that could change this for the host to 
device is macros, but we check those currently so it should always be in sync. 
The downside is if the user somehow only passes a macro to the device side 
it'll somewhat silently fail when we try to register it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125904

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

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

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

I'm very happy with how this patch is looking.




Comment at: clang/include/clang/Sema/DeclSpec.h:1926-1928
+assert(llvm::all_of(DeclarationAttrs, [](const ParsedAttr &AL) {
+  return AL.isStandardAttributeSyntax();
+}));

rsmith wrote:
> I think I recall seeing OpenMP pragma attributes being parsed as declaration 
> attributes too. FIXME update this comment after review is complete
Oops, forgot to circle back to this. I think I was wrong; please disregard :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126061

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

[PATCH] D126289: [Clang][Driver] Fix include paths for `--sysroot /` on Linux

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/include/clang/Driver/ToolChain.h:219
+
+  static std::string concat(const std::string &Path, const Twine &A,
+const Twine &B = "", const Twine &C = "",

This can use `llvm::sys::path::append`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126289

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

[PATCH] D125904: [Cuda] Use fallback method to mangle externalized decls if no CUID given

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D125904#3538163 , @yaxunl wrote:

> I am OK with this patch.

OK.

> That said, this patch provided a default CUID that do not depend on driver, 
> which is its advantage. It should be OK for most usecases.

I agree with this part.

> Driver provided CUID has more robustness, so it can serve as last resort.

I'd argue that it should be the default and, as such, the only mechanism for 
CUID generation. Doing it both in the driver and the codegen looks odd to me.

> If the fallback is not sufficient for the new driver then we can revisit this.

It would be great to have some compile-time checks for that, if possible. 
Otherwise it will only manifest at run-time and the end user will have no clue 
what's going on.




Comment at: clang/lib/CodeGen/CodeGenModule.cpp:6855
+  if (auto EC = llvm::sys::fs::getUniqueID(PLoc.getFilename(), ID))
+SM.getDiagnostics().Report(diag::err_cannot_open_file)
+<< PLoc.getFilename() << EC.message();

If we're compiling an already preprocessed file with `#line` directives, I 
assume that the source location will be pointing to the file specified by 
`#line`, not the input file itself.
If that file is not available (and it does not have to. E.g. I someone has 
preprocess the original CUDA source and use creduce on the preprocessed code) 
then we'll get an error when we should not have.

It also results in a different CUIDs being generated for different identifiers, 
which is different from one CUID for everything generated during the 
compilation. I can not tell whether it matters in practice. As long as they are 
in sync betwee the host and the device, it should be OK and it may even have 
the benefit of allowing commoning things that come from headers, while 
driver-set common CUID would make all such instances unique.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125904

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

[PATCH] D126331: [OpenMP] Add diagnostic for unterminated 'omp [begin] declare target'

[Mike Rice via Phabricator via cfe-commits]

mikerice added a comment.

In D126331#3538670 , @jdoerfert wrote:

> Cool! Can we have that for begin declare variant too :D ?

I'll take a look when I get a moment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126331

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

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

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: clang/include/clang/Sema/DeclSpec.h:1926-1928
+assert(llvm::all_of(DeclarationAttrs, [](const ParsedAttr &AL) {
+  return AL.isStandardAttributeSyntax();
+}));

I think I recall seeing OpenMP pragma attributes being parsed as declaration 
attributes too. FIXME update this comment after review is complete



Comment at: clang/lib/Parse/ParseDecl.cpp:1854
+ProhibitAttributes(DeclAttrs);
+ProhibitAttributes(OriginalDeclSpecAttrs);
 DeclEnd = Tok.getLocation();

This looks wrong to me (both before and after this patch; you've faithfully 
retained an incorrect behavior). If `DeclSpec` attributes aren't allowed, they 
should be diagnosed by `ParsedFreeStandingDeclSpec`. Before and after this 
patch, we'll diagnose attributes in a free-standing decl-specifier-seq if we 
happened to tentatively parse them, not if we happened to parse them in 
`ParseDeclarationSpecifiers`. For example:

```
__attribute__(()) struct X; // DeclSpecAttrs will be empty, no error
void f() {
  __attribute(()) struct X; // DeclSpecAttrs will contain the attribute 
specifier, error
}
```

Comparing to GCC's behavior for that example (which GCC silently accepts) and 
for this one:

```
__attribute__((packed)) struct X; // GCC warns that packed has no meaning here, 
clang produces a high-quality warning.
void f() {
  __attribute((packed)) struct X; // GCC warns that packed has no meaning here, 
clang produces a bogus error.
}
```

... I think the right thing to do is to delete this call (along with 
`OriginalDeclSpecAttrs`). GNU decl-specifier attributes *are* apparently 
syntactically permitted here, but most (maybe even all?) GNU attributes don't 
have any meaning in this position.



Comment at: clang/lib/Parse/ParseDeclCXX.cpp:2691-2695
+  // We need to keep these attributes for future diagnostics
+  // before they are taken over by the declaration.
+  ParsedAttributesView FnAttrs;
+  FnAttrs.addAll(DeclAttrs.begin(), DeclAttrs.end());
+  FnAttrs.Range = DeclAttrs.Range;

Oh, neat, I think this copy isn't necessary any more, given that we don't give 
ownership of the `DeclAttrs` to the `ParsingDeclarator` any more, and don't mix 
the declaration attributes and the `DeclSpec` attributes together any more. We 
should be able to just use `DeclAttrs` instead of `FnAttrs` below now, I think?



Comment at: clang/lib/Parse/ParseExprCXX.cpp:2745-2746
   // ptr-operators.
-  Declarator D(DS, DeclaratorContext::ConversionId);
+  ParsedAttributes EmptyDeclAttrs(AttrFactory);
+  Declarator D(DS, EmptyDeclAttrs, DeclaratorContext::ConversionId);
   ParseDeclaratorInternal(D, /*DirectDeclParser=*/nullptr);

Given the number of times this has come up, I wonder if it's worth adding 
something like:

```
class ParsedAttributes {
public:
  // ...
  static const ParsedAttributes &none() {
static AttributeFactory Factory;
static AttributePool Pool(Factory);
static const ParsedAttributes Attrs(Pool);
return Attrs;
  }
  // ...
};
```
(or, better, changing `Declarator` to hold a `const ParsedAttributesView&` 
rather than a `const ParsedAttributes&`) so that we can write
```
Declarator D(DS, ParsedAttributes::none(), DeclaratorContext::ConversionId);
```

Totally optional, though, and I'm happy for this cleanup to happen at some 
later point in time.



Comment at: clang/lib/Parse/ParseStmt.cpp:16
 #include "clang/Basic/Attributes.h"
+#include "clang/Basic/DiagnosticSema.h"
 #include "clang/Basic/PrettyStackTrace.h"

This header is intended to be private to Sema. If you want to use a diagnostic 
in both the parser and sema, please move it to `DiagnosticCommonKinds.td`.



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

mboehme wrote:
> aaron.ballman wrote:
> > rsmith wrote:
> > > I think this is the only place where we're passing decl-specifier-seq 
> > > attributes into `ParseDeclaration`. There are only two possible cases 
> > > here:
> > > 
> > > 1) We have a simple-declaration, and can `ParseSimpleDeclaration` 
> > > directly.
> > > 2) We have a static-assert, using, or namespace alias declaration, which 
> > > don't permit attributes at all.
> > > 
> > > So I think we *could* simplify this so that decl-spec attributes are 
> > > never passed into `ParseDeclaration`:
> > > 
> > > * If the next token is `kw_static_assert`, `kw_using`, or `kw_namespace`, 
> > > then prohibit attributes and call `ParseDeclaration`.
> > > * Otherwise

[PATCH] D126331: [OpenMP] Add diagnostic for unterminated 'omp [begin] declare target'

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

Cool! Can we have that for begin declare variant too :D ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126331

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

[PATCH] D102122: Support warn_unused_result on typedefs

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Sorry, with all the layers in the previous messages I'm not sure what the 
summary is. Sounds like the summary is "this is OK to continue work in the 
direction of supporting [[clang::warn_unused_result]] (only that spelling) for 
typedefs in C and C++ (for compatibility with C/so that C headers, like LLVM 
C's APIs can still be parsed as C++), while considering some of the 
complications raised in the WG21 discussion"?

OK, so I updated this patch with a very first draft/attempt at that - I wasn't 
sure how to add typedef support to only the clang spelling - so I created a new 
attribute name in the .td file (is there a better/more suitable way?) and that 
seems to have created one follow-on issue (partly because of my other choice: 
Clang still only /instantiates WarnUnusedResultAttr class, not the new 
WarnUnusedResultClangAttr class - even for the clang spelling, it uses the 
non-clang attribute class, so /most/ of the code looking at the attribute class 
type continues to work) - that issue is in  SemaCXX/cxx11-attr-print.cpp it 
doesn't print the attribute spelling correctly, it chooses the C++ standard 
spelling when trying to reproduce the Clang spelling, I guess because the 
ClangAttr class isn't used. (but of course if I change the code to actually 
instantiate the WarnUnusedResultClangAttr class - then all the existing code 
that handles the attribute class/implements the warning has to be updated to 
test both attribute classes... )

Is there another way I should be approaching this?

(oh, and there was something about a supported version number for the `has 
attribute` checking - where would that factor into all this?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102122

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

[PATCH] D102122: Support warn_unused_result on typedefs

[David Blaikie via Phabricator via cfe-commits]

dblaikie updated this revision to Diff 432131.
dblaikie added a comment.

Separate out clang:warn_unused_result so typedef support can be added to only 
that spelling.

This has one lingering issue (because it still currently instantiates both the 
clang:warn_unused_result as the same *Attr class as the others - and so 
SemaCXX/cxx11-attr-print.cpp is failing because the AST printing for the clang 
spelling decides to use the C++ standard spelling, I guess because the 
non-clang Attr class doesn't realize it was spelled with the clang spelling)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102122

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/AST/Expr.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaCXX/warn-unused-result.cpp


Index: clang/test/SemaCXX/warn-unused-result.cpp
===
--- clang/test/SemaCXX/warn-unused-result.cpp
+++ clang/test/SemaCXX/warn-unused-result.cpp
@@ -254,3 +254,15 @@
 
 void i([[nodiscard]] bool (*fp)()); // expected-warning {{'nodiscard' 
attribute only applies to functions, classes, or enumerations}}
 }
+
+namespace unused_typedef_result {
+[[clang::warn_unused_result]] typedef void *p;
+p f1();
+void f2() {
+  f1(); // expected-warning {{ignoring return value}}
+  void *(*p1)();
+  p1(); // no warning
+  p (*p2)();
+  p2(); // expected-warning {{ignoring return value}}
+}
+}
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
@@ -186,6 +186,7 @@
 // CHECK-NEXT: VecTypeHint (SubjectMatchRule_function)
 // CHECK-NEXT: WarnUnused (SubjectMatchRule_record)
 // CHECK-NEXT: WarnUnusedResult (SubjectMatchRule_objc_method, 
SubjectMatchRule_enum, SubjectMatchRule_record, 
SubjectMatchRule_hasType_functionType)
+// CHECK-NEXT: WarnUnusedResultClang (SubjectMatchRule_objc_method, 
SubjectMatchRule_enum, SubjectMatchRule_record, 
SubjectMatchRule_hasType_functionType, SubjectMatchRule_type_alias)
 // CHECK-NEXT: Weak (SubjectMatchRule_variable, SubjectMatchRule_function, 
SubjectMatchRule_record)
 // CHECK-NEXT: WeakRef (SubjectMatchRule_variable, SubjectMatchRule_function)
 // CHECK-NEXT: WebAssemblyExportName (SubjectMatchRule_function)
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -8740,6 +8740,9 @@
   case ParsedAttr::AT_WarnUnusedResult:
 handleWarnUnusedResult(S, D, AL);
 break;
+  case ParsedAttr::AT_WarnUnusedResultClang:
+handleWarnUnusedResult(S, D, AL);
+break;
   case ParsedAttr::AT_WeakRef:
 handleWeakRefAttr(S, D, AL);
 break;
Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -1522,6 +1522,10 @@
 if (const auto *A = TD->getAttr())
   return A;
 
+  if (const auto *TD = getCallReturnType(Ctx)->getAs())
+if (const auto *A = TD->getDecl()->getAttr())
+  return A;
+
   // Otherwise, see if the callee is marked nodiscard and return that attribute
   // instead.
   const Decl *D = getCalleeDecl();
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -2939,10 +2939,16 @@
   let SimpleHandler = 1;
 }
 
+def WarnUnusedResultClang : InheritableAttr {
+  let Spellings = [CXX11<"clang", "warn_unused_result">];
+  let Subjects = SubjectList<[ObjCMethod, Enum, Record, FunctionLike, 
TypedefName]>;
+  let Args = [StringArgument<"Message", 1>];
+  let Documentation = [WarnUnusedResultsDocs];
+}
+
 def WarnUnusedResult : InheritableAttr {
   let Spellings = [CXX11<"", "nodiscard", 201907>,
C2x<"", "nodiscard", 201904>,
-   CXX11<"clang", "warn_unused_result">,
GCC<"warn_unused_result">];
   let Subjects = SubjectList<[ObjCMethod, Enum, Record, FunctionLike]>;
   let Args = [StringArgument<"Message", 1>];


Index: clang/test/SemaCXX/warn-unused-result.cpp
===
--- clang/test/SemaCXX/warn-unused-result.cpp
+++ clang/test/SemaCXX/warn-unused-result.cpp
@@ -254,3 +254,15 @@
 
 void i([[nodiscard]] bool (*fp)()); // expected-warning {{'nodiscard' attribute only applies to functions, classes, or enumerations}}
 }
+
+namespace unused_typedef_result {
+[[clang::warn_unused_result]] typedef void *p;
+p f1();
+void f2() {
+  f1(); // expected-warning {{ignoring return value}}
+  void *(*p1)();
+  p1(); // no warning
+  p (*p2)();
+

[PATCH] D120244: [clang][sema] Enable first-class bool support for C2x

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D120244#3538380 , @ldionne wrote:

> We've started having several internal user complaints because their system 
> headers are suddenly triggering a warning for using ``. This seems 
> to be caused by the fact that `#warning` is surfaced to users even when the 
> `#warning` is present in a system header (which makes sense, because 
> otherwise there would be no way to issue a `#warning` from a system header). 
> This ends up causing problems because users have no way to suppress the 
> warning in the system headers they use without also disabling deprecation 
> warnings in their own code. Is this intended? Instead, it seems to me like 
> what we'd want is some way to mark the header as deprecated such that Clang 
> will not flag uses of `` from within system headers, but will flag 
> them from user code. This would be consistent with how the deprecated 
> attributes work for classes and functions.
>
> Thoughts?

This is what `_CLANG_DISABLE_CRT_DEPRECATION_WARNINGS` is for. We documented it 
here: 
https://clang.llvm.org/docs/UsersManual.html#controlling-deprecation-diagnostics-in-clang-provided-c-runtime-headers
 but the basic idea is, if that's defined before including the header, we don't 
issue those warnings. Does that suffice?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120244

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

[PATCH] D126406: [analyzer] Return from reAssume if State is posteriorly overconstrained

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

Thanks for the quick response!




Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:127
+
+public:
   bool isPosteriorlyOverconstrained() const {

This shouldnt be the way.
Consider fwd declaring and making it friend instead.
I really dont want to expose this api.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2547
   if (Constraint->encodesFalseRange())
 return State->assume(DefinedVal, false);
 

martong wrote:
> I am wondering, that maybe it would be better to check for 
> `isPosteriorlyOverconstrained` here. Because only `State->assume` can return 
> such States, and by checking it here, we could spare some instructions.
Play with it.



Comment at: clang/test/Analysis/runtime-regression.c:7
+// This test is here to check if there is no significant run-time regression
+// related to the assume machinery. This is an automatically reduced code. The
+// analysis should finish in less than 10 seconds.

Sadly, I had to manually track this down xD.



Comment at: clang/test/Analysis/runtime-regression.c:8
+// related to the assume machinery. This is an automatically reduced code. The
+// analysis should finish in less than 10 seconds.
+

Maybe the test infra has something to specify a timeout.



Comment at: clang/test/Analysis/runtime-regression.c:12
+
+typedef unsigned char uint8_t;
+typedef unsigned short uint16_t;

Pin the tartget triple.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126406

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

[PATCH] D126420: [clang][dataflow] Remove private-field filtering from `StorageLocation` creation.

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:407
 
 for (const FieldDecl *Field : getAccessibleObjectFields(Type)) {
   assert(Field != nullptr);

Why not use `getObjectFields` here too?



Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:523
 llvm::DenseMap FieldValues;
 for (const FieldDecl *Field : getAccessibleObjectFields(Type)) {
   assert(Field != nullptr);

Why not use `getObjectFields` here too?



Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:1252-1253
+   ASTContext &ASTCtx) {
+// The purpose of this test is to verify non-crashing behavior for the
+// analysis of the access to `Foo.Bar`. So, we
+EXPECT_THAT(Results, ElementsAre(Pair("p", _)));

Might as well check that `Foo.Bar` is assigned a value like the 
`DerivedBaseMemberClass` test case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126420

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

[PATCH] D126413: [clang][dataflow] Fix incorrect CXXThisExpr pointee for lambdas

[Eric Li 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 rG5520c5839016: [clang][dataflow] Fix incorrect CXXThisExpr 
pointee for lambdas (authored by li.zhe.hua).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126413

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

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1476,6 +1476,112 @@
   });
 }
 
+TEST_F(TransferTest, StructThisInLambda) {
+  std::string ThisCaptureCode = R"(
+struct A {
+  void frob() {
+[this]() {
+  int Foo = Bar;
+  // [[p1]]
+}();
+  }
+
+  int Bar;
+};
+  )";
+  runDataflow(
+  ThisCaptureCode,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p1", _)));
+const Environment &Env = Results[0].second.Env;
+
+const auto *ThisLoc = dyn_cast(
+Env.getThisPointeeStorageLocation());
+ASSERT_THAT(ThisLoc, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const auto *BarLoc =
+cast(&ThisLoc->getChild(*BarDecl));
+ASSERT_TRUE(isa_and_nonnull(BarLoc));
+
+const Value *BarVal = Env.getValue(*BarLoc);
+ASSERT_TRUE(isa_and_nonnull(BarVal));
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+EXPECT_EQ(Env.getValue(*FooDecl, SkipPast::None), BarVal);
+  },
+  LangStandard::lang_cxx17, /*ApplyBuiltinTransfer=*/true, "operator()");
+
+  std::string RefCaptureDefaultCode = R"(
+struct A {
+  void frob() {
+[&]() {
+  int Foo = Bar;
+  // [[p2]]
+}();
+  }
+
+  int Bar;
+};
+  )";
+  runDataflow(
+  RefCaptureDefaultCode,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p2", _)));
+const Environment &Env = Results[0].second.Env;
+
+const auto *ThisLoc = dyn_cast(
+Env.getThisPointeeStorageLocation());
+ASSERT_THAT(ThisLoc, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const auto *BarLoc =
+cast(&ThisLoc->getChild(*BarDecl));
+ASSERT_TRUE(isa_and_nonnull(BarLoc));
+
+const Value *BarVal = Env.getValue(*BarLoc);
+ASSERT_TRUE(isa_and_nonnull(BarVal));
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+EXPECT_EQ(Env.getValue(*FooDecl, SkipPast::None), BarVal);
+  },
+  LangStandard::lang_cxx17, /*ApplyBuiltinTransfer=*/true, "operator()");
+
+  std::string FreeFunctionLambdaCode = R"(
+void foo() {
+  int Bar;
+  [&]() {
+int Foo = Bar;
+// [[p3]]
+  }();
+}
+  )";
+  runDataflow(
+  FreeFunctionLambdaCode,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p3", _)));
+const Environment &Env = Results[0].second.Env;
+
+EXPECT_THAT(Env.getThisPointeeStorageLocation(), IsNull());
+  },
+  LangStandard::lang_cxx17, /*ApplyBuiltinTransfer=*/true, "operator()");
+}
+
 TEST_F(TransferTest, ConstructorInitializer) {
   std::string Code = R"(
 struct target {
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -216,7 +216,12 @@
   }
 
   if (const auto *MethodDecl = dyn_cast(&DeclCtx)) {
-if (!MethodDecl->isStatic()) {
+auto *Parent = MethodDecl->getParent();
+assert(Parent != nullptr);
+if (Parent->isLambda())
+  MethodDecl = dyn_cast(Parent->getDeclContext());
+
+if (MethodDecl && !MethodDecl->isStatic()) {
   QualType ThisPointeeType = MethodDecl->getThisObjectType();
   // FIXME: Add support for union types.
   if (!ThisPointeeType->isUnionType()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126405: [clang][dataflow] Relax assert on existence of `this` pointee storage

[Eric Li 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 rG33b598a808b9: [clang][dataflow] Relax assert on existence of 
`this` pointee storage (authored by li.zhe.hua).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126405

Files:
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -42,10 +42,11 @@
   template 
   void runDataflow(llvm::StringRef Code, Matcher Match,
LangStandard::Kind Std = LangStandard::lang_cxx17,
-   bool ApplyBuiltinTransfer = true) {
+   bool ApplyBuiltinTransfer = true,
+   llvm::StringRef TargetFun = "target") {
 ASSERT_THAT_ERROR(
 test::checkDataflow(
-Code, "target",
+Code, TargetFun,
 [ApplyBuiltinTransfer](ASTContext &C, Environment &) {
   return NoopAnalysis(C, ApplyBuiltinTransfer);
 },
@@ -3175,4 +3176,27 @@
 });
 }
 
+TEST_F(TransferTest, DoesNotCrashOnUnionThisExpr) {
+  std::string Code = R"(
+union Union {
+  int A;
+  float B;
+};
+
+void foo() {
+  Union A;
+  Union B;
+  A = B;
+}
+  )";
+  // This is a crash regression test when calling the transfer function on a
+  // `CXXThisExpr` that refers to a union.
+  runDataflow(
+  Code,
+  [](llvm::ArrayRef<
+ std::pair>>,
+ ASTContext &) {},
+  LangStandard::lang_cxx17, /*ApplyBuiltinTransfer=*/true, "operator=");
+}
+
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -279,7 +279,10 @@
 
   void VisitCXXThisExpr(const CXXThisExpr *S) {
 auto *ThisPointeeLoc = Env.getThisPointeeStorageLocation();
-assert(ThisPointeeLoc != nullptr);
+if (ThisPointeeLoc == nullptr)
+  // Unions are not supported yet, and will not have a location for the
+  // `this` expression's pointee.
+  return;
 
 auto &Loc = Env.createStorageLocation(*S);
 Env.setStorageLocation(*S, Loc);


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -42,10 +42,11 @@
   template 
   void runDataflow(llvm::StringRef Code, Matcher Match,
LangStandard::Kind Std = LangStandard::lang_cxx17,
-   bool ApplyBuiltinTransfer = true) {
+   bool ApplyBuiltinTransfer = true,
+   llvm::StringRef TargetFun = "target") {
 ASSERT_THAT_ERROR(
 test::checkDataflow(
-Code, "target",
+Code, TargetFun,
 [ApplyBuiltinTransfer](ASTContext &C, Environment &) {
   return NoopAnalysis(C, ApplyBuiltinTransfer);
 },
@@ -3175,4 +3176,27 @@
 });
 }
 
+TEST_F(TransferTest, DoesNotCrashOnUnionThisExpr) {
+  std::string Code = R"(
+union Union {
+  int A;
+  float B;
+};
+
+void foo() {
+  Union A;
+  Union B;
+  A = B;
+}
+  )";
+  // This is a crash regression test when calling the transfer function on a
+  // `CXXThisExpr` that refers to a union.
+  runDataflow(
+  Code,
+  [](llvm::ArrayRef<
+ std::pair>>,
+ ASTContext &) {},
+  LangStandard::lang_cxx17, /*ApplyBuiltinTransfer=*/true, "operator=");
+}
+
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -279,7 +279,10 @@
 
   void VisitCXXThisExpr(const CXXThisExpr *S) {
 auto *ThisPointeeLoc = Env.getThisPointeeStorageLocation();
-assert(ThisPointeeLoc != nullptr);
+if (ThisPointeeLoc == nullptr)
+  // Unions are not supported yet, and will not have a location for the
+  // `this` expression's pointee.
+  return;
 
 auto &Loc = Env.createStorageLocation(*S);
 Env.setStorageLocation(*S, Loc);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 5520c58 - [clang][dataflow] Fix incorrect CXXThisExpr pointee for lambdas

[Eric Li via cfe-commits]

Author: Eric Li
Date: 2022-05-25T20:58:02Z
New Revision: 5520c5839016d1d98d8e01789eb17c910381bd6f

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

LOG: [clang][dataflow] Fix incorrect CXXThisExpr pointee for lambdas

When constructing the `Environment`, the `this` pointee is established
for a `CXXMethodDecl` by looking at its parent. However, inside of
lambdas, a `CXXThisExpr` refers to the captured `this` coming from the
enclosing member function.

When establishing the `this` pointee for a function, we check whether
the function is a lambda, and check for an enclosing member function
to establish the `this` pointee storage location.

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

Added: 


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

Removed: 




diff  --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp 
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index e555bee0e8bf..fe573bbf9f37 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -216,7 +216,12 @@ Environment::Environment(DataflowAnalysisContext &DACtx,
   }
 
   if (const auto *MethodDecl = dyn_cast(&DeclCtx)) {
-if (!MethodDecl->isStatic()) {
+auto *Parent = MethodDecl->getParent();
+assert(Parent != nullptr);
+if (Parent->isLambda())
+  MethodDecl = dyn_cast(Parent->getDeclContext());
+
+if (MethodDecl && !MethodDecl->isStatic()) {
   QualType ThisPointeeType = MethodDecl->getThisObjectType();
   // FIXME: Add support for union types.
   if (!ThisPointeeType->isUnionType()) {

diff  --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp 
b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index bb6e269969da..bed6b975974c 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1476,6 +1476,112 @@ TEST_F(TransferTest, ClassThisMember) {
   });
 }
 
+TEST_F(TransferTest, StructThisInLambda) {
+  std::string ThisCaptureCode = R"(
+struct A {
+  void frob() {
+[this]() {
+  int Foo = Bar;
+  // [[p1]]
+}();
+  }
+
+  int Bar;
+};
+  )";
+  runDataflow(
+  ThisCaptureCode,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p1", _)));
+const Environment &Env = Results[0].second.Env;
+
+const auto *ThisLoc = dyn_cast(
+Env.getThisPointeeStorageLocation());
+ASSERT_THAT(ThisLoc, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const auto *BarLoc =
+cast(&ThisLoc->getChild(*BarDecl));
+ASSERT_TRUE(isa_and_nonnull(BarLoc));
+
+const Value *BarVal = Env.getValue(*BarLoc);
+ASSERT_TRUE(isa_and_nonnull(BarVal));
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+EXPECT_EQ(Env.getValue(*FooDecl, SkipPast::None), BarVal);
+  },
+  LangStandard::lang_cxx17, /*ApplyBuiltinTransfer=*/true, "operator()");
+
+  std::string RefCaptureDefaultCode = R"(
+struct A {
+  void frob() {
+[&]() {
+  int Foo = Bar;
+  // [[p2]]
+}();
+  }
+
+  int Bar;
+};
+  )";
+  runDataflow(
+  RefCaptureDefaultCode,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p2", _)));
+const Environment &Env = Results[0].second.Env;
+
+const auto *ThisLoc = dyn_cast(
+Env.getThisPointeeStorageLocation());
+ASSERT_THAT(ThisLoc, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const auto *BarLoc =
+cast(&ThisLoc->getChild(*BarDecl));
+ASSERT_TRUE(isa_and_nonnull(BarLoc));
+
+const Value *BarVal = Env.getValue(*BarLoc);
+ASSERT_TRUE(isa_and_nonnull(BarVal));
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+EXPECT_EQ(Env.getValue(*FooDecl, SkipPast::None), BarVal);
+  },
+  LangStandard::lang_cxx17, /*ApplyBuiltinTransfer=*/true, "operator()");
+
+  std::string FreeFunctionLambdaCode = R"(
+void foo() {
+  int Bar;
+  [&]() {
+int Foo = Bar;
+// [[p3]]
+  }();
+}
+  )";
+  runDataflow(
+  FreeFunctionLambdaCode,
+  [](llvm::Ar

[clang] 33b598a - [clang][dataflow] Relax assert on existence of `this` pointee storage

[Eric Li via cfe-commits]

Author: Eric Li
Date: 2022-05-25T20:58:02Z
New Revision: 33b598a808b9ef1a019e798f19855f203e09ad48

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

LOG: [clang][dataflow] Relax assert on existence of `this` pointee storage

Support for unions is incomplete (per 99f7d55e) and the `this` pointee
storage location is not set for unions. The assert in
`VisitCXXThisExpr` is then guaranteed to trigger when analyzing member
functions of a union.

This commit changes the assert to an early-return. Any expression may
be undefined, and so having a value for the `CXXThisExpr` is not a
postcondition of the transfer function.

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

Added: 


Modified: 
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Removed: 




diff  --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp 
b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index f88406595d728..fc04e0b55ffc7 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -279,7 +279,10 @@ class TransferVisitor : public 
ConstStmtVisitor {
 
   void VisitCXXThisExpr(const CXXThisExpr *S) {
 auto *ThisPointeeLoc = Env.getThisPointeeStorageLocation();
-assert(ThisPointeeLoc != nullptr);
+if (ThisPointeeLoc == nullptr)
+  // Unions are not supported yet, and will not have a location for the
+  // `this` expression's pointee.
+  return;
 
 auto &Loc = Env.createStorageLocation(*S);
 Env.setStorageLocation(*S, Loc);

diff  --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp 
b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index b6d2940a98daa..bb6e269969da7 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -42,10 +42,11 @@ class TransferTest : public ::testing::Test {
   template 
   void runDataflow(llvm::StringRef Code, Matcher Match,
LangStandard::Kind Std = LangStandard::lang_cxx17,
-   bool ApplyBuiltinTransfer = true) {
+   bool ApplyBuiltinTransfer = true,
+   llvm::StringRef TargetFun = "target") {
 ASSERT_THAT_ERROR(
 test::checkDataflow(
-Code, "target",
+Code, TargetFun,
 [ApplyBuiltinTransfer](ASTContext &C, Environment &) {
   return NoopAnalysis(C, ApplyBuiltinTransfer);
 },
@@ -3175,4 +3176,27 @@ TEST_F(TransferTest, 
LoopWithStructReferenceAssignmentConverges) {
 });
 }
 
+TEST_F(TransferTest, DoesNotCrashOnUnionThisExpr) {
+  std::string Code = R"(
+union Union {
+  int A;
+  float B;
+};
+
+void foo() {
+  Union A;
+  Union B;
+  A = B;
+}
+  )";
+  // This is a crash regression test when calling the transfer function on a
+  // `CXXThisExpr` that refers to a union.
+  runDataflow(
+  Code,
+  [](llvm::ArrayRef<
+ std::pair>>,
+ ASTContext &) {},
+  LangStandard::lang_cxx17, /*ApplyBuiltinTransfer=*/true, "operator=");
+}
+
 } // namespace



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

[PATCH] D125936: [Sema] Relax an assertion in BuildStmtExpr

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

ping

I think the assertion is assuming `StmtExpr` is created only for GNU statement 
expressions, but it's created for asm statements too (see 
https://github.com/llvm/llvm-project/commit/3050d9bdb84e322e122219d54aedfe2d8ef7c51c).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125936

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

[PATCH] D126413: [clang][dataflow] Fix incorrect CXXThisExpr pointee for lambdas

[Eric Li via Phabricator via cfe-commits]

li.zhe.hua updated this revision to Diff 432124.
li.zhe.hua added a comment.

Address reviewer comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126413

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

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1476,6 +1476,112 @@
   });
 }
 
+TEST_F(TransferTest, StructThisInLambda) {
+  std::string ThisCaptureCode = R"(
+struct A {
+  void frob() {
+[this]() {
+  int Foo = Bar;
+  // [[p1]]
+}();
+  }
+
+  int Bar;
+};
+  )";
+  runDataflow(
+  ThisCaptureCode,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p1", _)));
+const Environment &Env = Results[0].second.Env;
+
+const auto *ThisLoc = dyn_cast(
+Env.getThisPointeeStorageLocation());
+ASSERT_THAT(ThisLoc, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const auto *BarLoc =
+cast(&ThisLoc->getChild(*BarDecl));
+ASSERT_TRUE(isa_and_nonnull(BarLoc));
+
+const Value *BarVal = Env.getValue(*BarLoc);
+ASSERT_TRUE(isa_and_nonnull(BarVal));
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+EXPECT_EQ(Env.getValue(*FooDecl, SkipPast::None), BarVal);
+  },
+  LangStandard::lang_cxx17, /*ApplyBuiltinTransfer=*/true, "operator()");
+
+  std::string RefCaptureDefaultCode = R"(
+struct A {
+  void frob() {
+[&]() {
+  int Foo = Bar;
+  // [[p2]]
+}();
+  }
+
+  int Bar;
+};
+  )";
+  runDataflow(
+  RefCaptureDefaultCode,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p2", _)));
+const Environment &Env = Results[0].second.Env;
+
+const auto *ThisLoc = dyn_cast(
+Env.getThisPointeeStorageLocation());
+ASSERT_THAT(ThisLoc, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const auto *BarLoc =
+cast(&ThisLoc->getChild(*BarDecl));
+ASSERT_TRUE(isa_and_nonnull(BarLoc));
+
+const Value *BarVal = Env.getValue(*BarLoc);
+ASSERT_TRUE(isa_and_nonnull(BarVal));
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+EXPECT_EQ(Env.getValue(*FooDecl, SkipPast::None), BarVal);
+  },
+  LangStandard::lang_cxx17, /*ApplyBuiltinTransfer=*/true, "operator()");
+
+  std::string FreeFunctionLambdaCode = R"(
+void foo() {
+  int Bar;
+  [&]() {
+int Foo = Bar;
+// [[p3]]
+  }();
+}
+  )";
+  runDataflow(
+  FreeFunctionLambdaCode,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p3", _)));
+const Environment &Env = Results[0].second.Env;
+
+EXPECT_THAT(Env.getThisPointeeStorageLocation(), IsNull());
+  },
+  LangStandard::lang_cxx17, /*ApplyBuiltinTransfer=*/true, "operator()");
+}
+
 TEST_F(TransferTest, ConstructorInitializer) {
   std::string Code = R"(
 struct target {
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -216,7 +216,12 @@
   }
 
   if (const auto *MethodDecl = dyn_cast(&DeclCtx)) {
-if (!MethodDecl->isStatic()) {
+auto *Parent = MethodDecl->getParent();
+assert(Parent != nullptr);
+if (Parent->isLambda())
+  MethodDecl = dyn_cast(Parent->getDeclContext());
+
+if (MethodDecl && !MethodDecl->isStatic()) {
   QualType ThisPointeeType = MethodDecl->getThisObjectType();
   // FIXME: Add support for union types.
   if (!ThisPointeeType->isUnionType()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126405: [clang][dataflow] Relax assert on existence of `this` pointee storage

[Eric Li via Phabricator via cfe-commits]

li.zhe.hua updated this revision to Diff 432119.
li.zhe.hua added a comment.

Accidentally a word...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126405

Files:
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -42,10 +42,11 @@
   template 
   void runDataflow(llvm::StringRef Code, Matcher Match,
LangStandard::Kind Std = LangStandard::lang_cxx17,
-   bool ApplyBuiltinTransfer = true) {
+   bool ApplyBuiltinTransfer = true,
+   llvm::StringRef TargetFun = "target") {
 ASSERT_THAT_ERROR(
 test::checkDataflow(
-Code, "target",
+Code, TargetFun,
 [ApplyBuiltinTransfer](ASTContext &C, Environment &) {
   return NoopAnalysis(C, ApplyBuiltinTransfer);
 },
@@ -3175,4 +3176,27 @@
 });
 }
 
+TEST_F(TransferTest, DoesNotCrashOnUnionThisExpr) {
+  std::string Code = R"(
+union Union {
+  int A;
+  float B;
+};
+
+void foo() {
+  Union A;
+  Union B;
+  A = B;
+}
+  )";
+  // This is a crash regression test when calling the transfer function on a
+  // `CXXThisExpr` that refers to a union.
+  runDataflow(
+  Code,
+  [](llvm::ArrayRef<
+ std::pair>>,
+ ASTContext &) {},
+  LangStandard::lang_cxx17, /*ApplyBuiltinTransfer=*/true, "operator=");
+}
+
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -279,7 +279,10 @@
 
   void VisitCXXThisExpr(const CXXThisExpr *S) {
 auto *ThisPointeeLoc = Env.getThisPointeeStorageLocation();
-assert(ThisPointeeLoc != nullptr);
+if (ThisPointeeLoc == nullptr)
+  // Unions are not supported yet, and will not have a location for the
+  // `this` expression's pointee.
+  return;
 
 auto &Loc = Env.createStorageLocation(*S);
 Env.setStorageLocation(*S, Loc);


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -42,10 +42,11 @@
   template 
   void runDataflow(llvm::StringRef Code, Matcher Match,
LangStandard::Kind Std = LangStandard::lang_cxx17,
-   bool ApplyBuiltinTransfer = true) {
+   bool ApplyBuiltinTransfer = true,
+   llvm::StringRef TargetFun = "target") {
 ASSERT_THAT_ERROR(
 test::checkDataflow(
-Code, "target",
+Code, TargetFun,
 [ApplyBuiltinTransfer](ASTContext &C, Environment &) {
   return NoopAnalysis(C, ApplyBuiltinTransfer);
 },
@@ -3175,4 +3176,27 @@
 });
 }
 
+TEST_F(TransferTest, DoesNotCrashOnUnionThisExpr) {
+  std::string Code = R"(
+union Union {
+  int A;
+  float B;
+};
+
+void foo() {
+  Union A;
+  Union B;
+  A = B;
+}
+  )";
+  // This is a crash regression test when calling the transfer function on a
+  // `CXXThisExpr` that refers to a union.
+  runDataflow(
+  Code,
+  [](llvm::ArrayRef<
+ std::pair>>,
+ ASTContext &) {},
+  LangStandard::lang_cxx17, /*ApplyBuiltinTransfer=*/true, "operator=");
+}
+
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -279,7 +279,10 @@
 
   void VisitCXXThisExpr(const CXXThisExpr *S) {
 auto *ThisPointeeLoc = Env.getThisPointeeStorageLocation();
-assert(ThisPointeeLoc != nullptr);
+if (ThisPointeeLoc == nullptr)
+  // Unions are not supported yet, and will not have a location for the
+  // `this` expression's pointee.
+  return;
 
 auto &Loc = Env.createStorageLocation(*S);
 Env.setStorageLocation(*S, Loc);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126405: [clang][dataflow] Relax assert on existence of `this` pointee storage

[Eric Li via Phabricator via cfe-commits]

li.zhe.hua updated this revision to Diff 432117.
li.zhe.hua marked 2 inline comments as done.
li.zhe.hua added a comment.

Address reviewer comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126405

Files:
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -42,10 +42,11 @@
   template 
   void runDataflow(llvm::StringRef Code, Matcher Match,
LangStandard::Kind Std = LangStandard::lang_cxx17,
-   bool ApplyBuiltinTransfer = true) {
+   bool ApplyBuiltinTransfer = true,
+   llvm::StringRef TargetFun = "target") {
 ASSERT_THAT_ERROR(
 test::checkDataflow(
-Code, "target",
+Code, TargetFun,
 [ApplyBuiltinTransfer](ASTContext &C, Environment &) {
   return NoopAnalysis(C, ApplyBuiltinTransfer);
 },
@@ -3175,4 +3176,27 @@
 });
 }
 
+TEST_F(TransferTest, DoesNotCrashOnUnionThisExpr) {
+  std::string Code = R"(
+union Union {
+  int A;
+  float B;
+};
+
+void foo() {
+  Union A;
+  Union B;
+  A = B;
+}
+  )";
+  // This is a crash regression test when calling the transfer function on a
+  // `CXXThisExpr` that refers to a union.
+  runDataflow(
+  Code,
+  [](llvm::ArrayRef<
+ std::pair>>,
+ ASTContext &) {},
+  LangStandard::lang_cxx17, /*ApplyBuiltinTransfer=*/true, "operator=");
+}
+
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -279,7 +279,10 @@
 
   void VisitCXXThisExpr(const CXXThisExpr *S) {
 auto *ThisPointeeLoc = Env.getThisPointeeStorageLocation();
-assert(ThisPointeeLoc != nullptr);
+if (ThisPointeeLoc == nullptr)
+  // Unions are supported yet, and will not have a location for the
+  // expression's pointee.
+  return;
 
 auto &Loc = Env.createStorageLocation(*S);
 Env.setStorageLocation(*S, Loc);


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -42,10 +42,11 @@
   template 
   void runDataflow(llvm::StringRef Code, Matcher Match,
LangStandard::Kind Std = LangStandard::lang_cxx17,
-   bool ApplyBuiltinTransfer = true) {
+   bool ApplyBuiltinTransfer = true,
+   llvm::StringRef TargetFun = "target") {
 ASSERT_THAT_ERROR(
 test::checkDataflow(
-Code, "target",
+Code, TargetFun,
 [ApplyBuiltinTransfer](ASTContext &C, Environment &) {
   return NoopAnalysis(C, ApplyBuiltinTransfer);
 },
@@ -3175,4 +3176,27 @@
 });
 }
 
+TEST_F(TransferTest, DoesNotCrashOnUnionThisExpr) {
+  std::string Code = R"(
+union Union {
+  int A;
+  float B;
+};
+
+void foo() {
+  Union A;
+  Union B;
+  A = B;
+}
+  )";
+  // This is a crash regression test when calling the transfer function on a
+  // `CXXThisExpr` that refers to a union.
+  runDataflow(
+  Code,
+  [](llvm::ArrayRef<
+ std::pair>>,
+ ASTContext &) {},
+  LangStandard::lang_cxx17, /*ApplyBuiltinTransfer=*/true, "operator=");
+}
+
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -279,7 +279,10 @@
 
   void VisitCXXThisExpr(const CXXThisExpr *S) {
 auto *ThisPointeeLoc = Env.getThisPointeeStorageLocation();
-assert(ThisPointeeLoc != nullptr);
+if (ThisPointeeLoc == nullptr)
+  // Unions are supported yet, and will not have a location for the
+  // expression's pointee.
+  return;
 
 auto &Loc = Env.createStorageLocation(*S);
 Env.setStorageLocation(*S, Loc);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126420: [clang][dataflow] Remove private-field filtering from `StorageLocation` creation.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
ymandel added reviewers: sgatev, xazax.hun, li.zhe.hua.
Herald added subscribers: tschuett, steakhal, rnkovacs.
Herald added a project: All.
ymandel requested review of this revision.
Herald added a project: clang.

The API for `AggregateStorageLocation` does not allow for missing fields (it 
asserts). Therefore, it is incorrect to filter out any fields at 
location-creation time which may be accessed by the code. Currently, we limit 
filtering to private, base-calss fields on the assumption that those can never 
be accessed. However, `friend` declarations can invalidate that assumption, 
thereby breaking our invariants.

This patch removes said field filtering to avoid violating the invariant of "no 
missing fields" for `AggregateStorageLocation`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126420

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

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1226,6 +1226,35 @@
   });
 }
 
+TEST_F(TransferTest, DerivedBaseMemberPrivateFriend) {
+  // Include an access to `Foo.Bar` to verify the analysis doesn't crash on that
+  // access.
+  std::string Code = R"(
+class A {
+private:
+  friend void target();
+  int Bar;
+};
+struct B : public A {
+};
+
+void target() {
+  B Foo;
+  (void)Foo.Bar;
+  // [[p]]
+}
+  )";
+  runDataflow(
+  Code, [](llvm::ArrayRef<
+   std::pair>>
+   Results,
+   ASTContext &ASTCtx) {
+// The purpose of this test is to verify non-crashing behavior for the
+// analysis of the access to `Foo.Bar`. So, we
+EXPECT_THAT(Results, ElementsAre(Pair("p", _)));
+  });
+}
+
 TEST_F(TransferTest, ClassMember) {
   std::string Code = R"(
 class A {
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -148,6 +148,8 @@
   }
 }
 
+// FIXME: Does not precisely handle non-virtual diamond inheritance. A single
+// field decl will be modeled for all instances of the inherited field.
 static void
 getFieldsFromClassHierarchy(QualType Type, bool IgnorePrivateFields,
 llvm::DenseSet &Fields) {
@@ -162,24 +164,40 @@
   continue;
 Fields.insert(Field);
   }
-  if (auto *CXXRecord = Type->getAsCXXRecordDecl()) {
-for (const CXXBaseSpecifier &Base : CXXRecord->bases()) {
-  // Ignore private fields (including default access in C++ classes) in
-  // base classes, because they are not visible in derived classes.
-  getFieldsFromClassHierarchy(Base.getType(), /*IgnorePrivateFields=*/true,
-  Fields);
-}
-  }
+  if (auto *CXXRecord = Type->getAsCXXRecordDecl())
+for (const CXXBaseSpecifier &Base : CXXRecord->bases())
+  getFieldsFromClassHierarchy(
+  Base.getType(), IgnorePrivateFields, Fields);
 }
 
-/// Gets the set of all fields accesible from the type.
+/// Gets the set of all fields accesible from the type. Admits private fields of
+/// the type, but not of any base classes (which are presumably inaccessible.
 ///
-/// FIXME: Does not precisely handle non-virtual diamond inheritance. A single
-/// field decl will be modeled for all instances of the inherited field.
+/// FIXME: Does not account for friend declarations, which may make private
+/// fields visible.
 static llvm::DenseSet
 getAccessibleObjectFields(QualType Type) {
   llvm::DenseSet Fields;
-  // Don't ignore private fields for the class itself, only its super classes.
+  if (Type->isIncompleteType() || Type->isDependentType() ||
+  !Type->isRecordType())
+return Fields;
+
+  for (const FieldDecl *Field : Type->getAsRecordDecl()->fields())
+Fields.insert(Field);
+
+  if (auto *CXXRecord = Type->getAsCXXRecordDecl())
+for (const CXXBaseSpecifier &Base : CXXRecord->bases())
+  // Ignore private fields (including default access in C++ classes) in base
+  // classes, because they are not visible in derived classes.
+  getFieldsFromClassHierarchy(Base.getType(), /*IgnorePrivateFields=*/true,
+  Fields);
+
+  return Fields;
+}
+
+/// Gets the set of all fields in the type.
+static llvm::DenseSet getObjectFields(QualType Type) {
+  llvm::DenseSet Fields;
   getFieldsFromClassHierarchy(Type, /*IgnorePrivateFields=*/false, Fields);
   return Fields;
 }
@@ -319,9 +337,8 @@
 // FIXME: Explore options to avoid eager initialization of fields as some of
 // them might not be needed for a

[PATCH] D126419: [clang][dataflow] Improve handling of constructor initializers.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
ymandel added reviewers: sgatev, li.zhe.hua, xazax.hun.
Herald added subscribers: tschuett, steakhal, rnkovacs.
Herald added a project: All.
ymandel requested review of this revision.
Herald added a project: clang.

Currently, we assert that `CXXCtorInitializer`s are field initializers. Replace
the assertion with an early return. Otherwise, we crash every time we process a
constructor with a non-field (e.g. base class) initializer.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126419

Files:
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1274,6 +1274,49 @@
   });
 }
 
+TEST_F(TransferTest, BaseClassInitializer) {
+  using ast_matchers::cxxConstructorDecl;
+  using ast_matchers::hasName;
+  using ast_matchers::ofClass;
+
+  std::string Code = R"(
+class A {
+public:
+  A(int I) : Bar(I) {}
+  int Bar;
+};
+
+class B : public A {
+public:
+  B(int I) : A(I) {
+(void)0;
+// [[p]]
+  }
+};
+  )";
+  ASSERT_THAT_ERROR(
+  test::checkDataflow(
+  Code, cxxConstructorDecl(ofClass(hasName("B"))),
+  [](ASTContext &C, Environment &) {
+return NoopAnalysis(C, /*ApplyBuiltinTransfer=*/true);
+  },
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+// Regression test to verify that base-class initializers do not
+// trigger an assertion. If we add support for such initializers in
+// the future, we can expand this test to check more specific
+// properties.
+EXPECT_THAT(Results, ElementsAre(Pair("p", _)));
+  },
+  {"-fsyntax-only", "-fno-delayed-template-parsing",
+   "-std=" + std::string(LangStandard::getLangStandardForKind(
+ LangStandard::lang_cxx17)
+ .getName())}),
+  llvm::Succeeded());
+}
+
 TEST_F(TransferTest, ReferenceMember) {
   std::string Code = R"(
 struct A {
Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -257,6 +257,11 @@
   const CXXCtorInitializer *Initializer = CfgInit.getInitializer();
   assert(Initializer != nullptr);
 
+  const FieldDecl *Member = Initializer->getMember();
+  if (Member == nullptr)
+// Not a field initializer.
+return;
+
   auto *InitStmt = Initializer->getInit();
   assert(InitStmt != nullptr);
 
@@ -269,9 +274,6 @@
   if (InitStmtVal == nullptr)
 return;
 
-  const FieldDecl *Member = Initializer->getMember();
-  assert(Member != nullptr);
-
   if (Member->getType()->isReferenceType()) {
 auto &MemberLoc = ThisLoc.getChild(*Member);
 State.Env.setValue(MemberLoc,


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1274,6 +1274,49 @@
   });
 }
 
+TEST_F(TransferTest, BaseClassInitializer) {
+  using ast_matchers::cxxConstructorDecl;
+  using ast_matchers::hasName;
+  using ast_matchers::ofClass;
+
+  std::string Code = R"(
+class A {
+public:
+  A(int I) : Bar(I) {}
+  int Bar;
+};
+
+class B : public A {
+public:
+  B(int I) : A(I) {
+(void)0;
+// [[p]]
+  }
+};
+  )";
+  ASSERT_THAT_ERROR(
+  test::checkDataflow(
+  Code, cxxConstructorDecl(ofClass(hasName("B"))),
+  [](ASTContext &C, Environment &) {
+return NoopAnalysis(C, /*ApplyBuiltinTransfer=*/true);
+  },
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+// Regression test to verify that base-class initializers do not
+// trigger an assertion. If we add support for such initializers in
+// the future, we can expand this test to check more specific
+// properties.
+EXPECT_THAT(Results, ElementsAre(Pair("p", _)));
+  },
+  {"-fsyntax-only", "-fno-delayed-template-parsing",
+   "-std=" + std::string(LangStandard::getLangStandardForKind(
+ LangStandard::lang_cxx17)
+ .getName())}),
+  llvm::Succeeded());
+}
+
 TEST_F(TransferTest, Refer

[PATCH] D126413: [clang][dataflow] Fix incorrect CXXThisExpr pointee for lambdas

[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/unittests/Analysis/FlowSensitive/TransferTest.cpp:1526
+  int Foo = Bar;
+  // [[p]]
+}();

please use different name for the assertion (e.g. `p2`) when bundling two or 
more code snippets in a single test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126413

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

[PATCH] D126398: [Clang] Introduce `--offload-link` option to perform offload device linking

[Joseph Huber via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb7c8c4d8cf07: [Clang] Introduce `--offload-link` option to 
perform offload device linking (authored by jhuber6).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126398

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/cuda-openmp-driver.cu


Index: clang/test/Driver/cuda-openmp-driver.cu
===
--- clang/test/Driver/cuda-openmp-driver.cu
+++ clang/test/Driver/cuda-openmp-driver.cu
@@ -35,3 +35,8 @@
 
 // RUN: %clang -### -target x86_64-linux-gnu -nocudalib --cuda-feature=+ptx61 
--offload-arch=sm_70 %s 2>&1 | FileCheck -check-prefix MANUAL-FEATURE %s
 // MANUAL-FEATURE: -cc1{{.*}}-target-feature{{.*}}+ptx61
+
+// RUN: %clang -### -target x86_64-linux-gnu -nocudalib -ccc-print-bindings 
--offload-link %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE-LINK %s
+
+// DEVICE-LINK: "x86_64-unknown-linux-gnu" - "Offload::Linker", inputs: 
["[[INPUT:.+]]"], output: "a.out"
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4158,7 +4158,8 @@
 // Check if this Linker Job should emit a static library.
 if (ShouldEmitStaticLibrary(Args)) {
   LA = C.MakeAction(LinkerInputs, types::TY_Image);
-} else if (UseNewOffloadingDriver) {
+} else if (UseNewOffloadingDriver ||
+   Args.hasArg(options::OPT_offload_link)) {
   LA = C.MakeAction(LinkerInputs, types::TY_Image);
   LA->propagateHostOffloadInfo(C.getActiveOffloadKinds(),
/*BoundArch=*/nullptr);
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -820,6 +820,8 @@
 def z : Separate<["-"], "z">, Flags<[LinkerInput, RenderAsInput]>,
   HelpText<"Pass -z  to the linker">, MetaVarName<"">,
   Group;
+def offload_link : Flag<["--"], "offload-link">, Group,
+  HelpText<"Use the new offloading linker to perform the link job.">;
 def Xlinker : Separate<["-"], "Xlinker">, Flags<[LinkerInput, RenderAsInput]>,
   HelpText<"Pass  to the linker">, MetaVarName<"">,
   Group;
Index: clang/docs/ClangCommandLineReference.rst
===
--- clang/docs/ClangCommandLineReference.rst
+++ clang/docs/ClangCommandLineReference.rst
@@ -4209,6 +4209,10 @@
 
 Pass the comma separated arguments in  to the linker
 
+.. option:: --offload-link
+
+Use the linker supporting offloading device linking.
+
 .. option:: -X
 
 .. option:: -Xlinker , --for-linker , --for-linker=


Index: clang/test/Driver/cuda-openmp-driver.cu
===
--- clang/test/Driver/cuda-openmp-driver.cu
+++ clang/test/Driver/cuda-openmp-driver.cu
@@ -35,3 +35,8 @@
 
 // RUN: %clang -### -target x86_64-linux-gnu -nocudalib --cuda-feature=+ptx61 --offload-arch=sm_70 %s 2>&1 | FileCheck -check-prefix MANUAL-FEATURE %s
 // MANUAL-FEATURE: -cc1{{.*}}-target-feature{{.*}}+ptx61
+
+// RUN: %clang -### -target x86_64-linux-gnu -nocudalib -ccc-print-bindings --offload-link %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE-LINK %s
+
+// DEVICE-LINK: "x86_64-unknown-linux-gnu" - "Offload::Linker", inputs: ["[[INPUT:.+]]"], output: "a.out"
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4158,7 +4158,8 @@
 // Check if this Linker Job should emit a static library.
 if (ShouldEmitStaticLibrary(Args)) {
   LA = C.MakeAction(LinkerInputs, types::TY_Image);
-} else if (UseNewOffloadingDriver) {
+} else if (UseNewOffloadingDriver ||
+   Args.hasArg(options::OPT_offload_link)) {
   LA = C.MakeAction(LinkerInputs, types::TY_Image);
   LA->propagateHostOffloadInfo(C.getActiveOffloadKinds(),
/*BoundArch=*/nullptr);
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -820,6 +820,8 @@
 def z : Separate<["-"], "z">, Flags<[LinkerInput, RenderAsInput]>,
   HelpText<"Pass -z  to the linker">, MetaVarName<"">,
   Group;
+def offload_link : Flag<["--"], "offload-link">, Group,
+  HelpText<"Use the new offloading linker to perform the link job.">;
 def Xlinker : Separate<["-"], "Xlinker">, Flags<[LinkerInput, RenderAsInput]>,
   HelpText<"Pass  to the linker">, MetaVarName<"">,
   Group;
Index: clang/docs/ClangCommandLineReference.rst

[clang] b7c8c4d - [Clang] Introduce `--offload-link` option to perform offload device linking

[Joseph Huber via cfe-commits]

Author: Joseph Huber
Date: 2022-05-25T16:30:53-04:00
New Revision: b7c8c4d8cf07d2e9e8cd157bccc8bd9e7c76415a

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

LOG: [Clang] Introduce `--offload-link` option to perform offload device linking

The new driver uses an augmented linker wrapper to perform the device
linking phase, but to the user looks like a regular linker invocation.
Contrary to the old driver, the new driver contains all the information
necessary to produce a linked device image in the host object itself.
Currently, we infer the usage of the device linker by the user
specifying an offloading toolchain, e.g. (--offload-arch=...) or
(-fopenmp-targets=...), but this shouldn't be strictly necessary.
This patch introduces a new option `--offload-link` to tell
 the driver to use the offloading linker instead. So a compilation flow
 can now look like this,

```
clang foo.cu --offload-new-driver -fgpu-rdc --offload-arch=sm_70 -c
clang foo.o --offload-link -lcudart
```

I was considering if this could be merged into the `-fuse-ld` option,
but because the device linker wraps over the users linker it would
conflict with that. In the future it's possible to merge this into `lld`
completely or `gold` via a plugin and we would use this option to
enable the device linking feature. Let me know what you think for this.

Reviewed By: tra

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

Added: 


Modified: 
clang/docs/ClangCommandLineReference.rst
clang/include/clang/Driver/Options.td
clang/lib/Driver/Driver.cpp
clang/test/Driver/cuda-openmp-driver.cu

Removed: 




diff  --git a/clang/docs/ClangCommandLineReference.rst 
b/clang/docs/ClangCommandLineReference.rst
index 5926decc85fc..73dccf6a9a04 100644
--- a/clang/docs/ClangCommandLineReference.rst
+++ b/clang/docs/ClangCommandLineReference.rst
@@ -4209,6 +4209,10 @@ Set starting address of TEXT to 
 
 Pass the comma separated arguments in  to the linker
 
+.. option:: --offload-link
+
+Use the linker supporting offloading device linking.
+
 .. option:: -X
 
 .. option:: -Xlinker , --for-linker , --for-linker=

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index d34cec766a2e..e19baac5860a 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -820,6 +820,8 @@ def Xopenmp_target_EQ : JoinedAndSeparate<["-"], 
"Xopenmp-target=">, Group, Flags<[LinkerInput, RenderAsInput]>,
   HelpText<"Pass -z  to the linker">, MetaVarName<"">,
   Group;
+def offload_link : Flag<["--"], "offload-link">, Group,
+  HelpText<"Use the new offloading linker to perform the link job.">;
 def Xlinker : Separate<["-"], "Xlinker">, Flags<[LinkerInput, RenderAsInput]>,
   HelpText<"Pass  to the linker">, MetaVarName<"">,
   Group;

diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 3edbaed9ae7f..94925568aec2 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -4158,7 +4158,8 @@ void Driver::BuildActions(Compilation &C, DerivedArgList 
&Args,
 // Check if this Linker Job should emit a static library.
 if (ShouldEmitStaticLibrary(Args)) {
   LA = C.MakeAction(LinkerInputs, types::TY_Image);
-} else if (UseNewOffloadingDriver) {
+} else if (UseNewOffloadingDriver ||
+   Args.hasArg(options::OPT_offload_link)) {
   LA = C.MakeAction(LinkerInputs, types::TY_Image);
   LA->propagateHostOffloadInfo(C.getActiveOffloadKinds(),
/*BoundArch=*/nullptr);

diff  --git a/clang/test/Driver/cuda-openmp-driver.cu 
b/clang/test/Driver/cuda-openmp-driver.cu
index dec0288db263..b27195da2fb1 100644
--- a/clang/test/Driver/cuda-openmp-driver.cu
+++ b/clang/test/Driver/cuda-openmp-driver.cu
@@ -35,3 +35,8 @@
 
 // RUN: %clang -### -target x86_64-linux-gnu -nocudalib --cuda-feature=+ptx61 
--offload-arch=sm_70 %s 2>&1 | FileCheck -check-prefix MANUAL-FEATURE %s
 // MANUAL-FEATURE: -cc1{{.*}}-target-feature{{.*}}+ptx61
+
+// RUN: %clang -### -target x86_64-linux-gnu -nocudalib -ccc-print-bindings 
--offload-link %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE-LINK %s
+
+// DEVICE-LINK: "x86_64-unknown-linux-gnu" - "Offload::Linker", inputs: 
["[[INPUT:.+]]"], output: "a.out"



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

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

[John Ericson via Phabricator via cfe-commits]

Ericson2314 added inline comments.



Comment at: clang/utils/hmaptool/CMakeLists.txt:5
 
-add_custom_command(OUTPUT 
${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/${CLANG_HMAPTOOL}
-   COMMAND ${CMAKE_COMMAND} -E make_directory
- ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin
-   COMMAND ${CMAKE_COMMAND} -E copy
- ${CMAKE_CURRENT_SOURCE_DIR}/${CLANG_HMAPTOOL}
- ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/
-   DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/${CLANG_HMAPTOOL})
-
-list(APPEND Depends 
${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/${CLANG_HMAPTOOL})
-install(PROGRAMS ${CLANG_HMAPTOOL}
-DESTINATION "${CMAKE_INSTALL_BINDIR}"
-COMPONENT hmaptool)
-
-add_custom_target(hmaptool ALL DEPENDS ${Depends})
+install(PROGRAMS hmaptool DESTINATION "${LLVM_UTILS_INSTALL_DIR}}" COMPONENT 
hmaptool)
+add_custom_target(hmaptool ALL DEPENDS "${LLVM_TOOLS_BINARY_DIR}/hmaptool")

tstellar wrote:
> mizvekov wrote:
> > tstellar wrote:
> > > Should this be LLVM_TOOLS_BINARY_DIR ?
> > I think LLVM_UTILS_INSTALL_DIR is the path where the programs will be 
> > installed, and LLVM_TOOLS_BINARY_DIR is where they are located in the build 
> > tree.
> > 
> > So I think the idea is that this install invocation will install the file, 
> > for the packaging for example, while add_custom_command above will just 
> > copy this program into the build tree so that llvm-lit will find it when 
> > run from the build tree.
> OK, I see.  Makes sense.
That is right about `INSTALL_DIR` vs `BINARY_DIR`, but `LLVM_UTILS_INSTALL_DIR` 
and `LLVM_TOOLS_BINARY_DIR` both exist.

At least after https://reviews.llvm.org/D117977 `CLANG_` ones would exist, 
which would make more sense for this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126308

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

[PATCH] D126405: [clang][dataflow] Relax assert on existence of `this` pointee storage

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev accepted this revision.
sgatev added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:282
 auto *ThisPointeeLoc = Env.getThisPointeeStorageLocation();
-assert(ThisPointeeLoc != nullptr);
+if (ThisPointeeLoc == nullptr)
+  return;

Please document when that could happen.



Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:3181-3190
+union Union {
+int A;
+float B;
+};
+
+void foo() {
+Union A;




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126405

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

[PATCH] D120244: [clang][sema] Enable first-class bool support for C2x

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

We've started having several internal user complaints because their system 
headers are suddenly triggering a warning for using ``. This seems 
to be caused by the fact that `#warning` is surfaced to users even when the 
`#warning` is present in a system header (which makes sense, because otherwise 
there would be no way to issue a `#warning` from a system header). This ends up 
causing problems because users have no way to suppress the warning in the 
system headers they use without also disabling deprecation warnings in their 
own code. Is this intended? Instead, it seems to me like what we'd want is some 
way to mark the header as deprecated such that Clang will not flag uses of 
`` from within system headers, but will flag them from user code. 
This would be consistent with how the deprecated attributes work for classes 
and functions.

Thoughts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120244

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

[PATCH] D126405: [clang][dataflow] Relax assert on existence of `this` pointee storage

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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

Please wait for at least one more "accept". thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126405

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

[PATCH] D126413: [clang][dataflow] Fix incorrect CXXThisExpr pointee for lambdas

[Eric Li via Phabricator via cfe-commits]

li.zhe.hua created this revision.
li.zhe.hua added a reviewer: ymandel.
Herald added subscribers: tschuett, steakhal.
Herald added a project: All.
li.zhe.hua requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

When constructing the `Environment`, the `this` pointee is established
for a `CXXMethodDecl` by looking at its parent. However, inside of
lambdas, a `CXXThisExpr` refers to the captured `this` coming from the
enclosing member function.

When establishing the `this` pointee for a function, we check whether
the function is a lambda, and check for an enclosing member function
to establish the `this` pointee storage location.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126413

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

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1476,6 +1476,112 @@
   });
 }
 
+TEST_F(TransferTest, StructThisInLambda) {
+  std::string ThisCaptureCode = R"(
+struct A {
+  void frob() {
+[this]() {
+  int Foo = Bar;
+  // [[p]]
+}();
+  }
+
+  int Bar;
+};
+  )";
+  runDataflow(
+  ThisCaptureCode,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const auto *ThisLoc = dyn_cast(
+Env.getThisPointeeStorageLocation());
+ASSERT_THAT(ThisLoc, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const auto *BarLoc =
+cast(&ThisLoc->getChild(*BarDecl));
+ASSERT_TRUE(isa_and_nonnull(BarLoc));
+
+const Value *BarVal = Env.getValue(*BarLoc);
+ASSERT_TRUE(isa_and_nonnull(BarVal));
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+EXPECT_EQ(Env.getValue(*FooDecl, SkipPast::None), BarVal);
+  },
+  LangStandard::lang_cxx17, /*ApplyBuiltinTransfer=*/true, "operator()");
+
+  std::string RefCaptureDefaultCode = R"(
+struct A {
+  void frob() {
+[&]() {
+  int Foo = Bar;
+  // [[p]]
+}();
+  }
+
+  int Bar;
+};
+  )";
+  runDataflow(
+  RefCaptureDefaultCode,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const auto *ThisLoc = dyn_cast(
+Env.getThisPointeeStorageLocation());
+ASSERT_THAT(ThisLoc, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const auto *BarLoc =
+cast(&ThisLoc->getChild(*BarDecl));
+ASSERT_TRUE(isa_and_nonnull(BarLoc));
+
+const Value *BarVal = Env.getValue(*BarLoc);
+ASSERT_TRUE(isa_and_nonnull(BarVal));
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+EXPECT_EQ(Env.getValue(*FooDecl, SkipPast::None), BarVal);
+  },
+  LangStandard::lang_cxx17, /*ApplyBuiltinTransfer=*/true, "operator()");
+
+  std::string FreeFunctionLambdaCode = R"(
+void foo() {
+  int Bar;
+  [&]() {
+int Foo = Bar;
+// [[p]]
+  }();
+}
+  )";
+  runDataflow(
+  FreeFunctionLambdaCode,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+EXPECT_THAT(Env.getThisPointeeStorageLocation(), IsNull());
+  },
+  LangStandard::lang_cxx17, /*ApplyBuiltinTransfer=*/true, "operator()");
+}
+
 TEST_F(TransferTest, ConstructorInitializer) {
   std::string Code = R"(
 struct target {
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -216,7 +216,12 @@
   }
 
   if (const auto *MethodDecl = dyn_cast(&DeclCtx)) {
-if (!MethodDecl->isStatic()) {
+auto *Parent = MethodDecl->getParent();
+assert(Parent != nullptr);
+if (Parent->isLambda())
+  MethodDecl = dyn_cast(Parent->getDeclContext());
+
+if (MethodDecl && !MethodDecl->isStatic()) {
   QualType ThisPointeeType = MethodDecl->getThisObjectType();
   // FIX

[PATCH] D125788: [flang][driver] Rename `flang-new` as `flang`

[Damian Rouson via Phabricator via cfe-commits]

rouson added a comment.

+1 @kiranchandramohan

I have access to the NAG Fortran compiler and find it very helpful for checking 
standard conformance.  I was unaware of the NAG test suite and don't see any 
mention of it online. Is it an official product that can be purchased?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125788

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

[PATCH] D102122: Support warn_unused_result on typedefs

[David Blaikie via Phabricator via cfe-commits]

dblaikie updated this revision to Diff 432091.
dblaikie added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102122

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/AST/Expr.cpp
  clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p1.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/c2x-nodiscard.c
  clang/test/Sema/unused-expr.c
  clang/test/SemaCXX/warn-unused-result.cpp


Index: clang/test/SemaCXX/warn-unused-result.cpp
===
--- clang/test/SemaCXX/warn-unused-result.cpp
+++ clang/test/SemaCXX/warn-unused-result.cpp
@@ -254,3 +254,15 @@
 
 void i([[nodiscard]] bool (*fp)()); // expected-warning {{'nodiscard' 
attribute only applies to functions, classes, or enumerations}}
 }
+
+namespace unused_typedef_result {
+[[clang::warn_unused_result]] typedef void *p;
+p f1();
+void f2() {
+  f1(); // expected-warning {{ignoring return value}}
+  void *(*p1)();
+  p1(); // no warning
+  p (*p2)();
+  p2(); // expected-warning {{ignoring return value}}
+}
+}
Index: clang/test/Sema/unused-expr.c
===
--- clang/test/Sema/unused-expr.c
+++ clang/test/Sema/unused-expr.c
@@ -96,7 +96,7 @@
   return 0;
 }
 
-int t7 __attribute__ ((warn_unused_result)); // expected-warning 
{{'warn_unused_result' attribute only applies to Objective-C methods, enums, 
structs, unions, classes, functions, and function pointers}}
+int t7 __attribute__ ((warn_unused_result)); // expected-warning 
{{'warn_unused_result' attribute only applies to Objective-C methods, enums, 
structs, unions, classes, functions, function pointers, and typedefs}}
 
 // PR4010
 int (*fn4)(void) __attribute__ ((warn_unused_result));
Index: clang/test/Sema/c2x-nodiscard.c
===
--- clang/test/Sema/c2x-nodiscard.c
+++ clang/test/Sema/c2x-nodiscard.c
@@ -15,7 +15,7 @@
 [[nodiscard]] int f1(void);
 enum [[nodiscard]] E1 { One };
 
-[[nodiscard]] int i; // expected-warning {{'nodiscard' attribute only applies 
to Objective-C methods, enums, structs, unions, classes, functions, and 
function pointers}}
+[[nodiscard]] int i; // expected-warning {{'nodiscard' attribute only applies 
to Objective-C methods, enums, structs, unions, classes, functions, function 
pointers, and typedefs}}
 
 struct [[nodiscard]] S4 {
   int i;
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
@@ -185,7 +185,7 @@
 // CHECK-NEXT: VecReturn (SubjectMatchRule_record)
 // CHECK-NEXT: VecTypeHint (SubjectMatchRule_function)
 // CHECK-NEXT: WarnUnused (SubjectMatchRule_record)
-// CHECK-NEXT: WarnUnusedResult (SubjectMatchRule_objc_method, 
SubjectMatchRule_enum, SubjectMatchRule_record, 
SubjectMatchRule_hasType_functionType)
+// CHECK-NEXT: WarnUnusedResult (SubjectMatchRule_objc_method, 
SubjectMatchRule_enum, SubjectMatchRule_record, 
SubjectMatchRule_hasType_functionType, SubjectMatchRule_type_alias)
 // CHECK-NEXT: Weak (SubjectMatchRule_variable, SubjectMatchRule_function, 
SubjectMatchRule_record)
 // CHECK-NEXT: WeakRef (SubjectMatchRule_variable, SubjectMatchRule_function)
 // CHECK-NEXT: WebAssemblyExportName (SubjectMatchRule_function)
Index: clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p1.cpp
===
--- clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p1.cpp
+++ clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p1.cpp
@@ -7,4 +7,4 @@
 [[nodiscard]] int f();
 enum [[nodiscard]] E {};
 
-namespace [[nodiscard]] N {} // expected-warning {{'nodiscard' attribute only 
applies to Objective-C methods, enums, structs, unions, classes, functions, and 
function pointers}}
+namespace [[nodiscard]] N {} // expected-warning {{'nodiscard' attribute only 
applies to Objective-C methods, enums, structs, unions, classes, functions, 
function pointers, and typedefs}}
Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -1522,6 +1522,10 @@
 if (const auto *A = TD->getAttr())
   return A;
 
+  if (const auto *TD = getCallReturnType(Ctx)->getAs())
+if (const auto *A = TD->getDecl()->getAttr())
+  return A;
+
   // Otherwise, see if the callee is marked nodiscard and return that attribute
   // instead.
   const Decl *D = getCalleeDecl();
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -2944,7 +2944,7 @@
C2x<"", "no

[PATCH] D126406: [analyzer] Return from reAssume if State is posteriorly overconstrained

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2547
   if (Constraint->encodesFalseRange())
 return State->assume(DefinedVal, false);
 

I am wondering, that maybe it would be better to check for 
`isPosteriorlyOverconstrained` here. Because only `State->assume` can return 
such States, and by checking it here, we could spare some instructions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126406

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

[PATCH] D126332: [clang] Fix the begin location of concepts specialization expression

[Alex Lorenz 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 rG79e09af1d6e1: [clang] Fix the begin location of concepts 
specialization expression (authored by arphaman).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126332

Files:
  clang/include/clang/AST/ExprConcepts.h
  clang/test/Index/index-concepts.cpp


Index: clang/test/Index/index-concepts.cpp
===
--- clang/test/Index/index-concepts.cpp
+++ clang/test/Index/index-concepts.cpp
@@ -64,8 +64,8 @@
 // CHECK: index-concepts.cpp:[[@LINE-1]]:9: 
ConceptDecl=ConTwoTemplateParams:[[@LINE-1]]:9 (Definition) 
Extent=[[[@LINE-2]]:1 - [[@LINE-1]]:79]
 // CHECK: index-concepts.cpp:[[@LINE-3]]:17: 
TemplateTypeParameter=T1:[[@LINE-3]]:17 (Definition) Extent=[[[@LINE-3]]:11 - 
[[@LINE-3]]:19] [access=public]
 // CHECK: index-concepts.cpp:[[@LINE-4]]:27: 
TemplateTypeParameter=T2:[[@LINE-4]]:27 (Definition) Extent=[[[@LINE-4]]:21 - 
[[@LINE-4]]:29] [access=public]
-// CHECK: index-concepts.cpp:[[@LINE-4]]:36: BinaryOperator= 
Extent=[[[@LINE-4]]:36 - [[@LINE-4]]:79]
-// CHECK: index-concepts.cpp:[[@LINE-5]]:36: ConceptSpecializationExpr= 
Extent=[[[@LINE-5]]:36 - [[@LINE-5]]:54]
+// CHECK: index-concepts.cpp:[[@LINE-4]]:32: BinaryOperator= 
Extent=[[[@LINE-4]]:32 - [[@LINE-4]]:79]
+// CHECK: index-concepts.cpp:[[@LINE-5]]:32: ConceptSpecializationExpr= 
Extent=[[[@LINE-5]]:32 - [[@LINE-5]]:54]
 // CHECK: index-concepts.cpp:[[@LINE-6]]:32: NamespaceRef=ns:55:11 
Extent=[[[@LINE-6]]:32 - [[@LINE-6]]:34]
 // CHECK: index-concepts.cpp:[[@LINE-7]]:36: TemplateRef=ConInNamespace:58:9 
Extent=[[[@LINE-7]]:36 - [[@LINE-7]]:50]
 // CHECK: index-concepts.cpp:[[@LINE-8]]:58: ConceptSpecializationExpr= 
Extent=[[[@LINE-8]]:58 - [[@LINE-8]]:79]
@@ -107,7 +107,7 @@
 // CHECK: index-concepts.cpp:[[@LINE-4]]:16: 
TemplateTypeParameter=T:[[@LINE-4]]:16 (Definition) Extent=[[[@LINE-4]]:10 - 
[[@LINE-4]]:17] [access=public]
 // CHECK: index-concepts.cpp:[[@LINE-4]]:36: ParmDecl=x:[[@LINE-4]]:36 
(Definition) Extent=[[[@LINE-4]]:27 - [[@LINE-4]]:37]
 // CHECK: index-concepts.cpp:[[@LINE-5]]:33: TypeRef=T:[[@LINE-6]]:16 
Extent=[[[@LINE-5]]:33 - [[@LINE-5]]:34]
-// CHECK: index-concepts.cpp:[[@LINE-5]]:14: ConceptSpecializationExpr= 
Extent=[[[@LINE-5]]:14 - [[@LINE-5]]:31]
+// CHECK: index-concepts.cpp:[[@LINE-5]]:10: ConceptSpecializationExpr= 
Extent=[[[@LINE-5]]:10 - [[@LINE-5]]:31]
 // CHECK: index-concepts.cpp:[[@LINE-6]]:10: NamespaceRef=ns:55:11 
Extent=[[[@LINE-6]]:10 - [[@LINE-6]]:12]
 // CHECK: index-concepts.cpp:[[@LINE-7]]:14: TemplateRef=ConInNamespace:58:9 
Extent=[[[@LINE-7]]:14 - [[@LINE-7]]:28]
 // CHECK: index-concepts.cpp:[[@LINE-8]]:29: TypeRef=T:[[@LINE-10]]:16 
Extent=[[[@LINE-8]]:29 - [[@LINE-8]]:30]
Index: clang/include/clang/AST/ExprConcepts.h
===
--- clang/include/clang/AST/ExprConcepts.h
+++ clang/include/clang/AST/ExprConcepts.h
@@ -122,6 +122,8 @@
   }
 
   SourceLocation getBeginLoc() const LLVM_READONLY {
+if (auto QualifierLoc = getNestedNameSpecifierLoc())
+  return QualifierLoc.getBeginLoc();
 return ConceptName.getBeginLoc();
   }
 


Index: clang/test/Index/index-concepts.cpp
===
--- clang/test/Index/index-concepts.cpp
+++ clang/test/Index/index-concepts.cpp
@@ -64,8 +64,8 @@
 // CHECK: index-concepts.cpp:[[@LINE-1]]:9: ConceptDecl=ConTwoTemplateParams:[[@LINE-1]]:9 (Definition) Extent=[[[@LINE-2]]:1 - [[@LINE-1]]:79]
 // CHECK: index-concepts.cpp:[[@LINE-3]]:17: TemplateTypeParameter=T1:[[@LINE-3]]:17 (Definition) Extent=[[[@LINE-3]]:11 - [[@LINE-3]]:19] [access=public]
 // CHECK: index-concepts.cpp:[[@LINE-4]]:27: TemplateTypeParameter=T2:[[@LINE-4]]:27 (Definition) Extent=[[[@LINE-4]]:21 - [[@LINE-4]]:29] [access=public]
-// CHECK: index-concepts.cpp:[[@LINE-4]]:36: BinaryOperator= Extent=[[[@LINE-4]]:36 - [[@LINE-4]]:79]
-// CHECK: index-concepts.cpp:[[@LINE-5]]:36: ConceptSpecializationExpr= Extent=[[[@LINE-5]]:36 - [[@LINE-5]]:54]
+// CHECK: index-concepts.cpp:[[@LINE-4]]:32: BinaryOperator= Extent=[[[@LINE-4]]:32 - [[@LINE-4]]:79]
+// CHECK: index-concepts.cpp:[[@LINE-5]]:32: ConceptSpecializationExpr= Extent=[[[@LINE-5]]:32 - [[@LINE-5]]:54]
 // CHECK: index-concepts.cpp:[[@LINE-6]]:32: NamespaceRef=ns:55:11 Extent=[[[@LINE-6]]:32 - [[@LINE-6]]:34]
 // CHECK: index-concepts.cpp:[[@LINE-7]]:36: TemplateRef=ConInNamespace:58:9 Extent=[[[@LINE-7]]:36 - [[@LINE-7]]:50]
 // CHECK: index-concepts.cpp:[[@LINE-8]]:58: ConceptSpecializationExpr= Extent=[[[@LINE-8]]:58 - [[@LINE-8]]:79]
@@ -107,7 +107,7 @@
 // CHECK: index-concepts.cpp:[[@LINE-4]]:16: TemplateTypeParameter=T:[[@LINE-4]]:16 (Definition) Extent=

[clang] 79e09af - [clang] Fix the begin location of concepts specialization expression

[Alex Lorenz via cfe-commits]

Author: Alex Lorenz
Date: 2022-05-25T12:39:21-07:00
New Revision: 79e09af1d6e11b05c6484868f15a9a2db298699c

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

LOG: [clang] Fix the begin location of concepts specialization expression

The concept specialization expression should start at the location of
the  nested qualifiers when it has nested qualifiers.
This ensures that libclang reports correct source ranges that include
all subexpressions when visiting the expression.

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

Added: 


Modified: 
clang/include/clang/AST/ExprConcepts.h
clang/test/Index/index-concepts.cpp

Removed: 




diff  --git a/clang/include/clang/AST/ExprConcepts.h 
b/clang/include/clang/AST/ExprConcepts.h
index 6849b65b71c0c..fd9cd31f3b90b 100644
--- a/clang/include/clang/AST/ExprConcepts.h
+++ b/clang/include/clang/AST/ExprConcepts.h
@@ -122,6 +122,8 @@ class ConceptSpecializationExpr final : public Expr, public 
ConceptReference,
   }
 
   SourceLocation getBeginLoc() const LLVM_READONLY {
+if (auto QualifierLoc = getNestedNameSpecifierLoc())
+  return QualifierLoc.getBeginLoc();
 return ConceptName.getBeginLoc();
   }
 

diff  --git a/clang/test/Index/index-concepts.cpp 
b/clang/test/Index/index-concepts.cpp
index ba4b87111ff48..b8d41e841e619 100644
--- a/clang/test/Index/index-concepts.cpp
+++ b/clang/test/Index/index-concepts.cpp
@@ -64,8 +64,8 @@ concept ConTwoTemplateParams = ns::ConInNamespace && 
ConWithLogicalAnd;
 // CHECK: index-concepts.cpp:[[@LINE-1]]:9: 
ConceptDecl=ConTwoTemplateParams:[[@LINE-1]]:9 (Definition) 
Extent=[[[@LINE-2]]:1 - [[@LINE-1]]:79]
 // CHECK: index-concepts.cpp:[[@LINE-3]]:17: 
TemplateTypeParameter=T1:[[@LINE-3]]:17 (Definition) Extent=[[[@LINE-3]]:11 - 
[[@LINE-3]]:19] [access=public]
 // CHECK: index-concepts.cpp:[[@LINE-4]]:27: 
TemplateTypeParameter=T2:[[@LINE-4]]:27 (Definition) Extent=[[[@LINE-4]]:21 - 
[[@LINE-4]]:29] [access=public]
-// CHECK: index-concepts.cpp:[[@LINE-4]]:36: BinaryOperator= 
Extent=[[[@LINE-4]]:36 - [[@LINE-4]]:79]
-// CHECK: index-concepts.cpp:[[@LINE-5]]:36: ConceptSpecializationExpr= 
Extent=[[[@LINE-5]]:36 - [[@LINE-5]]:54]
+// CHECK: index-concepts.cpp:[[@LINE-4]]:32: BinaryOperator= 
Extent=[[[@LINE-4]]:32 - [[@LINE-4]]:79]
+// CHECK: index-concepts.cpp:[[@LINE-5]]:32: ConceptSpecializationExpr= 
Extent=[[[@LINE-5]]:32 - [[@LINE-5]]:54]
 // CHECK: index-concepts.cpp:[[@LINE-6]]:32: NamespaceRef=ns:55:11 
Extent=[[[@LINE-6]]:32 - [[@LINE-6]]:34]
 // CHECK: index-concepts.cpp:[[@LINE-7]]:36: TemplateRef=ConInNamespace:58:9 
Extent=[[[@LINE-7]]:36 - [[@LINE-7]]:50]
 // CHECK: index-concepts.cpp:[[@LINE-8]]:58: ConceptSpecializationExpr= 
Extent=[[[@LINE-8]]:58 - [[@LINE-8]]:79]
@@ -107,7 +107,7 @@ requires ns::ConInNamespace && ConTwoTemplateParams {}
 // CHECK: index-concepts.cpp:[[@LINE-4]]:16: 
TemplateTypeParameter=T:[[@LINE-4]]:16 (Definition) Extent=[[[@LINE-4]]:10 - 
[[@LINE-4]]:17] [access=public]
 // CHECK: index-concepts.cpp:[[@LINE-4]]:36: ParmDecl=x:[[@LINE-4]]:36 
(Definition) Extent=[[[@LINE-4]]:27 - [[@LINE-4]]:37]
 // CHECK: index-concepts.cpp:[[@LINE-5]]:33: TypeRef=T:[[@LINE-6]]:16 
Extent=[[[@LINE-5]]:33 - [[@LINE-5]]:34]
-// CHECK: index-concepts.cpp:[[@LINE-5]]:14: ConceptSpecializationExpr= 
Extent=[[[@LINE-5]]:14 - [[@LINE-5]]:31]
+// CHECK: index-concepts.cpp:[[@LINE-5]]:10: ConceptSpecializationExpr= 
Extent=[[[@LINE-5]]:10 - [[@LINE-5]]:31]
 // CHECK: index-concepts.cpp:[[@LINE-6]]:10: NamespaceRef=ns:55:11 
Extent=[[[@LINE-6]]:10 - [[@LINE-6]]:12]
 // CHECK: index-concepts.cpp:[[@LINE-7]]:14: TemplateRef=ConInNamespace:58:9 
Extent=[[[@LINE-7]]:14 - [[@LINE-7]]:28]
 // CHECK: index-concepts.cpp:[[@LINE-8]]:29: TypeRef=T:[[@LINE-10]]:16 
Extent=[[[@LINE-8]]:29 - [[@LINE-8]]:30]



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

[PATCH] D126406: [analyzer] Return from reAssume if State is posteriorly overconstrained

[Gabor Marton via Phabricator via cfe-commits]

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

Depends on D124758 . That patch introduced 
serious regression in the run-time in
some special cases. This fixes that.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126406

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/test/Analysis/runtime-regression.c


Index: clang/test/Analysis/runtime-regression.c
===
--- /dev/null
+++ clang/test/Analysis/runtime-regression.c
@@ -0,0 +1,54 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core,alpha.security.ArrayBoundV2 \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -verify
+
+// This test is here to check if there is no significant run-time regression
+// related to the assume machinery. This is an automatically reduced code. The
+// analysis should finish in less than 10 seconds.
+
+// expected-no-diagnostics
+
+typedef unsigned char uint8_t;
+typedef unsigned short uint16_t;
+typedef unsigned long uint64_t;
+
+int filter_slice_word(int sat_linesize, int sigma, int radius, uint64_t *sat,
+  uint64_t *square_sat, int width, int height,
+  int src_linesize, int dst_linesize, const uint16_t *src,
+  uint16_t *dst, int jobnr, int nb_jobs) {
+  const int starty = height * jobnr / nb_jobs;
+  const int endy = height * (jobnr + 1) / nb_jobs;
+
+  for (int y = starty; y < endy; y++) {
+
+int lower_y = y - radius < 0 ? 0 : y - radius;
+int higher_y = y + radius + 1 > height ? height : y + radius + 1;
+int dist_y = higher_y - lower_y;
+
+for (int x = 0; x < width; x++) {
+
+  int lower_x = x - radius < 0 ? 0 : x - radius;
+  int higher_x = x + radius + 1 > width ? width : x + radius + 1;
+  int count = dist_y * (higher_x - lower_x);
+
+  // The below hunk caused significant regression in run-time.
+#if 1
+  uint64_t sum = sat[higher_y * sat_linesize + higher_x] -
+ sat[higher_y * sat_linesize + lower_x] -
+ sat[lower_y * sat_linesize + higher_x] +
+ sat[lower_y * sat_linesize + lower_x];
+  uint64_t square_sum = square_sat[higher_y * sat_linesize + higher_x] -
+square_sat[higher_y * sat_linesize + lower_x] -
+square_sat[lower_y * sat_linesize + higher_x] +
+square_sat[lower_y * sat_linesize + lower_x];
+  uint64_t mean = sum / count;
+  uint64_t var = (square_sum - sum * sum / count) / count;
+  dst[y * dst_linesize + x] =
+  (sigma * mean + var * src[y * src_linesize + x]) / (sigma + var);
+#endif
+
+}
+  }
+  return 0;
+}
Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -2534,6 +2534,9 @@
 LLVM_NODISCARD ProgramStateRef reAssume(ProgramStateRef State,
 const RangeSet *Constraint,
 SVal TheValue) {
+  assert(State);
+  if (State->isPosteriorlyOverconstrained())
+return nullptr;
   if (!Constraint)
 return State;
 
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
@@ -123,11 +123,12 @@
   void setStore(const StoreRef &storeRef);
 
   ProgramStateRef cloneAsPosteriorlyOverconstrained() const;
+
+public:
   bool isPosteriorlyOverconstrained() const {
 return PosteriorlyOverconstrained;
   }
 
-public:
   /// This ctor is used when creating the first ProgramState object.
   ProgramState(ProgramStateManager *mgr, const Environment& env,
   StoreRef st, GenericDataMap gdm);


Index: clang/test/Analysis/runtime-regression.c
===
--- /dev/null
+++ clang/test/Analysis/runtime-regression.c
@@ -0,0 +1,54 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core,alpha.security.ArrayBoundV2 \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -verify
+
+// This test is here to check if there is no significant run-time regression
+// related to

[PATCH] D125904: [Cuda] Use fallback method to mangle externalized decls if no CUID given

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D125904#3537952 , @tra wrote:

> How much work would it take to add cuid generation in the new driver, similar 
> to what the old driver does, using the same logic, however imperfect it is? 
> I'd be OK with that as a possibly permanent solution.
>
> I'm somewhat wary of temporary solutions as they tend to become permanent and 
> age poorly.  
> That said, I'm OK with someone else tie-breaking us here. 
> @yaxunl  -- Sam, do you have an opinion?

I am OK with this patch.

I doubt it is possible to find a solution for static variable without some form 
of CUID. Assuming we can rename duplicate symbols in device linker. We still 
need to let the host compiler know which symbol is for which TU+option, then we 
still need some CUID to match device and host compilation.

That said, this patch provided a default CUID that do not depend on driver, 
which is its advantage. It should be OK for most usecases. Driver provided CUID 
has more robustness, so it can serve as last resort. If the fallback is not 
sufficient for the new driver then we can revisit this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125904

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

[PATCH] D126405: [clang][dataflow] Relax assert on existence of `this` pointee storage

[Eric Li via Phabricator via cfe-commits]

li.zhe.hua created this revision.
li.zhe.hua added a reviewer: ymandel.
Herald added subscribers: tschuett, steakhal.
Herald added a project: All.
li.zhe.hua requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Support for unions is incomplete (per 99f7d55e 
) and the 
`this` pointee
storage location is not set for unions. The assert in
`VisitCXXThisExpr` is then guaranteed to trigger when analyzing member
functions of a union.

This commit changes the assert to an early-return. Any expression may
be undefined, and so having a value for the `CXXThisExpr` is not a
postcondition of the transfer function.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126405

Files:
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -42,10 +42,11 @@
   template 
   void runDataflow(llvm::StringRef Code, Matcher Match,
LangStandard::Kind Std = LangStandard::lang_cxx17,
-   bool ApplyBuiltinTransfer = true) {
+   bool ApplyBuiltinTransfer = true,
+   llvm::StringRef TargetFun = "target") {
 ASSERT_THAT_ERROR(
 test::checkDataflow(
-Code, "target",
+Code, TargetFun,
 [ApplyBuiltinTransfer](ASTContext &C, Environment &) {
   return NoopAnalysis(C, ApplyBuiltinTransfer);
 },
@@ -3175,4 +3176,27 @@
 });
 }
 
+TEST_F(TransferTest, DoesNotCrashOnUnionThisExpr) {
+  std::string Code = R"(
+union Union {
+int A;
+float B;
+};
+
+void foo() {
+Union A;
+Union B;
+A = B;
+}
+  )";
+  // This is a crash regression test when calling the transfer function on a
+  // `CXXThisExpr` that refers to a union.
+  runDataflow(
+  Code,
+  [](llvm::ArrayRef<
+ std::pair>>,
+ ASTContext &) {},
+  LangStandard::lang_cxx17, /*ApplyBuiltinTransfer=*/true, "operator=");
+}
+
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -279,7 +279,8 @@
 
   void VisitCXXThisExpr(const CXXThisExpr *S) {
 auto *ThisPointeeLoc = Env.getThisPointeeStorageLocation();
-assert(ThisPointeeLoc != nullptr);
+if (ThisPointeeLoc == nullptr)
+  return;
 
 auto &Loc = Env.createStorageLocation(*S);
 Env.setStorageLocation(*S, Loc);


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -42,10 +42,11 @@
   template 
   void runDataflow(llvm::StringRef Code, Matcher Match,
LangStandard::Kind Std = LangStandard::lang_cxx17,
-   bool ApplyBuiltinTransfer = true) {
+   bool ApplyBuiltinTransfer = true,
+   llvm::StringRef TargetFun = "target") {
 ASSERT_THAT_ERROR(
 test::checkDataflow(
-Code, "target",
+Code, TargetFun,
 [ApplyBuiltinTransfer](ASTContext &C, Environment &) {
   return NoopAnalysis(C, ApplyBuiltinTransfer);
 },
@@ -3175,4 +3176,27 @@
 });
 }
 
+TEST_F(TransferTest, DoesNotCrashOnUnionThisExpr) {
+  std::string Code = R"(
+union Union {
+int A;
+float B;
+};
+
+void foo() {
+Union A;
+Union B;
+A = B;
+}
+  )";
+  // This is a crash regression test when calling the transfer function on a
+  // `CXXThisExpr` that refers to a union.
+  runDataflow(
+  Code,
+  [](llvm::ArrayRef<
+ std::pair>>,
+ ASTContext &) {},
+  LangStandard::lang_cxx17, /*ApplyBuiltinTransfer=*/true, "operator=");
+}
+
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -279,7 +279,8 @@
 
   void VisitCXXThisExpr(const CXXThisExpr *S) {
 auto *ThisPointeeLoc = Env.getThisPointeeStorageLocation();
-assert(ThisPointeeLoc != nullptr);
+if (ThisPointeeLoc == nullptr)
+  return;
 
 auto &Loc = Env.createStorageLocation(*S);
 Env.setStorageLocation(*S, Loc);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/lis

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

[Tom Stellard via Phabricator via cfe-commits]

tstellar added inline comments.



Comment at: clang/utils/hmaptool/CMakeLists.txt:5
 
-add_custom_command(OUTPUT 
${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/${CLANG_HMAPTOOL}
-   COMMAND ${CMAKE_COMMAND} -E make_directory
- ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin
-   COMMAND ${CMAKE_COMMAND} -E copy
- ${CMAKE_CURRENT_SOURCE_DIR}/${CLANG_HMAPTOOL}
- ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/
-   DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/${CLANG_HMAPTOOL})
-
-list(APPEND Depends 
${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/${CLANG_HMAPTOOL})
-install(PROGRAMS ${CLANG_HMAPTOOL}
-DESTINATION "${CMAKE_INSTALL_BINDIR}"
-COMPONENT hmaptool)
-
-add_custom_target(hmaptool ALL DEPENDS ${Depends})
+install(PROGRAMS hmaptool DESTINATION "${LLVM_UTILS_INSTALL_DIR}}" COMPONENT 
hmaptool)
+add_custom_target(hmaptool ALL DEPENDS "${LLVM_TOOLS_BINARY_DIR}/hmaptool")

mizvekov wrote:
> tstellar wrote:
> > Should this be LLVM_TOOLS_BINARY_DIR ?
> I think LLVM_UTILS_INSTALL_DIR is the path where the programs will be 
> installed, and LLVM_TOOLS_BINARY_DIR is where they are located in the build 
> tree.
> 
> So I think the idea is that this install invocation will install the file, 
> for the packaging for example, while add_custom_command above will just copy 
> this program into the build tree so that llvm-lit will find it when run from 
> the build tree.
OK, I see.  Makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126308

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

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

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added a comment.

In D126308#3538064 , 
@stephenneuendorffer wrote:

> LGTM... Does LLVM_TOOLS_BINARY_DIR include CMAKE_CFG_INTDIR?  Is this 
> actually NFC?

On a normal build where the CMakelists in the llvm subdir is used as the top 
level one, this should be NFC.

If the llvm CMakeLists is added from another CMakeLists, then mostly everything 
works, except that some odd cases like this one get copied somewhere the lit 
tests do not know how to find.




Comment at: clang/utils/hmaptool/CMakeLists.txt:1
-set(CLANG_HMAPTOOL hmaptool)
 

stephenneuendorffer wrote:
> Unused elsewhere, I assume?
Yeah, as far as I understand, `set` by default will only create / modify 
variables in the current scope, where it will be accessible only from this 
CMakeLists.txt and any others which you add_subdirectory from here on 
recursively. Variables are inherited from scope to scope by copy, not reference.

You need to otherwise pass PARENT_SCOPE argument into it so it will modify the 
copy in the enclosing scope, which sadly will not modify the copy on the 
current scope if one existed.



Comment at: clang/utils/hmaptool/CMakeLists.txt:5
 
-add_custom_command(OUTPUT 
${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/${CLANG_HMAPTOOL}
-   COMMAND ${CMAKE_COMMAND} -E make_directory
- ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin
-   COMMAND ${CMAKE_COMMAND} -E copy
- ${CMAKE_CURRENT_SOURCE_DIR}/${CLANG_HMAPTOOL}
- ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/
-   DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/${CLANG_HMAPTOOL})
-
-list(APPEND Depends 
${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/${CLANG_HMAPTOOL})
-install(PROGRAMS ${CLANG_HMAPTOOL}
-DESTINATION "${CMAKE_INSTALL_BINDIR}"
-COMPONENT hmaptool)
-
-add_custom_target(hmaptool ALL DEPENDS ${Depends})
+install(PROGRAMS hmaptool DESTINATION "${LLVM_UTILS_INSTALL_DIR}}" COMPONENT 
hmaptool)
+add_custom_target(hmaptool ALL DEPENDS "${LLVM_TOOLS_BINARY_DIR}/hmaptool")

tstellar wrote:
> Should this be LLVM_TOOLS_BINARY_DIR ?
I think LLVM_UTILS_INSTALL_DIR is the path where the programs will be 
installed, and LLVM_TOOLS_BINARY_DIR is where they are located in the build 
tree.

So I think the idea is that this install invocation will install the file, for 
the packaging for example, while add_custom_command above will just copy this 
program into the build tree so that llvm-lit will find it when run from the 
build tree.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126308

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

[PATCH] D126158: [MLIR][GPU] Replace fdiv on fp16 with promoted (fp32) multiplication with reciprocal plus one (conditional) Newton iteration.

[Christian Sigg via Phabricator via cfe-commits]

csigg updated this revision to Diff 432074.
csigg added a comment.

Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126158

Files:
  mlir/include/mlir/Dialect/LLVMIR/NVVMOps.td
  mlir/lib/Conversion/GPUToNVVM/LowerGpuOpsToNVVMOps.cpp
  mlir/test/Conversion/GPUToNVVM/gpu-to-nvvm.mlir
  mlir/test/Dialect/LLVMIR/nvvm.mlir
  mlir/test/Target/LLVMIR/nvvmir.mlir

Index: mlir/test/Target/LLVMIR/nvvmir.mlir
===
--- mlir/test/Target/LLVMIR/nvvmir.mlir
+++ mlir/test/Target/LLVMIR/nvvmir.mlir
@@ -1,5 +1,6 @@
 // RUN: mlir-translate -mlir-to-llvmir %s | FileCheck %s
 
+// CHECK-LABEL: @nvvm_special_regs
 llvm.func @nvvm_special_regs() -> i32 {
   // CHECK: %1 = call i32 @llvm.nvvm.read.ptx.sreg.tid.x()
   %1 = nvvm.read.ptx.sreg.tid.x : i32
@@ -32,12 +33,21 @@
   llvm.return %1 : i32
 }
 
+// CHECK-LABEL: @nvvm_rcp
+llvm.func @nvvm_rcp(%0: f32) -> f32 {
+  // CHECK: call float @llvm.nvvm.rcp.approx.ftz.f
+  %1 = nvvm.rcp.approx.ftz.f %0 : f32
+  llvm.return %1 : f32
+}
+
+// CHECK-LABEL: @llvm_nvvm_barrier0
 llvm.func @llvm_nvvm_barrier0() {
   // CHECK: call void @llvm.nvvm.barrier0()
   nvvm.barrier0
   llvm.return
 }
 
+// CHECK-LABEL: @nvvm_shfl
 llvm.func @nvvm_shfl(
 %0 : i32, %1 : i32, %2 : i32,
 %3 : i32, %4 : f32) -> i32 {
@@ -60,6 +70,7 @@
   llvm.return %6 : i32
 }
 
+// CHECK-LABEL: @nvvm_shfl_pred
 llvm.func @nvvm_shfl_pred(
 %0 : i32, %1 : i32, %2 : i32,
 %3 : i32, %4 : f32) -> !llvm.struct<(i32, i1)> {
@@ -82,6 +93,7 @@
   llvm.return %6 : !llvm.struct<(i32, i1)>
 }
 
+// CHECK-LABEL: @nvvm_vote
 llvm.func @nvvm_vote(%0 : i32, %1 : i1) -> i32 {
   // CHECK: call i32 @llvm.nvvm.vote.ballot.sync(i32 %{{.*}}, i1 %{{.*}})
   %3 = nvvm.vote.ballot.sync %0, %1 : i32
@@ -99,6 +111,7 @@
   llvm.return %0 : !llvm.struct<(f32, f32, f32, f32, f32, f32, f32, f32)>
 }
 
+// CHECK-LABEL: @nvvm_mma_m16n8k16_f16_f16
 llvm.func @nvvm_mma_m16n8k16_f16_f16(%a0 : vector<2xf16>, %a1 : vector<2xf16>,
 %a2 : vector<2xf16>, %a3 : vector<2xf16>,
 %b0 : vector<2xf16>, %b1 : vector<2xf16>,
@@ -111,6 +124,7 @@
 }
 
 // f32 return type, f16 accumulate type
+// CHECK-LABEL: @nvvm_mma_m16n8k16_f32_f16
 llvm.func @nvvm_mma_m16n8k16_f32_f16(%a0 : vector<2xf16>, %a1 : vector<2xf16>,
 %a2 : vector<2xf16>, %a3 : vector<2xf16>,
 %b0 : vector<2xf16>, %b1 : vector<2xf16>,
@@ -123,6 +137,7 @@
 }
 
 // f16 return type, f32 accumulate type
+// CHECK-LABEL: @nvvm_mma_m16n8k16_f16_f32
 llvm.func @nvvm_mma_m16n8k16_f16_f32(%a0 : vector<2xf16>, %a1 : vector<2xf16>,
 %a2 : vector<2xf16>, %a3 : vector<2xf16>,
 %b0 : vector<2xf16>, %b1 : vector<2xf16>,
@@ -135,6 +150,7 @@
 }
 
 // f32 return type, f32 accumulate type
+// CHECK-LABEL: @nvvm_mma_m16n8k16_f32_f32
 llvm.func @nvvm_mma_m16n8k16_f32_f32(%a0 : vector<2xf16>, %a1 : vector<2xf16>,
 %a2 : vector<2xf16>, %a3 : vector<2xf16>,
 %b0 : vector<2xf16>, %b1 : vector<2xf16>,
@@ -146,7 +162,8 @@
   llvm.return %0 : !llvm.struct<(f32, f32, f32, f32)>
 }
 
-llvm.func @nvvm_mma_m16n8k16_s8_s8(%a0 : i32, %a1 : i32,
+// CHECK-LABEL: @nvvm_mma_m16n8k16_s8_s8
+llvm.func @nvvm_mma_m16n8k16_s8_s8(%a0 : i32, %a1 : i32,
 %b0 : i32, 
 %c0 : i32, %c1 : i32, %c2 : i32, %c3 : i32) -> !llvm.struct<(i32, i32, i32, i32)> {
   // CHECK: call { i32, i32, i32, i32 } @llvm.nvvm.mma.m16n8k16.row.col.s8
@@ -158,7 +175,8 @@
   llvm.return %0 : !llvm.struct<(i32,i32,i32,i32)>
 }
 
-llvm.func @nvvm_mma_m16n8k16_s8_u8(%a0 : i32, %a1 : i32,
+// CHECK-LABEL: @nvvm_mma_m16n8k16_s8_u8
+llvm.func @nvvm_mma_m16n8k16_s8_u8(%a0 : i32, %a1 : i32,
 %b0 : i32, 
 %c0 : i32, %c1 : i32, %c2 : i32, %c3 : i32) -> !llvm.struct<(i32, i32, i32, i32)> {
   // CHECK: call { i32, i32, i32, i32 } @llvm.nvvm.mma.m16n8k16.row.col.satfinite.s8.u8
@@ -170,7 +188,8 @@
   llvm.return %0 : !llvm.struct<(i32,i32,i32,i32)>
 }
 
-llvm.func @nvvm_mma_m16n8k128_b1_b1(%a0 : i32, %a1 : i32, 
+// CHECK-LABEL: @nvvm_mma_m16n8k128_b1_b1
+llvm.func @nvvm_mma_m16n8k128_b1_b1(%a0 : i32, %a1 : i32,
 %b0 : i32,
 %c0 : i32, %c1 : i32, %c2 : i32, %c3 : i32) -> !llvm.struct<(i32,i32,i32,i32)> {  
   // CHECK: call { i32, i32, i32, i32 } @llvm.nvvm.mma.xor.popc.m16n8k128.row.col.b1
@@ -181,6 +200,7 @@
   llvm.return %0 : !llvm.struct<(i32,i32,i32,i32)>
 }
 
+// CHECK-LABEL: @nvvm_mma_m16n8k32_s4_s4
 llvm.func @nvvm_mma_m16n8k32_s4_s4(%a0 : i32, %a1 : i32,
%b0 : i32,
 

[PATCH] D124758: [analyzer] Implement assume in terms of assumeDual

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

> This change in itself reduced the run-time of the analysis to 16 seconds, on 
> my machine. However, the repetition of States should still be addressed. I am 
> going to upload the upper patch for a starter.

Sorry, in that 16s, I measured also the rebuild and linkage of the Clang 
binary. The time is actually way better, 2.8s, which is quite close to the 
original values we had before this change. So, perhaps it is not even needed to 
bother with the above mentioned cache mechanism.

   time ./bin/clang --analyze -Xclang 
-analyzer-checker=core,alpha.security.ArrayBoundV2,debug.ExprInspection test.c
  test.c:14:3: warning: 1 [debug.ExprInspection]
clang_analyzer_numTimesReached(); // 1 times
^~~~
  test.c:16:5: warning: 253 [debug.ExprInspection]
  clang_analyzer_numTimesReached(); // 285 times
  ^~~~
  test.c:21:5: warning: 805 [debug.ExprInspection]
  clang_analyzer_numTimesReached(); // 1128 times
  ^~~~
  test.c:24:7: warning: 487 [debug.ExprInspection]
clang_analyzer_numTimesReached(); // 560 times
^~~~
  4 warnings generated.
  ./bin/clang --analyze -Xclang  test.c  2.74s user 0.07s system 99% cpu 2.811 
total


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124758

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

[PATCH] D126369: [LLVM] Add rcp.approx.ftz.f32 intrinsic

[Christian Sigg 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 rGc4bc416418a2: [LLVM] Add rcp.approx.ftz.f32 intrinsic 
(authored by csigg).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126369

Files:
  clang/include/clang/Basic/BuiltinsNVPTX.def
  llvm/include/llvm/IR/IntrinsicsNVVM.td
  llvm/lib/Target/NVPTX/NVPTXIntrinsics.td


Index: llvm/lib/Target/NVPTX/NVPTXIntrinsics.td
===
--- llvm/lib/Target/NVPTX/NVPTXIntrinsics.td
+++ llvm/lib/Target/NVPTX/NVPTXIntrinsics.td
@@ -1034,6 +1034,8 @@
 def INT_NVVM_RCP_RP_D : F_MATH_1<"rcp.rp.f64 \t$dst, $src0;", Float64Regs,
   Float64Regs, int_nvvm_rcp_rp_d>;
 
+def INT_NVVM_RCP_APPROX_FTZ_F : F_MATH_1<"rcp.approx.ftz.f32 \t$dst, $src0;",
+  Float32Regs, Float32Regs, int_nvvm_rcp_approx_ftz_f>;
 def INT_NVVM_RCP_APPROX_FTZ_D : F_MATH_1<"rcp.approx.ftz.f64 \t$dst, $src0;",
   Float64Regs, Float64Regs, int_nvvm_rcp_approx_ftz_d>;
 
Index: llvm/include/llvm/IR/IntrinsicsNVVM.td
===
--- llvm/include/llvm/IR/IntrinsicsNVVM.td
+++ llvm/include/llvm/IR/IntrinsicsNVVM.td
@@ -933,6 +933,8 @@
   def int_nvvm_rcp_rp_d : GCCBuiltin<"__nvvm_rcp_rp_d">,
   DefaultAttrsIntrinsic<[llvm_double_ty], [llvm_double_ty], [IntrNoMem]>;
 
+  def int_nvvm_rcp_approx_ftz_f : GCCBuiltin<"__nvvm_rcp_approx_ftz_f">,
+  DefaultAttrsIntrinsic<[llvm_float_ty], [llvm_float_ty], [IntrNoMem]>;
   def int_nvvm_rcp_approx_ftz_d : GCCBuiltin<"__nvvm_rcp_approx_ftz_d">,
   DefaultAttrsIntrinsic<[llvm_double_ty], [llvm_double_ty], [IntrNoMem]>;
 
Index: clang/include/clang/Basic/BuiltinsNVPTX.def
===
--- clang/include/clang/Basic/BuiltinsNVPTX.def
+++ clang/include/clang/Basic/BuiltinsNVPTX.def
@@ -343,6 +343,8 @@
 BUILTIN(__nvvm_rcp_rz_d, "dd", "")
 BUILTIN(__nvvm_rcp_rm_d, "dd", "")
 BUILTIN(__nvvm_rcp_rp_d, "dd", "")
+
+BUILTIN(__nvvm_rcp_approx_ftz_f, "ff", "")
 BUILTIN(__nvvm_rcp_approx_ftz_d, "dd", "")
 
 // Sqrt


Index: llvm/lib/Target/NVPTX/NVPTXIntrinsics.td
===
--- llvm/lib/Target/NVPTX/NVPTXIntrinsics.td
+++ llvm/lib/Target/NVPTX/NVPTXIntrinsics.td
@@ -1034,6 +1034,8 @@
 def INT_NVVM_RCP_RP_D : F_MATH_1<"rcp.rp.f64 \t$dst, $src0;", Float64Regs,
   Float64Regs, int_nvvm_rcp_rp_d>;
 
+def INT_NVVM_RCP_APPROX_FTZ_F : F_MATH_1<"rcp.approx.ftz.f32 \t$dst, $src0;",
+  Float32Regs, Float32Regs, int_nvvm_rcp_approx_ftz_f>;
 def INT_NVVM_RCP_APPROX_FTZ_D : F_MATH_1<"rcp.approx.ftz.f64 \t$dst, $src0;",
   Float64Regs, Float64Regs, int_nvvm_rcp_approx_ftz_d>;
 
Index: llvm/include/llvm/IR/IntrinsicsNVVM.td
===
--- llvm/include/llvm/IR/IntrinsicsNVVM.td
+++ llvm/include/llvm/IR/IntrinsicsNVVM.td
@@ -933,6 +933,8 @@
   def int_nvvm_rcp_rp_d : GCCBuiltin<"__nvvm_rcp_rp_d">,
   DefaultAttrsIntrinsic<[llvm_double_ty], [llvm_double_ty], [IntrNoMem]>;
 
+  def int_nvvm_rcp_approx_ftz_f : GCCBuiltin<"__nvvm_rcp_approx_ftz_f">,
+  DefaultAttrsIntrinsic<[llvm_float_ty], [llvm_float_ty], [IntrNoMem]>;
   def int_nvvm_rcp_approx_ftz_d : GCCBuiltin<"__nvvm_rcp_approx_ftz_d">,
   DefaultAttrsIntrinsic<[llvm_double_ty], [llvm_double_ty], [IntrNoMem]>;
 
Index: clang/include/clang/Basic/BuiltinsNVPTX.def
===
--- clang/include/clang/Basic/BuiltinsNVPTX.def
+++ clang/include/clang/Basic/BuiltinsNVPTX.def
@@ -343,6 +343,8 @@
 BUILTIN(__nvvm_rcp_rz_d, "dd", "")
 BUILTIN(__nvvm_rcp_rm_d, "dd", "")
 BUILTIN(__nvvm_rcp_rp_d, "dd", "")
+
+BUILTIN(__nvvm_rcp_approx_ftz_f, "ff", "")
 BUILTIN(__nvvm_rcp_approx_ftz_d, "dd", "")
 
 // Sqrt
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] c4bc416 - [LLVM] Add rcp.approx.ftz.f32 intrinsic

[Christian Sigg via cfe-commits]

Author: Christian Sigg
Date: 2022-05-25T21:05:20+02:00
New Revision: c4bc416418a2d8a86b699060efa47515de5ffbb6

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

LOG: [LLVM] Add rcp.approx.ftz.f32 intrinsic

Split out from https://reviews.llvm.org/D126158.

Reviewed By: tra

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

Added: 


Modified: 
clang/include/clang/Basic/BuiltinsNVPTX.def
llvm/include/llvm/IR/IntrinsicsNVVM.td
llvm/lib/Target/NVPTX/NVPTXIntrinsics.td

Removed: 




diff  --git a/clang/include/clang/Basic/BuiltinsNVPTX.def 
b/clang/include/clang/Basic/BuiltinsNVPTX.def
index 04bed16c9958e..a5ec77a6112c0 100644
--- a/clang/include/clang/Basic/BuiltinsNVPTX.def
+++ b/clang/include/clang/Basic/BuiltinsNVPTX.def
@@ -343,6 +343,8 @@ BUILTIN(__nvvm_rcp_rn_d, "dd", "")
 BUILTIN(__nvvm_rcp_rz_d, "dd", "")
 BUILTIN(__nvvm_rcp_rm_d, "dd", "")
 BUILTIN(__nvvm_rcp_rp_d, "dd", "")
+
+BUILTIN(__nvvm_rcp_approx_ftz_f, "ff", "")
 BUILTIN(__nvvm_rcp_approx_ftz_d, "dd", "")
 
 // Sqrt

diff  --git a/llvm/include/llvm/IR/IntrinsicsNVVM.td 
b/llvm/include/llvm/IR/IntrinsicsNVVM.td
index 678001f44527d..6bdd0b0775be9 100644
--- a/llvm/include/llvm/IR/IntrinsicsNVVM.td
+++ b/llvm/include/llvm/IR/IntrinsicsNVVM.td
@@ -933,6 +933,8 @@ let TargetPrefix = "nvvm" in {
   def int_nvvm_rcp_rp_d : GCCBuiltin<"__nvvm_rcp_rp_d">,
   DefaultAttrsIntrinsic<[llvm_double_ty], [llvm_double_ty], [IntrNoMem]>;
 
+  def int_nvvm_rcp_approx_ftz_f : GCCBuiltin<"__nvvm_rcp_approx_ftz_f">,
+  DefaultAttrsIntrinsic<[llvm_float_ty], [llvm_float_ty], [IntrNoMem]>;
   def int_nvvm_rcp_approx_ftz_d : GCCBuiltin<"__nvvm_rcp_approx_ftz_d">,
   DefaultAttrsIntrinsic<[llvm_double_ty], [llvm_double_ty], [IntrNoMem]>;
 

diff  --git a/llvm/lib/Target/NVPTX/NVPTXIntrinsics.td 
b/llvm/lib/Target/NVPTX/NVPTXIntrinsics.td
index d30c3d13f7ce0..1192cc0784084 100644
--- a/llvm/lib/Target/NVPTX/NVPTXIntrinsics.td
+++ b/llvm/lib/Target/NVPTX/NVPTXIntrinsics.td
@@ -1034,6 +1034,8 @@ def INT_NVVM_RCP_RM_D : F_MATH_1<"rcp.rm.f64 \t$dst, 
$src0;", Float64Regs,
 def INT_NVVM_RCP_RP_D : F_MATH_1<"rcp.rp.f64 \t$dst, $src0;", Float64Regs,
   Float64Regs, int_nvvm_rcp_rp_d>;
 
+def INT_NVVM_RCP_APPROX_FTZ_F : F_MATH_1<"rcp.approx.ftz.f32 \t$dst, $src0;",
+  Float32Regs, Float32Regs, int_nvvm_rcp_approx_ftz_f>;
 def INT_NVVM_RCP_APPROX_FTZ_D : F_MATH_1<"rcp.approx.ftz.f64 \t$dst, $src0;",
   Float64Regs, Float64Regs, int_nvvm_rcp_approx_ftz_d>;
 



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

[PATCH] D126369: [LLVM] Add rcp.approx.ftz.f32 intrinsic

[Christian Sigg via Phabricator via cfe-commits]

csigg added a comment.

Thanks Artem. I think I should be able to land it myself.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126369

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

[PATCH] D126398: [Clang] Introduce `--offload-link` option to perform offload device linking

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 updated this revision to Diff 432071.
jhuber6 added a comment.

Removing `-dlink`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126398

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/cuda-openmp-driver.cu


Index: clang/test/Driver/cuda-openmp-driver.cu
===
--- clang/test/Driver/cuda-openmp-driver.cu
+++ clang/test/Driver/cuda-openmp-driver.cu
@@ -35,3 +35,8 @@
 
 // RUN: %clang -### -target x86_64-linux-gnu -nocudalib --cuda-feature=+ptx61 
--offload-arch=sm_70 %s 2>&1 | FileCheck -check-prefix MANUAL-FEATURE %s
 // MANUAL-FEATURE: -cc1{{.*}}-target-feature{{.*}}+ptx61
+
+// RUN: %clang -### -target x86_64-linux-gnu -nocudalib -ccc-print-bindings 
--offload-link %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE-LINK %s
+
+// DEVICE-LINK: "x86_64-unknown-linux-gnu" - "Offload::Linker", inputs: 
["[[INPUT:.+]]"], output: "a.out"
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4158,7 +4158,8 @@
 // Check if this Linker Job should emit a static library.
 if (ShouldEmitStaticLibrary(Args)) {
   LA = C.MakeAction(LinkerInputs, types::TY_Image);
-} else if (UseNewOffloadingDriver) {
+} else if (UseNewOffloadingDriver ||
+   Args.hasArg(options::OPT_offload_link)) {
   LA = C.MakeAction(LinkerInputs, types::TY_Image);
   LA->propagateHostOffloadInfo(C.getActiveOffloadKinds(),
/*BoundArch=*/nullptr);
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -820,6 +820,8 @@
 def z : Separate<["-"], "z">, Flags<[LinkerInput, RenderAsInput]>,
   HelpText<"Pass -z  to the linker">, MetaVarName<"">,
   Group;
+def offload_link : Flag<["--"], "offload-link">, Group,
+  HelpText<"Use the new offloading linker to perform the link job.">;
 def Xlinker : Separate<["-"], "Xlinker">, Flags<[LinkerInput, RenderAsInput]>,
   HelpText<"Pass  to the linker">, MetaVarName<"">,
   Group;
Index: clang/docs/ClangCommandLineReference.rst
===
--- clang/docs/ClangCommandLineReference.rst
+++ clang/docs/ClangCommandLineReference.rst
@@ -4209,6 +4209,10 @@
 
 Pass the comma separated arguments in  to the linker
 
+.. option:: --offload-link
+
+Use the linker supporting offloading device linking.
+
 .. option:: -X
 
 .. option:: -Xlinker , --for-linker , --for-linker=


Index: clang/test/Driver/cuda-openmp-driver.cu
===
--- clang/test/Driver/cuda-openmp-driver.cu
+++ clang/test/Driver/cuda-openmp-driver.cu
@@ -35,3 +35,8 @@
 
 // RUN: %clang -### -target x86_64-linux-gnu -nocudalib --cuda-feature=+ptx61 --offload-arch=sm_70 %s 2>&1 | FileCheck -check-prefix MANUAL-FEATURE %s
 // MANUAL-FEATURE: -cc1{{.*}}-target-feature{{.*}}+ptx61
+
+// RUN: %clang -### -target x86_64-linux-gnu -nocudalib -ccc-print-bindings --offload-link %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE-LINK %s
+
+// DEVICE-LINK: "x86_64-unknown-linux-gnu" - "Offload::Linker", inputs: ["[[INPUT:.+]]"], output: "a.out"
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4158,7 +4158,8 @@
 // Check if this Linker Job should emit a static library.
 if (ShouldEmitStaticLibrary(Args)) {
   LA = C.MakeAction(LinkerInputs, types::TY_Image);
-} else if (UseNewOffloadingDriver) {
+} else if (UseNewOffloadingDriver ||
+   Args.hasArg(options::OPT_offload_link)) {
   LA = C.MakeAction(LinkerInputs, types::TY_Image);
   LA->propagateHostOffloadInfo(C.getActiveOffloadKinds(),
/*BoundArch=*/nullptr);
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -820,6 +820,8 @@
 def z : Separate<["-"], "z">, Flags<[LinkerInput, RenderAsInput]>,
   HelpText<"Pass -z  to the linker">, MetaVarName<"">,
   Group;
+def offload_link : Flag<["--"], "offload-link">, Group,
+  HelpText<"Use the new offloading linker to perform the link job.">;
 def Xlinker : Separate<["-"], "Xlinker">, Flags<[LinkerInput, RenderAsInput]>,
   HelpText<"Pass  to the linker">, MetaVarName<"">,
   Group;
Index: clang/docs/ClangCommandLineReference.rst
===
--- clang/docs/ClangCommandLineReference.rst
+++ clang/d

[PATCH] D126398: [Clang] Introduce `-dlink` option to perform offload device linking

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added inline comments.



Comment at: clang/include/clang/Driver/Options.td:825-826
+  HelpText<"Use the new offloading linker to perform the link job.">;
+def device_link : Flag<["-"], "dlink">, Group,
+  Alias;
 def Xlinker : Separate<["-"], "Xlinker">, Flags<[LinkerInput, RenderAsInput]>,

tra wrote:
> We typically use option aliases to provide compatibility with the legacy 
> options. AFAICT there are no current uses of `Alias` for the sake of saving a 
> few characters in an option name.
> 
> Is `-dlink` really needed? It's not going to be typed manually all that 
> often, and a slightly longer option does not make any difference for cmake or 
> make files.
> 
> I assume that partial motivation for `-dlink` is that it's a shortened alias 
> used by NVCC for its functionally similar --device-link option. I do not 
> think it buys us anything. We never intended to be option-compatible with 
> nvcc and clang's CUDA compilation and relevant options have only partial 
> overlap with NVCC's functionality-wise and almost none syntax-wise. Adding 
> one rarely used option for the same of matching NVCC's is not worth it, IMO.
Yeah, it's somewhat mental compatibility with Nvidia parlance, see `-fgpu-rdc` 
and `-rdc=true`, if you think that's not necessary I'll just make it 
`--offload-link`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126398

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

[PATCH] D126170: C++ DR2394: Const-default-constructible for members.

[James Y Knight 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 rG997b072e10d2: C++ DR2394: Const-default-constructible for 
members. (authored by jyknight).

Changed prior to commit:
  https://reviews.llvm.org/D126170?vs=431239&id=432068#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126170

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/drs/dr23xx.cpp
  clang/test/CXX/special/class.ctor/p5-0x.cpp
  clang/test/SemaCXX/cxx0x-deleted-default-ctor.cpp
  clang/www/cxx_dr_status.html

Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -14178,7 +14178,7 @@
 https://wg21.link/cwg2394";>2394
 CD5
 Const-default-constructible for members
-Unknown
+Clang 15
   
   
 https://wg21.link/cwg2395";>2395
Index: clang/test/SemaCXX/cxx0x-deleted-default-ctor.cpp
===
--- clang/test/SemaCXX/cxx0x-deleted-default-ctor.cpp
+++ clang/test/SemaCXX/cxx0x-deleted-default-ctor.cpp
@@ -48,8 +48,12 @@
 };
 good g;
 
+struct bad_const_inner {
+  int x;
+};
+
 struct bad_const {
-  const good g; // expected-note {{field 'g' of const-qualified type 'const good' would not be initialized}}
+  const bad_const_inner g; // expected-note {{field 'g' of const-qualified type 'const bad_const_inner' would not be initialized}}
 };
 bad_const bc; // expected-error {{call to implicitly-deleted default constructor}}
 
Index: clang/test/CXX/special/class.ctor/p5-0x.cpp
===
--- clang/test/CXX/special/class.ctor/p5-0x.cpp
+++ clang/test/CXX/special/class.ctor/p5-0x.cpp
@@ -2,6 +2,8 @@
 
 struct DefaultedDefCtor1 {};
 struct DefaultedDefCtor2 { DefaultedDefCtor2() = default; };
+struct DefaultedDefCtorUninitialized1 { int x; };
+struct DefaultedDefCtorUninitialized2 { int x; DefaultedDefCtorUninitialized2() = default; };
 struct DeletedDefCtor { DeletedDefCtor() = delete; DeletedDefCtor(int); }; // expected-note {{explicitly marked deleted here}}
 class PrivateDefCtor { PrivateDefCtor() = default; public: PrivateDefCtor(int); };
 struct DeletedDtor { ~DeletedDtor() = delete; }; // expected-note 4{{explicitly marked deleted here}}
@@ -51,21 +53,20 @@
 NotDeleted2d nd2d; // expected-note {{first required here}}
 
 // - any non-variant non-static data member of const qualified type (or array
-// thereof) with no brace-or-equal-initializer does not have a user-provided
-// default constructor,
+// thereof) with no brace-or-equal-initializer is not const-default-constructible
 class Deleted3a { const int a; }; // expected-note {{because field 'a' of const-qualified type 'const int' would not be initialized}} \
  expected-warning {{does not declare any constructor}} \
  expected-note {{will never be initialized}}
 Deleted3a d3a; // expected-error {{implicitly-deleted default constructor}}
-class Deleted3b { const DefaultedDefCtor1 a[42]; }; // expected-note {{because field 'a' of const-qualified type 'const DefaultedDefCtor1[42]' would not be initialized}}
+class Deleted3b { const DefaultedDefCtorUninitialized1 a[42]; }; // expected-note {{because field 'a' of const-qualified type 'const DefaultedDefCtorUninitialized1[42]' would not be initialized}}
 Deleted3b d3b; // expected-error {{implicitly-deleted default constructor}}
-class Deleted3c { const DefaultedDefCtor2 a; }; // expected-note {{because field 'a' of const-qualified type 'const DefaultedDefCtor2' would not be initialized}}
+class Deleted3c { const DefaultedDefCtorUninitialized2 a; }; // expected-note {{because field 'a' of const-qualified type 'const DefaultedDefCtorUninitialized2' would not be initialized}}
 Deleted3c d3c; // expected-error {{implicitly-deleted default constructor}}
 class NotDeleted3a { const int a = 0; };
 NotDeleted3a nd3a;
-class NotDeleted3b { const DefaultedDefCtor1 a[42] = {}; };
+class NotDeleted3b { const DefaultedDefCtorUninitialized1 a[42] = {}; };
 NotDeleted3b nd3b;
-class NotDeleted3c { const DefaultedDefCtor2 a = DefaultedDefCtor2(); };
+class NotDeleted3c { const DefaultedDefCtorUninitialized2 a = DefaultedDefCtorUninitialized2(); };
 NotDeleted3c nd3c;
 union NotDeleted3d { const int a; int b; };
 NotDeleted3d nd3d;
@@ -75,6 +76,10 @@
 NotDeleted3f nd3f;
 struct NotDeleted3g { union { const int a; int b; }; };
 NotDeleted3g nd3g;
+struct NotDeleted3h { const DefaultedDefCtor1 a[42]; };
+NotDeleted3h nd3h;
+struct NotDeleted3i { const DefaultedDefCtor2 a; };
+NotDeleted3i nd3i;
 
 // - X is a union and all of its variant members are of const-qualified type (or
 // array thereof),
Index: clang/test/CXX/drs/dr23xx.cpp

[clang] 997b072 - C++ DR2394: Const-default-constructible for members.

[James Y Knight via cfe-commits]

Author: James Y Knight
Date: 2022-05-25T14:20:11-04:00
New Revision: 997b072e10d2be09c0e1a5bf4d6b92e2da3b8cc6

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

LOG: C++ DR2394: Const-default-constructible for members.

Const class members may be initialized with a defaulted default
constructor under the same conditions it would be allowed for a const
object elsewhere.

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CXX/drs/dr23xx.cpp
clang/test/CXX/special/class.ctor/p5-0x.cpp
clang/test/SemaCXX/cxx0x-deleted-default-ctor.cpp
clang/www/cxx_dr_status.html

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a457a8fb2efe4..c34d29c8762d4 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -155,6 +155,9 @@ Bug Fixes
   a coroutine will no longer generate a call to a global allocation function
   with the signature (std::size_t, p0, ..., pn).
   This fixes Issue `Issue 54881 
`_.
+- Implement `CWG 2394 `_: Const class members
+  may be initialized with a defaulted default constructor under the same
+  conditions it would be allowed for a const object elsewhere.
 
 Improvements to Clang's diagnostics
 ^^^

diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index a16fe0b1b72a4..c470a46f58477 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -9213,13 +9213,12 @@ bool 
SpecialMemberDeletionInfo::shouldDeleteForField(FieldDecl *FD) {
   << !!ICI << MD->getParent() << FD << FieldType << /*Reference*/0;
   return true;
 }
-// C++11 [class.ctor]p5: any non-variant non-static data member of
-// const-qualified type (or array thereof) with no
-// brace-or-equal-initializer does not have a user-provided default
-// constructor.
+// C++11 [class.ctor]p5 (modified by DR2394): any non-variant non-static
+// data member of const-qualified type (or array thereof) with no
+// brace-or-equal-initializer is not const-default-constructible.
 if (!inUnion() && FieldType.isConstQualified() &&
 !FD->hasInClassInitializer() &&
-(!FieldRecord || !FieldRecord->hasUserProvidedDefaultConstructor())) {
+(!FieldRecord || !FieldRecord->allowConstDefaultInit())) {
   if (Diagnose)
 S.Diag(FD->getLocation(), diag::note_deleted_default_ctor_uninit_field)
   << !!ICI << MD->getParent() << FD << FD->getType() << /*Const*/1;

diff  --git a/clang/test/CXX/drs/dr23xx.cpp b/clang/test/CXX/drs/dr23xx.cpp
index a7ff2a5813ed0..8d6b4a5dc16ea 100644
--- a/clang/test/CXX/drs/dr23xx.cpp
+++ b/clang/test/CXX/drs/dr23xx.cpp
@@ -158,3 +158,14 @@ void g() {
 }
 } //namespace dr2303
 #endif
+
+namespace dr2394 { // dr2394: 15
+
+struct A {};
+const A a;
+
+// Now allowed to default-init B.
+struct B { const A a; };
+B b;
+
+}

diff  --git a/clang/test/CXX/special/class.ctor/p5-0x.cpp 
b/clang/test/CXX/special/class.ctor/p5-0x.cpp
index 67f0d2a935744..c433df64a7cf4 100644
--- a/clang/test/CXX/special/class.ctor/p5-0x.cpp
+++ b/clang/test/CXX/special/class.ctor/p5-0x.cpp
@@ -2,6 +2,8 @@
 
 struct DefaultedDefCtor1 {};
 struct DefaultedDefCtor2 { DefaultedDefCtor2() = default; };
+struct DefaultedDefCtorUninitialized1 { int x; };
+struct DefaultedDefCtorUninitialized2 { int x; 
DefaultedDefCtorUninitialized2() = default; };
 struct DeletedDefCtor { DeletedDefCtor() = delete; DeletedDefCtor(int); }; // 
expected-note {{explicitly marked deleted here}}
 class PrivateDefCtor { PrivateDefCtor() = default; public: 
PrivateDefCtor(int); };
 struct DeletedDtor { ~DeletedDtor() = delete; }; // expected-note 
4{{explicitly marked deleted here}}
@@ -51,21 +53,20 @@ class NotDeleted2d { int &&a = 0; }; // expected-error 
{{reference member 'a' bi
 NotDeleted2d nd2d; // expected-note {{first required here}}
 
 // - any non-variant non-static data member of const qualified type (or array
-// thereof) with no brace-or-equal-initializer does not have a user-provided
-// default constructor,
+// thereof) with no brace-or-equal-initializer is not 
const-default-constructible
 class Deleted3a { const int a; }; // expected-note {{because field 'a' of 
const-qualified type 'const int' would not be initialized}} \
  expected-warning {{does not declare any 
constructor}} \
  expected-note {{will never be 
initialized}}
 Deleted3a d3a; // expected-error {{implicitly-deleted default constructor}}
-class Deleted3b { const DefaultedDefCtor1 a[42]; };

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

[Stephen Neuendorffer via Phabricator via cfe-commits]

stephenneuendorffer added a comment.

LGTM... Does LLVM_TOOLS_BINARY_DIR include CMAKE_CFG_INTDIR?  Is this actually 
NFC?




Comment at: clang/utils/hmaptool/CMakeLists.txt:1
-set(CLANG_HMAPTOOL hmaptool)
 

Unused elsewhere, I assume?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126308

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

[PATCH] D126398: [Clang] Introduce `-dlink` option to perform offload device linking

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/include/clang/Driver/Options.td:825-826
+  HelpText<"Use the new offloading linker to perform the link job.">;
+def device_link : Flag<["-"], "dlink">, Group,
+  Alias;
 def Xlinker : Separate<["-"], "Xlinker">, Flags<[LinkerInput, RenderAsInput]>,

We typically use option aliases to provide compatibility with the legacy 
options. AFAICT there are no current uses of `Alias` for the sake of saving a 
few characters in an option name.

Is `-dlink` really needed? It's not going to be typed manually all that often, 
and a slightly longer option does not make any difference for cmake or make 
files.

I assume that partial motivation for `-dlink` is that it's a shortened alias 
used by NVCC for its functionally similar --device-link option. I do not think 
it buys us anything. We never intended to be option-compatible with nvcc and 
clang's CUDA compilation and relevant options have only partial overlap with 
NVCC's functionality-wise and almost none syntax-wise. Adding one rarely used 
option for the same of matching NVCC's is not worth it, IMO.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126398

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

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

[Tom Stellard via Phabricator via cfe-commits]

tstellar added a comment.

This




Comment at: clang/utils/hmaptool/CMakeLists.txt:5
 
-add_custom_command(OUTPUT 
${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/${CLANG_HMAPTOOL}
-   COMMAND ${CMAKE_COMMAND} -E make_directory
- ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin
-   COMMAND ${CMAKE_COMMAND} -E copy
- ${CMAKE_CURRENT_SOURCE_DIR}/${CLANG_HMAPTOOL}
- ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/
-   DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/${CLANG_HMAPTOOL})
-
-list(APPEND Depends 
${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/${CLANG_HMAPTOOL})
-install(PROGRAMS ${CLANG_HMAPTOOL}
-DESTINATION "${CMAKE_INSTALL_BINDIR}"
-COMPONENT hmaptool)
-
-add_custom_target(hmaptool ALL DEPENDS ${Depends})
+install(PROGRAMS hmaptool DESTINATION "${LLVM_UTILS_INSTALL_DIR}}" COMPONENT 
hmaptool)
+add_custom_target(hmaptool ALL DEPENDS "${LLVM_TOOLS_BINARY_DIR}/hmaptool")

Should this be LLVM_TOOLS_BINARY_DIR ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126308

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

[PATCH] D125848: [clang-format] Handle attributes in enum declaration.

[Tyler Chatow via Phabricator via cfe-commits]

tchatow added a comment.

No, I'll need someone to commit it on my behalf.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125848

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

[PATCH] D124758: [analyzer] Implement assume in terms of assumeDual

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Thanks Balazs for the report.

Here is my analysis. Looks like during the recursive simplification, `reAssume` 
 produces `State`s that had been created by a previous `reAssume`. Before this 
change, to stop the recursion it was enough to to check if the `OldState` 
equals to the actual `State` in `reAssume`. Now, with this change, each 
`assume` call evaluates both the true and the false branches, thus it is not 
necessary that the subsequent `reAssume` could detect an already "visited" 
State.
So, the obvious solution would be to have a `State` cache in the `reAssume` 
machinery, though, implementation details are not clear yet.

There is another really important thing. We should not continue with `reAssume` 
if the `State` is `posteriorlyOverConstrained`.

   LLVM_NODISCARD ProgramStateRef reAssume(ProgramStateRef State,
   const RangeSet *Constraint,
   SVal TheValue) {
  +  if (State->isPosteriorlyOverconstrained())
  +return nullptr;
 if (!Constraint)
   return State;

This change in itself reduced the run-time of the analysis to 16 seconds, on my 
machine. However, the repetition of States should still be addressed. I am 
going to upload the upper patch for a starter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124758

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

[PATCH] D125839: [gmodules] Skip CXXDeductionGuideDecls when visiting FunctionDecls in DebugTypeVisitor

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

ping.

Does debug info need information on deduction guides? I think it's safe to skip 
deduction guides as they are used only for template argument deduction.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125839

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

[PATCH] D126398: [Clang] Introduce `-dlink` option to perform offload device linking

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 updated this revision to Diff 432060.
jhuber6 added a comment.

Changing to use `--offload-link` and use `-dlink` as an alias.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126398

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/cuda-openmp-driver.cu


Index: clang/test/Driver/cuda-openmp-driver.cu
===
--- clang/test/Driver/cuda-openmp-driver.cu
+++ clang/test/Driver/cuda-openmp-driver.cu
@@ -35,3 +35,8 @@
 
 // RUN: %clang -### -target x86_64-linux-gnu -nocudalib --cuda-feature=+ptx61 
--offload-arch=sm_70 %s 2>&1 | FileCheck -check-prefix MANUAL-FEATURE %s
 // MANUAL-FEATURE: -cc1{{.*}}-target-feature{{.*}}+ptx61
+
+// RUN: %clang -### -target x86_64-linux-gnu -nocudalib -ccc-print-bindings 
-dlink %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE-LINK %s
+
+// DEVICE-LINK: "x86_64-unknown-linux-gnu" - "Offload::Linker", inputs: 
["[[INPUT:.+]]"], output: "a.out"
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4158,7 +4158,8 @@
 // Check if this Linker Job should emit a static library.
 if (ShouldEmitStaticLibrary(Args)) {
   LA = C.MakeAction(LinkerInputs, types::TY_Image);
-} else if (UseNewOffloadingDriver) {
+} else if (UseNewOffloadingDriver ||
+   Args.hasArg(options::OPT_offload_link)) {
   LA = C.MakeAction(LinkerInputs, types::TY_Image);
   LA->propagateHostOffloadInfo(C.getActiveOffloadKinds(),
/*BoundArch=*/nullptr);
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -820,6 +820,10 @@
 def z : Separate<["-"], "z">, Flags<[LinkerInput, RenderAsInput]>,
   HelpText<"Pass -z  to the linker">, MetaVarName<"">,
   Group;
+def offload_link : Flag<["--"], "offload-link">, Group,
+  HelpText<"Use the new offloading linker to perform the link job.">;
+def device_link : Flag<["-"], "dlink">, Group,
+  Alias;
 def Xlinker : Separate<["-"], "Xlinker">, Flags<[LinkerInput, RenderAsInput]>,
   HelpText<"Pass  to the linker">, MetaVarName<"">,
   Group;
Index: clang/docs/ClangCommandLineReference.rst
===
--- clang/docs/ClangCommandLineReference.rst
+++ clang/docs/ClangCommandLineReference.rst
@@ -4209,6 +4209,10 @@
 
 Pass the comma separated arguments in  to the linker
 
+.. option:: -dlink, --offload-link
+
+Use the linker supporting offloading device linking.
+
 .. option:: -X
 
 .. option:: -Xlinker , --for-linker , --for-linker=


Index: clang/test/Driver/cuda-openmp-driver.cu
===
--- clang/test/Driver/cuda-openmp-driver.cu
+++ clang/test/Driver/cuda-openmp-driver.cu
@@ -35,3 +35,8 @@
 
 // RUN: %clang -### -target x86_64-linux-gnu -nocudalib --cuda-feature=+ptx61 --offload-arch=sm_70 %s 2>&1 | FileCheck -check-prefix MANUAL-FEATURE %s
 // MANUAL-FEATURE: -cc1{{.*}}-target-feature{{.*}}+ptx61
+
+// RUN: %clang -### -target x86_64-linux-gnu -nocudalib -ccc-print-bindings -dlink %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE-LINK %s
+
+// DEVICE-LINK: "x86_64-unknown-linux-gnu" - "Offload::Linker", inputs: ["[[INPUT:.+]]"], output: "a.out"
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4158,7 +4158,8 @@
 // Check if this Linker Job should emit a static library.
 if (ShouldEmitStaticLibrary(Args)) {
   LA = C.MakeAction(LinkerInputs, types::TY_Image);
-} else if (UseNewOffloadingDriver) {
+} else if (UseNewOffloadingDriver ||
+   Args.hasArg(options::OPT_offload_link)) {
   LA = C.MakeAction(LinkerInputs, types::TY_Image);
   LA->propagateHostOffloadInfo(C.getActiveOffloadKinds(),
/*BoundArch=*/nullptr);
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -820,6 +820,10 @@
 def z : Separate<["-"], "z">, Flags<[LinkerInput, RenderAsInput]>,
   HelpText<"Pass -z  to the linker">, MetaVarName<"">,
   Group;
+def offload_link : Flag<["--"], "offload-link">, Group,
+  HelpText<"Use the new offloading linker to perform the link job.">;
+def device_link : Flag<["-"], "dlink">, Group,
+  Alias;
 def Xlinker : Separate<["-"], "Xlinker">, Flags<[LinkerInput, RenderAsInput]>,
   HelpText<"Pass  to the linker">, MetaVarName<"">,
   Group;
Index: clang/docs

[PATCH] D126397: [pseudo] Fix pseudo-gen usage when cross-compiling

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

Thanks for the fix!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126397

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

[PATCH] D125848: [clang-format] Handle attributes in enum declaration.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.

LGTM. Thanks a lot!
Do you have commit rights or need some help landing this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125848

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

[PATCH] D126358: clang-format][NFC] Refactor UnwrappedLineParser::parseBlock()

[Marek Kurdej via Phabricator via cfe-commits]

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

LGTM.




Comment at: clang/lib/Format/UnwrappedLineParser.cpp:874
 
-  if (SimpleBlock && !KeepBraces) {
+  auto RemoveBraces = [=]() mutable {
+if (KeepBraces || !SimpleBlock)

owenpan wrote:
> curdeius wrote:
> > Are there many captures here? Wouldn't it be better to be explicit and/or 
> > capture by ref? Do we need a mutable lambda?
> Yes, too many to be explicit for me. They are all integrals/pointers, a 
> couple of which are modified in the lambda, hence by copy and mutable.
Ok. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126358

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

[PATCH] D126358: clang-format][NFC] Refactor UnwrappedLineParser::parseBlock()

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:874
 
-  if (SimpleBlock && !KeepBraces) {
+  auto RemoveBraces = [=]() mutable {
+if (KeepBraces || !SimpleBlock)

curdeius wrote:
> Are there many captures here? Wouldn't it be better to be explicit and/or 
> capture by ref? Do we need a mutable lambda?
Yes, too many to be explicit for me. They are all integrals/pointers, a couple 
of which are modified in the lambda, hence by copy and mutable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126358

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

[PATCH] D125904: [Cuda] Use fallback method to mangle externalized decls if no CUID given

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

In D125904#3537952 , @tra wrote:

> How much work would it take to add cuid generation in the new driver, similar 
> to what the old driver does, using the same logic, however imperfect it is? 
> I'd be OK with that as a possibly permanent solution.

Probably wouldn't be too difficult, primarily just setting up the glue since 
the rest of the infrastructure is in place. I was hoping it would become 
unnecessary, but it seems like that's not happening. I'm tempted to have OpenMP 
handle it on its own do we don't need to port this to the OpenMP case, I think 
we already do something similar there with the kernel names.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125904

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

[PATCH] D126364: Fix interaction of pragma FENV_ACCESS with other pragmas

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Could you lay out the expected interaction between "STDC FENV_ACCESS", "clang 
fp exceptions", "float_control", and "fenv_access"?  If there's some way to map 
everything to "#pragma clang fp", please lay that out; if that isn't possible, 
please explain why.

As far as I can tell, "STDC FENV_ACCESS" and "STDC FENV_ROUND" don't directly 
interact.  FENV_ROUND just overrides the rounding mode for specific 
floating-point operations; it doesn't impact whether environment access is 
allowed.  So the call to setRoundingModeOverride seems dubious.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126364

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

[PATCH] D126084: [Sema] Reject implicit conversions between different scoped enum types in list initialization

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:4506
 !S.Context.hasSameUnqualifiedType(E->getType(), DestType) &&
-(E->getType()->isIntegralOrEnumerationType() ||
+(E->getType()->isIntegralOrUnscopedEnumerationType() ||
  E->getType()->isFloatingType())) {

aaron.ballman wrote:
> This doesn't match the comments immediately above here and I don't think is 
> the correct fix.
> 
> We're handling this case: http://eel.is/c++draft/dcl.init.list#3.8
> 
> A scoped enumeration has a fixed underlying type 
> (https://eel.is/c++draft/dcl.enum#5.sentence-5). The initializer list has a 
> single element and that element can be implicitly converted to the underlying 
> type (`int` in all of the test cases changed in this patch). And this is a 
> direct initialization case, so I think we should be performing the conversion 
> here rather than skipping to the next bullet.
Can scoped enums be implicitly converted to integer types? Unscoped enums can 
be converted to an integer type, but I don't see any mention of scoped enums 
here: https://eel.is/c++draft/conv.integral

It seems that the original paper was trying to change the rules about 
conversions from the underlying type to a scoped enum. It doesn't look like 
it's allowing conversion from a scope enum to another scope enum.

https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0138r2.pdf


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126084

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

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5596
+  DefaultedSMFArePOD = false;
+  }
+

probinson wrote:
> Does this mean `-fclang-abi-compat` will override the PS4/Darwin special 
> case?  I think we don't want to do that.
I don't think so - this expression will make `DefaultedSMFArePOD` false when 
it's already false due to the target not being PS4 or Darwin, yeah? So it'll 
redundantly/harmlessly set it to false.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119051

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

[PATCH] D126398: [Clang] Introduce `-dl` option to perform offload device linking

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

In D126398#3537942 , @tra wrote:

> Naming, as usual, is hard. I would prefer a more explicit `--offload-link` 
> which would be in line with other --offload* options we have by now. 
> `-dl` is cryptic for uninitiated and is uncomfortably close to commonly used 
> `-ldl`. If it gets mistyped as `-ld`, it would lead to a legitimate but 
> unrelated error about missing `libd`. Or it might silently succeed linking 
> with `libd` without actually doing any device linking.

Yeah, I can see your point, `--offload-link` definitely works but it would be 
nice to have something less verbose. Maybe could just use `-dlink` or something.

>> I was considering if this could be merged into the -fuse-ld option,
>> but becuse the device linker wraps over the users linker it would
>> conflict with that. In the future it's possible to merge this into lld
>> completely or gold via a plugin. Let me know what you think for this.

I should also add, even if we built-in support for this into LLD or Gold, we'll 
probably still need a flag like this to tell Gold to use the plugin, or LLD to 
do the extra processing.

Unrelated, but in the future I'm also considering making the linker wrapper add 
the necessary libraries for whatever offloading kinds it found. E.g. if the 
link job finds embedded `OpenMP` code it will add `-lomp -lomptarget` if not 
already present.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126398

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

[PATCH] D125904: [Cuda] Use fallback method to mangle externalized decls if no CUID given

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

How much work would it take to add cuid generation in the new driver, similar 
to what the old driver does, using the same logic, however imperfect it is? I'd 
be OK with that as a possibly permanent solution.

I'm somewhat wary of temporary solutions as they tend to become permanent and 
age poorly.  
That said, I'm OK with someone else tie-breaking us here. 
@yaxunl  -- Sam, do you have an opinion?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125904

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

[PATCH] D126341: Order implicitly instantiated global variable's initializer by the reverse instantiation order

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a subscriber: alexander-shaposhnikov.
rnk added a comment.

I'm somewhat supportive of the goal here, but I think there are still some 
underlying issues.

First, why should these guarantees be limited to instantiations and not inline 
variables? Such as:

  int f();
  inline int gv1 = f();
  inline int gv2 = gv1 + 1; // rely on previous

Second, LLVM doesn't guarantee that global_ctors at the same priority execute 
in order. See the langref: 
https://llvm.org/docs/LangRef.html#the-llvm-global-ctors-global-variable So, 
without a guarantee from LLVM, Clang can't rely on this behavior. LLVM relies 
on this lack of an ordering guarantee to power globalopt.

Last, what happens when the same global is implicitly instantiated in some 
other TU? Won't that disrupt the ordering?

+@alexander-shaposhnikov, who is working on global opt changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126341

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

[PATCH] D126397: [pseudo] Fix pseudo-gen usage when cross-compiling

[Shoaib Meenai via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4baae166ce33: [pseudo] Fix pseudo-gen usage when 
cross-compiling (authored by smeenai).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126397

Files:
  clang-tools-extra/pseudo/include/CMakeLists.txt


Index: clang-tools-extra/pseudo/include/CMakeLists.txt
===
--- clang-tools-extra/pseudo/include/CMakeLists.txt
+++ clang-tools-extra/pseudo/include/CMakeLists.txt
@@ -1,25 +1,35 @@
 # The cxx.bnf grammar file
 set(cxx_bnf ${CMAKE_CURRENT_SOURCE_DIR}/../lib/cxx.bnf)
 
+# Using CMAKE_CROSSCOMPILING and not LLVM_USE_HOST_TOOLS because the latter is
+# also set for LLVM_OPTIMIZED_TABLEGEN, which we don't care about here.
+if(CMAKE_CROSSCOMPILING)
+  build_native_tool(pseudo-gen pseudo_gen)
+  set(pseudo_gen_target "${pseudo_gen}")
+else()
+  set(pseudo_gen $)
+  set(pseudo_gen_target pseudo-gen)
+endif()
+
 # Generate inc files.
 set(cxx_symbols_inc ${CMAKE_CURRENT_BINARY_DIR}/CXXSymbols.inc)
 add_custom_command(OUTPUT ${cxx_symbols_inc}
-   COMMAND "${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/pseudo-gen"
+   COMMAND "${pseudo_gen}"
  --grammar ${cxx_bnf}
  --emit-symbol-list
  -o ${cxx_symbols_inc}
COMMENT "Generating nonterminal symbol file for cxx grammar..."
-   DEPENDS pseudo-gen
+   DEPENDS ${pseudo_gen_target}
VERBATIM)
 
 set(cxx_bnf_inc ${CMAKE_CURRENT_BINARY_DIR}/CXXBNF.inc)
 add_custom_command(OUTPUT ${cxx_bnf_inc}
-   COMMAND "${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/pseudo-gen"
+   COMMAND "${pseudo_gen}"
  --grammar ${cxx_bnf}
  --emit-grammar-content
  -o ${cxx_bnf_inc}
COMMENT "Generating bnf string file for cxx grammar..."
-   DEPENDS pseudo-gen
+   DEPENDS ${pseudo_gen_target}
VERBATIM)
 
 # add_custom_command does not create a new target, we need to deine a target


Index: clang-tools-extra/pseudo/include/CMakeLists.txt
===
--- clang-tools-extra/pseudo/include/CMakeLists.txt
+++ clang-tools-extra/pseudo/include/CMakeLists.txt
@@ -1,25 +1,35 @@
 # The cxx.bnf grammar file
 set(cxx_bnf ${CMAKE_CURRENT_SOURCE_DIR}/../lib/cxx.bnf)
 
+# Using CMAKE_CROSSCOMPILING and not LLVM_USE_HOST_TOOLS because the latter is
+# also set for LLVM_OPTIMIZED_TABLEGEN, which we don't care about here.
+if(CMAKE_CROSSCOMPILING)
+  build_native_tool(pseudo-gen pseudo_gen)
+  set(pseudo_gen_target "${pseudo_gen}")
+else()
+  set(pseudo_gen $)
+  set(pseudo_gen_target pseudo-gen)
+endif()
+
 # Generate inc files.
 set(cxx_symbols_inc ${CMAKE_CURRENT_BINARY_DIR}/CXXSymbols.inc)
 add_custom_command(OUTPUT ${cxx_symbols_inc}
-   COMMAND "${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/pseudo-gen"
+   COMMAND "${pseudo_gen}"
  --grammar ${cxx_bnf}
  --emit-symbol-list
  -o ${cxx_symbols_inc}
COMMENT "Generating nonterminal symbol file for cxx grammar..."
-   DEPENDS pseudo-gen
+   DEPENDS ${pseudo_gen_target}
VERBATIM)
 
 set(cxx_bnf_inc ${CMAKE_CURRENT_BINARY_DIR}/CXXBNF.inc)
 add_custom_command(OUTPUT ${cxx_bnf_inc}
-   COMMAND "${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/pseudo-gen"
+   COMMAND "${pseudo_gen}"
  --grammar ${cxx_bnf}
  --emit-grammar-content
  -o ${cxx_bnf_inc}
COMMENT "Generating bnf string file for cxx grammar..."
-   DEPENDS pseudo-gen
+   DEPENDS ${pseudo_gen_target}
VERBATIM)
 
 # add_custom_command does not create a new target, we need to deine a target
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 4baae16 - [pseudo] Fix pseudo-gen usage when cross-compiling

[Shoaib Meenai via cfe-commits]

Author: Shoaib Meenai
Date: 2022-05-25T11:08:21-07:00
New Revision: 4baae166ce33ac763bfc1813cf56b8d26e07fb1f

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

LOG: [pseudo] Fix pseudo-gen usage when cross-compiling

Use the LLVM build system's cross-compilation support for the tool, so
that the build works for both host and cross-compilation scenarios.

Reviewed By: sammccall

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

Added: 


Modified: 
clang-tools-extra/pseudo/include/CMakeLists.txt

Removed: 




diff  --git a/clang-tools-extra/pseudo/include/CMakeLists.txt 
b/clang-tools-extra/pseudo/include/CMakeLists.txt
index a79dd0dab8184..e4265b18c2b2e 100644
--- a/clang-tools-extra/pseudo/include/CMakeLists.txt
+++ b/clang-tools-extra/pseudo/include/CMakeLists.txt
@@ -1,25 +1,35 @@
 # The cxx.bnf grammar file
 set(cxx_bnf ${CMAKE_CURRENT_SOURCE_DIR}/../lib/cxx.bnf)
 
+# Using CMAKE_CROSSCOMPILING and not LLVM_USE_HOST_TOOLS because the latter is
+# also set for LLVM_OPTIMIZED_TABLEGEN, which we don't care about here.
+if(CMAKE_CROSSCOMPILING)
+  build_native_tool(pseudo-gen pseudo_gen)
+  set(pseudo_gen_target "${pseudo_gen}")
+else()
+  set(pseudo_gen $)
+  set(pseudo_gen_target pseudo-gen)
+endif()
+
 # Generate inc files.
 set(cxx_symbols_inc ${CMAKE_CURRENT_BINARY_DIR}/CXXSymbols.inc)
 add_custom_command(OUTPUT ${cxx_symbols_inc}
-   COMMAND "${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/pseudo-gen"
+   COMMAND "${pseudo_gen}"
  --grammar ${cxx_bnf}
  --emit-symbol-list
  -o ${cxx_symbols_inc}
COMMENT "Generating nonterminal symbol file for cxx grammar..."
-   DEPENDS pseudo-gen
+   DEPENDS ${pseudo_gen_target}
VERBATIM)
 
 set(cxx_bnf_inc ${CMAKE_CURRENT_BINARY_DIR}/CXXBNF.inc)
 add_custom_command(OUTPUT ${cxx_bnf_inc}
-   COMMAND "${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/pseudo-gen"
+   COMMAND "${pseudo_gen}"
  --grammar ${cxx_bnf}
  --emit-grammar-content
  -o ${cxx_bnf_inc}
COMMENT "Generating bnf string file for cxx grammar..."
-   DEPENDS pseudo-gen
+   DEPENDS ${pseudo_gen_target}
VERBATIM)
 
 # add_custom_command does not create a new target, we need to deine a target



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

[PATCH] D126398: [Clang] Introduce `-dl` option to perform offload device linking

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

> Currently, we infer the usage of the device linker by the user
> specifying an offloading toolchain, e.g. (--offload-arch=...) or
> (-fopenmp-targets=...), but this shouldn't be strictly necessary.

Yup. Whether we want to perform device link or not is orthogonal to those 
options.

> This patch introduces a new option -dl to tell the driver to use the
> offloading linker instead. So a compilation flow can now look like this,
>
> clang foo.cu --offload-new-driver -fgpu-rdc --offload-arch=sm_70 -c
> clang foo.o -dl -lcudart

It's an essential feature, as we do want to be able to produce libraries with 
host object files, but with fully linked GPU executables.
Naming, as usual, is hard. I would prefer a more explicit `--offload-link` 
which would be in line with other --offload* options we have by now. 
`-dl` is cryptic for uninitiated and is uncomfortably close to commonly used 
`-ldl`. If it gets mistyped as `-ld`, it would lead to a legitimate but 
unrelated error about missing `libd`. Or it might silently succeed linking with 
`libd` without actually doing any device linking.

> I was considering if this could be merged into the -fuse-ld option,
> but becuse the device linker wraps over the users linker it would
> conflict with that. In the future it's possible to merge this into lld
> completely or gold via a plugin. Let me know what you think for this.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126398

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

[PATCH] D125052: [HLSL] Enable vector types for hlsl.

[Xiang Li via Phabricator via cfe-commits]

python3kgae updated this revision to Diff 432051.
python3kgae added a comment.
Herald added a subscriber: MaskRay.

Add hlsl-no-stdinc to not include the hlsl header.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125052

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/LangOptions.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Headers/hlsl.h
  clang/lib/Headers/hlsl/hlsl_basic_types.h
  clang/test/CodeGenHLSL/basic_types.hlsl
  clang/test/Driver/hlsl_no_stdinc.hlsl

Index: clang/test/Driver/hlsl_no_stdinc.hlsl
===
--- /dev/null
+++ clang/test/Driver/hlsl_no_stdinc.hlsl
@@ -0,0 +1,12 @@
+// RUN: %clang_dxc  -Tlib_6_7 -fcgl -Fo - %s -### 2>&1 | FileCheck %s --check-prefix=STDINC
+// RUN: %clang_dxc  -Tlib_6_7 -hlsl-no-stdinc -fcgl -Fo - %s -### 2>&1 | FileCheck %s --check-prefix=NOSTDINC
+
+// RUN: %clang -cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -ast-dump -o - %s -verify
+
+// Make sure hlsl-no-stdinc is translated into finclude-default-header.
+// STDINC:"-finclude-default-header"
+// NOSTDINC-NOT:"-finclude-default-header"
+
+// Make sure uint not work when finclude-default-header is off.
+// expected-error@+1 {{unknown type name 'uint'}}
+uint a;
\ No newline at end of file
Index: clang/test/CodeGenHLSL/basic_types.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/basic_types.hlsl
@@ -0,0 +1,76 @@
+// RUN: %clang_dxc  -Tlib_6_7 -fcgl -Fo - %s | FileCheck %s
+
+// FIXME: check 16bit types once enable-16bit-types is ready.
+
+// CHECK:@uint_Val = global i32 0, align 4
+// CHECK:@uint64_t_Val = global i64 0, align 8
+// CHECK:@int64_t_Val = global i64 0, align 8
+// CHECK:@int2_Val = global <2 x i32> zeroinitializer, align 8
+// CHECK:@int3_Val = global <3 x i32> zeroinitializer, align 16
+// CHECK:@int4_Val = global <4 x i32> zeroinitializer, align 16
+// CHECK:@uint2_Val = global <2 x i32> zeroinitializer, align 8
+// CHECK:@uint3_Val = global <3 x i32> zeroinitializer, align 16
+// CHECK:@uint4_Val = global <4 x i32> zeroinitializer, align 16
+// CHECK:@int64_t2_Val = global <2 x i64> zeroinitializer, align 16
+// CHECK:@int64_t3_Val = global <3 x i64> zeroinitializer, align 32
+// CHECK:@int64_t4_Val = global <4 x i64> zeroinitializer, align 32
+// CHECK:@uint64_t2_Val = global <2 x i64> zeroinitializer, align 16
+// CHECK:@uint64_t3_Val = global <3 x i64> zeroinitializer, align 32
+// CHECK:@uint64_t4_Val = global <4 x i64> zeroinitializer, align 32
+// CHECK:@float2_Val = global <2 x float> zeroinitializer, align 8
+// CHECK:@float3_Val = global <3 x float> zeroinitializer, align 16
+// CHECK:@float4_Val = global <4 x float> zeroinitializer, align 16
+// CHECK:@double2_Val = global <2 x double> zeroinitializer, align 16
+// CHECK:@double3_Val = global <3 x double> zeroinitializer, align 32
+// CHECK:@double4_Val = global <4 x double> zeroinitializer, align 32
+
+#define TYPE_DECL(T)  T T##_Val
+
+#ifdef __HLSL_ENABLE_16_BIT
+TYPE_DECL(uint16_t);
+TYPE_DECL(int16_t);
+#endif
+
+// unsigned 32-bit integer.
+TYPE_DECL(uint);
+
+// 64-bit integer.
+TYPE_DECL(uint64_t);
+TYPE_DECL(int64_t);
+
+// built-in vector data types:
+
+#ifdef __HLSL_ENABLE_16_BIT
+TYPE_DECL(int16_t2   );
+TYPE_DECL(int16_t3   );
+TYPE_DECL(int16_t4   );
+TYPE_DECL( uint16_t2 );
+TYPE_DECL( uint16_t3 );
+TYPE_DECL( uint16_t4 );
+#endif
+
+TYPE_DECL( int2  );
+TYPE_DECL( int3  );
+TYPE_DECL( int4  );
+TYPE_DECL( uint2 );
+TYPE_DECL( uint3 );
+TYPE_DECL( uint4 );
+TYPE_DECL( int64_t2  );
+TYPE_DECL( int64_t3  );
+TYPE_DECL( int64_t4  );
+TYPE_DECL( uint64_t2 );
+TYPE_DECL( uint64_t3 );
+TYPE_DECL( uint64_t4 );
+
+#ifdef __HLSL_ENABLE_16_BIT
+TYPE_DECL(half2 );
+TYPE_DECL(half3 );
+TYPE_DECL(half4 );
+#endif
+
+TYPE_DECL( float2  );
+TYPE_DECL( float3  );
+TYPE_DECL( float4  );
+TYPE_DECL( double2 );
+TYPE_DECL( double3 );
+TYPE_DECL( double4 );
Index: clang/lib/Headers/hlsl/hlsl_basic_types.h
===
--- /dev/null
+++ clang/lib/Headers/hlsl/hlsl_basic_types.h
@@ -0,0 +1,64 @@
+//===- hlsl_basic_types.h - HLSL definitions for basic types --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef _HLSL_HLSL_BASIC_TYPES_H_
+#define _HLSL_HLSL_BASIC_TYPES_H_
+
+// built-in scalar data types:
+
+#ifdef __HLSL_ENABLE_16_BIT
+// 16-bit integer.
+typedef unsigned short uint16_t;
+typedef short int16_t;
+#endif
+
+// unsigned 32-bit integer.
+typedef unsi

[PATCH] D125814: Improve the strict prototype diagnostic behavior

[James Y Knight via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaDecl.cpp:3959
+  // it's a user-visible declaration. There is one exception to this:
+  // when the new declaration is one without a prototype, the old
+  // declaration with a prototype is not the cause of the issue, and

"when the new declaration is a definition without a prototype" perhaps?


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

https://reviews.llvm.org/D125814

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

[PATCH] D125678: [clang][extract-api] Don't emit symbols prefixed with an underscore

[Daniel Grumberg 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 rG504736cedff3: [clang][extract-api] Don't emit symbols 
prefixed with an underscore (authored by dang).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125678

Files:
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/test/ExtractAPI/underscored.c

Index: clang/test/ExtractAPI/underscored.c
===
--- /dev/null
+++ clang/test/ExtractAPI/underscored.c
@@ -0,0 +1,396 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
+// RUN: %t/reference.output.json.in >> %t/reference.output.json
+// RUN: %clang_cc1 -extract-api -triple arm64-apple-macosx \
+// RUN:   -x c-header %t/input.h -o %t/output.json -verify
+
+// Generator version is not consistent across test runs, normalize it.
+// RUN: sed -e "s@\"generator\": \".*\"@\"generator\": \"?\"@g" \
+// RUN: %t/output.json >> %t/output-normalized.json
+// RUN: diff %t/reference.output.json %t/output-normalized.json
+
+//--- input.h
+// expected-no-diagnostics
+
+// Global record
+int _HiddenGlobal;
+int exposed_global;
+
+// Record type
+struct _HiddenRecord {
+  int a;
+};
+
+struct ExposedRecord {
+  int a;
+};
+
+// Typedef
+typedef struct {} _HiddenTypedef;
+typedef int ExposedTypedef;
+typedef _HiddenTypedef ExposedTypedefToHidden;
+
+// Macros
+#define _HIDDEN_MACRO 5
+#define EXPOSED_MACRO 5
+
+// Symbols that start with '_' should not appear in the reference output
+//--- reference.output.json.in
+{
+  "metadata": {
+"formatVersion": {
+  "major": 0,
+  "minor": 5,
+  "patch": 3
+},
+"generator": "?"
+  },
+  "module": {
+"name": "",
+"platform": {
+  "architecture": "arm64",
+  "operatingSystem": {
+"minimumVersion": {
+  "major": 11,
+  "minor": 0,
+  "patch": 0
+},
+"name": "macosx"
+  },
+  "vendor": "apple"
+}
+  },
+  "relationships": [
+{
+  "kind": "memberOf",
+  "source": "c:@S@ExposedRecord@FI@a",
+  "target": "c:@S@ExposedRecord"
+}
+  ],
+  "symbols": [
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:I",
+  "spelling": "int"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "exposed_global"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "c",
+"precise": "c:@exposed_global"
+  },
+  "kind": {
+"displayName": "Global Variable",
+"identifier": "c.var"
+  },
+  "location": {
+"position": {
+  "character": 5,
+  "line": 5
+},
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"navigator": [
+  {
+"kind": "identifier",
+"spelling": "exposed_global"
+  }
+],
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "exposed_global"
+  }
+],
+"title": "exposed_global"
+  },
+  "pathComponents": [
+"exposed_global"
+  ]
+},
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "keyword",
+  "spelling": "struct"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "ExposedRecord"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "c",
+"precise": "c:@S@ExposedRecord"
+  },
+  "kind": {
+"displayName": "Structure",
+"identifier": "c.struct"
+  },
+  "location": {
+"position": {
+  "character": 8,
+  "line": 12
+},
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"navigator": [
+  {
+"kind": "identifier",
+"spelling": "ExposedRecord"
+  }
+],
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "ExposedRecord"
+  }
+],
+"title": "ExposedRecord"
+  },
+  "pathComponents": [
+"ExposedRecord"
+  ]
+},
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:I",
+  "spelling": "int"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "a"
+}
+  ],
+  "identifier": {
+

[clang] 504736c - [clang][extract-api] Don't emit symbols prefixed with an underscore

[Daniel Grumberg via cfe-commits]

Author: Daniel Grumberg
Date: 2022-05-25T19:02:18+01:00
New Revision: 504736cedff39d46fffc1293a35602ba140b19a8

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

LOG: [clang][extract-api] Don't emit symbols prefixed with an underscore

These symbols are understood to not be used for client API consumption
by convention so they should not appear in the generated symbol graph.

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

Added: 
clang/test/ExtractAPI/underscored.c

Modified: 
clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp

Removed: 




diff  --git a/clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp 
b/clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
index 045d5d68e733b..39d885c7c90aa 100644
--- a/clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
+++ b/clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
@@ -467,6 +467,11 @@ bool SymbolGraphSerializer::shouldSkip(const APIRecord 
&Record) const {
   if (Record.Availability.isUnconditionallyUnavailable())
 return true;
 
+  // Filter out symbols prefixed with an underscored as they are understood to
+  // be symbols clients should not use.
+  if (Record.Name.startswith("_"))
+return true;
+
   return false;
 }
 

diff  --git a/clang/test/ExtractAPI/underscored.c 
b/clang/test/ExtractAPI/underscored.c
new file mode 100644
index 0..47f1893cdb029
--- /dev/null
+++ b/clang/test/ExtractAPI/underscored.c
@@ -0,0 +1,396 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
+// RUN: %t/reference.output.json.in >> %t/reference.output.json
+// RUN: %clang_cc1 -extract-api -triple arm64-apple-macosx \
+// RUN:   -x c-header %t/input.h -o %t/output.json -verify
+
+// Generator version is not consistent across test runs, normalize it.
+// RUN: sed -e "s@\"generator\": \".*\"@\"generator\": \"?\"@g" \
+// RUN: %t/output.json >> %t/output-normalized.json
+// RUN: 
diff  %t/reference.output.json %t/output-normalized.json
+
+//--- input.h
+// expected-no-diagnostics
+
+// Global record
+int _HiddenGlobal;
+int exposed_global;
+
+// Record type
+struct _HiddenRecord {
+  int a;
+};
+
+struct ExposedRecord {
+  int a;
+};
+
+// Typedef
+typedef struct {} _HiddenTypedef;
+typedef int ExposedTypedef;
+typedef _HiddenTypedef ExposedTypedefToHidden;
+
+// Macros
+#define _HIDDEN_MACRO 5
+#define EXPOSED_MACRO 5
+
+// Symbols that start with '_' should not appear in the reference output
+//--- reference.output.json.in
+{
+  "metadata": {
+"formatVersion": {
+  "major": 0,
+  "minor": 5,
+  "patch": 3
+},
+"generator": "?"
+  },
+  "module": {
+"name": "",
+"platform": {
+  "architecture": "arm64",
+  "operatingSystem": {
+"minimumVersion": {
+  "major": 11,
+  "minor": 0,
+  "patch": 0
+},
+"name": "macosx"
+  },
+  "vendor": "apple"
+}
+  },
+  "relationships": [
+{
+  "kind": "memberOf",
+  "source": "c:@S@ExposedRecord@FI@a",
+  "target": "c:@S@ExposedRecord"
+}
+  ],
+  "symbols": [
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:I",
+  "spelling": "int"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "exposed_global"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "c",
+"precise": "c:@exposed_global"
+  },
+  "kind": {
+"displayName": "Global Variable",
+"identifier": "c.var"
+  },
+  "location": {
+"position": {
+  "character": 5,
+  "line": 5
+},
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"navigator": [
+  {
+"kind": "identifier",
+"spelling": "exposed_global"
+  }
+],
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "exposed_global"
+  }
+],
+"title": "exposed_global"
+  },
+  "pathComponents": [
+"exposed_global"
+  ]
+},
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "keyword",
+  "spelling": "struct"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "ExposedRecord"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "c",
+"precise": "c:@S@ExposedRecord"
+  },
+  "kind": {
+"displayNam

[PATCH] D124486: [clangd] ExtractVariable support for C and Objective-C

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Please go ahead and land this, the remaining open comment doesn't really matter.




Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:418
+  if (const auto *ME = dyn_cast(E))
+if (const auto *TE = dyn_cast(ME->getBase()))
+  if (TE->isImplicit())

dgoldman wrote:
> sammccall wrote:
> > dgoldman wrote:
> > > sammccall wrote:
> > > > oops, I forgot one detail: we want ME->getBase()->IgnoreImpCasts()
> > > > 
> > > > (in `void nonConstMethod() { constMethod(); }` there's an 
> > > > ImplicitCastExpr in there...
> > > Hmm, is this right, I tested out that example and it seems like that's 
> > > actually a `CXXMemberCallExpr` which this doesn't handle?
> > The callee of the CXXMemberCallExpr is the MemberExpr i'm talking about 
> > here.
> Right but this code here doesn't handle CXXMemberCallExpr, only MemberExpr - 
> so we don't even see the callee here. If I add the following above:
> 
> ```
>   if (const auto *MCE = dyn_cast(E))
> E = MCE->getCallee();
> ```
> 
> and do `ME->getBase()->IgnoreImpCasts()` then:
> 
> 
> ```
> class Test {
>   int constMethod() const { return 1; }
>   int nonConstMethod() { return [[constMethod()]]; }
> };
> ```
> 
> will be unavailable.
> Right but this code here doesn't handle CXXMemberCallExpr, only MemberExpr - 
> so we don't even see the callee here.

I don't understand what you mean.

If you select `  int nonConstMethod() { return [[constMethod]](); }`, then 
isn't N such a MemberExpr?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124486

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

[PATCH] D125904: [Cuda] Use fallback method to mangle externalized decls if no CUID given

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

In D125904#3537872 , @tra wrote:

> Is this patch in its current form blocking any of your other work? no-cuid 
> approach, even if we figure out how to do it, will likely take some time. Do 
> you need an interim solution until then?
>
> In D125904#3537385 , @jhuber6 wrote:
>
>> Also, for the OpenMP case, we already pass the host-IR as a dependency for 
>> the device compilation. So it would be relatively easy for us to just 
>> generate these names on the host and then read them from the IR for the 
>> device. The problem is that CUDA / HIP doesn't use this approach so it 
>> wouldn't be a great solution to have two different ways to do this. So we 
>> would either need to make CUDA / HIP take the host IR and use that, or move 
>> OpenMP to use the driver. The benefit of passing the IR is that we can much 
>> more stably generate some arbitrary string to mangle these names and we're 
>> guarunteed to have them match up because we read them from the host. The 
>> downside is that it'd be somewhat of a regression because now we have an 
>> extra call to Clang for CUDA / HIP when we previously didn't need to.
>
> Yeah. The different compilation flows are a bit of a problem. So is the 
> closeness of NVIDIA's binary format, which limits what we can do with them. 
> E.g. we can't currently modify GPU binary and rename of add new symbols.
>
> I'll need to think about the no-cuid solution. If we can solve it, not 
> deviating from C++ linking would be a valuable benefit and would save us some 
> headaches down the road. Extra clang invocation may be worth it, but it's too 
> early to tell.

It's blocking the new driver from handling static variables correctly because I 
haven't written the CUID support there yet. I could go ahead and copy over some 
necessary code to get it to work there, but a no-CUID solution would definitely 
be ideal. Personally, I think this is fine as a fallback so clang at least 
generates **something** that works rather than just leaving it completely 
blank. The code changed in this patch is pretty minimal. But I will probably 
get rid of the environment variable check, since I can't verify exactly that it 
will be the same between the host and device if we were to land this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125904

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

[PATCH] D125904: [Cuda] Use fallback method to mangle externalized decls if no CUID given

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

Is this patch in its current form blocking any of your other work? no-cuid 
approach, even if we figure out how to do it, will likely take some time. Do 
you need an interim solution until then?

In D125904#3537385 , @jhuber6 wrote:

> Also, for the OpenMP case, we already pass the host-IR as a dependency for 
> the device compilation. So it would be relatively easy for us to just 
> generate these names on the host and then read them from the IR for the 
> device. The problem is that CUDA / HIP doesn't use this approach so it 
> wouldn't be a great solution to have two different ways to do this. So we 
> would either need to make CUDA / HIP take the host IR and use that, or move 
> OpenMP to use the driver. The benefit of passing the IR is that we can much 
> more stably generate some arbitrary string to mangle these names and we're 
> guarunteed to have them match up because we read them from the host. The 
> downside is that it'd be somewhat of a regression because now we have an 
> extra call to Clang for CUDA / HIP when we previously didn't need to.

Yeah. The different compilation flows are a bit of a problem. So is the 
closeness of NVIDIA's binary format, which limits what we can do with them. 
E.g. we can't currently modify GPU binary and rename of add new symbols.

I'll need to think about the no-cuid solution. If we can solve it, not 
deviating from C++ linking would be a valuable benefit and would save us some 
headaches down the road. Extra clang invocation may be worth it, but it's too 
early to tell.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125904

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

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D123235#3537820 , @aaron.ballman 
wrote:

> In D123235#3537636 , @aaron.ballman 
> wrote:
>
>> Please revert the changes while investigating how to resolve the crashes 
>> though.
>
> I just noticed that you needed someone to commit on your behalf for this, so 
> I went ahead and did the revert myself in 
> 69da3b6aead2e7a18a2578aad661d6d36b8d30cf 
> .

There was a follow-up I also needed to revert in 
9368bf9023eee0dc6fcfa007e157fe30e1540fcc 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123235

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

[clang] 9368bf9 - Removing this as part of the revert done in 69da3b6aead2e7a18a2578aad661d6d36b8d30cf

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2022-05-25T13:45:17-04:00
New Revision: 9368bf9023eee0dc6fcfa007e157fe30e1540fcc

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

LOG: Removing this as part of the revert done in 
69da3b6aead2e7a18a2578aad661d6d36b8d30cf

This appears to have been added in a follow-up commit that I missed.

Added: 


Modified: 
clang/lib/CodeGen/CGStmtOpenMP.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGStmtOpenMP.cpp 
b/clang/lib/CodeGen/CGStmtOpenMP.cpp
index ebe65e5f72b3e..61e3661e59be0 100644
--- a/clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ b/clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -6317,7 +6317,6 @@ static void emitOMPAtomicExpr(CodeGenFunction &CGF, 
OpenMPClauseKind Kind,
   case OMPC_bind:
   case OMPC_align:
   case OMPC_cancellation_construct_type:
-  case OMPC_fail:
 llvm_unreachable("Clause is not allowed in 'omp atomic'.");
   }
 }



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

[PATCH] D126341: Order implicitly instantiated global variable's initializer by the reverse instantiation order

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

In D126341#3537675 , @aaron.ballman 
wrote:

> Adding the language WG as a reviewer in case others have opinions.
>
>> The underlying problem is basically wg21.link/cwg362 which has no concensus 
>> yet.
>
> According to 
> https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#362 this was 
> resolved in CD1 and we track it as being not applicable to us 
> (https://clang.llvm.org/cxx_dr_status.html#362). (Is our status actually 
> correct for this?)
>
>> Will the reviewers be supportive if I make the original cwg362 test case 
>> work too?
>
> To me, it depends on what it does to compile times and memory overhead for 
> the compiler when run on large projects. If the extra tracking is cheap and 
> doesn't really impact anything, I think it's reasonable to want to match 
> GCC's behavior. If it turns out this is expensive, I'm less keen on matching 
> GCC.

Thanks for the opinion @aaron.ballman. I think the cost would be low. I'll get 
back with some numbers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126341

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

[PATCH] D126397: [pseudo] Fix pseudo-gen usage when cross-compiling

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

In D126397#3537843 , @sammccall wrote:

> Thank you! I have been banging my head against this, I'm not entirely sure 
> why this works and my version doesn't.

Ah, sorry for the duplicated work :( Thanks for the review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126397

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

[PATCH] D126397: [pseudo] Fix pseudo-gen usage when cross-compiling

[Sam McCall via Phabricator via cfe-commits]

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

Thank you! I have been banging my head against this, I'm not entirely sure why 
this works and my version doesn't.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126397

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

[PATCH] D123924: [clang-apply-replacements] Added an option to ignore insert conflict.

[Aaron Ballman via Phabricator via cfe-commits]

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

I think this has sat for long enough and my LGTM will have to suffice. :-)


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

https://reviews.llvm.org/D123924

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

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D123235#3537636 , @aaron.ballman 
wrote:

> Please revert the changes while investigating how to resolve the crashes 
> though.

I just noticed that you needed someone to commit on your behalf for this, so I 
went ahead and did the revert myself in 
69da3b6aead2e7a18a2578aad661d6d36b8d30cf 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123235

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

[PATCH] D126331: [OpenMP] Add diagnostic for unterminated 'omp [begin] declare target'

[Mike Rice via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGba3f85390bde: [OpenMP] Add diagnostic for unterminated 
'omp [begin] declare target' (authored by mikerice).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126331

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/Inputs/unterminated_declare_target_include.h
  clang/test/OpenMP/declare_target_messages.cpp


Index: clang/test/OpenMP/declare_target_messages.cpp
===
--- clang/test/OpenMP/declare_target_messages.cpp
+++ clang/test/OpenMP/declare_target_messages.cpp
@@ -6,6 +6,9 @@
 // RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 
-verify=expected,omp5,host5 -fopenmp-simd -fopenmp-is-device 
-fopenmp-targets=x86_64-apple-macos10.7.0 -fnoopenmp-use-tls -ferror-limit 100 
-o - %s
 // RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp45 
-fopenmp-version=45 -fopenmp-simd -fnoopenmp-use-tls -ferror-limit 100 -o - %s
 // RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp51 
-fopenmp -fopenmp-version=51 -fnoopenmp-use-tls -ferror-limit 100 -o - %s
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp51 
-fopenmp -fopenmp-version=51 -fnoopenmp-use-tls -ferror-limit 100 -DTESTEND=1 
-o - %s
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp51 
-fopenmp -fopenmp-version=51 -fnoopenmp-use-tls -ferror-limit 100 -I%S/Inputs 
-DTESTENDINC=1 -o - %s
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp51 
-fopenmp-version=51 -fopenmp-simd -fnoopenmp-use-tls -ferror-limit 100 -o - %s
 // RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp51 
-fopenmp-version=51 -fopenmp-simd -fnoopenmp-use-tls -ferror-limit 100 -o - %s
 
 // RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp5 
-fopenmp -fnoopenmp-use-tls -ferror-limit 100 -o - %s
@@ -228,5 +231,12 @@
 #pragma omp declare target to(MultiDevTy) device_type(host)   // omp45-error 
{{unexpected 'device_type' clause, only 'to' or 'link' clauses expected}} 
omp5-error {{'device_type(host)' does not match previously specified 
'device_type(any)' for the same declaration}} omp51-error {{'device_type(host)' 
does not match previously specified 'device_type(any)' for the same 
declaration}}
 #pragma omp declare target to(MultiDevTy) device_type(nohost) // omp45-error 
{{unexpected 'device_type' clause, only 'to' or 'link' clauses expected}} 
omp5-error {{'device_type(nohost)' does not match previously specified 
'device_type(any)' for the same declaration}} // omp51-error 
{{'device_type(nohost)' does not match previously specified 'device_type(any)' 
for the same declaration}}
 
-// TODO: Issue an error message error {{expected '#pragma omp end declare 
target'}} note {{to match this '#pragma omp declare target'}}
+#if TESTENDINC
+#include "unterminated_declare_target_include.h"
+#elif TESTEND
+// expected-warning@+1 {{expected '#pragma omp end declare target' at end of 
file to match '#pragma omp declare target'}}
 #pragma omp declare target
+#else
+// expected-warning@+1 {{expected '#pragma omp end declare target' at end of 
file to match '#pragma omp begin declare target'}}
+#pragma omp begin declare target
+#endif
Index: clang/test/OpenMP/Inputs/unterminated_declare_target_include.h
===
--- /dev/null
+++ clang/test/OpenMP/Inputs/unterminated_declare_target_include.h
@@ -0,0 +1,3 @@
+// expected-warning@+1 {{expected '#pragma omp end declare target' at end of 
file to match '#pragma omp declare target'}}
+#pragma omp declare target
+void zxy();
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -22101,6 +22101,14 @@
 ActOnOpenMPDeclareTargetName(It.first, It.second.Loc, It.second.MT, DTCI);
 }
 
+void Sema::DiagnoseUnterminatedOpenMPDeclareTarget() {
+  if (DeclareTargetNesting.empty())
+return;
+  DeclareTargetContextInfo &DTCI = DeclareTargetNesting.back();
+  Diag(DTCI.Loc, diag::warn_omp_unterminated_declare_target)
+  << getOpenMPDirectiveName(DTCI.Kind);
+}
+
 NamedDecl *Sema::lookupOpenMPDeclareTargetName(Scope *CurScope,
CXXScopeSpec &ScopeSpec,
const DeclarationNameInfo &Id) {
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -1157,6 +1157,7 @@
 
   DiagnoseUnterminatedPragmaAlignPack();
   DiagnoseUnterminated

[clang] 69da3b6 - Revert "[OpenMP] atomic compare fail : Parser & AST support"

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2022-05-25T13:34:34-04:00
New Revision: 69da3b6aead2e7a18a2578aad661d6d36b8d30cf

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

LOG: Revert "[OpenMP] atomic compare fail : Parser & AST support"

This reverts commit 232bf8189ef7d574a468bd5bfd1e84e962f7f16e.

It broke the sanitize buildbot: 
https://lab.llvm.org/buildbot/#/builders/5/builds/24074

It also reproduces on Windows debug builds as a crash.

Added: 


Modified: 
clang/include/clang/AST/ASTNodeTraverser.h
clang/include/clang/AST/OpenMPClause.h
clang/include/clang/AST/RecursiveASTVisitor.h
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Parse/Parser.h
clang/include/clang/Sema/Sema.h
clang/lib/AST/OpenMPClause.cpp
clang/lib/AST/StmtProfile.cpp
clang/lib/Basic/OpenMPKinds.cpp
clang/lib/Parse/ParseOpenMP.cpp
clang/lib/Sema/SemaOpenMP.cpp
clang/lib/Sema/TreeTransform.h
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/test/OpenMP/atomic_ast_print.cpp
clang/test/OpenMP/atomic_messages.cpp
clang/tools/libclang/CIndex.cpp
flang/lib/Semantics/check-omp-structure.cpp
llvm/include/llvm/Frontend/OpenMP/OMP.td

Removed: 




diff  --git a/clang/include/clang/AST/ASTNodeTraverser.h 
b/clang/include/clang/AST/ASTNodeTraverser.h
index cb5bbeb644812..f2c5c01ac88de 100644
--- a/clang/include/clang/AST/ASTNodeTraverser.h
+++ b/clang/include/clang/AST/ASTNodeTraverser.h
@@ -214,10 +214,6 @@ class ASTNodeTraverser
   }
 
   void Visit(const OMPClause *C) {
-if (OMPFailClause::classof(C)) {
-  Visit(static_cast(C));
-  return;
-}
 getNodeDelegate().AddChild([=] {
   getNodeDelegate().Visit(C);
   for (const auto *S : C->children())
@@ -225,13 +221,6 @@ class ASTNodeTraverser
 });
   }
 
-  void Visit(const OMPFailClause *C) {
-getNodeDelegate().AddChild([=] {
-  getNodeDelegate().Visit(C);
-  const OMPClause *MOC = C->const_getMemoryOrderClause();
-  Visit(MOC);
-});
-  }
   void Visit(const GenericSelectionExpr::ConstAssociation &A) {
 getNodeDelegate().AddChild([=] {
   getNodeDelegate().Visit(A);

diff  --git a/clang/include/clang/AST/OpenMPClause.h 
b/clang/include/clang/AST/OpenMPClause.h
index aa9503c565779..a745df1143468 100644
--- a/clang/include/clang/AST/OpenMPClause.h
+++ b/clang/include/clang/AST/OpenMPClause.h
@@ -2266,133 +2266,6 @@ class OMPCompareClause final : public OMPClause {
   }
 };
 
-/// This represents 'fail' clause in the '#pragma omp atomic'
-/// directive.
-///
-/// \code
-/// #pragma omp atomic compare fail
-/// \endcode
-/// In this example directive '#pragma omp atomic compare' has 'fail' clause.
-class OMPFailClause final
-: public OMPClause,
-  private llvm::TrailingObjects {
-  OMPClause *MemoryOrderClause;
-
-  friend class OMPClauseReader;
-  friend TrailingObjects;
-
-  /// Define the sizes of each trailing object array except the last one. This
-  /// is required for TrailingObjects to work properly.
-  size_t numTrailingObjects(OverloadToken) const {
-// 2 locations: for '(' and argument location.
-return 2;
-  }
-
-  /// Sets the location of '(' in fail clause.
-  void setLParenLoc(SourceLocation Loc) {
-*getTrailingObjects() = Loc;
-  }
-
-  /// Sets the location of memoryOrder clause argument in fail clause.
-  void setArgumentLoc(SourceLocation Loc) {
-*std::next(getTrailingObjects(), 1) = Loc;
-  }
-
-  /// Sets the mem_order clause for 'atomic compare fail' directive.
-  void setMemOrderClauseKind(OpenMPClauseKind MemOrder) {
-OpenMPClauseKind *MOCK = getTrailingObjects();
-*MOCK = MemOrder;
-  }
-
-  /// Sets the mem_order clause for 'atomic compare fail' directive.
-  void setMemOrderClause(OMPClause *MemoryOrderClauseParam) {
-MemoryOrderClause = MemoryOrderClauseParam;
-  }
-public:
-  /// Build 'fail' clause.
-  ///
-  /// \param StartLoc Starting location of the clause.
-  /// \param EndLoc Ending location of the clause.
-  OMPFailClause(SourceLocation StartLoc, SourceLocation EndLoc)
-  : OMPClause(llvm::omp::OMPC_fail, StartLoc, EndLoc) {}
-
-  /// Build an empty clause.
-  OMPFailClause()
-  : OMPClause(llvm::omp::OMPC_fail, SourceLocation(), SourceLocation()) {}
-
-  static OMPFailClause *CreateEmpty(const ASTContext &C);
-  static OMPFailClause *Create(const ASTContext &C, SourceLocation StartLoc,
-   SourceLocation EndLoc);
-
-  child_range children() {
-return child_range(child_iterator(), child_iterator());
-  }
-
-
-  const_child_range children() const {
-return const_child_range(const_child_iterator(), const_child_iterator());
-  }
-
-  child_range used_children() {
-return child_range(c