[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-18 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment.

In D137181#3938913 , @goldstein.w.n 
wrote:

> Anything left for me todo (can't imagine I have push access).

See here . We will 
need your name and email.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137181

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


[PATCH] D138350: [scudo] Add loongarch64 support for scudo

2022-11-18 Thread Youling Tang via Phabricator via cfe-commits
tangyouling created this revision.
tangyouling added reviewers: cryptoad, eugenis, SixWeining, xen0n, xry111, 
MaskRay.
Herald added subscribers: Enna1, StephenFan.
Herald added a project: All.
tangyouling requested review of this revision.
Herald added projects: clang, Sanitizers.
Herald added subscribers: Sanitizers, cfe-commits.

Enable scudo on LoongArch64 on both clang side and compiler-rt side.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138350

Files:
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/test/Driver/fsanitize.c
  compiler-rt/cmake/Modules/AllSupportedArchDefs.cmake


Index: compiler-rt/cmake/Modules/AllSupportedArchDefs.cmake
===
--- compiler-rt/cmake/Modules/AllSupportedArchDefs.cmake
+++ compiler-rt/cmake/Modules/AllSupportedArchDefs.cmake
@@ -68,9 +68,9 @@
 set(ALL_CFI_SUPPORTED_ARCH ${X86} ${X86_64} ${ARM32} ${ARM64} ${MIPS64}
 ${HEXAGON})
 set(ALL_SCUDO_SUPPORTED_ARCH ${X86} ${X86_64} ${ARM32} ${ARM64} ${MIPS32}
-${MIPS64} ${PPC64} ${HEXAGON})
+${MIPS64} ${PPC64} ${HEXAGON} ${LOONGARCH64})
 set(ALL_SCUDO_STANDALONE_SUPPORTED_ARCH ${X86} ${X86_64} ${ARM32} ${ARM64}
-${MIPS32} ${MIPS64} ${PPC64} ${HEXAGON})
+${MIPS32} ${MIPS64} ${PPC64} ${HEXAGON} ${LOONGARCH64})
 if(APPLE)
 set(ALL_XRAY_SUPPORTED_ARCH ${X86_64})
 else()
Index: clang/test/Driver/fsanitize.c
===
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -873,6 +873,7 @@
 // RUN: %clang --target=arm-linux-androideabi -fsanitize=scudo %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-SCUDO
 // RUN: %clang --target=x86_64-linux-gnu -fsanitize=scudo %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-SCUDO
 // RUN: %clang --target=i386-linux-gnu -fsanitize=scudo %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-SCUDO
+// RUN: %clang --target=loongarch64-unknown-linux-gnu -fsanitize=scudo %s -### 
2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
 // RUN: %clang --target=mips64-unknown-linux-gnu -fsanitize=scudo %s -### 2>&1 
| FileCheck %s --check-prefix=CHECK-SCUDO
 // RUN: %clang --target=mips64el-unknown-linux-gnu -fsanitize=scudo %s -### 
2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
 // RUN: %clang --target=mips-unknown-linux-gnu -fsanitize=scudo %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-SCUDO
Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -749,6 +749,7 @@
  getTriple().getArch() == llvm::Triple::thumb ||
  getTriple().getArch() == llvm::Triple::armeb ||
  getTriple().getArch() == llvm::Triple::thumbeb;
+  const bool IsLoongArch64 = getTriple().getArch() == 
llvm::Triple::loongarch64;
   const bool IsRISCV64 = getTriple().getArch() == llvm::Triple::riscv64;
   const bool IsSystemZ = getTriple().getArch() == llvm::Triple::systemz;
   const bool IsHexagon = getTriple().getArch() == llvm::Triple::hexagon;
@@ -774,7 +775,7 @@
   if (IsX86 || IsX86_64)
 Res |= SanitizerKind::Function;
   if (IsX86_64 || IsMIPS64 || IsAArch64 || IsX86 || IsMIPS || IsArmArch ||
-  IsPowerPC64 || IsHexagon)
+  IsPowerPC64 || IsHexagon || IsLoongArch64)
 Res |= SanitizerKind::Scudo;
   if (IsX86_64 || IsAArch64) {
 Res |= SanitizerKind::HWAddress;


Index: compiler-rt/cmake/Modules/AllSupportedArchDefs.cmake
===
--- compiler-rt/cmake/Modules/AllSupportedArchDefs.cmake
+++ compiler-rt/cmake/Modules/AllSupportedArchDefs.cmake
@@ -68,9 +68,9 @@
 set(ALL_CFI_SUPPORTED_ARCH ${X86} ${X86_64} ${ARM32} ${ARM64} ${MIPS64}
 ${HEXAGON})
 set(ALL_SCUDO_SUPPORTED_ARCH ${X86} ${X86_64} ${ARM32} ${ARM64} ${MIPS32}
-${MIPS64} ${PPC64} ${HEXAGON})
+${MIPS64} ${PPC64} ${HEXAGON} ${LOONGARCH64})
 set(ALL_SCUDO_STANDALONE_SUPPORTED_ARCH ${X86} ${X86_64} ${ARM32} ${ARM64}
-${MIPS32} ${MIPS64} ${PPC64} ${HEXAGON})
+${MIPS32} ${MIPS64} ${PPC64} ${HEXAGON} ${LOONGARCH64})
 if(APPLE)
 set(ALL_XRAY_SUPPORTED_ARCH ${X86_64})
 else()
Index: clang/test/Driver/fsanitize.c
===
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -873,6 +873,7 @@
 // RUN: %clang --target=arm-linux-androideabi -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
 // RUN: %clang --target=x86_64-linux-gnu -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
 // RUN: %clang --target=i386-linux-gnu -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
+// RUN: %clang --target=loongarch64-unknown-linux-gnu -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
 // RUN: %clang --target=mips64-unknown-linux-gnu -fsanitize=scudo %s -### 2>&1 

[PATCH] D138137: [CodeGen][ARM] Fix ARMABIInfo::EmitVAAarg crash with empty record type variadic arg

2022-11-18 Thread Lin Yurong via Phabricator via cfe-commits
yronglin added a comment.

Thank you so much for your review, @rjmccall


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138137

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


[PATCH] D138137: [CodeGen][ARM] Fix ARMABIInfo::EmitVAAarg crash with empty record type variadic arg

2022-11-18 Thread Gold Cat via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG80f444646c62: [CodeGen][ARM] Fix ARMABIInfo::EmitVAAarg 
crash with empty record type variadic… (authored by yronglin, committed by 
objwyh).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138137

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/arm-vaarg.c


Index: clang/test/CodeGen/arm-vaarg.c
===
--- /dev/null
+++ clang/test/CodeGen/arm-vaarg.c
@@ -0,0 +1,23 @@
+// RUN: %clang -Xclang -no-opaque-pointers -mfloat-abi=soft -target 
arm-linux-gnu -emit-llvm -S -o - %s | FileCheck %s
+
+struct Empty {};
+
+struct Empty emptyvar;
+
+void take_args(int a, ...) {
+// CHECK: [[ALLOCA_VA_LIST:%[a-zA-Z0-9._]+]] = alloca %struct.__va_list, align 
4
+// CHECK: call void @llvm.va_start
+// CHECK-NEXT: [[AP_ADDR:%[a-zA-Z0-9._]+]] = bitcast %struct.__va_list* 
[[ALLOCA_VA_LIST]] to i8**
+// CHECK-NEXT: [[LOAD_AP:%[a-zA-Z0-9._]+]] = load i8*, i8** [[AP_ADDR]], align 
4
+// CHECK-NEXT: [[EMPTY_PTR:%[a-zA-Z0-9._]+]] = bitcast i8* [[LOAD_AP]] to 
%struct.Empty*
+
+  // It's conceivable that EMPTY_PTR may not actually be a valid pointer
+  // (e.g. it's at the very bottom of the stack and the next page is
+  // invalid). This doesn't matter provided it's never loaded (there's no
+  // well-defined way to tell), but it becomes a problem if we do try to use 
it.
+// CHECK-NOT: load %struct.Empty, %struct.Empty* [[EMPTY_PTR]]
+  __builtin_va_list l;
+  __builtin_va_start(l, a);
+  emptyvar = __builtin_va_arg(l, struct Empty);
+  __builtin_va_end(l);
+}
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -7050,10 +7050,10 @@
 
   // Empty records are ignored for parameter passing purposes.
   if (isEmptyRecord(getContext(), Ty, true)) {
-Address Addr = Address(CGF.Builder.CreateLoad(VAListAddr),
-   getVAListElementType(CGF), SlotSize);
-Addr = CGF.Builder.CreateElementBitCast(Addr, CGF.ConvertTypeForMem(Ty));
-return Addr;
+VAListAddr = CGF.Builder.CreateElementBitCast(VAListAddr, CGF.Int8PtrTy);
+auto *Load = CGF.Builder.CreateLoad(VAListAddr);
+Address Addr = Address(Load, CGF.Int8Ty, SlotSize);
+return CGF.Builder.CreateElementBitCast(Addr, CGF.ConvertTypeForMem(Ty));
   }
 
   CharUnits TySize = getContext().getTypeSizeInChars(Ty);


Index: clang/test/CodeGen/arm-vaarg.c
===
--- /dev/null
+++ clang/test/CodeGen/arm-vaarg.c
@@ -0,0 +1,23 @@
+// RUN: %clang -Xclang -no-opaque-pointers -mfloat-abi=soft -target arm-linux-gnu -emit-llvm -S -o - %s | FileCheck %s
+
+struct Empty {};
+
+struct Empty emptyvar;
+
+void take_args(int a, ...) {
+// CHECK: [[ALLOCA_VA_LIST:%[a-zA-Z0-9._]+]] = alloca %struct.__va_list, align 4
+// CHECK: call void @llvm.va_start
+// CHECK-NEXT: [[AP_ADDR:%[a-zA-Z0-9._]+]] = bitcast %struct.__va_list* [[ALLOCA_VA_LIST]] to i8**
+// CHECK-NEXT: [[LOAD_AP:%[a-zA-Z0-9._]+]] = load i8*, i8** [[AP_ADDR]], align 4
+// CHECK-NEXT: [[EMPTY_PTR:%[a-zA-Z0-9._]+]] = bitcast i8* [[LOAD_AP]] to %struct.Empty*
+
+  // It's conceivable that EMPTY_PTR may not actually be a valid pointer
+  // (e.g. it's at the very bottom of the stack and the next page is
+  // invalid). This doesn't matter provided it's never loaded (there's no
+  // well-defined way to tell), but it becomes a problem if we do try to use it.
+// CHECK-NOT: load %struct.Empty, %struct.Empty* [[EMPTY_PTR]]
+  __builtin_va_list l;
+  __builtin_va_start(l, a);
+  emptyvar = __builtin_va_arg(l, struct Empty);
+  __builtin_va_end(l);
+}
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -7050,10 +7050,10 @@
 
   // Empty records are ignored for parameter passing purposes.
   if (isEmptyRecord(getContext(), Ty, true)) {
-Address Addr = Address(CGF.Builder.CreateLoad(VAListAddr),
-   getVAListElementType(CGF), SlotSize);
-Addr = CGF.Builder.CreateElementBitCast(Addr, CGF.ConvertTypeForMem(Ty));
-return Addr;
+VAListAddr = CGF.Builder.CreateElementBitCast(VAListAddr, CGF.Int8PtrTy);
+auto *Load = CGF.Builder.CreateLoad(VAListAddr);
+Address Addr = Address(Load, CGF.Int8Ty, SlotSize);
+return CGF.Builder.CreateElementBitCast(Addr, CGF.ConvertTypeForMem(Ty));
   }
 
   CharUnits TySize = getContext().getTypeSizeInChars(Ty);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 80f4446 - [CodeGen][ARM] Fix ARMABIInfo::EmitVAAarg crash with empty record type variadic arg

2022-11-18 Thread via cfe-commits

Author: yronglin
Date: 2022-11-19T15:14:10+08:00
New Revision: 80f444646c62ccc8b2399d60ac91e62e6e576da6

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

LOG: [CodeGen][ARM] Fix ARMABIInfo::EmitVAAarg crash with empty record type 
variadic arg

Fix ARMABIInfo::EmitVAAarg crash with empty record type variadic arg

Open issue: https://github.com/llvm/llvm-project/issues/58794

Reviewed By: rjmccall

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

Added: 
clang/test/CodeGen/arm-vaarg.c

Modified: 
clang/lib/CodeGen/TargetInfo.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/TargetInfo.cpp 
b/clang/lib/CodeGen/TargetInfo.cpp
index fb6eb4aba60f..b0b51ee231f1 100644
--- a/clang/lib/CodeGen/TargetInfo.cpp
+++ b/clang/lib/CodeGen/TargetInfo.cpp
@@ -7050,10 +7050,10 @@ Address ARMABIInfo::EmitVAArg(CodeGenFunction , 
Address VAListAddr,
 
   // Empty records are ignored for parameter passing purposes.
   if (isEmptyRecord(getContext(), Ty, true)) {
-Address Addr = Address(CGF.Builder.CreateLoad(VAListAddr),
-   getVAListElementType(CGF), SlotSize);
-Addr = CGF.Builder.CreateElementBitCast(Addr, CGF.ConvertTypeForMem(Ty));
-return Addr;
+VAListAddr = CGF.Builder.CreateElementBitCast(VAListAddr, CGF.Int8PtrTy);
+auto *Load = CGF.Builder.CreateLoad(VAListAddr);
+Address Addr = Address(Load, CGF.Int8Ty, SlotSize);
+return CGF.Builder.CreateElementBitCast(Addr, CGF.ConvertTypeForMem(Ty));
   }
 
   CharUnits TySize = getContext().getTypeSizeInChars(Ty);

diff  --git a/clang/test/CodeGen/arm-vaarg.c b/clang/test/CodeGen/arm-vaarg.c
new file mode 100644
index ..4dab397a20de
--- /dev/null
+++ b/clang/test/CodeGen/arm-vaarg.c
@@ -0,0 +1,23 @@
+// RUN: %clang -Xclang -no-opaque-pointers -mfloat-abi=soft -target 
arm-linux-gnu -emit-llvm -S -o - %s | FileCheck %s
+
+struct Empty {};
+
+struct Empty emptyvar;
+
+void take_args(int a, ...) {
+// CHECK: [[ALLOCA_VA_LIST:%[a-zA-Z0-9._]+]] = alloca %struct.__va_list, align 
4
+// CHECK: call void @llvm.va_start
+// CHECK-NEXT: [[AP_ADDR:%[a-zA-Z0-9._]+]] = bitcast %struct.__va_list* 
[[ALLOCA_VA_LIST]] to i8**
+// CHECK-NEXT: [[LOAD_AP:%[a-zA-Z0-9._]+]] = load i8*, i8** [[AP_ADDR]], align 
4
+// CHECK-NEXT: [[EMPTY_PTR:%[a-zA-Z0-9._]+]] = bitcast i8* [[LOAD_AP]] to 
%struct.Empty*
+
+  // It's conceivable that EMPTY_PTR may not actually be a valid pointer
+  // (e.g. it's at the very bottom of the stack and the next page is
+  // invalid). This doesn't matter provided it's never loaded (there's no
+  // well-defined way to tell), but it becomes a problem if we do try to use 
it.
+// CHECK-NOT: load %struct.Empty, %struct.Empty* [[EMPTY_PTR]]
+  __builtin_va_list l;
+  __builtin_va_start(l, a);
+  emptyvar = __builtin_va_arg(l, struct Empty);
+  __builtin_va_end(l);
+}



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


[PATCH] D137826: [clang] Allow comparing pointers to string literals

2022-11-18 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:12951-12954
+  // ObjC's @encode()
+  if (isa(E->getLHS()->IgnoreParenImpCasts()) ||
+  isa(E->getRHS()->IgnoreParenImpCasts()))
 return Error(E);

tahonermann wrote:
> tbaeder wrote:
> > tahonermann wrote:
> > > A comment to explain this change would be helpful. It isn't intuitive 
> > > (for me anyway) why Objective-C's `@encode` would require special 
> > > handling here. Was this needed to avoid a test failure?
> > In `../clang/test/CodeGenObjC/encode-test-4.m`:
> > 
> > ```
> > int a(void) {
> >   return @encode(int) == @encode(int) &&
> > @encode(Color) == @encode(long) &&
> > @encode(PrintColor) == @encode(int);
> > }
> > ```
> > 
> > The comparisons need to be rejected here and are folded to a `1` later on, 
> > it seems. Letting the comparison happen will lead to a `0`.
> Apologies, I meant it would be helpful to add a comment to the code :)
> 
> I think `@encode(...)` expressions should be treated the same as string 
> literals; the former is lowered to the latter; both contribute to the string 
> table.
You mean the special case for them here should go away?

I've tracked this down to `CodeGenFunction::ConstantFoldsToSimpleInteger()`. 
Without this special case for endcode expressions, we will return `true` here 
and fold the comparison to `false`, so we will just emit that. 

This is a funny case though, which also exists in C++. With this patch, `"a" == 
"a"` evaluates to `true`, as one would expect.
However, `"a" == "a" && "b" == "b"` will evaluate to `false`.

That's pretty wrong so I think the patch needs more work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137826

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


[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-18 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n marked an inline comment as done.
goldstein.w.n added a comment.

In D137181#3938904 , @owenpan wrote:

> LGTM

Awesome! Thanks for sticking with it through all these revisions :)

Anything left for me todo (can't imagine I have push access).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137181

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


[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-18 Thread Owen Pan via Phabricator via cfe-commits
owenpan accepted this revision.
owenpan added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137181

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


[PATCH] D137488: [clang][Interp] Array initialization via string literal

2022-11-18 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1098-1099
+
+unsigned N = SL->getLength();
+for (size_t I = 0; I != NumElems; ++I) {
+  uint32_t CodePoint = I < N ? SL->getCodeUnit(I) : 0;

tahonermann wrote:
> tahonermann wrote:
> > Aren't `N` and `NumElems` guaranteed to have the same value here? Both are 
> > derived from `SL`. The code seems to be written with the expectation that 
> > `NumElems` corresponds to the number of elements to be iniitialized in the 
> > target array.
> I see the change to now use the minimum of `SL->getLength()` and 
> `CAT->getSize().getZExtValue()`. Based on https://godbolt.org/z/5sTWExTac 
> this looks to be unnecessary. When a string literal is used as an array 
> initializer, it appears that the type of the string literal is adjusted to 
> match the size of the array being initialized. I suggest using only 
> `CAT->getSize().getZExtValue()` and adding a comment that this code depends 
> on that adjustment.
That is good to know and  makes sense, thanks!


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

https://reviews.llvm.org/D137488

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


[PATCH] D137488: [clang][Interp] Array initialization via string literal

2022-11-18 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder updated this revision to Diff 476660.
tbaeder marked 3 inline comments as done.

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

https://reviews.llvm.org/D137488

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/test/AST/Interp/literals.cpp


Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -350,6 +350,46 @@
 #endif
 
 #pragma clang diagnostic pop
+
+  constexpr char foo[12] = "abc";
+  static_assert(foo[0] == 'a', "");
+  static_assert(foo[1] == 'b', "");
+  static_assert(foo[2] == 'c', "");
+  static_assert(foo[3] == 0, "");
+  static_assert(foo[11] == 0, "");
+
+  constexpr char foo2[] = "abc\0def";
+  static_assert(foo2[0] == 'a', "");
+  static_assert(foo2[3] == '\0', "");
+  static_assert(foo2[6] == 'f', "");
+  static_assert(foo2[7] == '\0', "");
+  static_assert(foo2[8] == '\0', ""); // expected-error {{not an integral 
constant expression}} \
+  // expected-note {{read of dereferenced 
one-past-the-end pointer}} \
+  // ref-error {{not an integral constant 
expression}} \
+  // ref-note {{read of dereferenced 
one-past-the-end pointer}}
+
+  constexpr char foo3[4] = "abc";
+  static_assert(foo3[3] == '\0', "");
+  static_assert(foo3[4] == '\0', ""); // expected-error {{not an integral 
constant expression}} \
+  // expected-note {{read of dereferenced 
one-past-the-end pointer}} \
+  // ref-error {{not an integral constant 
expression}} \
+  // ref-note {{read of dereferenced 
one-past-the-end pointer}}
+
+  constexpr char foo4[2] = "abcd"; // expected-error {{initializer-string for 
char array is too long}} \
+   // ref-error {{initializer-string for char 
array is too long}}
+  static_assert(foo4[0] == 'a', "");
+  static_assert(foo4[1] == 'b', "");
+  static_assert(foo4[2] == '\0', ""); // expected-error {{not an integral 
constant expression}} \
+  // expected-note {{read of dereferenced 
one-past-the-end pointer}} \
+  // ref-error {{not an integral constant 
expression}} \
+  // ref-note {{read of dereferenced 
one-past-the-end pointer}}
+
+constexpr char foo5[12] = "abc\xff";
+#if defined(__CHAR_UNSIGNED__) || __CHAR_BIT__ > 8
+static_assert(foo5[3] == 255, "");
+#else
+static_assert(foo5[3] == -1, "");
+#endif
 };
 
 #if __cplusplus > 201402L
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -1073,6 +1073,33 @@
 return false;
 }
 return true;
+  } else if (const auto *SL = dyn_cast(Initializer)) {
+const ConstantArrayType *CAT =
+Ctx.getASTContext().getAsConstantArrayType(SL->getType());
+assert(CAT && "a string literal that's not a constant array?");
+
+// If the initializer string is too long, a diagnostic has already been
+// emitted. Read only the array length from the string literal.
+unsigned N = CAT->getSize().getZExtValue();
+size_t CharWidth = SL->getCharByteWidth();
+
+for (unsigned I = 0; I != N; ++I) {
+  uint32_t CodeUnit = SL->getCodeUnit(I);
+
+  if (CharWidth == 1) {
+this->emitConstSint8(CodeUnit, SL);
+this->emitInitElemSint8(I, SL);
+  } else if (CharWidth == 2) {
+this->emitConstUint16(CodeUnit, SL);
+this->emitInitElemUint16(I, SL);
+  } else if (CharWidth == 4) {
+this->emitConstUint32(CodeUnit, SL);
+this->emitInitElemUint32(I, SL);
+  } else {
+llvm_unreachable("unsupported character width");
+  }
+}
+return true;
   }
 
   assert(false && "Unknown expression for array initialization");


Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -350,6 +350,46 @@
 #endif
 
 #pragma clang diagnostic pop
+
+  constexpr char foo[12] = "abc";
+  static_assert(foo[0] == 'a', "");
+  static_assert(foo[1] == 'b', "");
+  static_assert(foo[2] == 'c', "");
+  static_assert(foo[3] == 0, "");
+  static_assert(foo[11] == 0, "");
+
+  constexpr char foo2[] = "abc\0def";
+  static_assert(foo2[0] == 'a', "");
+  static_assert(foo2[3] == '\0', "");
+  static_assert(foo2[6] == 'f', "");
+  static_assert(foo2[7] == '\0', "");
+  static_assert(foo2[8] == '\0', ""); // expected-error {{not an integral constant expression}} \
+  // expected-note {{read of dereferenced one-past-the-end pointer}} \
+  

[PATCH] D138249: [WebAssembly] Update relaxed-simd instruction names

2022-11-18 Thread Marat Dukhan via Phabricator via cfe-commits
maratyszcza accepted this revision.
maratyszcza added a comment.

Intrinsic naming LGTM. Not qualified to review the rest.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138249

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


[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-18 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n marked 2 inline comments as done.
goldstein.w.n added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:838
+  // If this changes PPLevel needs to be used for get correct indentation.
+  assert(!Line.InMacroBody && !InPPDirective);
   return Line.Level * Style.IndentWidth + Length <= ColumnLimit;

owenpan wrote:
> ```
> error: use of undeclared identifier 'InPPDirective'
>   assert(!Line.InMacroBody && !InPPDirective);
>^
> ```
> Also, can you break it into two assertions as suggested so that we know which 
> one failed even without a debugger?
Fixed. Sorry. Had only been rebuilting formattests. Figured everything else 
must have a dep. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137181

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


[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-18 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n marked an inline comment as done.
goldstein.w.n added a comment.

In D137181#3938771 , @owenpan wrote:

> In D137181#3935951 , @owenpan wrote:
>
>> In D137181#3935856 , 
>> @goldstein.w.n wrote:
>>
>>> I could remove either the `PPDIS_BeforeHash` or `PPDIS_AfterHash` though as 
>>> the they really no independent logic in this commit.
>>
>> Yes, please. Or better yet, alternate a little bit between the two.
>
> My bad. I meant splitting the test cases between the two. Can you regroup 
> them (lines 5137-5258) as follows?
>
>   style.IndentPPDirectives = FormatStyle::PPDIS_AfterHash;
>   style.IndentWidth = 4;
>   style.PPIndentWidth = 1;
>   verifyFormat("#ifdef foo\n"
>"# define bar() \\\n"
>" if (A) {  \\\n"
>" B();  \\\n"
>" } \\\n"
>" C();\n"
>"#endif",
>style);
>   verifyFormat("#if abc\n"
>"# ifdef foo\n"
>"#  define bar()\\\n"
>"  if (A) { \\\n"
>"  if (B) { \\\n"
>"  C(); \\\n"
>"  }\\\n"
>"  }\\\n"
>"  D();\n"
>"# endif\n"
>"#endif",
>style);
>   verifyFormat("#ifndef foo\n"
>"#define foo\n"
>"if (emacs) {\n"
>"#ifdef is\n"
>"# define lit   \\\n"
>" if (af) { \\\n"
>" return duh(); \\\n"
>" }\n"
>"#endif\n"
>"}\n"
>"#endif",
>style);
>   verifyFormat("#define X  \\\n"
>"{  \\\n"
>"x; \\\n"
>"x; \\\n"
>"}",
>style);
>   
>   style.IndentWidth = 8;
>   style.PPIndentWidth = 2;
>   verifyFormat("#ifdef foo\n"
>"#  define bar()\\\n"
>"  if (A) { \\\n"
>"  B(); \\\n"
>"  }\\\n"
>"  C();\n"
>"#endif",
>style);
>   
>   style.IndentWidth = 1;
>   style.PPIndentWidth = 4;
>   verifyFormat("#define X \\\n"
>" {\\\n"
>"  x;  \\\n"
>"  x;  \\\n"
>" }",
>style);
>   
>   style.IndentPPDirectives = FormatStyle::PPDIS_BeforeHash;
>   style.IndentWidth = 4;
>   style.PPIndentWidth = 1;
>   verifyFormat("if (emacs) {\n"
>"#ifdef is\n"
>" #define lit   \\\n"
>" if (af) { \\\n"
>" return duh(); \\\n"
>" }\n"
>"#endif\n"
>"}",
>style);
>   verifyFormat("#if abc\n"
>" #ifdef foo\n"
>"  #define bar() \\\n"
>"  if (A) {  \\\n"
>"  B();  \\\n"
>"  } \\\n"
>"  C();\n"
>" #endif\n"
>"#endif",
>style);
>   verifyFormat("#if 1\n"
>" #define X  \\\n"
>" {  \\\n"
>" x; \\\n"
>" x; \\\n"
>" }\n"
>"#endif",
>style);
>   
>   style.PPIndentWidth = 2;
>   verifyFormat("#ifdef foo\n"
>"  #define bar() \\\n"
>"  if (A) {  \\\n"
>"  B();  \\\n"
>"  } \\\n"
>"  C();\n"
>"#endif",
>style);
>   
>   style.IndentWidth = 1;
>   style.PPIndentWidth = 4;
>   verifyFormat("#if 1\n"
>"#define X \\\n"
>" {\\\n"
>"  x;  \\\n"
>"  x;  \\\n"
>" }\n"
>"#endif",
>style);

Yes done.




Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1281
+  Line->PPLevel = PPBranchLevel + (IncludeGuard == IG_Defined ? 0 : 1);
   Line->InMacroBody = true;
 

owenpan wrote:
> In case `PPLevel` is negative.
Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137181

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


[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-18 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n updated this revision to Diff 476654.
goldstein.w.n added a comment.



1. Updating D137181 : [clang-format] Don't 
use 'PPIndentWidth' inside multi-line macros #
2. Enter a brief description of the changes included in this update.
3. The first line is used as subject, next lines as comment. #
4. If you intended to create a new revision, use:
5. $ arc diff --create

De-interleave test cases.
Cleanup asserts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137181

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -5040,6 +5040,213 @@
"  int y = 0;\n"
"}",
style);
+
+  style.IndentPPDirectives = FormatStyle::PPDIS_None;
+  verifyFormat("#ifdef foo\n"
+   "#define bar() \\\n"
+   "if (A) {  \\\n"
+   "B();  \\\n"
+   "} \\\n"
+   "C();\n"
+   "#endif",
+   style);
+  verifyFormat("if (emacs) {\n"
+   "#ifdef is\n"
+   "#define lit   \\\n"
+   "if (af) { \\\n"
+   "return duh(); \\\n"
+   "}\n"
+   "#endif\n"
+   "}",
+   style);
+  verifyFormat("#if abc\n"
+   "#ifdef foo\n"
+   "#define bar()\\\n"
+   "if (A) { \\\n"
+   "if (B) { \\\n"
+   "C(); \\\n"
+   "}\\\n"
+   "}\\\n"
+   "D();\n"
+   "#endif\n"
+   "#endif",
+   style);
+  verifyFormat("#ifndef foo\n"
+   "#define foo\n"
+   "if (emacs) {\n"
+   "#ifdef is\n"
+   "#define lit   \\\n"
+   "if (af) { \\\n"
+   "return duh(); \\\n"
+   "}\n"
+   "#endif\n"
+   "}\n"
+   "#endif",
+   style);
+  verifyFormat("#if 1\n"
+   "#define X  \\\n"
+   "{  \\\n"
+   "x; \\\n"
+   "x; \\\n"
+   "}\n"
+   "#endif",
+   style);
+  verifyFormat("#define X  \\\n"
+   "{  \\\n"
+   "x; \\\n"
+   "x; \\\n"
+   "}",
+   style);
+
+  style.PPIndentWidth = 2;
+  verifyFormat("#ifdef foo\n"
+   "#define bar() \\\n"
+   "if (A) {  \\\n"
+   "B();  \\\n"
+   "} \\\n"
+   "C();\n"
+   "#endif",
+   style);
+  style.IndentWidth = 8;
+  verifyFormat("#ifdef foo\n"
+   "#define bar()\\\n"
+   "if (A) { \\\n"
+   "B(); \\\n"
+   "}\\\n"
+   "C();\n"
+   "#endif",
+   style);
+
+  style.IndentWidth = 1;
+  style.PPIndentWidth = 4;
+  verifyFormat("#if 1\n"
+   "#define X \\\n"
+   " {\\\n"
+   "  x;  \\\n"
+   "  x;  \\\n"
+   " }\n"
+   "#endif",
+   style);
+  verifyFormat("#define X \\\n"
+   " {\\\n"
+   "  x;  \\\n"
+   "  x;  \\\n"
+   " }",
+   style);
+
+  style.IndentWidth = 4;
+  style.PPIndentWidth = 1;
+  style.IndentPPDirectives = FormatStyle::PPDIS_AfterHash;
+  verifyFormat("#ifdef foo\n"
+   "# define bar() \\\n"
+   " if (A) {  \\\n"
+   " B();  \\\n"
+   " } \\\n"
+   " C();\n"
+   "#endif",
+   style);
+  verifyFormat("#if abc\n"
+   "# ifdef foo\n"
+   "#  define bar()\\\n"
+   "  if (A) { \\\n"
+   "  if (B) { \\\n"
+   "  C(); \\\n"
+   "  }\\\n"
+   "  }\\\n"
+   "  D();\n"
+   "# endif\n"
+   "#endif",
+   style);
+  verifyFormat("#ifndef foo\n"
+   "#define foo\n"
+   "if (emacs) {\n"
+   "#ifdef is\n"
+   "# 

[PATCH] D138278: TableGen: honor LLVM_LINK_LLVM_DYLIB by default

2022-11-18 Thread Stephen Neuendorffer via Phabricator via cfe-commits
stephenneuendorffer added a comment.

I think the MLIR parts of this code were written before attempting to create 
libMLIR.so.  Probably these tablegen binaries would have to also avoid linking 
against libMLIR.so and be statically linked to both llvmSupport and MLIRsupport 
to work properly.I think the proposed solution is reasonable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138278

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


[PATCH] D137753: [Clang][GNU][AIX][p]Enable -p Functionality

2022-11-18 Thread Michael Francis via Phabricator via cfe-commits
francii updated this revision to Diff 476649.
francii added a comment.

Remove Linux changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137753

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Driver/ToolChains/AIX.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/aix-ld.c

Index: clang/test/Driver/aix-ld.c
===
--- clang/test/Driver/aix-ld.c
+++ clang/test/Driver/aix-ld.c
@@ -135,6 +135,8 @@
 // CHECK-LD32-PROF-NOT: "--no-as-needed"
 // CHECK-LD32-PROF-NOT: "-lm"
 // CHECK-LD32-PROF: "-lc"
+// CHECK-LD32-PROF: "-L[[SYSROOT]]/lib/profiled"
+// CHECK-LD32-PROF: "-L[[SYSROOT]]/usr/lib/profiled"
 
 // Check powerpc64-ibm-aix7.1.0.0, 64-bit. Enable profiling.
 // RUN: %clang %s -### 2>&1 \
@@ -162,6 +164,8 @@
 // CHECK-LD64-PROF-NOT: "--no-as-needed"
 // CHECK-LD64-PROF-NOT: "-lm"
 // CHECK-LD64-PROF: "-lc"
+// CHECK-LD64-PROF: "-L[[SYSROOT]]/lib/profiled"
+// CHECK-LD64-PROF: "-L[[SYSROOT]]/usr/lib/profiled"
 
 // Check powerpc-ibm-aix7.1.0.0, 32-bit. Enable g-profiling.
 // RUN: %clang %s -### 2>&1 \
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -521,7 +521,8 @@
 
 static bool useFramePointerForTargetByDefault(const ArgList ,
   const llvm::Triple ) {
-  if (Args.hasArg(options::OPT_pg) && !Args.hasArg(options::OPT_mfentry))
+  if (Args.hasArg(options::OPT_p, options::OPT_pg) &&
+  !Args.hasArg(options::OPT_mfentry))
 return true;
 
   switch (Triple.getArch()) {
@@ -7388,7 +7389,7 @@
 C.getJobs().getJobs().back()->PrintInputFilenames = true;
   }
 
-  if (Arg *A = Args.getLastArg(options::OPT_pg))
+  if (Arg *A = Args.getLastArg(options::OPT_p, options::OPT_pg))
 if (FPKeepKind == CodeGenOptions::FramePointerKind::None &&
 !Args.hasArg(options::OPT_mfentry))
   D.Diag(diag::err_drv_argument_not_allowed_with) << "-fomit-frame-pointer"
Index: clang/lib/Driver/ToolChains/AIX.cpp
===
--- clang/lib/Driver/ToolChains/AIX.cpp
+++ clang/lib/Driver/ToolChains/AIX.cpp
@@ -250,6 +250,13 @@
   CmdArgs.push_back("-lm");
 
 CmdArgs.push_back("-lc");
+
+if (Args.hasArg(options::OPT_p, options::OPT_pg)) {
+  CmdArgs.push_back(Args.MakeArgString((llvm::Twine("-L") + D.SysRoot) +
+   "/lib/profiled"));
+  CmdArgs.push_back(Args.MakeArgString((llvm::Twine("-L") + D.SysRoot) +
+   "/usr/lib/profiled"));
+}
   }
 
   const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -1043,7 +1043,8 @@
   // inlining, we just add an attribute to insert a mcount call in backend.
   // The attribute "counting-function" is set to mcount function name which is
   // architecture dependent.
-  if (CGM.getCodeGenOpts().InstrumentForProfiling) {
+  if (CGM.getCodeGenOpts().InstrumentForProfiling ||
+  CGM.getCodeGenOpts().InstrumentForProfilingGraph) {
 // Calls to fentry/mcount should not be generated if function has
 // the no_instrument_function attribute.
 if (!CurFuncDecl || !CurFuncDecl->hasAttr()) {
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -923,7 +923,8 @@
 if (CodeGenOpts.InstrumentFunctions ||
 CodeGenOpts.InstrumentFunctionEntryBare ||
 CodeGenOpts.InstrumentFunctionsAfterInlining ||
-CodeGenOpts.InstrumentForProfiling) {
+CodeGenOpts.InstrumentForProfiling ||
+CodeGenOpts.InstrumentForProfilingGraph) {
   PB.registerPipelineStartEPCallback(
   [](ModulePassManager , OptimizationLevel Level) {
 MPM.addPass(createModuleToFunctionPassAdaptor(
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -4094,8 +4094,10 @@
   MarshallingInfoFlag>;
 def pedantic : Flag<["-", "--"], "pedantic">, Group, Flags<[CC1Option,FlangOption,FC1Option]>,
   HelpText<"Warn on language extensions">, MarshallingInfoFlag>;
-def pg : Flag<["-"], "pg">, HelpText<"Enable mcount instrumentation">, Flags<[CC1Option]>,
+def p : Flag<["-"], "p">, HelpText<"Enable mcount instrumentation 

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-18 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment.

In D137181#3935951 , @owenpan wrote:

> In D137181#3935856 , @goldstein.w.n 
> wrote:
>
>> I could remove either the `PPDIS_BeforeHash` or `PPDIS_AfterHash` though as 
>> the they really no independent logic in this commit.
>
> Yes, please. Or better yet, alternate a little bit between the two.

My bad. I meant splitting the test cases between the two. Can you regroup them 
(lines 5137-5258) as follows?

  style.IndentPPDirectives = FormatStyle::PPDIS_AfterHash;
  style.IndentWidth = 4;
  style.PPIndentWidth = 1;
  verifyFormat("#ifdef foo\n"
   "# define bar() \\\n"
   " if (A) {  \\\n"
   " B();  \\\n"
   " } \\\n"
   " C();\n"
   "#endif",
   style);
  verifyFormat("#if abc\n"
   "# ifdef foo\n"
   "#  define bar()\\\n"
   "  if (A) { \\\n"
   "  if (B) { \\\n"
   "  C(); \\\n"
   "  }\\\n"
   "  }\\\n"
   "  D();\n"
   "# endif\n"
   "#endif",
   style);
  verifyFormat("#ifndef foo\n"
   "#define foo\n"
   "if (emacs) {\n"
   "#ifdef is\n"
   "# define lit   \\\n"
   " if (af) { \\\n"
   " return duh(); \\\n"
   " }\n"
   "#endif\n"
   "}\n"
   "#endif",
   style);
  verifyFormat("#define X  \\\n"
   "{  \\\n"
   "x; \\\n"
   "x; \\\n"
   "}",
   style);
  
  style.IndentWidth = 8;
  style.PPIndentWidth = 2;
  verifyFormat("#ifdef foo\n"
   "#  define bar()\\\n"
   "  if (A) { \\\n"
   "  B(); \\\n"
   "  }\\\n"
   "  C();\n"
   "#endif",
   style);
  
  style.IndentWidth = 1;
  style.PPIndentWidth = 4;
  verifyFormat("#define X \\\n"
   " {\\\n"
   "  x;  \\\n"
   "  x;  \\\n"
   " }",
   style);
  
  style.IndentPPDirectives = FormatStyle::PPDIS_BeforeHash;
  style.IndentWidth = 4;
  style.PPIndentWidth = 1;
  verifyFormat("if (emacs) {\n"
   "#ifdef is\n"
   " #define lit   \\\n"
   " if (af) { \\\n"
   " return duh(); \\\n"
   " }\n"
   "#endif\n"
   "}",
   style);
  verifyFormat("#if abc\n"
   " #ifdef foo\n"
   "  #define bar() \\\n"
   "  if (A) {  \\\n"
   "  B();  \\\n"
   "  } \\\n"
   "  C();\n"
   " #endif\n"
   "#endif",
   style);
  verifyFormat("#if 1\n"
   " #define X  \\\n"
   " {  \\\n"
   " x; \\\n"
   " x; \\\n"
   " }\n"
   "#endif",
   style);
  
  style.PPIndentWidth = 2;
  verifyFormat("#ifdef foo\n"
   "  #define bar() \\\n"
   "  if (A) {  \\\n"
   "  B();  \\\n"
   "  } \\\n"
   "  C();\n"
   "#endif",
   style);
  
  style.IndentWidth = 1;
  style.PPIndentWidth = 4;
  verifyFormat("#if 1\n"
   "#define X \\\n"
   " {\\\n"
   "  x;  \\\n"
   "  x;  \\\n"
   " }\n"
   "#endif",
   style);




Comment at: clang/lib/Format/UnwrappedLineParser.cpp:838
+  // If this changes PPLevel needs to be used for get correct indentation.
+  assert(!Line.InMacroBody && !InPPDirective);
   return Line.Level * Style.IndentWidth + Length <= ColumnLimit;

```
error: use of undeclared identifier 'InPPDirective'
  assert(!Line.InMacroBody && !InPPDirective);
   ^
```
Also, can you break it into two assertions as suggested so that we know which 
one failed even without a debugger?



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1281
+  Line->PPLevel = PPBranchLevel + (IncludeGuard == IG_Defined ? 0 : 1);
   Line->InMacroBody = true;
 

In case `PPLevel` is negative.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137181

___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D137473: [vfs] Allow root paths relative to the directory of the vfsoverlay YAML file

2022-11-18 Thread Petr Hosek via Phabricator via cfe-commits
phosek added inline comments.



Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1933-1935
 path_style = sys::path::is_absolute(Name, sys::path::Style::posix)
  ? sys::path::Style::posix
  : sys::path::Style::windows_backslash;

haowei wrote:
> bnbarham wrote:
> > `windows_slash` is going to be `windows_backslash` here. I assume this was 
> > fine since `sys::fs::make_absolute(Name)` would always return the native 
> > style, but that isn't the case now. It's also unfortunate we calculate this 
> > again when `makeAbsolute` only just did it.
> Thanks for pointing that out. I need to consider the windows forward slash 
> case.
> 
> There is a bit in consistency in the VFS implementation. for Windows. we can 
> use forward slashes('/') on Windows. Most Windows API/Syscalls accept it. But 
> some APIs may not work if you mix '/' with '\' in the path on Windows. What 
> RedirectFileSystem::makeAbsolute is trying to do is, it first determine the 
> slashes used in the WorkDir(either from process working directory or from 
> command line flags, or OverlayDir), in which it can be a forward slash on 
> Windows. Then it uses the same separator when appending the relative 
> directory.  Using sys::fs::make_absolute will break that. 
> 
> The reason that I use makeAbsolute here is that the OverlayDir will likely 
> use forward slash on Windows if people uses CMake options 
> LINK_FLAG="/vfsoverlay:C:/path/to/overlay.xml". In that case, 
> sys::fs::make_absolute will generate some paths like C:/path/to/foo\\bar, 
> which may cause issues.
> 
> I will rewrite this to consider the forward slashes.
This was also discussed in D87732 which may provide some additional context.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137473

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


[PATCH] D136176: Implement support for option 'fexcess-precision'.

2022-11-18 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

At some point, we're going to have to figure out how this impacts 
`FLT_EVAL_METHOD`, but we can do that in a separate patch.




Comment at: clang/docs/UsersManual.rst:1732
+.. option:: -fexcess-precision:
+
+   By default, Clang uses excess precision to calculate ``_Float16``


   The C and C++ standards allow floating-point expressions to be computed
   as if intermediate results had more precision (and/or a wider range) than the
   type of the expression strictly allows.  This is called excess precision 
arithmetic.
   Excess precision arithmetic can improve the accuracy of results (although not
   always), and it can make computation significantly faster if the target lacks
   direct hardware support for arithmetic in a particular type.  However, it can
   also undermine strict floating-point reproducibility.

   Under the standards, assignments and explicit casts force the operand to be
   converted to its formal type, discarding any excess precision.  Because data
   can only flow between statements via an assignment, this means that the
   use of excess precision arithmetic is a reliable local property of a single
   statement, and results do not change based on optimization.  However, when
   excess precision arithmetic is in use, Clang does not guarantee strict
   reproducibility, and future compiler releases may recognize more 
opportunities
   to use excess precision arithmetic, e.g. with floating-point builtins.

   Clang does not use excess precision arithmetic for most types or on most 
targets.
   For example, even on pre-SSE X86 targets where ``float`` and ``double``
   computations must be performed in the 80-bit X87 format, Clang rounds
   all intermediate results correctly for their type.  Clang currently uses 
excess
   precision arithmetic by default only for the following types and targets:

   * ``_Float16`` on X86 targets without ``AVX512-FP16``

   The ``-fexcess-precision=`` option can be used to control the use of 
excess
   precision arithmetic.  Valid values are:

   * ``standard`` - The default.  Allow the use of excess precision arithmetic 
under
 the constraints of the C and C++ standards. Has no effect except on the 
types
 and targets listed above.
   * ``fast`` - Accepted for GCC compatibility, but currently treated as an 
alias
 for ``standard``.
   * ``16`` - Forces ``_Float16`` operations to be emitted without using excess
 precision arithmetic.
   



Comment at: clang/include/clang/Basic/LangOptions.h:301
 
+  enum Float16ExcessPrecisionKind { FPP_Standard, FPP_Fast, FPP_None };
+

You can leave this named `ExcessPrecisionKind` — if we introduce excess 
precision for other types, they'll have the same set of options.



Comment at: clang/include/clang/Driver/Options.td:1581
+def ffloat16_excess_precision_EQ : Joined<["-"], "ffloat16-excess-precision=">,
+  Group, Flags<[CC1Option]>,
+  HelpText<"Allows control over excess precision on targets where native "

I think you have to be explicit about `NoDriverOption` here to make sure it's 
only for `-cc1`.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2795
   bool StrictFPModel = false;
+  StringRef Float16ExcessPrecision = "standard";
 

It's minor, but let's default this to empty so that we don't put any work 
towards rendering this option unless the user actually passes something.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2997
+ << A->getAsString(Args) << TC.getTriple().str();
+   if (Val.equals("standard") || Val.equals("fast") || Val.equals("none"))
+ Float16ExcessPrecision = Val;

GCC doesn't allow `none` here, right?  Let's keep that.  But our type-specific 
frontend option will accept `none`.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2995
+  StringRef Val = A->getValue();
+   if (TC.getTriple().getArch() == llvm::Triple::x86 && Val.equals("16"))
+D.Diag(diag::err_drv_unsupported_opt_for_target)

zahiraam wrote:
> andrew.w.kaylor wrote:
> > Why is 16 only supported for x86? Is it only here for gcc compatibility?
> Yes for gcc compatibility (although we are using here that value "none" to 
> disable excess precision instead of using "16") and also because we are 
> dealing with excess precision for _Float16 types only, so sticking to X86.
`llvm::Triple::x86` is just i386, and I think you want to include x86_64, right?


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

https://reviews.llvm.org/D136176

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


[PATCH] D138312: [OPENMP5.1] Initial support for message clause.

2022-11-18 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 closed this revision.
jyu2 added a comment.

checked with 9d90cf2fca44 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138312

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


[clang] 9d90cf2 - [OPENMP5.1] Initial support for message clause.

2022-11-18 Thread Jennifer Yu via cfe-commits

Author: Jennifer Yu
Date: 2022-11-18T17:59:23-08:00
New Revision: 9d90cf2fca4446f5c227f6e3217e96c00665cb72

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

LOG: [OPENMP5.1] Initial support for message clause.

Added: 


Modified: 
clang/include/clang/AST/OpenMPClause.h
clang/include/clang/AST/RecursiveASTVisitor.h
clang/include/clang/Basic/DiagnosticParseKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/AST/OpenMPClause.cpp
clang/lib/AST/StmtProfile.cpp
clang/lib/CodeGen/CGStmtOpenMP.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/error_ast_print.cpp
clang/test/OpenMP/error_message.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/OpenMPClause.h 
b/clang/include/clang/AST/OpenMPClause.h
index 21d9f740eddf1..2bf3e91a15a20 100644
--- a/clang/include/clang/AST/OpenMPClause.h
+++ b/clang/include/clang/AST/OpenMPClause.h
@@ -1722,6 +1722,72 @@ class OMPSeverityClause final : public OMPClause {
   }
 };
 
+/// This represents 'message' clause in the '#pragma omp error' directive
+///
+/// \code
+/// #pragma omp error message("GNU compiler required.")
+/// \endcode
+/// In this example directive '#pragma omp error' has simple
+/// 'message' clause with user error message of "GNU compiler required.".
+class OMPMessageClause final : public OMPClause {
+  friend class OMPClauseReader;
+
+  /// Location of '('
+  SourceLocation LParenLoc;
+
+  // Expression of the 'message' clause.
+  Stmt *MessageString = nullptr;
+
+  /// Set message string of the clause.
+  void setMessageString(Expr *MS) { MessageString = MS; }
+
+  /// Sets the location of '('.
+  void setLParenLoc(SourceLocation Loc) { LParenLoc = Loc; }
+
+public:
+  /// Build 'message' clause with message string argument
+  ///
+  /// \param A Argument of the clause (message string).
+  /// \param StartLoc Starting location of the clause.
+  /// \param LParenLoc Location of '('.
+  /// \param EndLoc Ending location of the clause.
+  OMPMessageClause(Expr *MS, SourceLocation StartLoc, SourceLocation LParenLoc,
+   SourceLocation EndLoc)
+  : OMPClause(llvm::omp::OMPC_message, StartLoc, EndLoc),
+LParenLoc(LParenLoc), MessageString(MS) {}
+
+  /// Build an empty clause.
+  OMPMessageClause()
+  : OMPClause(llvm::omp::OMPC_message, SourceLocation(), SourceLocation()) 
{
+  }
+
+  /// Returns the locaiton of '('.
+  SourceLocation getLParenLoc() const { return LParenLoc; }
+
+  /// Returns message string of the clause.
+  Expr *getMessageString() const { return cast_or_null(MessageString); }
+
+  child_range children() {
+return child_range(,  + 1);
+  }
+
+  const_child_range children() const {
+return const_child_range(,  + 1);
+  }
+
+  child_range used_children() {
+return child_range(child_iterator(), child_iterator());
+  }
+
+  const_child_range used_children() const {
+return const_child_range(const_child_iterator(), const_child_iterator());
+  }
+
+  static bool classof(const OMPClause *T) {
+return T->getClauseKind() == llvm::omp::OMPC_message;
+  }
+};
+
 /// This represents 'schedule' clause in the '#pragma omp ...' directive.
 ///
 /// \code

diff  --git a/clang/include/clang/AST/RecursiveASTVisitor.h 
b/clang/include/clang/AST/RecursiveASTVisitor.h
index 9baee8b72fa11..7475d29cd98d0 100644
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -3320,6 +3320,12 @@ bool 
RecursiveASTVisitor::VisitOMPSeverityClause(OMPSeverityClause *) {
   return true;
 }
 
+template 
+bool RecursiveASTVisitor::VisitOMPMessageClause(OMPMessageClause *C) {
+  TRY_TO(TraverseStmt(C->getMessageString()));
+  return true;
+}
+
 template 
 bool
 RecursiveASTVisitor::VisitOMPScheduleClause(OMPScheduleClause *C) {

diff  --git a/clang/include/clang/Basic/DiagnosticParseKinds.td 
b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 6f46ede56f66c..ffbc859162270 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1342,6 +1342,8 @@ def err_omp_unexpected_directive : Error<
   "unexpected OpenMP directive %select{|'#pragma omp %1'}0">;
 def err_omp_expected_punc : Error<
   "expected ',' or ')' in '%0' %select{clause|directive}1">;
+def warn_clause_expected_string : Warning<
+  "expected string literal in 'clause %0' - ignoring">, 
InGroup;
 def err_omp_unexpected_clause : Error<
   "unexpected OpenMP clause '%0' in directive '#pragma omp %1'">;
 

[PATCH] D138088: [clang][docs] Use `option` directive in User's Manual

2022-11-18 Thread KAWASHIMA Takahiro via Phabricator via cfe-commits
kawashima-fj added a comment.

In D138088#3937680 , @aaron.ballman 
wrote:

> Thank you for this cleanup! In general, I thin this looks correct. However, I 
> know we've had to fix a bunch of options that cause the sphinx build to fail 
> (IIRC, oftentimes due to duplicate options) and our precommit CI doesn't test 
> the documentation build. Did you try building the docs locally to ensure 
> there are no new warnings/errors from Sphinx?

Yes. I confirmed no new warnings/errors with the following commands. I used 
Sphinx packaged by distributions (Ubuntu 22.04 and Debian GNU/Linux 11). Is it 
sufficient? If no, let me know.

  cmake -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_PROJECTS=clang 
-DLLVM_ENABLE_SPHINX=ON -DLLVM_INCLUDE_DOCS=ON [other unrelated options ...]
  ninja docs-clang-html


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138088

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


[PATCH] D138329: [-Wunsafe-buffer-usage] Add a new recursive matcher to replace `forEachDescendant` in unsafe buffer check

2022-11-18 Thread Ziqing Luo via Phabricator via cfe-commits
ziqingluo-90 added a comment.

In D138329#3938351 , @xazax.hun wrote:

> Is the problem `forEachDescendant` matching statements inside blocks and 
> lambdas? I wonder if this behavior would surprise people, so I think it would 
> be better to:
>
> - Potentially add a template bool parameter to `forEachDescendant` 
> controlling this behavior.
> - Review existing uses because I am not entirely sure if the current behavior 
> is the right default.

Let me try to describe the problem in more detail.   The goal is to recursively 
visit every descendant of a node `n` but skip all nested callable declarations 
in `n`.  With `forEachDescendant`, one needs to explicitly test and skip, using 
`forCallable`,  if a descendant does not belong to the same "callable" as `n`.  
`forCallable` looks for ancestors of a node through the AST so it requires the 
AST to be complete.  But in our case, the AST is under construction.   Given a 
node, we want the visitor only strictly look at descendants of the node.

For the questions,  I think blocks and lambdas are the only cases we will have 
in analyzing "C/C++" + "blocks" programs.   Our plan is to generalize this 
matcher to be an optional mode of `forEachDescendant` once it becomes stable.  
So this implementation also skips other declarations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138329

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


[PATCH] D137872: Try to implement lambdas with inalloca parameters by forwarding without use of inallocas.

2022-11-18 Thread Amy Huang via Phabricator via cfe-commits
akhuang added a comment.

In D137872#3925723 , @efriedma wrote:

> I'm not quite sure I understand what's happening here.  Does this actually 
> avoid generating two copies of the function body if both the call operator 
> and the conversion are used?

That was intended but missing; patch is now updated to make both the invoker 
and the call operator call the new function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137872

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


[PATCH] D137872: Try to implement lambdas with inalloca parameters by forwarding without use of inallocas.

2022-11-18 Thread Amy Huang via Phabricator via cfe-commits
akhuang updated this revision to Diff 476634.
akhuang added a comment.

cleanup


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137872

Files:
  clang/include/clang/CodeGen/CGFunctionInfo.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGCall.h
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenABITypes.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenTypes.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGenCXX/inalloca-lambda.cpp

Index: clang/test/CodeGenCXX/inalloca-lambda.cpp
===
--- clang/test/CodeGenCXX/inalloca-lambda.cpp
+++ clang/test/CodeGenCXX/inalloca-lambda.cpp
@@ -1,7 +1,4 @@
-// RUN: not %clang_cc1 -triple i686-windows-msvc -emit-llvm -o /dev/null %s  2>&1 | FileCheck %s
-
-// PR28299
-// CHECK: error: cannot compile this forwarded non-trivially copyable parameter yet
+// RUN: %clang_cc1 -triple i686-windows-msvc -emit-llvm -o - %s  2>&1 | FileCheck %s
 
 class A {
   A(const A &);
@@ -9,3 +6,23 @@
 typedef void (*fptr_t)(A);
 fptr_t fn1() { return [](A) {}; }
 
+
+// CHECK: define internal void @"?__invoke@@?0??fn1@@YAP6AXVA@@@ZXZ@CA?A?@@0@Z"
+// CHECK-SAME:  (ptr inalloca(<{ %class.A, [3 x i8] }>) %0)
+// CHECK: %unused.capture = alloca %class.anon, align 1
+// CHECK: %1 = getelementptr inbounds <{ %class.A, [3 x i8] }>, ptr %0, i32 0, i32 0
+// CHECK: call x86_thiscallcc void @"??R@?0??fn1@@YAP6AXVA@@@ZXZ@QBE?A?@@0...@z.impl"
+// CHECK-SAME: (ptr noundef %unused.capture, ptr noundef %1)
+// CHECK: ret void
+
+// CHECK: define internal x86_thiscallcc void @"??R@?0??fn1@@YAP6AXVA@@@ZXZ@QBE?A?@@0@Z"
+// CHECK-SAME:  (ptr noundef %this, ptr inalloca(<{ %class.A, [3 x i8] }>) %0)
+// CHECK: call x86_thiscallcc void @"??R@?0??fn1@@YAP6AXVA@@@ZXZ@QBE?A?@@0...@z.impl"
+// CHECK-SAME: (ptr noundef %unused.capture, ptr noundef %1)
+
+// CHECK: define internal x86_thiscallcc void @"??R@?0??fn1@@YAP6AXVA@@@ZXZ@QBE?A?@@0...@z.impl"
+// CHECK-SAME: (ptr noundef %this, ptr noundef %0)
+// CHECK: %this.addr = alloca ptr, align 4
+// CHECK: store ptr %this, ptr %this.addr, align 4
+// CHECK: %this1 = load ptr, ptr %this.addr, align 4
+// CHECK: ret void
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -1187,7 +1187,7 @@
 
   Class classify(QualType Ty) const;
   ABIArgInfo classifyReturnType(QualType RetTy, CCState ) const;
-  ABIArgInfo classifyArgumentType(QualType RetTy, CCState ) const;
+  ABIArgInfo classifyArgumentType(QualType RetTy, CCState , bool NoInAlloca) const;
 
   /// Updates the number of available free registers, returns
   /// true if any registers were allocated.
@@ -1824,7 +1824,8 @@
 }
 
 ABIArgInfo X86_32ABIInfo::classifyArgumentType(QualType Ty,
-   CCState ) const {
+   CCState ,
+   bool NoInAlloca) const {
   // FIXME: Set alignment on indirect arguments.
   bool IsFastCall = State.CC == llvm::CallingConv::X86_FastCall;
   bool IsRegCall = State.CC == llvm::CallingConv::X86_RegCall;
@@ -1840,6 +1841,8 @@
 if (RAA == CGCXXABI::RAA_Indirect) {
   return getIndirectResult(Ty, false, State);
 } else if (RAA == CGCXXABI::RAA_DirectInMemory) {
+  if (NoInAlloca)
+return getIndirectResult(Ty, false, State);
   // The field index doesn't matter, we'll fix it up later.
   return ABIArgInfo::getInAlloca(/*FieldIndex=*/0);
 }
@@ -2014,7 +2017,7 @@
 if (State.IsPreassigned.test(I))
   continue;
 
-Args[I].info = classifyArgumentType(Args[I].type, State);
+Args[I].info = classifyArgumentType(Args[I].type, State, FI.isAvoidingInAlloca());
 UsedInAlloca |= (Args[I].info.getKind() == ABIArgInfo::InAlloca);
   }
 
Index: clang/lib/CodeGen/CodeGenTypes.h
===
--- clang/lib/CodeGen/CodeGenTypes.h
+++ clang/lib/CodeGen/CodeGenTypes.h
@@ -260,8 +260,7 @@
   ///
   /// \param argTypes - must all actually be canonical as params
   const CGFunctionInfo (CanQualType returnType,
-bool instanceMethod,
-bool chainCall,
+FnInfoOptions opts,
 ArrayRef argTypes,
 FunctionType::ExtInfo info,
 ArrayRef paramInfos,
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -2219,10 +2219,16 @@
   void 

[PATCH] D138263: [clang-format] Supress aligning of trailing namespace comments

2022-11-18 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment.

Something like the following:

  diff --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h
  index 87515372046d..3dc5e411df55 100644
  --- a/clang/lib/Format/FormatToken.h
  +++ b/clang/lib/Format/FormatToken.h
  @@ -98,6 +98,8 @@ namespace format {
 TYPE(MacroBlockBegin)  
  \
 TYPE(MacroBlockEnd)
  \
 TYPE(ModulePartitionColon) 
  \
  +  TYPE(NamespaceLBrace)  
  \
  +  TYPE(NamespaceRBrace)  
  \
 TYPE(NamespaceMacro)   
  \
 TYPE(NonNullAssertion) 
  \
 TYPE(NullCoalescingEqual)  
  \
  diff --git a/clang/lib/Format/UnwrappedLineParser.cpp 
b/clang/lib/Format/UnwrappedLineParser.cpp
  index 18ec0844db3d..6eb1086015f0 100644
  --- a/clang/lib/Format/UnwrappedLineParser.cpp
  +++ b/clang/lib/Format/UnwrappedLineParser.cpp
  @@ -920,6 +920,9 @@ FormatToken *UnwrappedLineParser::parseBlock(
   return IfLBrace;
 }
   
  +  if (FormatTok->is(tok::r_brace) && Tok->is(TT_NamespaceLBrace))
  +FormatTok->setFinalizedType(TT_NamespaceRBrace);
  +
 const bool IsFunctionRBrace =
 FormatTok->is(tok::r_brace) && Tok->is(TT_FunctionLBrace);
   
  @@ -2961,6 +2964,7 @@ void UnwrappedLineParser::parseNamespace() {
   }
 }
 if (FormatTok->is(tok::l_brace)) {
  +FormatTok->setFinalizedType(TT_NamespaceLBrace);
   if (ShouldBreakBeforeBrace(Style, InitialToken))
 addUnwrappedLine();
   
  diff --git a/clang/lib/Format/WhitespaceManager.cpp 
b/clang/lib/Format/WhitespaceManager.cpp
  index 1f29f7ab917c..0867a8585b50 100644
  --- a/clang/lib/Format/WhitespaceManager.cpp
  +++ b/clang/lib/Format/WhitespaceManager.cpp
  @@ -990,8 +990,7 @@ void WhitespaceManager::alignTrailingComments() {
   // If this comment follows an } in column 0, it probably documents the
   // closing of a namespace and we don't want to align it.
   bool FollowsRBraceInColumn0 = i > 0 && Changes[i].NewlinesBefore == 0 &&
  -  Changes[i - 1].Tok->is(tok::r_brace) &&
  -  Changes[i - 1].StartOfTokenColumn == 0;
  +  Changes[i - 1].Tok->is(TT_NamespaceRBrace);
   bool WasAlignedWithStartOfNextLine = false;
   if (Changes[i].NewlinesBefore >= 1) { // A comment on its own line.
 unsigned CommentColumn = SourceMgr.getSpellingColumnNumber(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138263

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


[PATCH] D138073: [clang-doc] Move file layout to the generators.

2022-11-18 Thread Paul Kirth via Phabricator via cfe-commits
paulkirth accepted this revision.
paulkirth added a comment.
This revision is now accepted and ready to land.

This is mostly LGTM, modulo the small change to the test that I've suggested. 
Assuming the presubmit tests still pass with that small edit, you're good on my 
end.




Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:890-892
+// TODO If there are multiple Infos for this file name (for example,
+// template specializations), this will generate multiple complete web 
pages
+// (with  and , etc.) concatenated together. This generator

brettw wrote:
> paulkirth wrote:
> > brettw wrote:
> > > paulkirth wrote:
> > > > So does this patch make HTML unusable? Currently, if there is a 
> > > > conflict, the file gets replaced, right? 
> > > > 
> > > > I read this as we're going from a situation where we lost data, but 
> > > > still had valid HTML pages to one where we have complete, but incorrect 
> > > > HTML, is that correct?
> > > > 
> > > > Also, why does that happen under template specialization? USRs are SHA1 
> > > > hashes of the symbol. I would expect that hashing the symbol would be 
> > > > unambiguous and unique, given C++'s ODR. 
> > > > 
> > > > That last point made me wonder if an alternate way to handle this 
> > > > without using the hash would be to use the mangled names?
> > > I think this makes HTML strictly better (though not ideal).
> > > 
> > > Previously if there was a collision, they got written over the top of 
> > > each other without truncation (not sure why). So if the longer page was 
> > > written second, you got lucky and you get a complete valid page and just 
> > > lost the shorter struct's definition. If you got unlucky the shorter page 
> > > was written second and you get a corrupted file with the shorter content, 
> > > followed by the end of the longer content peeking out from the end.
> > > 
> > > With this change, you get both content concatenated without corruption, 
> > > you just have duplicate doctype and title tags in between them. Browsers 
> > > should generally handle this so the page should be usable.
> > > 
> > > Fixing this requires a refactor of the HTML generator such that the 
> > > concept of writing a struct is separate from writing a page. Even if I 
> > > was going to work on the HTML generator, I would probably want to do this 
> > > in a separate patch anyway.
> > > 
> > > The markdown generator does the same, but markdown doesn't have all of 
> > > the headers stuff that shouldn't be in the middle of the file, you just 
> > > get a new `# ...` for the second section and it should be reasonable.
> > > 
> > > The HTML and MD generators name files according to the `Name` of the 
> > > structure. USR is not involved. So any time to things in the same 
> > > namespace have the same name, you'll get a collision. This happens most 
> > > commonly with template specialization because the names are identical 
> > > (the name doesn't account for the template parameters).
> > > 
> > > All of this complexity is why I changed the YAML output to use USR which 
> > > avoids the problem. I didn't want to make this change for the HTML and MD 
> > > generators because it will make ugly file names that the user will see, 
> > > and you probably want the template specializations to be in the same file 
> > > anyway.
> > > I think this makes HTML strictly better (though not ideal).
> > > 
> > > Previously if there was a collision, they got written over the top of 
> > > each other without truncation (not sure why). So if the longer page was 
> > > written second, you got lucky and you get a complete valid page and just 
> > > lost the shorter struct's definition. If you got unlucky the shorter page 
> > > was written second and you get a corrupted file with the shorter content, 
> > > followed by the end of the longer content peeking out from the end.
> > > 
> > > With this change, you get both content concatenated without corruption, 
> > > you just have duplicate doctype and title tags in between them. Browsers 
> > > should generally handle this so the page should be usable.
> > > 
> > 
> > Thanks for the explanation. My concern was that this change made HTML 
> > unusable, if it is still valid (if not ideal) then I don't think there is 
> > an issue.
> > 
> > > Fixing this requires a refactor of the HTML generator such that the 
> > > concept of writing a struct is separate from writing a page. Even if I 
> > > was going to work on the HTML generator, I would probably want to do this 
> > > in a separate patch anyway.
> > > 
> > 
> > No arguments re splitting up work. Can you file an issue to track this on 
> > github and reference it in the TODO? That should help make sure this gets 
> > addressed.
> > 
> > > The markdown generator does the same, but markdown doesn't have all of 
> > > the headers stuff that shouldn't be in the middle of the file, you just 
> > > get a new `# ...` for the second section and 

[PATCH] D137872: Try to implement lambdas with inalloca parameters by forwarding without use of inallocas.

2022-11-18 Thread Amy Huang via Phabricator via cfe-commits
akhuang updated this revision to Diff 476633.
akhuang added a comment.

Clean up existing code and add code to make the call operator also call the new 
function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137872

Files:
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/CodeGen/CGFunctionInfo.h
  clang/lib/AST/DeclCXX.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGCall.h
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenABITypes.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenTypes.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGenCXX/inalloca-lambda.cpp

Index: clang/test/CodeGenCXX/inalloca-lambda.cpp
===
--- clang/test/CodeGenCXX/inalloca-lambda.cpp
+++ clang/test/CodeGenCXX/inalloca-lambda.cpp
@@ -1,7 +1,4 @@
-// RUN: not %clang_cc1 -triple i686-windows-msvc -emit-llvm -o /dev/null %s  2>&1 | FileCheck %s
-
-// PR28299
-// CHECK: error: cannot compile this forwarded non-trivially copyable parameter yet
+// RUN: %clang_cc1 -triple i686-windows-msvc -emit-llvm -o - %s  2>&1 | FileCheck %s
 
 class A {
   A(const A &);
@@ -9,3 +6,23 @@
 typedef void (*fptr_t)(A);
 fptr_t fn1() { return [](A) {}; }
 
+
+// CHECK: define internal void @"?__invoke@@?0??fn1@@YAP6AXVA@@@ZXZ@CA?A?@@0@Z"
+// CHECK-SAME:  (ptr inalloca(<{ %class.A, [3 x i8] }>) %0)
+// CHECK: %unused.capture = alloca %class.anon, align 1
+// CHECK: %1 = getelementptr inbounds <{ %class.A, [3 x i8] }>, ptr %0, i32 0, i32 0
+// CHECK: call x86_thiscallcc void @"??R@?0??fn1@@YAP6AXVA@@@ZXZ@QBE?A?@@0...@z.impl"
+// CHECK-SAME: (ptr noundef %unused.capture, ptr noundef %1)
+// CHECK: ret void
+
+// CHECK: define internal x86_thiscallcc void @"??R@?0??fn1@@YAP6AXVA@@@ZXZ@QBE?A?@@0@Z"
+// CHECK-SAME:  (ptr noundef %this, ptr inalloca(<{ %class.A, [3 x i8] }>) %0)
+// CHECK: call x86_thiscallcc void @"??R@?0??fn1@@YAP6AXVA@@@ZXZ@QBE?A?@@0...@z.impl"
+// CHECK-SAME: (ptr noundef %unused.capture, ptr noundef %1)
+
+// CHECK: define internal x86_thiscallcc void @"??R@?0??fn1@@YAP6AXVA@@@ZXZ@QBE?A?@@0...@z.impl"
+// CHECK-SAME: (ptr noundef %this, ptr noundef %0)
+// CHECK: %this.addr = alloca ptr, align 4
+// CHECK: store ptr %this, ptr %this.addr, align 4
+// CHECK: %this1 = load ptr, ptr %this.addr, align 4
+// CHECK: ret void
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -1187,7 +1187,7 @@
 
   Class classify(QualType Ty) const;
   ABIArgInfo classifyReturnType(QualType RetTy, CCState ) const;
-  ABIArgInfo classifyArgumentType(QualType RetTy, CCState ) const;
+  ABIArgInfo classifyArgumentType(QualType RetTy, CCState , bool NoInAlloca) const;
 
   /// Updates the number of available free registers, returns
   /// true if any registers were allocated.
@@ -1824,7 +1824,8 @@
 }
 
 ABIArgInfo X86_32ABIInfo::classifyArgumentType(QualType Ty,
-   CCState ) const {
+   CCState ,
+   bool NoInAlloca) const {
   // FIXME: Set alignment on indirect arguments.
   bool IsFastCall = State.CC == llvm::CallingConv::X86_FastCall;
   bool IsRegCall = State.CC == llvm::CallingConv::X86_RegCall;
@@ -1840,6 +1841,8 @@
 if (RAA == CGCXXABI::RAA_Indirect) {
   return getIndirectResult(Ty, false, State);
 } else if (RAA == CGCXXABI::RAA_DirectInMemory) {
+  if (NoInAlloca)
+return getIndirectResult(Ty, false, State);
   // The field index doesn't matter, we'll fix it up later.
   return ABIArgInfo::getInAlloca(/*FieldIndex=*/0);
 }
@@ -2014,7 +2017,7 @@
 if (State.IsPreassigned.test(I))
   continue;
 
-Args[I].info = classifyArgumentType(Args[I].type, State);
+Args[I].info = classifyArgumentType(Args[I].type, State, FI.isAvoidingInAlloca());
 UsedInAlloca |= (Args[I].info.getKind() == ABIArgInfo::InAlloca);
   }
 
Index: clang/lib/CodeGen/CodeGenTypes.h
===
--- clang/lib/CodeGen/CodeGenTypes.h
+++ clang/lib/CodeGen/CodeGenTypes.h
@@ -260,8 +260,7 @@
   ///
   /// \param argTypes - must all actually be canonical as params
   const CGFunctionInfo (CanQualType returnType,
-bool instanceMethod,
-bool chainCall,
+FnInfoOptions opts,
 ArrayRef argTypes,
 FunctionType::ExtInfo info,
 ArrayRef paramInfos,
Index: clang/lib/CodeGen/CodeGenFunction.h

[PATCH] D138328: [OpenMP] Initial parsing and semantic analysis support for 'strict' modifier with 'num_tasks' clause on 'taskloop' construct

2022-11-18 Thread Mike Rice via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG56c166017055: [OpenMP] Initial parsing/sema for 
strict modifier with num_tasks clause (authored by 
mdfazlay, committed 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/D138328/new/

https://reviews.llvm.org/D138328

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/Basic/OpenMPKinds.def
  clang/include/clang/Basic/OpenMPKinds.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/OpenMPClause.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/taskloop_strict_modifier_ast_print.cpp
  clang/test/OpenMP/taskloop_strict_modifier_messages.cpp
  llvm/include/llvm/Frontend/OpenMP/OMP.td

Index: llvm/include/llvm/Frontend/OpenMP/OMP.td
===
--- llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -246,9 +246,18 @@
 def OMPC_NoGroup : Clause<"nogroup"> {
   let clangClass = "OMPNogroupClause";
 }
+
+def OMP_NUMTASKS_Strict : ClauseVal<"strict", 1, 1> {}
+def OMP_NUMTASKS_Unknown : ClauseVal<"unkonwn", 2, 0> { let isDefault = 1; }
+
 def OMPC_NumTasks : Clause<"num_tasks"> {
   let clangClass = "OMPNumTasksClause";
   let flangClass = "ScalarIntExpr";
+  let enumClauseValue = "NumTasksType";
+  let allowedClauseValues = [
+OMP_NUMTASKS_Strict,
+OMP_NUMTASKS_Unknown
+  ];
 }
 def OMPC_Hint : Clause<"hint"> {
   let clangClass = "OMPHintClause";
Index: clang/test/OpenMP/taskloop_strict_modifier_messages.cpp
===
--- clang/test/OpenMP/taskloop_strict_modifier_messages.cpp
+++ clang/test/OpenMP/taskloop_strict_modifier_messages.cpp
@@ -1,8 +1,8 @@
-// RUN: %clang_cc1 -verify=expected,omp51 -fopenmp -fopenmp-version=51 -ferror-limit 100 %s -Wuninitialized
-// RUN: %clang_cc1 -verify=expected,omp50 -fopenmp -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,omp51 -fopenmp -fopenmp-version=51 -ferror-limit 150 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,omp50 -fopenmp -fopenmp-version=50 -ferror-limit 150 %s -Wuninitialized
 
-// RUN: %clang_cc1 -verify=expected,omp51 -fopenmp -fopenmp-version=51 -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
-// RUN: %clang_cc1 -verify=expected,omp50 -fopenmp -fopenmp-version=50 -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,omp51 -fopenmp -fopenmp-version=51 -fopenmp-simd -ferror-limit 150 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,omp50 -fopenmp -fopenmp-version=50 -fopenmp-simd -ferror-limit 150 %s -Wuninitialized
 
 void foo() {
 }
@@ -23,7 +23,15 @@
   #pragma omp taskloop grainsize(aa: 10) // omp51-error {{expected 'strict' in OpenMP clause 'grainsize'}} omp50-error {{use of undeclared identifier 'aa'}} omp50-error {{expected ')'}} omp50-note {{to match this '('}}
   for (int i = 0; i < 10; ++i)
  foo();
-
+  #pragma omp taskloop num_tasks(strict 10) // omp51-error {{missing ':' after strict modifier}} omp50-error {{use of undeclared identifier 'strict'}} omp50-error {{expected ')'}} omp50-note {{to match this '('}}
+  for (int i = 0; i < 10; ++i)
+foo();
+  #pragma omp taskloop num_tasks(aa 10) // omp51-error {{use of undeclared identifier 'aa'}} omp51-error {{expected ')'}} omp51-note {{to match this '('}} omp50-error {{use of undeclared identifier 'aa'}} omp50-error {{expected ')'}} omp50-note {{to match this '('}}
+  for (int i = 0; i < 10; ++i)
+foo();
+  #pragma omp taskloop num_tasks(aa: 10) // omp51-error {{expected 'strict' in OpenMP clause 'num_tasks'}} omp50-error {{use of undeclared identifier 'aa'}} omp50-error {{expected ')'}} omp50-note {{to match this '('}}
+  for (int i = 0; i < 10; ++i)
+ foo();
   return 0;
 }
 
@@ -108,6 +116,83 @@
   #pragma omp parallel master taskloop simd grainsize(aa: 10) // omp51-error {{expected 'strict' in OpenMP clause 'grainsize'}} omp50-error {{use of undeclared identifier 'aa'}} omp50-error {{expected ')'}} omp50-note {{to match this '('}}
   for (int i = 0; i < 10; ++i)
  foo();
-
+  #pragma omp masked taskloop num_tasks(strict 10) // omp51-error {{missing ':' after strict modifier}} omp50-error {{use of undeclared identifier 'strict'}} omp50-error {{expected ')'}} omp50-note {{to match this '('}}
+  for (int i = 0; i < 10; ++i)
+foo();
+  #pragma omp masked taskloop num_tasks(aa 10) // omp51-error {{use of undeclared identifier 'aa'}} omp51-error {{expected ')'}} omp51-note {{to match this '('}} omp50-error {{use of undeclared identifier 'aa'}} omp50-error {{expected ')'}} omp50-note {{to match this '('}}
+  for 

[clang] 56c1660 - [OpenMP] Initial parsing/sema for 'strict' modifier with 'num_tasks' clause

2022-11-18 Thread Mike Rice via cfe-commits

Author: Fazlay Rabbi
Date: 2022-11-18T16:26:47-08:00
New Revision: 56c166017055595a9f26933e85bfd89e30c528d0

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

LOG: [OpenMP] Initial parsing/sema for 'strict' modifier with 'num_tasks' clause

This patch gives basic parsing and semantic analysis support for 'strict'
modifier with 'num_tasks' clause of 'taskloop' construct introduced in
OpenMP 5.1 (section 2.12.2)

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

Added: 


Modified: 
clang/include/clang/AST/OpenMPClause.h
clang/include/clang/Basic/OpenMPKinds.def
clang/include/clang/Basic/OpenMPKinds.h
clang/include/clang/Sema/Sema.h
clang/lib/AST/OpenMPClause.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/taskloop_strict_modifier_ast_print.cpp
clang/test/OpenMP/taskloop_strict_modifier_messages.cpp
llvm/include/llvm/Frontend/OpenMP/OMP.td

Removed: 




diff  --git a/clang/include/clang/AST/OpenMPClause.h 
b/clang/include/clang/AST/OpenMPClause.h
index 1c9f62355e4ee..21d9f740eddf1 100644
--- a/clang/include/clang/AST/OpenMPClause.h
+++ b/clang/include/clang/AST/OpenMPClause.h
@@ -6486,26 +6486,43 @@ class OMPNumTasksClause : public OMPClause, public 
OMPClauseWithPreInit {
   /// Location of '('.
   SourceLocation LParenLoc;
 
+  /// Modifiers for 'num_tasks' clause.
+  OpenMPNumTasksClauseModifier Modifier = OMPC_NUMTASKS_unknown;
+
+  /// Location of the modifier.
+  SourceLocation ModifierLoc;
+
   /// Safe iteration space distance.
   Stmt *NumTasks = nullptr;
 
   /// Set safelen.
   void setNumTasks(Expr *Size) { NumTasks = Size; }
 
+  /// Sets modifier.
+  void setModifier(OpenMPNumTasksClauseModifier M) { Modifier = M; }
+
+  /// Sets modifier location.
+  void setModifierLoc(SourceLocation Loc) { ModifierLoc = Loc; }
+
 public:
   /// Build 'num_tasks' clause.
   ///
+  /// \param Modifier Clause modifier.
   /// \param Size Expression associated with this clause.
   /// \param HelperSize Helper grainsize for the construct.
   /// \param CaptureRegion Innermost OpenMP region where expressions in this
   /// clause must be captured.
   /// \param StartLoc Starting location of the clause.
   /// \param EndLoc Ending location of the clause.
-  OMPNumTasksClause(Expr *Size, Stmt *HelperSize,
-OpenMPDirectiveKind CaptureRegion, SourceLocation StartLoc,
-SourceLocation LParenLoc, SourceLocation EndLoc)
+  /// \param ModifierLoc Modifier location.
+  /// \param LParenLoc Location of '('.
+  OMPNumTasksClause(OpenMPNumTasksClauseModifier Modifier, Expr *Size,
+Stmt *HelperSize, OpenMPDirectiveKind CaptureRegion,
+SourceLocation StartLoc, SourceLocation LParenLoc,
+SourceLocation ModifierLoc, SourceLocation EndLoc)
   : OMPClause(llvm::omp::OMPC_num_tasks, StartLoc, EndLoc),
-OMPClauseWithPreInit(this), LParenLoc(LParenLoc), NumTasks(Size) {
+OMPClauseWithPreInit(this), LParenLoc(LParenLoc), Modifier(Modifier),
+ModifierLoc(ModifierLoc), NumTasks(Size) {
 setPreInitStmt(HelperSize, CaptureRegion);
   }
 
@@ -6524,6 +6541,12 @@ class OMPNumTasksClause : public OMPClause, public 
OMPClauseWithPreInit {
   /// Return safe iteration space distance.
   Expr *getNumTasks() const { return cast_or_null(NumTasks); }
 
+  /// Gets modifier.
+  OpenMPNumTasksClauseModifier getModifier() const { return Modifier; }
+
+  /// Gets modifier location.
+  SourceLocation getModifierLoc() const { return ModifierLoc; }
+
   child_range children() { return child_range(,  + 1); }
 
   const_child_range children() const {

diff  --git a/clang/include/clang/Basic/OpenMPKinds.def 
b/clang/include/clang/Basic/OpenMPKinds.def
index 339139abe2a49..a084e9686f5ee 100644
--- a/clang/include/clang/Basic/OpenMPKinds.def
+++ b/clang/include/clang/Basic/OpenMPKinds.def
@@ -74,6 +74,9 @@
 #ifndef OPENMP_GRAINSIZE_MODIFIER
 #define OPENMP_GRAINSIZE_MODIFIER(Name)
 #endif
+#ifndef OPENMP_NUMTASKS_MODIFIER
+#define OPENMP_NUMTASKS_MODIFIER(Name)
+#endif
 
 // Static attributes for 'schedule' clause.
 OPENMP_SCHEDULE_KIND(static)
@@ -187,6 +190,10 @@ OPENMP_BIND_KIND(thread)
 // Modifiers for the 'grainsize' clause.
 OPENMP_GRAINSIZE_MODIFIER(strict)
 
+// Modifiers for the 'num_tasks' clause.
+OPENMP_NUMTASKS_MODIFIER(strict)
+
+#undef OPENMP_NUMTASKS_MODIFIER
 #undef OPENMP_GRAINSIZE_MODIFIER
 #undef OPENMP_BIND_KIND
 #undef OPENMP_ADJUST_ARGS_KIND

diff  --git a/clang/include/clang/Basic/OpenMPKinds.h 
b/clang/include/clang/Basic/OpenMPKinds.h
index 

[PATCH] D137826: [clang] Allow comparing pointers to string literals

2022-11-18 Thread Tom Honermann via Phabricator via cfe-commits
tahonermann added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:12951-12954
+  // ObjC's @encode()
+  if (isa(E->getLHS()->IgnoreParenImpCasts()) ||
+  isa(E->getRHS()->IgnoreParenImpCasts()))
 return Error(E);

tbaeder wrote:
> tahonermann wrote:
> > A comment to explain this change would be helpful. It isn't intuitive (for 
> > me anyway) why Objective-C's `@encode` would require special handling here. 
> > Was this needed to avoid a test failure?
> In `../clang/test/CodeGenObjC/encode-test-4.m`:
> 
> ```
> int a(void) {
>   return @encode(int) == @encode(int) &&
> @encode(Color) == @encode(long) &&
> @encode(PrintColor) == @encode(int);
> }
> ```
> 
> The comparisons need to be rejected here and are folded to a `1` later on, it 
> seems. Letting the comparison happen will lead to a `0`.
Apologies, I meant it would be helpful to add a comment to the code :)

I think `@encode(...)` expressions should be treated the same as string 
literals; the former is lowered to the latter; both contribute to the string 
table.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137826

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


[PATCH] D138337: Add support for kcfi-seal optimization with LTO

2022-11-18 Thread Sami Tolvanen via Phabricator via cfe-commits
samitolvanen added reviewers: nickdesaulniers, pcc, MaskRay, kees, joaomoreira.
samitolvanen added a comment.
Herald added a subscriber: StephenFan.

ClangBuiltLinux issue: https://github.com/ClangBuiltLinux/linux/issues/1737


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138337

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


[PATCH] D138337: Add support for kcfi-seal optimization with LTO

2022-11-18 Thread Sami Tolvanen via Phabricator via cfe-commits
samitolvanen created this revision.
Herald added subscribers: pengfei, hiraditya, inglorion.
Herald added a project: All.
samitolvanen requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, MaskRay.
Herald added projects: clang, LLVM.

With -fsanitize=kcfi, Clang emits a !kcfi_type attribute for all global
functions, making them valid indirect call targets, whether the program
ends up calling them indirectly or not. With LTO, we can "seal" the
program by dropping types from non-address-taken globals, thus limiting
the possible targets that can be called.

Add -fsanitize-kcfi-seal command line flag to Clang that's only allowed
with -flto, and drop types from non-address-taken globals in AsmPrinter
when the kcfi-seal module attribute is set.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138337

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/SanitizerArgs.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  clang/test/CodeGen/kcfi.c
  clang/test/Driver/fsanitize.c
  llvm/include/llvm/CodeGen/AsmPrinter.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/test/CodeGen/X86/kcfi-seal.ll

Index: llvm/test/CodeGen/X86/kcfi-seal.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/kcfi-seal.ll
@@ -0,0 +1,30 @@
+; RUN: llc -mtriple=x86_64-unknown-linux-gnu -verify-machineinstrs < %s | FileCheck %s
+
+@llvm.used = appending global [1 x ptr] [ptr @f3], section "llvm.metadata"
+
+; CHECK-LABEL:  __cfi_f1:
+; CHECK-LABEL:  f1:
+define void @f1(ptr noundef %x) !kcfi_type !2 {
+  ret void
+}
+
+; CHECK-NOT:__cfi_f2:
+; CHECK-LABEL:  f2:
+define void @f2(ptr noundef %x) !kcfi_type !2 {
+  ret void
+}
+
+; CHECK-LABEL:  __cfi_f3:
+; CHECK-LABEL:  f3:
+define void @f3(ptr noundef %x) !kcfi_type !2 {
+  ret void
+}
+
+define dso_local ptr @g1() {
+ret ptr @f1
+}
+
+!llvm.module.flags = !{!0, !1}
+!0 = !{i32 4, !"kcfi", i32 1}
+!1 = !{i32 4, !"kcfi-seal", i32 1}
+!2 = !{i32 12345678}
Index: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -943,7 +943,8 @@
   }
 
   // Emit KCFI type information before patchable-function-prefix nops.
-  emitKCFITypeId(*MF);
+  if (needsKCFITypeId(*MF))
+emitKCFITypeId(*MF);
 
   // Emit M NOPs for -fpatchable-function-entry=N,M where M>0. We arbitrarily
   // place prefix data before NOPs.
@@ -1378,6 +1379,14 @@
   OutStreamer->popSection();
 }
 
+bool AsmPrinter::needsKCFITypeId(const MachineFunction ) {
+  const Function  = MF.getFunction();
+  // With kcfi-seal, drop the type identifier for non-address-taken functions
+  // even if they have a !kcfi_type attribute to reduce the number of
+  // indirectly callable functions.
+  return !F.getParent()->getModuleFlag("kcfi-seal") || F.hasAddressTaken();
+}
+
 void AsmPrinter::emitKCFITypeId(const MachineFunction ) {
   const Function  = MF.getFunction();
   if (const MDNode *MD = F.getMetadata(LLVMContext::MD_kcfi_type))
Index: llvm/include/llvm/CodeGen/AsmPrinter.h
===
--- llvm/include/llvm/CodeGen/AsmPrinter.h
+++ llvm/include/llvm/CodeGen/AsmPrinter.h
@@ -412,6 +412,7 @@
   void emitBBAddrMapSection(const MachineFunction );
 
   void emitKCFITrapEntry(const MachineFunction , const MCSymbol *Symbol);
+  bool needsKCFITypeId(const MachineFunction );
   virtual void emitKCFITypeId(const MachineFunction );
 
   void emitPseudoProbe(const MachineInstr );
Index: clang/test/Driver/fsanitize.c
===
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -653,6 +653,12 @@
 // RUN: %clang --target=x86_64-linux-gnu -fsanitize=kcfi %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-KCFI
 // CHECK-KCFI: "-fsanitize=kcfi"
 
+// RUN: %clang --target=x86_64-linux-gnu -fsanitize=kcfi -fsanitize-kcfi-seal %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-KCFI-SEAL-NOLTO
+// CHECK-KCFI-SEAL-NOLTO: error: invalid argument '-fsanitize-kcfi-seal' only allowed with '-flto'
+
+// RUN: %clang --target=x86_64-linux-gnu -fsanitize=kcfi -fsanitize-kcfi-seal -flto %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-KCFI-SEAL-LTO
+// CHECK-KCFI-SEAL-LTO: "-fsanitize-kcfi-seal"
+
 // RUN: %clang --target=x86_64-linux-gnu -fsanitize=kcfi -fno-sanitize-recover=kcfi %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-KCFI-RECOVER
 // CHECK-KCFI-RECOVER: error: unsupported argument 'kcfi' to option '-fno-sanitize-recover='
 
Index: clang/test/CodeGen/kcfi.c
===
--- clang/test/CodeGen/kcfi.c
+++ clang/test/CodeGen/kcfi.c
@@ -1,5 +1,6 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm 

[PATCH] D129531: [clang][C++20] P0960R3 and P1975R0: Allow initializing aggregates from a parenthesized list of values

2022-11-18 Thread Alan Zhao via Phabricator via cfe-commits
ayzhao added inline comments.



Comment at: clang/lib/AST/JSONNodeDumper.cpp:852
 case VarDecl::ListInit: JOS.attribute("init", "list"); break;
+case VarDecl::ParenListInit:
+  JOS.attribute("init", "paren-list");

ilya-biryukov wrote:
> NIT: maybe use the same formatting as other switch cases for constistency?
Unfortunately clang-format insists that these be on separate lines.



Comment at: clang/lib/CodeGen/CGExprAgg.cpp:581
+  Expr *filler = nullptr;
+  if (auto *ILE = dyn_cast(ExprToVisit))
+filler = ILE->getArrayFiller();

ilya-biryukov wrote:
> - Should we have a filler for `CXXParenInitListExpr` too?
>   It seems like an important optimization and could have large effect on 
> compile times if we don't 
> 
> - Same question for semantic and syntactic-form (similar to `InitListExpr`): 
> should we have it here?
>   I am not sure if it's semantically required (probably not?), but that's 
> definitely something that `clang-tidy` and other source tools will rely on.
> 
> We should probably share the code there, I suggest moving it to a shared base 
> class and using it where appropriate instead of the derived classes.
> Should we have a filler for CXXParenInitListExpr too? It seems like an 
> important optimization and could have large effect on compile times if we 
> don't

This feels like premature optimization - presumably, wouldn't this only be an 
issue with extraordinarily large (say, O(1000)) arrays?

> Same question for semantic and syntactic-form (similar to InitListExpr): 
> should we have it here? I am not sure if it's semantically required (probably 
> not?), but that's definitely something that clang-tidy and other source tools 
> will rely on

IIRC this doesn't apply to paren list aggregate expressions, as the syntatic 
form would be the enclosing `ParenListExpr`.



Comment at: clang/lib/Sema/SemaInit.cpp:6169
+  Failure == FK_ConstructorOverloadFailed &&
+  getFailedCandidateSet().size() == 3) {
+// C++20 [dcl.init] 17.6.2.2:

ilya-biryukov wrote:
> It seems shaky to rely on the size of `getFailedCandidateSet`.
> What are we trying to achieve here? Is there a more direct way to specify it?
Done.

As mentioned in the above comment, we try to parse this as a `CXXParenListInit` 
if the only failed overload constructor candidates are the default constructor, 
the default copy constructor, and the default move constructor).



Comment at: clang/lib/Sema/SemaInit.cpp:6188
+//  (6.7.7), and there is no brace elision.
+TryOrBuildParenListInitialization(S, Entity, Kind, Args, *this,
+  /*VerifyOnly=*/true);

ilya-biryukov wrote:
> NIT: maybe move the full comment into the body of the function?
> It describes in detail what the body of the function does and it would be 
> easier to understand whether the code fits the intention in the standard.
> Having the first part of the comment here would still be useful, probably.
Done. Most of this comment is spread out throughout the body of the function 
anyways.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129531

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


[PATCH] D129531: [clang][C++20] P0960R3 and P1975R0: Allow initializing aggregates from a parenthesized list of values

2022-11-18 Thread Alan Zhao via Phabricator via cfe-commits
ayzhao updated this revision to Diff 476625.
ayzhao marked 9 inline comments as done.
ayzhao added a comment.

address code review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129531

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang-c/Index.h
  clang/include/clang/AST/ComputeDependence.h
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/StmtNodes.td
  clang/include/clang/Sema/Initialization.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/ComputeDependence.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/AST/ExprClassification.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExceptionSpec.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/test/CXX/class/class.compare/class.spaceship/p1.cpp
  clang/test/CXX/drs/dr2xx.cpp
  clang/test/CXX/temp/temp.decls/temp.variadic/p4.cpp
  clang/test/CodeGen/paren-list-agg-init.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/PCH/cxx_paren_init.cpp
  clang/test/PCH/cxx_paren_init.h
  clang/test/SemaCXX/cxx2a-explicit-bool.cpp
  clang/test/SemaCXX/paren-list-agg-init.cpp
  clang/tools/libclang/CIndex.cpp
  clang/tools/libclang/CXCursor.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1156,7 +1156,7 @@
 
   Parenthesized initialization of aggregates
   https://wg21.link/p0960r3;>P0960R3
-  No
+  Clang 16
 

 https://wg21.link/p1975r0;>P1975R0
Index: clang/tools/libclang/CXCursor.cpp
===
--- clang/tools/libclang/CXCursor.cpp
+++ clang/tools/libclang/CXCursor.cpp
@@ -643,6 +643,10 @@
 K = CXCursor_RequiresExpr;
 break;
 
+  case Stmt::CXXParenListInitExprClass:
+K = CXCursor_CXXParenListInitExpr;
+break;
+
   case Stmt::MSDependentExistsStmtClass:
 K = CXCursor_UnexposedStmt;
 break;
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -2139,6 +2139,7 @@
   void VisitLambdaExpr(const LambdaExpr *E);
   void VisitConceptSpecializationExpr(const ConceptSpecializationExpr *E);
   void VisitRequiresExpr(const RequiresExpr *E);
+  void VisitCXXParenListInitExpr(const CXXParenListInitExpr *E);
   void VisitOMPExecutableDirective(const OMPExecutableDirective *D);
   void VisitOMPLoopBasedDirective(const OMPLoopBasedDirective *D);
   void VisitOMPLoopDirective(const OMPLoopDirective *D);
@@ -3000,6 +3001,9 @@
   for (ParmVarDecl *VD : E->getLocalParameters())
 AddDecl(VD);
 }
+void EnqueueVisitor::VisitCXXParenListInitExpr(const CXXParenListInitExpr *E) {
+  EnqueueChildren(E);
+}
 void EnqueueVisitor::VisitPseudoObjectExpr(const PseudoObjectExpr *E) {
   // Treat the expression like its syntactic form.
   Visit(E->getSyntacticForm());
@@ -5595,6 +5599,8 @@
 return cxstring::createRef("ConceptSpecializationExpr");
   case CXCursor_RequiresExpr:
 return cxstring::createRef("RequiresExpr");
+  case CXCursor_CXXParenListInitExpr:
+return cxstring::createRef("CXXParenListInitExpr");
   case CXCursor_UnexposedStmt:
 return cxstring::createRef("UnexposedStmt");
   case CXCursor_DeclStmt:
Index: clang/test/SemaCXX/paren-list-agg-init.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/paren-list-agg-init.cpp
@@ -0,0 +1,172 @@
+// RUN: %clang_cc1 -verify -std=c++20 %s -fsyntax-only
+// RUN: %clang_cc1 -verify=expected,beforecxx20 -Wc++20-extensions -std=c++20 %s -fsyntax-only
+
+struct A { // expected-note 4{{candidate constructor}}
+  char i;
+  double j;
+};
+
+struct B {
+  A a;
+  int b[20];
+  int & // expected-note {{reference member declared here}}
+};
+
+struct C { // expected-note 2{{candidate constructor}}
+  A a;
+  int b[20];
+};
+
+struct D : public C, public A {
+  int a;
+};
+
+struct E { // expected-note 3{{candidate constructor}}
+  struct F {
+F(int, int);
+  };
+  int a;
+  F f;
+};
+
+int getint(); // expected-note {{declared here}}
+
+struct F {
+  int a;
+  int b = getint(); // expected-note {{non-constexpr function 'getint' 

[PATCH] D138326: [CodeView] Don't generate dummy unnamed strcut/class/union type.

2022-11-18 Thread Zequan Wu via Phabricator via cfe-commits
zequanwu added a comment.

In D138326#3938128 , @rnk wrote:

> Since Clang creates the member list and only adds types for the benefit of 
> codeview, should we change clang instead?

Yes, that's something clang does only for codeview, changed in clang side. It 
will still emit type info for unnamed records but not for anonymous records.




Comment at: llvm/test/DebugInfo/COFF/nested-types.ll:214
+; CHECK-NEXT:   }
 ; CHECK-NEXT:   NestedType {
 ; CHECK-NEXT: TypeLeafKind: LF_NESTTYPE (0x1510)

rnk wrote:
> So the test shows that we don't have NestedType records for unnamed unions & 
> structs, right?
Updated. We want NestedType records for unnamed unions & structs, but not for 
anonymous unions & structs. That's what MSVC does.
```
 0x1004 | LF_FIELDLIST [size = 168]
- LF_MEMBER [name = `i1`, Type = 0x0074 (int), offset = 0, attrs = 
public]
- LF_NESTTYPE [name = ``, parent = 
0x1002]
- LF_MEMBER [name = `unnamed_union`, Type = 0x1002, offset = 4, 
attrs = public]
- LF_MEMBER [name = `i3`, Type = 0x0074 (int), offset = 8, attrs = 
public]
- LF_NESTTYPE [name = ``, parent = 
0x1003]
- LF_MEMBER [name = `unnamed_struct`, Type = 0x1003, offset = 12, 
attrs = public]
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138326

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


[PATCH] D137753: [Clang][GNU][AIX][p]Enable -p Functionality

2022-11-18 Thread Michael Francis via Phabricator via cfe-commits
francii added a comment.

In D137753#3935391 , @MaskRay wrote:

> In D137753#3935305 , @francii wrote:
>
>> Recall that the goal with `-p` is to create parity with GCC (at least with 
>> Linux and AIX), as per the RFC discussion.
>>
>> In D137753#3935138 , @MaskRay 
>> wrote:
>>
>>> In D137753#3935126 , @francii 
>>> wrote:
>>>
 In D137753#3934932 , @MaskRay 
 wrote:

> Please make `-p` accepted for AIX only and don't change the semantics for 
> other targets in this patch. For FreeBSD and Linux (musl and gnu) we can 
> try rejecting `-p`. If OpenBSD wants to make `-p` an alias for `-pg`, 
> that's fine.

 We can make `-p` emit a message on Linux while also accepting it as an 
 alias to `-pg`. Do you have a suggestion as to what that message would be?
>>>
>>> The current `warning: argument unused during compilation: '-p' 
>>> [-Wunused-command-line-argument]` is good for Linux.
>>> In the future Linux can try removing `-p`.
>>
>> The current behaviour of ignoring the option without stopping with an error 
>> return code is not a good one.
>>
>> Recall that the goal is to create parity with GCC, as per the RFC post.
>>
>> Is there a reason this flag shouldn't be supported on Linux? Specifically, 
>> what is your justification for diverging from GCC on this matter?
>
> It's a legacy option (at least for Linux, FreeBSD, etc) and we don't want the 
> usage to grow. I objected in the RFC, either. Note that the objection is not 
> only from me, also from a Linux distro folk I checked with.

If we aren't adding Linux functionality, we should make it throw an error at 
the same time.

Once again, ignoring the option without stopping with an error code is not 
ideal.

I can update this patch to throw an error, much like this patch for z/OS: 
https://reviews.llvm.org/D137756

In D137753#3935674 , @MaskRay wrote:

> In D137753#3935617 , @francii wrote:
>
>> In D137753#3935391 , @MaskRay 
>> wrote:
>>
>>> In D137753#3935305 , @francii 
>>> wrote:
>>>
 Recall that the goal with `-p` is to create parity with GCC (at least with 
 Linux and AIX), as per the RFC discussion.

 In D137753#3935138 , @MaskRay 
 wrote:

> In D137753#3935126 , @francii 
> wrote:
>
>> In D137753#3934932 , @MaskRay 
>> wrote:
>>
>>> Please make `-p` accepted for AIX only and don't change the semantics 
>>> for other targets in this patch. For FreeBSD and Linux (musl and gnu) 
>>> we can try rejecting `-p`. If OpenBSD wants to make `-p` an alias for 
>>> `-pg`, that's fine.
>>
>> We can make `-p` emit a message on Linux while also accepting it as an 
>> alias to `-pg`. Do you have a suggestion as to what that message would 
>> be?
>
> The current `warning: argument unused during compilation: '-p' 
> [-Wunused-command-line-argument]` is good for Linux.
> In the future Linux can try removing `-p`.

 The current behaviour of ignoring the option without stopping with an 
 error return code is not a good one.

 Recall that the goal is to create parity with GCC, as per the RFC post.

 Is there a reason this flag shouldn't be supported on Linux? Specifically, 
 what is your justification for diverging from GCC on this matter?
>>>
>>> It's a legacy option (at least for Linux, FreeBSD, etc) and we don't want 
>>> the usage to grow. I objected in the RFC, either. Note that the objection 
>>> is not only from me, also from a Linux distro folk I checked with.
>>
>> If we aren't adding Linux functionality, we should make it throw an error at 
>> the same time.
>>
>> Once again, ignoring the option without stopping with an error code is not 
>> ideal. I can update this patch to throw an error on Linux, much like this 
>> patch for z/OS: https://reviews.llvm.org/D137756
>
> I acknowledge that the current state is bad. Reject it for FreeBSD/Linux 
> (perhaps most OSes. An OS can opt in if their platform really needs this) is 
> likely fine. I don't think `-p` has many uses.
>
> Something like D137756  will be nice, but I 
> think it can be done in clang/lib/Driver/ToolChains/Clang.cpp: D138255 
> 

Thank you, I will remove the Linux/Gnu changes from this patch shortly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137753


[PATCH] D138326: [CodeView] Don't generate dummy unnamed strcut/class/union type.

2022-11-18 Thread Zequan Wu via Phabricator via cfe-commits
zequanwu updated this revision to Diff 476621.
zequanwu added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Reverted previous change and change clang side so that it doesn't emit nested 
anonymous record type when emitting code-view.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138326

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/debug-info-codeview-nested-types.cpp


Index: clang/test/CodeGenCXX/debug-info-codeview-nested-types.cpp
===
--- clang/test/CodeGenCXX/debug-info-codeview-nested-types.cpp
+++ clang/test/CodeGenCXX/debug-info-codeview-nested-types.cpp
@@ -5,6 +5,11 @@
   typedef int InnerTypedef;
   enum { InnerEnumerator = 2 };
   struct InnerStruct { };
+
+  union { int i1; };
+  union { int i2; } unnamed_union;
+  struct { int i3; };
+  struct { int i4; } unnamed_struct;
 };
 HasNested f;
 
@@ -12,7 +17,7 @@
 // CHECK: ![[HASNESTED:[0-9]+]] = distinct !DICompositeType(tag: 
DW_TAG_structure_type, name: "HasNested",
 // CHECK-SAME: elements: ![[MEMBERS:[0-9]+]],
 //
-// CHECK: ![[MEMBERS]] = !{![[INNERENUM]], ![[INNERTYPEDEF:[0-9]+]], 
![[UNNAMEDENUM:[0-9]+]], ![[INNERSTRUCT:[0-9]+]]}
+// CHECK: ![[MEMBERS]] = !{![[INNERENUM]], ![[INNERTYPEDEF:[0-9]+]], 
![[UNNAMEDENUM:[0-9]+]], ![[INNERSTRUCT:[0-9]+]], ![[ANONYMOUS_UNION:[0-9]+]], 
![[UNNAMED_UNION_TYPE:[0-9]+]], ![[UNNAMED_UNION:[0-9]+]], 
![[ANONYMOUS_STRUCT:[0-9]+]], ![[UNNAMED_STRUCT_TYPE:[0-9]+]], 
![[UNNAMED_STRUCT:[0-9]+]]}
 //
 // CHECK: ![[INNERTYPEDEF]] = !DIDerivedType(tag: DW_TAG_typedef, name: 
"InnerTypedef", scope: ![[HASNESTED]]{{.*}})
 //
@@ -23,3 +28,31 @@
 //
 // CHECK: ![[INNERSTRUCT]] = !DICompositeType(tag: DW_TAG_structure_type, 
name: "InnerStruct"
 // CHECK-SAME: flags: DIFlagFwdDecl
+
+// CHECK: ![[ANONYMOUS_UNION]] = !DIDerivedType(tag: DW_TAG_member, 
+// CHECK-SAME: baseType: ![[ANONYMOUS_UNION_TYPE:[0-9]+]]
+// CHECK: ![[ANONYMOUS_UNION_TYPE]] = distinct !DICompositeType(tag: 
DW_TAG_union_type,
+// CHECK-SAME: elements: ![[ANONYMOUS_UNION_MEMBERS:[0-9]+]], identifier: 
".?AT@HasNested@@")
+// CHECK: ![[ANONYMOUS_UNION_MEMBERS]] = !{![[ANONYMOUS_UNION_MEMBER:[0-9]+]]}
+// CHECK: ![[ANONYMOUS_UNION_MEMBER]] = !DIDerivedType(tag: DW_TAG_member, 
name: "i1"
+
+// CHECK: ![[UNNAMED_UNION_TYPE]] = distinct !DICompositeType(tag: 
DW_TAG_union_type, name: ""
+// CHECK-SAME: elements: ![[UNNAMED_UNION_MEMBERS:[0-9]+]], identifier: 
".?AT@HasNested@@")
+// CHECK: ![[UNNAMED_UNION_MEMBERS]] = !{![[UNNAMED_UNION_MEMBER:[0-9]+]]}
+// CHECK: ![[UNNAMED_UNION_MEMBER]] = !DIDerivedType(tag: DW_TAG_member, name: 
"i2"
+// CHECK: ![[UNNAMED_UNION]] = !DIDerivedType(tag: DW_TAG_member, name: 
"unnamed_union"
+// CHECK-SAME: baseType: ![[UNNAMED_UNION_TYPE]]
+
+// CHECK: ![[ANONYMOUS_STRUCT]] = !DIDerivedType(tag: DW_TAG_member
+// CHECK-SAME: baseType: ![[ANONYMOUS_STRUCT_TYPE:[0-9]+]]
+// CHECK: ![[ANONYMOUS_STRUCT_TYPE]] = distinct !DICompositeType(tag: 
DW_TAG_structure_type
+// CHECK-SAME: elements: ![[ANONYMOUS_STRUCT_MEMBERS:[0-9]+]], identifier: 
".?AU@HasNested@@")
+// CHECK: ![[ANONYMOUS_STRUCT_MEMBERS]] = 
!{![[ANONYMOUS_STRUCT_MEMBER:[0-9]+]]}
+// CHECK: ![[ANONYMOUS_STRUCT_MEMBER]] = !DIDerivedType(tag: DW_TAG_member, 
name: "i3"
+
+// CHECK: ![[UNNAMED_STRUCT_TYPE]] = distinct !DICompositeType(tag: 
DW_TAG_structure_type, name: ""
+// CHECK-SAME: elements: ![[UNNAMED_STRUCT_MEMBERS:[0-9]+]], identifier: 
".?AU@HasNested@@")
+// CHECK: ![[UNNAMED_STRUCT_MEMBERS]] = !{![[UNNAMED_STRUCT_MEMBER:[0-9]+]]}
+// CHECK: ![[UNNAMED_STRUCT_MEMBER]] = !DIDerivedType(tag: DW_TAG_member, 
name: "i4"
+// CHECK: ![[UNNAMED_STRUCT]] = !DIDerivedType(tag: DW_TAG_member, name: 
"unnamed_struct"
+// CHECK-SAME: baseType: ![[UNNAMED_STRUCT_TYPE]]
\ No newline at end of file
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1652,10 +1652,15 @@
   } else if (CGM.getCodeGenOpts().EmitCodeView) {
 // Debug info for nested types is included in the member list only for
 // CodeView.
-if (const auto *nestedType = dyn_cast(I))
+if (const auto *nestedType = dyn_cast(I)) {
+  // MSVC doesn't generate nested type for anonymous struct/union.
+  if (isa(I) &&
+  cast(I)->isAnonymousStructOrUnion())
+continue;
   if (!nestedType->isImplicit() &&
   nestedType->getDeclContext() == record)
 CollectRecordNestedType(nestedType, elements);
+}
   }
   }
 }


Index: clang/test/CodeGenCXX/debug-info-codeview-nested-types.cpp
===
--- clang/test/CodeGenCXX/debug-info-codeview-nested-types.cpp
+++ 

[PATCH] D137488: [clang][Interp] Array initialization via string literal

2022-11-18 Thread Tom Honermann via Phabricator via cfe-commits
tahonermann added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1098-1099
+
+unsigned N = SL->getLength();
+for (size_t I = 0; I != NumElems; ++I) {
+  uint32_t CodePoint = I < N ? SL->getCodeUnit(I) : 0;

tahonermann wrote:
> Aren't `N` and `NumElems` guaranteed to have the same value here? Both are 
> derived from `SL`. The code seems to be written with the expectation that 
> `NumElems` corresponds to the number of elements to be iniitialized in the 
> target array.
I see the change to now use the minimum of `SL->getLength()` and 
`CAT->getSize().getZExtValue()`. Based on https://godbolt.org/z/5sTWExTac this 
looks to be unnecessary. When a string literal is used as an array initializer, 
it appears that the type of the string literal is adjusted to match the size of 
the array being initialized. I suggest using only 
`CAT->getSize().getZExtValue()` and adding a comment that this code depends on 
that adjustment.



Comment at: clang/test/AST/Interp/literals.cpp:354-359
+  constexpr char foo[12] = "abc";
+  static_assert(foo[0] == 'a', "");
+  static_assert(foo[1] == 'b', "");
+  static_assert(foo[2] == 'c', "");
+  static_assert(foo[3] == 0, "");
+  static_assert(foo[11] == 0, "");

tahonermann wrote:
> aaron.ballman wrote:
> > I'd like to see some tests for the other encodings, as well as a test with 
> > embedded null characters in the literal.
> > 
> > Testing a string literal that's longer than the array is something we 
> > should think about. That code is ill-formed in C++, so I don't think we can 
> > add a test for it yet, but it's only a warning in C.
> I agree with Aaron's requests. Please also extend the test to include a 
> `char` element that would be negative for a `signed` 8-bit `char`. Something 
> like:
>   constexpr char foo[12] = "abc\xff";
>   ...
>   #if defined(__CHAR_UNSIGNED__) || __CHAR_BIT__ > 8
>   static_assert(foo[3] == 255, "");
>   #else
>   static_assert(foo[3] == -1, "");
>   #endif
> 
> A couple of more tests to add:
> - One where the string literal has the same length (including the implicit 
> terminator) as the array; to ensure that the implicit terminator is properly 
> accounted for.
> - One where the target array size is deduced from the string literal; to 
> ensure there are no dependencies on an explicit array size.
These cases all look to have been added now. Thank you!


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

https://reviews.llvm.org/D137488

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


[PATCH] D135411: Add generic KCFI operand bundle lowering

2022-11-18 Thread Sami Tolvanen via Phabricator via cfe-commits
samitolvanen updated this revision to Diff 476615.
samitolvanen added a comment.

Fixed clang-format warnings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135411

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChain.cpp
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/Transforms/Instrumentation/KCFI.h
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Passes/PassRegistry.def
  llvm/lib/Transforms/Instrumentation/CMakeLists.txt
  llvm/lib/Transforms/Instrumentation/KCFI.cpp
  llvm/test/Transforms/KCFI/kcfi-patchable-function-prefix.ll
  llvm/test/Transforms/KCFI/kcfi.ll
  llvm/utils/gn/secondary/llvm/lib/Transforms/Instrumentation/BUILD.gn

Index: llvm/utils/gn/secondary/llvm/lib/Transforms/Instrumentation/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/lib/Transforms/Instrumentation/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/lib/Transforms/Instrumentation/BUILD.gn
@@ -20,6 +20,7 @@
 "InstrOrderFile.cpp",
 "InstrProfiling.cpp",
 "Instrumentation.cpp",
+"KCFI.cpp",
 "MemProfiler.cpp",
 "MemorySanitizer.cpp",
 "PGOInstrumentation.cpp",
Index: llvm/test/Transforms/KCFI/kcfi.ll
===
--- /dev/null
+++ llvm/test/Transforms/KCFI/kcfi.ll
@@ -0,0 +1,21 @@
+; RUN: opt -S -passes=kcfi %s | FileCheck %s
+
+; CHECK-LABEL: define void @f1(
+define void @f1(ptr noundef %x) {
+  ; CHECK:  %[[#GEPI:]] = getelementptr inbounds i32, ptr %x, i32 -1
+  ; CHECK-NEXT: %[[#LOAD:]] = load i32, ptr %[[#GEPI]], align 4
+  ; CHECK-NEXT: %[[#ICMP:]] = icmp ne i32 %[[#LOAD]], 12345678
+  ; CHECK-NEXT: br i1 %[[#ICMP]], label %[[#TRAP:]], label %[[#CALL:]], !prof ![[#WEIGHTS:]]
+  ; CHECK:  [[#TRAP]]:
+  ; CHECK-NEXT: call void @llvm.trap()
+  ; CHECK-NEXT: br label %[[#CALL]]
+  ; CHECK:  [[#CALL]]:
+  ; CHECK-NEXT: call void %x()
+  ; CHECK-NOT:  [ "kcfi"(i32 12345678) ]
+  call void %x() [ "kcfi"(i32 12345678) ]
+  ret void
+}
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 4, !"kcfi", i32 1}
+; CHECK: ![[#WEIGHTS]] = !{!"branch_weights", i32 1, i32 1048575}
Index: llvm/test/Transforms/KCFI/kcfi-patchable-function-prefix.ll
===
--- /dev/null
+++ llvm/test/Transforms/KCFI/kcfi-patchable-function-prefix.ll
@@ -0,0 +1,12 @@
+; RUN: not opt -S -passes=kcfi %s 2>&1 | FileCheck %s
+
+; CHECK: error: -fpatchable-function-entry=N,M, where M>0 is not compatible with -fsanitize=kcfi on this target
+define void @f1(ptr noundef %x) #0 {
+  call void %x() [ "kcfi"(i32 12345678) ]
+  ret void
+}
+
+attributes #0 = { "patchable-function-prefix"="1" }
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 4, !"kcfi", i32 1}
Index: llvm/lib/Transforms/Instrumentation/KCFI.cpp
===
--- /dev/null
+++ llvm/lib/Transforms/Instrumentation/KCFI.cpp
@@ -0,0 +1,111 @@
+//===-- KCFI.cpp - Generic KCFI operand bundle lowering -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This pass emits generic KCFI indirect call checks for targets that don't
+// support lowering KCFI operand bundles in the back-end.
+//
+//===--===//
+
+#include "llvm/Transforms/Instrumentation/KCFI.h"
+#include "llvm/ADT/Statistic.h"
+#include "llvm/IR/Constants.h"
+#include "llvm/IR/DiagnosticInfo.h"
+#include "llvm/IR/DiagnosticPrinter.h"
+#include "llvm/IR/Function.h"
+#include "llvm/IR/GlobalObject.h"
+#include "llvm/IR/IRBuilder.h"
+#include "llvm/IR/InstIterator.h"
+#include "llvm/IR/Instructions.h"
+#include "llvm/IR/Intrinsics.h"
+#include "llvm/IR/MDBuilder.h"
+#include "llvm/IR/Module.h"
+#include "llvm/InitializePasses.h"
+#include "llvm/Pass.h"
+#include "llvm/Target/TargetMachine.h"
+#include "llvm/Transforms/Instrumentation.h"
+#include "llvm/Transforms/Utils/BasicBlockUtils.h"
+
+using namespace llvm;
+
+#define DEBUG_TYPE "kcfi"
+
+STATISTIC(NumKCFIChecks, "Number of kcfi operands transformed into checks");
+
+namespace {
+class DiagnosticInfoKCFI : public DiagnosticInfo {
+  const Twine 
+
+public:
+  DiagnosticInfoKCFI(const Twine ,
+ DiagnosticSeverity Severity = DS_Error)
+  : DiagnosticInfo(DK_Linker, Severity), Msg(DiagMsg) {}
+  void print(DiagnosticPrinter ) const override { DP << Msg; }
+};
+} // namespace
+
+PreservedAnalyses KCFIPass::run(Function , FunctionAnalysisManager ) {
+  Module  = *F.getParent();
+  if (!M.getModuleFlag("kcfi"))
+return PreservedAnalyses::all();
+
+  // Find call instructions with KCFI 

[PATCH] D138322: [Index] Add various missing USR generation

2022-11-18 Thread Ben Barham via Phabricator via cfe-commits
bnbarham updated this revision to Diff 476614.
bnbarham added a comment.

Add missing --target


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138322

Files:
  clang/lib/Index/USRGeneration.cpp
  clang/test/Index/index-builtin-fixedpoint.c
  clang/test/Index/index-builtin-opencl.clcpp
  clang/test/Index/index-builtin-ppc.cpp
  clang/test/Index/index-builtin-riscv.c
  clang/test/Index/index-builtin-sve.cpp
  clang/test/Index/index-msguid.cpp

Index: clang/test/Index/index-msguid.cpp
===
--- /dev/null
+++ clang/test/Index/index-msguid.cpp
@@ -0,0 +1,14 @@
+template 
+class GUIDHolder {
+public:
+  virtual ~GUIDHolder() {}
+};
+
+class  __declspec(uuid("12345678-1234-5678-ABCD-12345678ABCD")) GUIDInterface {};
+
+class GUIDUse : public GUIDHolder {
+  ~GUIDUse() {}
+  // CHECK: RelOver | ~GUIDHolder | c:@S@GUIDHolder>#$@S@GUIDInterface#@MG@GUID{12345678-1234-5678-abcd-12345678abcd}@F@~GUIDHolder#
+};
+
+// RUN: c-index-test core -print-source-symbols -- -std=c++11 -fms-extensions %s | FileCheck %s
Index: clang/test/Index/index-builtin-sve.cpp
===
--- /dev/null
+++ clang/test/Index/index-builtin-sve.cpp
@@ -0,0 +1,7 @@
+void testSve(__SVInt8_t sve);
+// CHECK: USR: c:@F@testSve#@BT@__SVInt8_t#
+
+void testBf16(__bf16);
+// CHECK: USR: c:@F@testBf16#@BT@__bf16#
+
+// RUN: c-index-test -index-file %s --target=aarch64 -target-feature +bf16 -target-feature +sve -std=c++11 | FileCheck %s
Index: clang/test/Index/index-builtin-riscv.c
===
--- /dev/null
+++ clang/test/Index/index-builtin-riscv.c
@@ -0,0 +1,5 @@
+__attribute__((overloadable))
+void testRiscv(__rvv_int8mf8_t);
+// CHECK: USR: c:@F@testRiscv#@BT@__rvv_int8mf8_t#
+
+// RUN: c-index-test -index-file %s --target=riscv64 -target-feature +v | FileCheck %s
Index: clang/test/Index/index-builtin-ppc.cpp
===
--- /dev/null
+++ clang/test/Index/index-builtin-ppc.cpp
@@ -0,0 +1,7 @@
+void testPpc(__vector_quad);
+// CHECK: USR: c:@F@testPpc#@BT@__vector_quad#
+
+void testIBM(__ibm128);
+// CHECK: USR: c:@F@testIBM#@BT@__ibm128#
+//
+// RUN: c-index-test -index-file %s --target=powerpc64 -target-cpu pwr10 | FileCheck %s
Index: clang/test/Index/index-builtin-opencl.clcpp
===
--- /dev/null
+++ clang/test/Index/index-builtin-opencl.clcpp
@@ -0,0 +1,7 @@
+void testImage(read_only image1d_t);
+// CHECK: USR: c:@F@testImage#@BT@ro_image1d#
+
+void testExt(intel_sub_group_avc_mce_payload_t) {}
+// CHECK: USR: c:@F@testExt#@BT@intel_sub_group_avc_mce_payload_t#
+
+// RUN: c-index-test -index-file %s --target=spir | FileCheck %s
Index: clang/test/Index/index-builtin-fixedpoint.c
===
--- /dev/null
+++ clang/test/Index/index-builtin-fixedpoint.c
@@ -0,0 +1,5 @@
+__attribute__((overloadable))
+void testFixedPoint(_Accum);
+// CHECK: USR: c:@F@testFixedPoint#@BT@Accum#
+
+// RUN: c-index-test -index-file %s -ffixed-point | FileCheck %s
Index: clang/lib/Index/USRGeneration.cpp
===
--- clang/lib/Index/USRGeneration.cpp
+++ clang/lib/Index/USRGeneration.cpp
@@ -168,6 +168,8 @@
   void VisitTemplateName(TemplateName Name);
   void VisitTemplateArgument(const TemplateArgument );
 
+  void VisitMSGuidDecl(const MSGuidDecl *D);
+
   /// Emit a Decl's name using NamedDecl::printName() and return true if
   ///  the decl had no name.
   bool EmitDeclName(const NamedDecl *D);
@@ -658,120 +660,157 @@
 }
 
 if (const BuiltinType *BT = T->getAs()) {
-  unsigned char c = '\0';
   switch (BT->getKind()) {
 case BuiltinType::Void:
-  c = 'v'; break;
+  Out << 'v'; break;
 case BuiltinType::Bool:
-  c = 'b'; break;
+  Out << 'b'; break;
 case BuiltinType::UChar:
-  c = 'c'; break;
+  Out << 'c'; break;
 case BuiltinType::Char8:
-  c = 'u'; break; // FIXME: Check this doesn't collide
+  Out << 'u'; break;
 case BuiltinType::Char16:
-  c = 'q'; break;
+  Out << 'q'; break;
 case BuiltinType::Char32:
-  c = 'w'; break;
+  Out << 'w'; break;
 case BuiltinType::UShort:
-  c = 's'; break;
+  Out << 's'; break;
 case BuiltinType::UInt:
-  c = 'i'; break;
+  Out << 'i'; break;
 case BuiltinType::ULong:
-  c = 'l'; break;
+  Out << 'l'; break;
 case BuiltinType::ULongLong:
-  c = 'k'; break;
+  Out << 'k'; break;
 case BuiltinType::UInt128:
-  c = 'j'; break;
+  Out << 'j'; break;
 case 

[PATCH] D138073: [clang-doc] Move file layout to the generators.

2022-11-18 Thread Brett Wilson via Phabricator via cfe-commits
brettw added inline comments.



Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:890-892
+// TODO If there are multiple Infos for this file name (for example,
+// template specializations), this will generate multiple complete web 
pages
+// (with  and , etc.) concatenated together. This generator

paulkirth wrote:
> brettw wrote:
> > paulkirth wrote:
> > > So does this patch make HTML unusable? Currently, if there is a conflict, 
> > > the file gets replaced, right? 
> > > 
> > > I read this as we're going from a situation where we lost data, but still 
> > > had valid HTML pages to one where we have complete, but incorrect HTML, 
> > > is that correct?
> > > 
> > > Also, why does that happen under template specialization? USRs are SHA1 
> > > hashes of the symbol. I would expect that hashing the symbol would be 
> > > unambiguous and unique, given C++'s ODR. 
> > > 
> > > That last point made me wonder if an alternate way to handle this without 
> > > using the hash would be to use the mangled names?
> > I think this makes HTML strictly better (though not ideal).
> > 
> > Previously if there was a collision, they got written over the top of each 
> > other without truncation (not sure why). So if the longer page was written 
> > second, you got lucky and you get a complete valid page and just lost the 
> > shorter struct's definition. If you got unlucky the shorter page was 
> > written second and you get a corrupted file with the shorter content, 
> > followed by the end of the longer content peeking out from the end.
> > 
> > With this change, you get both content concatenated without corruption, you 
> > just have duplicate doctype and title tags in between them. Browsers should 
> > generally handle this so the page should be usable.
> > 
> > Fixing this requires a refactor of the HTML generator such that the concept 
> > of writing a struct is separate from writing a page. Even if I was going to 
> > work on the HTML generator, I would probably want to do this in a separate 
> > patch anyway.
> > 
> > The markdown generator does the same, but markdown doesn't have all of the 
> > headers stuff that shouldn't be in the middle of the file, you just get a 
> > new `# ...` for the second section and it should be reasonable.
> > 
> > The HTML and MD generators name files according to the `Name` of the 
> > structure. USR is not involved. So any time to things in the same namespace 
> > have the same name, you'll get a collision. This happens most commonly with 
> > template specialization because the names are identical (the name doesn't 
> > account for the template parameters).
> > 
> > All of this complexity is why I changed the YAML output to use USR which 
> > avoids the problem. I didn't want to make this change for the HTML and MD 
> > generators because it will make ugly file names that the user will see, and 
> > you probably want the template specializations to be in the same file 
> > anyway.
> > I think this makes HTML strictly better (though not ideal).
> > 
> > Previously if there was a collision, they got written over the top of each 
> > other without truncation (not sure why). So if the longer page was written 
> > second, you got lucky and you get a complete valid page and just lost the 
> > shorter struct's definition. If you got unlucky the shorter page was 
> > written second and you get a corrupted file with the shorter content, 
> > followed by the end of the longer content peeking out from the end.
> > 
> > With this change, you get both content concatenated without corruption, you 
> > just have duplicate doctype and title tags in between them. Browsers should 
> > generally handle this so the page should be usable.
> > 
> 
> Thanks for the explanation. My concern was that this change made HTML 
> unusable, if it is still valid (if not ideal) then I don't think there is an 
> issue.
> 
> > Fixing this requires a refactor of the HTML generator such that the concept 
> > of writing a struct is separate from writing a page. Even if I was going to 
> > work on the HTML generator, I would probably want to do this in a separate 
> > patch anyway.
> > 
> 
> No arguments re splitting up work. Can you file an issue to track this on 
> github and reference it in the TODO? That should help make sure this gets 
> addressed.
> 
> > The markdown generator does the same, but markdown doesn't have all of the 
> > headers stuff that shouldn't be in the middle of the file, you just get a 
> > new `# ...` for the second section and it should be reasonable.
> > 
> > The HTML and MD generators name files according to the `Name` of the 
> > structure. USR is not involved. So any time to things in the same namespace 
> > have the same name, you'll get a collision. This happens most commonly with 
> > template specialization because the names are identical (the name doesn't 
> > account for the template parameters).
> > 
> 
> I think there should be 

[PATCH] D138073: [clang-doc] Move file layout to the generators.

2022-11-18 Thread Brett Wilson via Phabricator via cfe-commits
brettw updated this revision to Diff 476612.
brettw marked an inline comment as done.

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

https://reviews.llvm.org/D138073

Files:
  clang-tools-extra/clang-doc/Generators.h
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/MDGenerator.cpp
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
  clang-tools-extra/test/clang-doc/single-file-public.cpp
  clang-tools-extra/test/clang-doc/single-file.cpp

Index: clang-tools-extra/test/clang-doc/single-file.cpp
===
--- clang-tools-extra/test/clang-doc/single-file.cpp
+++ clang-tools-extra/test/clang-doc/single-file.cpp
@@ -3,7 +3,7 @@
 // RUN: echo "" > %t/compile_flags.txt
 // RUN: cp "%s" "%t/test.cpp"
 // RUN: clang-doc --doxygen --executor=standalone -p %t %t/test.cpp -output=%t/docs
-// RUN: cat %t/docs/GlobalNamespace/index.yaml | FileCheck %s --check-prefix=CHECK
+// RUN: cat %t/docs/index.yaml | FileCheck %s --check-prefix=CHECK
 // RUN: rm -rf %t
 
 void function(int x);
Index: clang-tools-extra/test/clang-doc/single-file-public.cpp
===
--- clang-tools-extra/test/clang-doc/single-file-public.cpp
+++ clang-tools-extra/test/clang-doc/single-file-public.cpp
@@ -3,7 +3,10 @@
 // RUN: echo "" > %t/compile_flags.txt
 // RUN: cp "%s" "%t/test.cpp"
 // RUN: clang-doc --doxygen --public --executor=standalone -p %t %t/test.cpp -output=%t/docs
-// RUN: cat %t/docs/GlobalNamespace/Record.yaml | FileCheck %s --check-prefix=CHECK
+//   This produces two files, index.yaml and one for the record named by its USR
+//   (which we don't know in advance). This checks the record file by searching
+//   for a name with a 40-char USR name.
+// RUN: find %t -regex ".*[0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z].*" -exec cat {} \; | FileCheck %s --check-prefix=CHECK
 // RUN: rm -rf %t
 
 class Record {
Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -43,6 +43,7 @@
 #include "llvm/Support/ThreadPool.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
+#include 
 #include 
 
 using namespace clang::ast_matchers;
@@ -130,54 +131,6 @@
   return llvm::sys::fs::getMainExecutable(Argv0, MainAddr);
 }
 
-bool CreateDirectory(const Twine , bool ClearDirectory = false) {
-  std::error_code OK;
-  llvm::SmallString<128> DocsRootPath;
-  if (ClearDirectory) {
-std::error_code RemoveStatus = llvm::sys::fs::remove_directories(DirName);
-if (RemoveStatus != OK) {
-  llvm::errs() << "Unable to remove existing documentation directory for "
-   << DirName << ".\n";
-  return true;
-}
-  }
-  std::error_code DirectoryStatus = llvm::sys::fs::create_directories(DirName);
-  if (DirectoryStatus != OK) {
-llvm::errs() << "Unable to create documentation directories.\n";
-return true;
-  }
-  return false;
-}
-
-// A function to extract the appropriate file name for a given info's
-// documentation. The path returned is a composite of the output directory, the
-// info's relative path and name and the extension. The relative path should
-// have been constructed in the serialization phase.
-//
-// Example: Given the below, the  path for class C will be
-// /A/B/C.
-//
-// namespace A {
-// namespace B {
-//
-// class C {};
-//
-// }
-// }
-llvm::Expected> getInfoOutputFile(StringRef Root,
- StringRef RelativePath,
- StringRef Name,
- StringRef Ext) {
-  llvm::SmallString<128> Path;
-  llvm::sys::path::native(Root, Path);
-  llvm::sys::path::append(Path, RelativePath);
-  if (CreateDirectory(Path))
-return llvm::createStringError(llvm::inconvertibleErrorCode(),
-   "failed to create directory");
-  llvm::sys::path::append(Path, Name + Ext);
-  return Path;
-}
-
 int main(int argc, const char **argv) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   std::error_code OK;
@@ -274,6 +227,11 @@
 R.first->second.emplace_back(Value);
   });
 
+  // Collects all Infos according to their unique USR value. This map is added
+  // to from the thread pool below and is protected by the USRToInfoMutex.
+  llvm::sys::Mutex USRToInfoMutex;
+  llvm::StringMap> USRToInfo;
+
   // First reducing phase (reduce all decls into one info per 

[PATCH] D138329: [-Wunsafe-buffer-usage] Add a new recursive matcher to replace `forEachDescendant` in unsafe buffer check

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

Is the problem `forEachDescendant` matching statements inside blocks and 
lambdas? I wonder if this behavior would surprise people, so I think it would 
be better to:

- Potentially add a template bool parameter to `forEachDescendant` controlling 
this behavior.
- Review existing uses because I am not entirely sure if the current behavior 
is the right default.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138329

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


[PATCH] D138321: [-Wunsafe-buffer-usage] Ignore array subscript on literal zero

2022-11-18 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added inline comments.



Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:219
+arraySubscriptExpr(hasBase(ignoringParenImpCasts(hasPointerType())),
+   unless(hasIndex(integerLiteral(equals(0)
 .bind("arraySubscr"));

Isn't it the case you still want to cover zero indices but as a safe gadget to 
make sure fixits can be emitted? 
Having to add another gadget for this makes me think maybe categorizing the 
safety of gadgets upfront is not the right model. 


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

https://reviews.llvm.org/D138321

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


[PATCH] D138318: [-Wunsafe-buffer-usage] Improve pointer match pattern

2022-11-18 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added inline comments.



Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:31-39
+hasType(pointerType()),
+hasType(autoType(
+hasDeducedType(hasUnqualifiedDesugaredType(pointerType(),
+// DecayedType, e.g., array type in formal parameter decl
+hasType(decayedType(hasDecayedType(pointerType(,
+// ElaboratedType, e.g., typedef
+hasType(elaboratedType(hasUnqualifiedDesugaredType(pointerType(,

Isn't it possible to reduce the number of cases by using `hasCanonicalType`?


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

https://reviews.llvm.org/D138318

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


[PATCH] D138253: [-Wunsafe-buffer-usage] NFC: Implement fix-strategies and variable-use-claiming.

2022-11-18 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added inline comments.



Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:236
+  using UseSetTy = SmallSet;
+  using DefMapTy = DenseMap;
+

NoQ wrote:
> NoQ wrote:
> > This extra payload wasn't advertised but it's very useful because it's hard 
> > to jump from `VarDecl` to `DeclStmt` without it, and we need to do such 
> > jump because our analysis starts from unsafe *uses* of a variable, which 
> > only give us a `VarDecl`.
> Another way to look at this, is to notice that currently `DeclStmt` of the 
> variable isn't a `Gadget` on its own, and ask whether it *should* be a 
> `Gadget`.
> 
> I currently believe that yes, it should be a `Gadget`, because it can 
> potentially be a "multi-variable" `Gadget` if, for example, the 
> initializer-expression is itself another raw pointer `DeclRefExpr`. So there 
> needs to be a way to communicate that the fix for the variable declaration 
> may depend on `Strategy` for more than one variable, and that the `Strategy` 
> itself should be chosen carefully, considering that choices for different 
> variables may interact this way. And from the point of view of the *other* 
> variable, this definitely needs to be a `Gadget`, it's no different from any 
> other `Gadget`.
> 
> So, this part needs some more work.
What about `DeclStmt`s that declare multiple variables, some safe some unsafe. 
Do you plan to generate fixits in that case exploding the DeclStmt? 


Repository:
  rC Clang

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

https://reviews.llvm.org/D138253

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


[PATCH] D137473: [vfs] Allow root paths relative to the directory of the vfsoverlay YAML file

2022-11-18 Thread Haowei Wu via Phabricator via cfe-commits
haowei added inline comments.



Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1922
+  EC = FS->makeAbsolute(FullPath, Name);
+  Name = canonicalize(Name);
+} else {

bnbarham wrote:
> Why the canonicalization?
If the Name here somehow has something like //foo/./../bar, it will cause match 
failures. Canonicalize function removes these dots.
The setOverlayFileDir does not canonicalize the dir so this is quite common to 
see.



Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1933-1935
 path_style = sys::path::is_absolute(Name, sys::path::Style::posix)
  ? sys::path::Style::posix
  : sys::path::Style::windows_backslash;

bnbarham wrote:
> `windows_slash` is going to be `windows_backslash` here. I assume this was 
> fine since `sys::fs::make_absolute(Name)` would always return the native 
> style, but that isn't the case now. It's also unfortunate we calculate this 
> again when `makeAbsolute` only just did it.
Thanks for pointing that out. I need to consider the windows forward slash case.

There is a bit in consistency in the VFS implementation. for Windows. we can 
use forward slashes('/') on Windows. Most Windows API/Syscalls accept it. But 
some APIs may not work if you mix '/' with '\' in the path on Windows. What 
RedirectFileSystem::makeAbsolute is trying to do is, it first determine the 
slashes used in the WorkDir(either from process working directory or from 
command line flags, or OverlayDir), in which it can be a forward slash on 
Windows. Then it uses the same separator when appending the relative directory. 
 Using sys::fs::make_absolute will break that. 

The reason that I use makeAbsolute here is that the OverlayDir will likely use 
forward slash on Windows if people uses CMake options 
LINK_FLAG="/vfsoverlay:C:/path/to/overlay.xml". In that case, 
sys::fs::make_absolute will generate some paths like C:/path/to/foo\\bar, which 
may cause issues.

I will rewrite this to consider the forward slashes.



Comment at: llvm/unittests/Support/VirtualFileSystemTest.cpp:1878
+  // with native path seperator, regardless of the actual path seperator
+  // used in YAMLFilePath field.
+#ifndef _WIN32

rnk wrote:
> dexonsmith wrote:
> > bnbarham wrote:
> > > `separator` -> `separator`
> > > 
> > > As per my comment above, this is only true because we're either using 
> > > posix or windows_backslash. I'm not sure what we really want here, should 
> > > we always convert to native? Any thoughts @dexonsmith?
> > > 
> > > FWIW the non-windows case is identical to the above block. It'd be 
> > > interesting to test `/` and `\` on both filesystems.
> > I think we might need a Windows person to help answer; @rnk, can you 
> > suggest someone to help answer @bnbarham's question?
> I can try suggesting @hans, @compnerd, or @Bigcheese 
The non-windows case enable the overlay-relative option. The above block 
didn't. And that is where I discover the issue with overlay-relative always 
uses native separator instead of trying to match OverlayDir.

The best approach for root-relative and overlay-relative option would probably 
be matching the style of OverlayDir/WorkDir and unify the path separator if 
they are mixed in a path.
But this change will likely affects a few tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137473

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


[PATCH] D137379: [-Wunsafe-buffer-usage] Add warnings for unsafe buffer accesses by array subscript operations

2022-11-18 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added inline comments.



Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:168-169
+  static Matcher matcher() {
+// FIXME: What if the index is integer literal 0? Should this be
+// a safe gadget in this case?
+return stmt(

As per some of the discussions, in the future the compiler might be able to 
recognize certain safe patterns, e.g., when there is a simple for loop with 
known bounds, or when both the index and the array size is statically known.

I think here we need to make a very important design decision: Do we want the 
gadgets to have the right "safety" category when it is created (e.g., we have 
to be able to decide if a gadget is safe or not using matchers), or do we want 
some mechanisms to be able to promote an unsafe gadget to be a safe one? (E.g., 
do we want to be able to prove some unsafe gadgets safe using dataflow analysis 
in a later pass?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137379

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


[PATCH] D137348: [-Wunsafe-buffer-usage] Introduce an abstraction for fixable code patterns.

2022-11-18 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added inline comments.



Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:70
+
+  virtual ~Gadget() {}
+





Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:107
+/// out of bounds.
+class IncrementGadget : public UnsafeGadget {
+  const UnaryOperator *Op;

How deep will the `Gadget` Hierarchy be? Using inheritance only to classify 
safe and unsafe gadgets feels like a very heavy weight solution to a relatively 
simple problem.


Repository:
  rC Clang

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

https://reviews.llvm.org/D137348

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


[PATCH] D138322: [Index] Add various missing USR generation

2022-11-18 Thread Ben Langmuir via Phabricator via cfe-commits
benlangmuir accepted this revision.
benlangmuir added a comment.
This revision is now accepted and ready to land.

LGTM once the test failure is fixed




Comment at: clang/test/Index/index-builtin-sve.cpp:7
+
+// RUN: c-index-test -index-file %s -target-feature +bf16 -target-feature +sve 
-std=c++11 | FileCheck %s

Needs a --target=aarch64 or similar to use sve.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138322

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


[PATCH] D136811: [-Wunsafe-buffer-usage] WIP: RFC: NFC: User documentation.

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

Overall, the document looks good to me, I like the general direction. I still 
see some pending comments (mostly small wording fixes) from Aaron.


Repository:
  rC Clang

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

https://reviews.llvm.org/D136811

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


[PATCH] D138297: [Clang][OpenMP] Add default map type for target enter/exit data

2022-11-18 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
doru1004 closed this revision.
doru1004 added a comment.

Commit: 9e595e911eb539caad99fd8642328007d47c6f4e 



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

https://reviews.llvm.org/D138297

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


[PATCH] D137473: [vfs] Allow root paths relative to the directory of the vfsoverlay YAML file

2022-11-18 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added subscribers: hans, compnerd, Bigcheese.
rnk added inline comments.



Comment at: llvm/unittests/Support/VirtualFileSystemTest.cpp:1878
+  // with native path seperator, regardless of the actual path seperator
+  // used in YAMLFilePath field.
+#ifndef _WIN32

dexonsmith wrote:
> bnbarham wrote:
> > `separator` -> `separator`
> > 
> > As per my comment above, this is only true because we're either using posix 
> > or windows_backslash. I'm not sure what we really want here, should we 
> > always convert to native? Any thoughts @dexonsmith?
> > 
> > FWIW the non-windows case is identical to the above block. It'd be 
> > interesting to test `/` and `\` on both filesystems.
> I think we might need a Windows person to help answer; @rnk, can you suggest 
> someone to help answer @bnbarham's question?
I can try suggesting @hans, @compnerd, or @Bigcheese 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137473

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


[clang] 9e595e9 - [Clang][OpenMP] Add support for default to/from map types on target enter/exit data

2022-11-18 Thread Doru Bercea via cfe-commits

Author: Doru Bercea
Date: 2022-11-18T16:12:35-06:00
New Revision: 9e595e911eb539caad99fd8642328007d47c6f4e

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

LOG: [Clang][OpenMP] Add support for default to/from map types on target 
enter/exit data

Added: 
clang/test/OpenMP/target_enter_data_ast_print_openmp52.cpp
clang/test/OpenMP/target_exit_data_ast_print_openmp52.cpp

Modified: 
clang/lib/Parse/ParseOpenMP.cpp
clang/lib/Sema/SemaOpenMP.cpp
clang/test/OpenMP/target_enter_data_ast_print.cpp
clang/test/OpenMP/target_exit_data_ast_print.cpp

Removed: 




diff  --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp
index d6998548cf518..820dd179610b6 100644
--- a/clang/lib/Parse/ParseOpenMP.cpp
+++ b/clang/lib/Parse/ParseOpenMP.cpp
@@ -4392,6 +4392,12 @@ bool Parser::ParseOpenMPVarList(OpenMPDirectiveKind 
DKind,
 }
 if (Data.ExtraModifier == OMPC_MAP_unknown) {
   Data.ExtraModifier = OMPC_MAP_tofrom;
+  if (getLangOpts().OpenMP >= 52) {
+if (DKind == OMPD_target_enter_data)
+  Data.ExtraModifier = OMPC_MAP_to;
+else if (DKind == OMPD_target_exit_data)
+  Data.ExtraModifier = OMPC_MAP_from;
+  }
   Data.IsMapTypeImplicit = true;
 }
 

diff  --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index edeca632ef6d5..a67983da2b66e 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -21724,10 +21724,12 @@ static void checkMappableExpressionList(
   // target enter data
   // OpenMP [2.10.2, Restrictions, p. 99]
   // A map-type must be specified in all map clauses and must be either
-  // to or alloc.
+  // to or alloc. Starting with OpenMP 5.2 the default map type is `to` if
+  // no map type is present.
   OpenMPDirectiveKind DKind = DSAS->getCurrentDirective();
   if (DKind == OMPD_target_enter_data &&
-  !(MapType == OMPC_MAP_to || MapType == OMPC_MAP_alloc)) {
+  !(MapType == OMPC_MAP_to || MapType == OMPC_MAP_alloc ||
+SemaRef.getLangOpts().OpenMP >= 52)) {
 SemaRef.Diag(StartLoc, diag::err_omp_invalid_map_type_for_directive)
 << (IsMapTypeImplicit ? 1 : 0)
 << getOpenMPSimpleClauseTypeName(OMPC_map, MapType)
@@ -21738,10 +21740,11 @@ static void checkMappableExpressionList(
   // target exit_data
   // OpenMP [2.10.3, Restrictions, p. 102]
   // A map-type must be specified in all map clauses and must be either
-  // from, release, or delete.
+  // from, release, or delete. Starting with OpenMP 5.2 the default map
+  // type is `from` if no map type is present.
   if (DKind == OMPD_target_exit_data &&
   !(MapType == OMPC_MAP_from || MapType == OMPC_MAP_release ||
-MapType == OMPC_MAP_delete)) {
+MapType == OMPC_MAP_delete || SemaRef.getLangOpts().OpenMP >= 52)) 
{
 SemaRef.Diag(StartLoc, diag::err_omp_invalid_map_type_for_directive)
 << (IsMapTypeImplicit ? 1 : 0)
 << getOpenMPSimpleClauseTypeName(OMPC_map, MapType)

diff  --git a/clang/test/OpenMP/target_enter_data_ast_print.cpp 
b/clang/test/OpenMP/target_enter_data_ast_print.cpp
index 0ccafaef5b59a..b11d5de13de67 100644
--- a/clang/test/OpenMP/target_enter_data_ast_print.cpp
+++ b/clang/test/OpenMP/target_enter_data_ast_print.cpp
@@ -6,6 +6,10 @@
 // RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=45 -x c++ -std=c++11 
-emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=45 -std=c++11 -include-pch 
%t -fsyntax-only -verify %s -ast-print | FileCheck %s
 
+// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-version=52 -ast-print %s | 
FileCheck %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=52 -x c++ -std=c++11 
-emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=52 -std=c++11 -include-pch 
%t -fsyntax-only -verify %s -ast-print | FileCheck %s
+
 // RUN: %clang_cc1 -verify -fopenmp -ast-print %s | FileCheck %s
 // RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp -std=c++11 -include-pch %t -fsyntax-only -verify 
%s -ast-print | FileCheck %s

diff  --git a/clang/test/OpenMP/target_enter_data_ast_print_openmp52.cpp 
b/clang/test/OpenMP/target_enter_data_ast_print_openmp52.cpp
new file mode 100644
index 0..578f9a2542744
--- /dev/null
+++ b/clang/test/OpenMP/target_enter_data_ast_print_openmp52.cpp
@@ -0,0 +1,65 @@
+// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-version=52 -ast-print %s | 
FileCheck %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=52 -x c++ -std=c++11 
-emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=52 -std=c++11 -include-pch 
%t -fsyntax-only -verify %s -ast-print | 

[PATCH] D137996: Add support for a backdoor driver option that enables emitting header usage information in JSON to a file

2022-11-18 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 added inline comments.



Comment at: clang/lib/Frontend/HeaderIncludeGen.cpp:289-290
+SrcMgr::CharacteristicKind NewFileType, FileID PrevFID) {
+  if (!shouldRecordNewFile(NewFileType, SM.getLocForStartOfFile(PrevFID), SM))
+return;
+

Hmm, this new logic seems orthogonal to the output format. Changing the set of 
headers only based on the desired format seems counter-intuitive. I think we 
should either allow specifying the desired logic via a separate command-line 
flag, or decouple the new feature from the `-header-include-file` flag 
entirely. What are your thoughts?

(Sorry I didn't notice this earlier.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137996

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


[PATCH] D138274: Add version to all LLVM cmake package

2022-11-18 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm accepted this revision.
arsenm added a comment.
This revision is now accepted and ready to land.

I thought we had this already. The amount of boiler plate required repeated in 
each component is depressing, but I wouldn't be surprised if it's really needed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138274

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


[PATCH] D138329: [-Wunsafe-buffer-usage] Add a new recursive matcher to replace `forEachDescendant` in unsafe buffer check

2022-11-18 Thread Ziqing Luo via Phabricator via cfe-commits
ziqingluo-90 created this revision.
ziqingluo-90 added reviewers: NoQ, jkorous, t-rasmud, malavikasamak, 
aaron.ballman, xazax.hun, gribozavr.
Herald added a subscriber: rnkovacs.
Herald added a project: All.
ziqingluo-90 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Add a new matcher `forEveryDescendant` that recursively matches descendants of 
a `Stmt` but skips nested callable definitions.
This matcher has same effect as using `forEachDescendant` and skipping 
`forCallable` explicitly but does not require the AST construction to be 
complete.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138329

Files:
  clang/lib/Analysis/UnsafeBufferUsage.cpp
  clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp

Index: clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
===
--- clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
+++ clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -include %s -verify %s
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fblocks -include %s -verify %s
 #ifndef INCLUDED
 #define INCLUDED
 #pragma clang system_header
@@ -133,15 +133,15 @@
 
 int garray[10];
 int * gp = garray;
-int gvar = gp[1];  // TODO: this is not warned
+int gvar = gp[1];  // FIXME: file scope unsafe buffer access is not warned
 
 void testLambdaCaptureAndGlobal(int * p) {
   int a[10];
 
   auto Lam = [p, a]() {
-return p[1] // expected-warning2{{unchecked operation on raw buffer in expression}}
+return p[1] // expected-warning{{unchecked operation on raw buffer in expression}}
   + a[1] + garray[1]
-  + gp[1];  // expected-warning2{{unchecked operation on raw buffer in expression}}
+  + gp[1];  // expected-warning{{unchecked operation on raw buffer in expression}}
   };
 }
 
@@ -163,4 +163,66 @@
 		expected-note{{in instantiation of function template specialization 'f' requested here}}
 }
 
+
+// test that nested callable definitions are scanned only once
+void testNestedCallableDefinition(int * p) {
+  class A {
+void inner(int * p) {
+  p++; // expected-warning{{unchecked operation on raw buffer in expression}}
+}
+
+static void innerStatic(int * p) {
+  p++; // expected-warning{{unchecked operation on raw buffer in expression}}
+}
+
+void innerInner(int * p) {
+  auto Lam = [p]() {
+int * q = p;
+q++;   // expected-warning{{unchecked operation on raw buffer in expression}}
+return *q;
+  };
+}
+  };
+
+  auto Lam = [p]() {
+int * q = p;
+q++;  // expected-warning{{unchecked operation on raw buffer in expression}}
+return *q;
+  };
+
+  auto LamLam = [p]() {
+auto Lam = [p]() {
+  int * q = p;
+  q++;  // expected-warning{{unchecked operation on raw buffer in expression}}
+  return *q;
+};
+  };
+
+  void (^Blk)(int*) = ^(int *p) {
+p++;   // expected-warning{{unchecked operation on raw buffer in expression}}
+  };
+
+  void (^BlkBlk)(int*) = ^(int *p) {
+void (^Blk)(int*) = ^(int *p) {
+  p++;   // expected-warning{{unchecked operation on raw buffer in expression}}
+};
+Blk(p);
+  };
+
+  // lambda and block as call arguments...
+  foo( [p]() { int * q = p;
+  q++;  // expected-warning{{unchecked operation on raw buffer in expression}}
+  return *q;
+   },
+   ^(int *p) { p++;   // expected-warning{{unchecked operation on raw buffer in expression}}
+   }
+ );
+}
+
+void testVariableDecls(int * p) {
+  int * q = p++;  // expected-warning{{unchecked operation on raw buffer in expression}}
+  int a[p[1]];// expected-warning{{unchecked operation on raw buffer in expression}}
+  int b = p[1];   // expected-warning{{unchecked operation on raw buffer in expression}}
+}
+
 #endif
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -7,12 +7,111 @@
 //===--===//
 
 #include "clang/Analysis/Analyses/UnsafeBufferUsage.h"
+#include "clang/AST/RecursiveASTVisitor.h"
 #include "llvm/ADT/SmallVector.h"
 
 using namespace llvm;
 using namespace clang;
 using namespace ast_matchers;
 
+namespace clang::ast_matchers::internal {
+// A `RecursiveASTVisitor` that traverses all descendants of a given node "n"
+// except for those belonging to a different callable of "n".
+class MatchDescendantVisitor
+: public RecursiveASTVisitor {
+public:
+  typedef RecursiveASTVisitor VisitorBase;
+
+  // Creates an AST visitor that matches `Matcher` on all
+  // descendants of a given node "n" except for the ones
+  // belonging to a different callable of "n".
+  MatchDescendantVisitor(const DynTypedMatcher *Matcher, ASTMatchFinder 

[PATCH] D136176: Implement support for option 'fexcess-precision'.

2022-11-18 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 476587.
zahiraam marked 2 inline comments as done.

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

https://reviews.llvm.org/D136176

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/FPOptions.def
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/X86/fexcess-precision.c
  clang/test/Driver/clang_f_opts.c
  clang/test/Driver/fexcess-precision.c

Index: clang/test/Driver/fexcess-precision.c
===
--- /dev/null
+++ clang/test/Driver/fexcess-precision.c
@@ -0,0 +1,22 @@
+// RUN: %clang -### -target i386 -fexcess-precision=fast -c %s 2>&1  \
+// RUN:   | FileCheck --check-prefix=CHECK-FAST %s
+// RUN: %clang -### -target i386 -fexcess-precision=standard -c %s 2>&1  \
+// RUN:   | FileCheck --check-prefix=CHECK-STD %s
+// RUN: %clang -### -target x86_64 -fexcess-precision=standard -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-STD %s
+// RUN: %clang -### -target x86_64 -fexcess-precision=fast -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FAST %s
+// RUN: %clang -### -target i386 -fexcess-precision=none -c %s 2>&1  \
+// RUN:   | FileCheck --check-prefix=CHECK-ERR-NONE %s
+// RUN: %clang -### -target x86_64 -fexcess-precision=none -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-NONE %s
+// RUN: %clang -### -target i386 -fexcess-precision=16 -c %s 2>&1  \
+// RUN:   | FileCheck --check-prefix=CHECK-ERR-16 %s
+// RUN: %clang -### -target x86_64 -fexcess-precision=16 -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-NONE %s
+
+// CHECK-FAST: "-ffloat16-excess-precision=fast"
+// CHECK-STD: "-ffloat16-excess-precision=standard"
+// CHECK-NONE: "-ffloat16-excess-precision=none"
+// CHECK-ERR-NONE: error: unsupported option '-fexcess-precision=none' for target 'i386'
+// CHECK-ERR-16: error: unsupported option '-fexcess-precision=16' for target 'i386'
Index: clang/test/Driver/clang_f_opts.c
===
--- clang/test/Driver/clang_f_opts.c
+++ clang/test/Driver/clang_f_opts.c
@@ -398,7 +398,7 @@
 // CHECK-WARNING-DAG: optimization flag '-falign-loops' is not supported
 // CHECK-WARNING-DAG: optimization flag '-falign-jumps' is not supported
 // CHECK-WARNING-DAG: optimization flag '-falign-jumps=100' is not supported
-// CHECK-WARNING-DAG: optimization flag '-fexcess-precision=100' is not supported
+// CHECK-WARNING-DAG: unsupported argument '100' to option '-fexcess-precision='
 // CHECK-WARNING-DAG: optimization flag '-fbranch-count-reg' is not supported
 // CHECK-WARNING-DAG: optimization flag '-fcaller-saves' is not supported
 // CHECK-WARNING-DAG: optimization flag '-fno-default-inline' is not supported
Index: clang/test/CodeGen/X86/fexcess-precision.c
===
--- /dev/null
+++ clang/test/CodeGen/X86/fexcess-precision.c
@@ -0,0 +1,286 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ffloat16-excess-precision=fast \
+// RUN: -emit-llvm -o - %s \
+// RUN: | FileCheck -check-prefixes=CHECK-EXT %s
+
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ffloat16-excess-precision=standard \
+// RUN: -emit-llvm -o - %s \
+// RUN: | FileCheck -check-prefixes=CHECK-EXT %s
+
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ffloat16-excess-precision=none \
+// RUN: -emit-llvm -o - %s \
+// RUN: | FileCheck -check-prefixes=CHECK-NO-EXT %s
+
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ffloat16-excess-precision=fast \
+// RUN: -emit-llvm -ffp-eval-method=source -o - %s \
+// RUN: | FileCheck -check-prefixes=CHECK-EXT %s
+
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ffloat16-excess-precision=standard \
+// RUN: -emit-llvm -ffp-eval-method=source -o - %s \
+// RUN: | FileCheck -check-prefixes=CHECK-EXT %s
+
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ffloat16-excess-precision=none \
+// RUN: -emit-llvm -ffp-eval-method=source -o - %s \
+// RUN: | FileCheck -check-prefixes=CHECK-NO-EXT %s
+
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ffloat16-excess-precision=fast \
+// RUN: -emit-llvm -ffp-eval-method=double -o - %s \
+// RUN: | FileCheck -check-prefixes=CHECK-EXT-DBL %s
+
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ffloat16-excess-precision=standard \
+// RUN: -emit-llvm -ffp-eval-method=double -o - %s \
+// RUN: | FileCheck -check-prefixes=CHECK-EXT-DBL %s
+
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ffloat16-excess-precision=none \
+// RUN: -emit-llvm -ffp-eval-method=double -o - %s \
+// RUN: | FileCheck -check-prefixes=CHECK-EXT-DBL %s
+
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ffloat16-excess-precision=fast \
+// RUN: 

[PATCH] D137473: [vfs] Allow root paths relative to the directory of the vfsoverlay YAML file

2022-11-18 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a subscriber: rnk.
dexonsmith added inline comments.



Comment at: llvm/unittests/Support/VirtualFileSystemTest.cpp:1878
+  // with native path seperator, regardless of the actual path seperator
+  // used in YAMLFilePath field.
+#ifndef _WIN32

bnbarham wrote:
> `separator` -> `separator`
> 
> As per my comment above, this is only true because we're either using posix 
> or windows_backslash. I'm not sure what we really want here, should we always 
> convert to native? Any thoughts @dexonsmith?
> 
> FWIW the non-windows case is identical to the above block. It'd be 
> interesting to test `/` and `\` on both filesystems.
I think we might need a Windows person to help answer; @rnk, can you suggest 
someone to help answer @bnbarham's question?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137473

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


[PATCH] D138321: [-Wunsafe-buffer-usage] Ignore array subscript on literal zero

2022-11-18 Thread Ziqing Luo via Phabricator via cfe-commits
ziqingluo-90 updated this revision to Diff 476581.

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

https://reviews.llvm.org/D138321

Files:
  clang/lib/Analysis/UnsafeBufferUsage.cpp
  clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp


Index: clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
===
--- clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
+++ clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
@@ -29,10 +29,10 @@
 char * baz(void);
 
 void testArraySubscripts(int *p, int **pp) {
-  foo(p[0], // expected-warning{{unchecked operation on raw buffer 
in expression}}
-  pp[0][0], // expected-warning2{{unchecked operation on raw 
buffer in expression}}
-  0[0[pp]], // expected-warning2{{unchecked operation on raw 
buffer in expression}}
-  0[pp][0]  // expected-warning2{{unchecked operation on raw 
buffer in expression}}
+  foo(p[1], // expected-warning{{unchecked operation on raw buffer 
in expression}}
+  pp[1][1], // expected-warning2{{unchecked operation on raw 
buffer in expression}}
+  1[1[pp]], // expected-warning2{{unchecked operation on raw 
buffer in expression}}
+  1[pp][1]  // expected-warning2{{unchecked operation on raw 
buffer in expression}}
   );
 
   if (p[3]) {   // expected-warning{{unchecked operation on raw buffer 
in expression}}
@@ -50,11 +50,18 @@
   int a[10], b[10][10];
 
   // Not to warn subscripts on arrays
-  foo(a[0], a[1],
-  0[a], 1[a],
+  foo(a[1], 1[a],
   b[3][4],
   4[b][3],
   4[3[b]]);
+
+  // Not to warn when index is zero
+  foo(p[0], pp[0][0], 0[0[pp]], 0[pp][0],
+  ((int*)bar())[0],
+  0[(int*)bar()],
+  baz()[0],
+  0[baz()]
+  );
 }
 
 void testArraySubscriptsWithAuto(int *p, int **pp) {
@@ -62,19 +69,19 @@
 
   auto ap1 = a;
 
-  foo(ap1[0]);  // expected-warning{{unchecked operation on raw buffer in 
expression}}
+  foo(ap1[1]);  // expected-warning{{unchecked operation on raw buffer in 
expression}}
 
   auto ap2 = p;
 
-  foo(ap2[0]);  // expected-warning{{unchecked operation on raw buffer in 
expression}}
+  foo(ap2[1]);   // expected-warning{{unchecked operation on raw buffer in 
expression}}
 
   auto ap3 = pp;
 
-  foo(ap3[0][0]); // expected-warning2{{unchecked operation on raw buffer in 
expression}}
+  foo(ap3[1][1]); // expected-warning2{{unchecked operation on raw buffer in 
expression}}
 
   auto ap4 = *pp;
 
-  foo(ap4[0]);  // expected-warning{{unchecked operation on raw buffer in 
expression}}
+  foo(ap4[1]);   // expected-warning{{unchecked operation on raw buffer in 
expression}}
 }
 
 void testQualifiedParameters(const int * p, const int * const q,
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -214,10 +214,9 @@
   }
 
   static Matcher matcher() {
-// FIXME: What if the index is integer literal 0? Should this be
-// a safe gadget in this case?
 return stmt(
-arraySubscriptExpr(hasBase(ignoringParenImpCasts(hasPointerType(
+arraySubscriptExpr(hasBase(ignoringParenImpCasts(hasPointerType())),
+   unless(hasIndex(integerLiteral(equals(0)
 .bind("arraySubscr"));
   }
 


Index: clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
===
--- clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
+++ clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
@@ -29,10 +29,10 @@
 char * baz(void);
 
 void testArraySubscripts(int *p, int **pp) {
-  foo(p[0], // expected-warning{{unchecked operation on raw buffer in expression}}
-  pp[0][0], // expected-warning2{{unchecked operation on raw buffer in expression}}
-  0[0[pp]], // expected-warning2{{unchecked operation on raw buffer in expression}}
-  0[pp][0]  // expected-warning2{{unchecked operation on raw buffer in expression}}
+  foo(p[1], // expected-warning{{unchecked operation on raw buffer in expression}}
+  pp[1][1], // expected-warning2{{unchecked operation on raw buffer in expression}}
+  1[1[pp]], // expected-warning2{{unchecked operation on raw buffer in expression}}
+  1[pp][1]  // expected-warning2{{unchecked operation on raw buffer in expression}}
   );
 
   if (p[3]) {   // expected-warning{{unchecked operation on raw buffer in expression}}
@@ -50,11 +50,18 @@
   int a[10], b[10][10];
 
   // Not to warn subscripts on arrays
-  foo(a[0], a[1],
-  0[a], 1[a],
+  foo(a[1], 1[a],
   b[3][4],
   4[b][3],
   4[3[b]]);
+
+  // Not to warn when index is zero
+  foo(p[0], pp[0][0], 0[0[pp]], 0[pp][0],
+  ((int*)bar())[0],
+  0[(int*)bar()],
+  baz()[0],
+  0[baz()]
+   

[PATCH] D138219: [include-cleaner] Show includes matched by refs in HTML report. Demo: https://htmlpreview.github.io/?https://gist.githubusercontent.com/sam-mccall/ecee6869e37af3db28089b64d8dce806/ra

2022-11-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

In D138219#3936279 , @hokein wrote:

> Thanks, I like the idea of marking missing-include refs.
>
> I haven't read the code yet, and below are all my findings when I played with 
> the demo html (some of them are unrelated to the current patch, just want to 
> dump all):

Sure thing, here are my notes :-)

> 1. size_t shows duplicated entries, line 465

This is because we're using the same decl-specifier with multiple declarators, 
but the AST doesn't model that. I don't think this is a big deal, we should 
deduplicate before actually displaying/applying findings but no need in this 
too imo.

> 2. we are missing refs in some using declarations (line 31, line32), but line 
> 33 does have a link which seems weird

They are different types of symbol (template vs function), may make a 
difference?

> 3. UI related:
>   - it would be nice to highlight the whole line, if user clicks the line 
> link from the shown-up panel;

Hmm, i think i can do this with CSS3 `:target`. Otherwise I don't think it's 
worth adding extra JS for.

> - for `Included path/to/header.h`, I think adding the `""`/`<>` around the 
> spelling string will be nicer;

Agree, but we don't know which, and i don't think it's worth adding it to the 
library for that purpose only.

> - for main-file symbols, showing (`Header  ASTTests.cpp`) is suboptimal, but 
> I don't have better solution;

I think `MainFile` should be a `Header` variant maybe. (Not just for this)

> 4. The handling of std symbols seems inconsistent:
>   - click on a vector `push_back` will give the mapping `vector` header;
>   - click on the type `std::vector` will give the `iosfwd` header;

At first glance this indeed looks like a bug (not in this patch i guess)

> 5. I was confused why the type `Annotation` has the `Included` file, but not 
> the method call `.code()`, since both shown-up panel are showing 
> `Annotations.h` header, then I realized that the `code` is from the base 
> class `llvm::Annotation`. This brings up a policy question (Not sure it has 
> been discussed before): If we have already included the header of a subclass, 
> and we have some base-class method calls via the subclass object, do we need 
> to insert the header of the base class?

Right, that's silly. I do think policy on member access in general is still a 
bit up in the air.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138219

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


[PATCH] D138318: [-Wunsafe-buffer-usage] Improve pointer match pattern

2022-11-18 Thread Ziqing Luo via Phabricator via cfe-commits
ziqingluo-90 updated this revision to Diff 476578.

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

https://reviews.llvm.org/D138318

Files:
  clang/lib/Analysis/UnsafeBufferUsage.cpp
  clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp

Index: clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
===
--- clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
+++ clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
@@ -1,4 +1,22 @@
-// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -verify %s
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -include %s -verify %s
+#ifndef INCLUDED
+#define INCLUDED
+#pragma clang system_header
+
+// no spanification warnings for system headers
+void foo(...);
+void testAsSystemHeader(char *p) {
+  ++p;
+
+  auto ap1 = p;
+  auto ap2 = 
+
+  foo(p[1],
+  ap1[1],
+  ap2[2][3]);
+}
+
+#else
 
 void testIncrement(char *p) {
   ++p; // expected-warning{{unchecked operation on raw buffer in expression}}
@@ -7,8 +25,6 @@
   p--; // expected-warning{{unchecked operation on raw buffer in expression}}
 }
 
-void foo(...);
-
 void * bar(void);
 char * baz(void);
 
@@ -33,7 +49,7 @@
 
   int a[10], b[10][10];
 
-  // not to warn subscripts on arrays
+  // Not to warn subscripts on arrays
   foo(a[0], a[1],
   0[a], 1[a],
   b[3][4],
@@ -54,9 +70,90 @@
 
   auto ap3 = pp;
 
-  foo(pp[0][0]); // expected-warning2{{unchecked operation on raw buffer in expression}}
+  foo(ap3[0][0]); // expected-warning2{{unchecked operation on raw buffer in expression}}
 
   auto ap4 = *pp;
 
   foo(ap4[0]);  // expected-warning{{unchecked operation on raw buffer in expression}}
 }
+
+void testQualifiedParameters(const int * p, const int * const q,
+			 const int a[10], const int b[10][10]) {
+  foo(p[1], 1[p], p[-1],   // expected-warning3{{unchecked operation on raw buffer in expression}}
+  q[1], 1[q], q[-1],   // expected-warning3{{unchecked operation on raw buffer in expression}}
+  a[1],// expected-warning{{unchecked operation on raw buffer in expression}} `a` is of pointer type
+  b[1][2]  // expected-warning{{unchecked operation on raw buffer in expression}} `b[1]` is of array type
+  );
+}
+
+struct T {
+  int a[10];
+  int * b;
+  struct {
+int a[10];
+int * b;
+  } c;
+};
+
+typedef struct T T_t;
+
+T_t funRetT();
+T_t * funRetTStar();
+
+void testStructMembers(struct T * sp, struct T s, T_t * sp2, T_t s2) {
+  foo(sp->a[1],
+  sp->b[1], // expected-warning{{unchecked operation on raw buffer in expression}}
+  sp->c.a[1],
+  sp->c.b[1],   // expected-warning{{unchecked operation on raw buffer in expression}}
+  s.a[1],
+  s.b[1],   // expected-warning{{unchecked operation on raw buffer in expression}}
+  s.c.a[1],
+  s.c.b[1], // expected-warning{{unchecked operation on raw buffer in expression}}
+  sp2->a[1],
+  sp2->b[1],// expected-warning{{unchecked operation on raw buffer in expression}}
+  sp2->c.a[1],
+  sp2->c.b[1],  // expected-warning{{unchecked operation on raw buffer in expression}}
+  s2.a[1],
+  s2.b[1],  // expected-warning{{unchecked operation on raw buffer in expression}}
+  s2.c.a[1],
+  s2.c.b[1],   // expected-warning{{unchecked operation on raw buffer in expression}}
+  funRetT().a[1],
+  funRetT().b[1],  // expected-warning{{unchecked operation on raw buffer in expression}}
+  funRetTStar()->a[1],
+  funRetTStar()->b[1]  // expected-warning{{unchecked operation on raw buffer in expression}}
+  );
+}
+
+int garray[10];
+int * gp = garray;
+int gvar = gp[1];  // TODO: this is not warned
+
+void testLambdaCaptureAndGlobal(int * p) {
+  int a[10];
+
+  auto Lam = [p, a]() {
+return p[1] // expected-warning2{{unchecked operation on raw buffer in expression}}
+  + a[1] + garray[1]
+  + gp[1];  // expected-warning2{{unchecked operation on raw buffer in expression}}
+  };
+}
+
+typedef T_t * T_ptr_t;
+
+void testTypedefs(T_ptr_t p) {
+  foo(p[1],  // expected-warning{{unchecked operation on raw buffer in expression}}
+  p[1].a[1], // expected-warning{{unchecked operation on raw buffer in expression}}
+  p[1].b[1]  // expected-warning2{{unchecked operation on raw buffer in expression}}
+  );
+}
+
+template T f(T t) {
+  return [1]; // expected-warning{{unchecked operation on raw buffer in expression}}
+}
+
+void testTemplate(int * p) {
+  foo(f(p)[1]); // expected-warning{{unchecked operation on raw buffer in expression}}, \
+		expected-note{{in instantiation of function template specialization 'f' requested here}}
+}
+
+#endif
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -27,9 +27,16 @@
 
 // Because we're dealing with raw pointers, 

[PATCH] D137473: [vfs] Allow root paths relative to the directory of the vfsoverlay YAML file

2022-11-18 Thread Ben Barham via Phabricator via cfe-commits
bnbarham added inline comments.



Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1922
+  EC = FS->makeAbsolute(FullPath, Name);
+  Name = canonicalize(Name);
+} else {

Why the canonicalization?



Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1933-1935
 path_style = sys::path::is_absolute(Name, sys::path::Style::posix)
  ? sys::path::Style::posix
  : sys::path::Style::windows_backslash;

`windows_slash` is going to be `windows_backslash` here. I assume this was fine 
since `sys::fs::make_absolute(Name)` would always return the native style, but 
that isn't the case now. It's also unfortunate we calculate this again when 
`makeAbsolute` only just did it.



Comment at: llvm/unittests/Support/VirtualFileSystemTest.cpp:1878
+  // with native path seperator, regardless of the actual path seperator
+  // used in YAMLFilePath field.
+#ifndef _WIN32

`separator` -> `separator`

As per my comment above, this is only true because we're either using posix or 
windows_backslash. I'm not sure what we really want here, should we always 
convert to native? Any thoughts @dexonsmith?

FWIW the non-windows case is identical to the above block. It'd be interesting 
to test `/` and `\` on both filesystems.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137473

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


[PATCH] D138322: [Index] Add various missing USR generation

2022-11-18 Thread Ben Barham via Phabricator via cfe-commits
bnbarham created this revision.
bnbarham added a reviewer: arphaman.
Herald added subscribers: frasercrmck, luismarques, apazos, sameer.abuasal, 
s.egerton, Jim, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, 
edward-jones, zzheng, jrtc27, niosHD, sabuasal, simoncook, johnrusso, rbar, 
asb, kbarton, nemanjai.
Herald added a project: All.
bnbarham requested review of this revision.
Herald added subscribers: cfe-commits, pcwang-thead.
Herald added a project: clang.

Over the years there's been many builtin types added without
corresponding USRs. Add a `@BT@` USR for all these types. Also add
a comment so that hopefully this doesn't continue happening.

`MSGuid` was also missing a USR, use `@MG@GUID{}` for it.

Resolves rdar://102198268.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138322

Files:
  clang/lib/Index/USRGeneration.cpp
  clang/test/Index/index-builtin-fixedpoint.c
  clang/test/Index/index-builtin-opencl.clcpp
  clang/test/Index/index-builtin-ppc.cpp
  clang/test/Index/index-builtin-riscv.c
  clang/test/Index/index-builtin-sve.cpp
  clang/test/Index/index-msguid.cpp

Index: clang/test/Index/index-msguid.cpp
===
--- /dev/null
+++ clang/test/Index/index-msguid.cpp
@@ -0,0 +1,14 @@
+template 
+class GUIDHolder {
+public:
+  virtual ~GUIDHolder() {}
+};
+
+class  __declspec(uuid("12345678-1234-5678-ABCD-12345678ABCD")) GUIDInterface {};
+
+class GUIDUse : public GUIDHolder {
+  ~GUIDUse() {}
+  // CHECK: RelOver | ~GUIDHolder | c:@S@GUIDHolder>#$@S@GUIDInterface#@MG@GUID{12345678-1234-5678-abcd-12345678abcd}@F@~GUIDHolder#
+};
+
+// RUN: c-index-test core -print-source-symbols -- -std=c++11 -fms-extensions %s | FileCheck %s
Index: clang/test/Index/index-builtin-sve.cpp
===
--- /dev/null
+++ clang/test/Index/index-builtin-sve.cpp
@@ -0,0 +1,7 @@
+void testSve(__SVInt8_t sve);
+// CHECK: USR: c:@F@testSve#@BT@__SVInt8_t#
+
+void testBf16(__bf16);
+// CHECK: USR: c:@F@testBf16#@BT@__bf16#
+
+// RUN: c-index-test -index-file %s -target-feature +bf16 -target-feature +sve -std=c++11 | FileCheck %s
Index: clang/test/Index/index-builtin-riscv.c
===
--- /dev/null
+++ clang/test/Index/index-builtin-riscv.c
@@ -0,0 +1,5 @@
+__attribute__((overloadable))
+void testRiscv(__rvv_int8mf8_t);
+// CHECK: USR: c:@F@testRiscv#@BT@__rvv_int8mf8_t#
+
+// RUN: c-index-test -index-file %s --target=riscv64 -target-feature +v | FileCheck %s
Index: clang/test/Index/index-builtin-ppc.cpp
===
--- /dev/null
+++ clang/test/Index/index-builtin-ppc.cpp
@@ -0,0 +1,7 @@
+void testPpc(__vector_quad);
+// CHECK: USR: c:@F@testPpc#@BT@__vector_quad#
+
+void testIBM(__ibm128);
+// CHECK: USR: c:@F@testIBM#@BT@__ibm128#
+//
+// RUN: c-index-test -index-file %s --target=powerpc64 -target-cpu pwr10 | FileCheck %s
Index: clang/test/Index/index-builtin-opencl.clcpp
===
--- /dev/null
+++ clang/test/Index/index-builtin-opencl.clcpp
@@ -0,0 +1,7 @@
+void testImage(read_only image1d_t);
+// CHECK: USR: c:@F@testImage#@BT@ro_image1d#
+
+void testExt(intel_sub_group_avc_mce_payload_t) {}
+// CHECK: USR: c:@F@testExt#@BT@intel_sub_group_avc_mce_payload_t#
+
+// RUN: c-index-test -index-file %s --target=spir | FileCheck %s
Index: clang/test/Index/index-builtin-fixedpoint.c
===
--- /dev/null
+++ clang/test/Index/index-builtin-fixedpoint.c
@@ -0,0 +1,5 @@
+__attribute__((overloadable))
+void testFixedPoint(_Accum);
+// CHECK: USR: c:@F@testFixedPoint#@BT@Accum#
+
+// RUN: c-index-test -index-file %s -ffixed-point | FileCheck %s
Index: clang/lib/Index/USRGeneration.cpp
===
--- clang/lib/Index/USRGeneration.cpp
+++ clang/lib/Index/USRGeneration.cpp
@@ -168,6 +168,8 @@
   void VisitTemplateName(TemplateName Name);
   void VisitTemplateArgument(const TemplateArgument );
 
+  void VisitMSGuidDecl(const MSGuidDecl *D);
+
   /// Emit a Decl's name using NamedDecl::printName() and return true if
   ///  the decl had no name.
   bool EmitDeclName(const NamedDecl *D);
@@ -658,120 +660,157 @@
 }
 
 if (const BuiltinType *BT = T->getAs()) {
-  unsigned char c = '\0';
   switch (BT->getKind()) {
 case BuiltinType::Void:
-  c = 'v'; break;
+  Out << 'v'; break;
 case BuiltinType::Bool:
-  c = 'b'; break;
+  Out << 'b'; break;
 case BuiltinType::UChar:
-  c = 'c'; break;
+  Out << 'c'; break;
 case BuiltinType::Char8:
-  c = 'u'; break; // FIXME: Check this doesn't collide
+  Out << 'u'; break;
 case BuiltinType::Char16:
-  c = 'q'; break;
+  Out << 'q'; break;
 

[PATCH] D138321: [-Wunsafe-buffer-usage] Ignore array subscript on literal zero

2022-11-18 Thread Ziqing Luo via Phabricator via cfe-commits
ziqingluo-90 created this revision.
ziqingluo-90 added reviewers: NoQ, jkorous, t-rasmud, malavikasamak, 
aaron.ballman, gribozavr, xazax.hun.
Herald added a subscriber: rnkovacs.
Herald added a project: All.
ziqingluo-90 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Improving unsafe array subscript warning reporting.
For array subscripts with a literal zero index, no warning will be emitted.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138321

Files:
  clang/lib/Analysis/UnsafeBufferUsage.cpp
  clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp


Index: clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
===
--- clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
+++ clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
@@ -29,10 +29,10 @@
 char * baz(void);
 
 void testArraySubscripts(int *p, int **pp) {
-  foo(p[0], // expected-warning{{unchecked operation on raw buffer 
in expression}}
-  pp[0][0], // expected-warning2{{unchecked operation on raw 
buffer in expression}}
-  0[0[pp]], // expected-warning2{{unchecked operation on raw 
buffer in expression}}
-  0[pp][0]  // expected-warning2{{unchecked operation on raw 
buffer in expression}}
+  foo(p[1], // expected-warning{{unchecked operation on raw buffer 
in expression}}
+  pp[1][1], // expected-warning2{{unchecked operation on raw 
buffer in expression}}
+  1[1[pp]], // expected-warning2{{unchecked operation on raw 
buffer in expression}}
+  1[pp][1]  // expected-warning2{{unchecked operation on raw 
buffer in expression}}
   );
 
   if (p[3]) {   // expected-warning{{unchecked operation on raw buffer 
in expression}}
@@ -50,11 +50,18 @@
   int a[10], b[10][10];
 
   // Not to warn subscripts on arrays
-  foo(a[0], a[1],
-  0[a], 1[a],
+  foo(a[1], 1[a],
   b[3][4],
   4[b][3],
   4[3[b]]);
+
+  // Not to warn when index is zero
+  foo(p[0], pp[0][0], 0[0[pp]], 0[pp][0],
+  ((int*)bar())[0],
+  0[(int*)bar()],
+  baz()[0],
+  0[baz()]
+  );
 }
 
 void testArraySubscriptsWithAuto(int *p, int **pp) {
@@ -62,19 +69,19 @@
 
   auto ap1 = a; // expected-warning{{variable 'ap1' participates in unchecked 
buffer operations}}
 
-  foo(ap1[0]);  
+  foo(ap1[1]);
 
   auto ap2 = p; // expected-warning{{variable 'ap2' participates in unchecked 
buffer operations}}
 
-  foo(ap2[0]);  
+  foo(ap2[1]);
 
   auto ap3 = pp;  // expected-warning{{variable 'ap3' participates in 
unchecked buffer operations}}
 
-  foo(ap3[0][0]); // expected-warning{{unchecked operation on raw buffer in 
expression}} 
+  foo(ap3[1][1]); // expected-warning{{unchecked operation on raw buffer in 
expression}}
 
   auto ap4 = *pp; // expected-warning{{variable 'ap4' participates in 
unchecked buffer operations}}
 
-  foo(ap4[0]);  
+  foo(ap4[1]);
 }
 
 void testQualifiedParameters(const int * p, const int * const q,
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -228,10 +228,9 @@
   }
 
   static Matcher matcher() {
-// FIXME: What if the index is integer literal 0? Should this be
-// a safe gadget in this case?
 return stmt(
-arraySubscriptExpr(hasBase(ignoringParenImpCasts(hasPointerType(
+arraySubscriptExpr(hasBase(ignoringParenImpCasts(hasPointerType())),
+   unless(hasIndex(integerLiteral(equals(0)
 .bind("arraySubscr"));
   }
 


Index: clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
===
--- clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
+++ clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
@@ -29,10 +29,10 @@
 char * baz(void);
 
 void testArraySubscripts(int *p, int **pp) {
-  foo(p[0], // expected-warning{{unchecked operation on raw buffer in expression}}
-  pp[0][0], // expected-warning2{{unchecked operation on raw buffer in expression}}
-  0[0[pp]], // expected-warning2{{unchecked operation on raw buffer in expression}}
-  0[pp][0]  // expected-warning2{{unchecked operation on raw buffer in expression}}
+  foo(p[1], // expected-warning{{unchecked operation on raw buffer in expression}}
+  pp[1][1], // expected-warning2{{unchecked operation on raw buffer in expression}}
+  1[1[pp]], // expected-warning2{{unchecked operation on raw buffer in expression}}
+  1[pp][1]  // expected-warning2{{unchecked operation on raw buffer in expression}}
   );
 
   if (p[3]) {   // expected-warning{{unchecked operation on raw buffer in expression}}
@@ -50,11 +50,18 @@
   int a[10], b[10][10];
 
   // Not to warn subscripts 

[PATCH] D138319: [analyzer] Prepare structures for integral cast feature introducing

2022-11-18 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov created this revision.
ASDenysPetrov added reviewers: NoQ, MaskRay, martong, steakhal.
ASDenysPetrov added a project: clang.
Herald added subscribers: manas, StephenFan, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: All.
ASDenysPetrov requested review of this revision.
Herald added a subscriber: cfe-commits.

Change structures of storing bindings between Symbols and Equivalent Classes.
Add a Bitwidth characteristic as a common feature of integral Symbol to make 
`(char)(int x)` and `(uchar)(int x)` treated under the same Equivalent Class.

The link **Symbol **- **Class **was direct and now it depends on the 
effective(minimal) Bitwidth of the Symbol.
The link **Class **- **Symbol **stays as previously.

Some test cases are affected by regression for the sake of the next patch which 
will fix it.

This patch does not introduce integral casts but prepares the field for the 
core patch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138319

Files:
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/test/Analysis/svalbuilder-casts.cpp

Index: clang/test/Analysis/svalbuilder-casts.cpp
===
--- clang/test/Analysis/svalbuilder-casts.cpp
+++ clang/test/Analysis/svalbuilder-casts.cpp
@@ -39,17 +39,17 @@
 
   // These are not truncated to short as zero.
   static_assert((short)1 != 0, "");
-  clang_analyzer_eval(x == 1);   // expected-warning{{FALSE}}
+  clang_analyzer_eval(x == 1);   // expected-warning{{UNKNOWN}}
   static_assert((short)-1 != 0, "");
-  clang_analyzer_eval(x == -1);  // expected-warning{{FALSE}}
+  clang_analyzer_eval(x == -1);  // expected-warning{{UNKNOWN}}
   static_assert((short)65537 != 0, "");
-  clang_analyzer_eval(x == 65537);   // expected-warning{{FALSE}}
+  clang_analyzer_eval(x == 65537);   // expected-warning{{UNKNOWN}}
   static_assert((short)-65537 != 0, "");
-  clang_analyzer_eval(x == -65537);  // expected-warning{{FALSE}}
+  clang_analyzer_eval(x == -65537);  // expected-warning{{UNKNOWN}}
   static_assert((short)131073 != 0, "");
-  clang_analyzer_eval(x == 131073);  // expected-warning{{FALSE}}
+  clang_analyzer_eval(x == 131073);  // expected-warning{{UNKNOWN}}
   static_assert((short)-131073 != 0, "");
-  clang_analyzer_eval(x == -131073); // expected-warning{{FALSE}}
+  clang_analyzer_eval(x == -131073); // expected-warning{{UNKNOWN}}
 
   // Check for implicit cast.
   short s = y;
Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -13,6 +13,7 @@
 
 #include "clang/Basic/JsonSupport.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/APSIntType.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h"
@@ -872,17 +873,18 @@
 }
 LLVM_DUMP_METHOD void RangeSet::dump() const { dump(llvm::errs()); }
 
-REGISTER_SET_FACTORY_WITH_PROGRAMSTATE(SymbolSet, SymbolRef)
-
 namespace {
 class EquivalenceClass;
+using BitWidthType = uint32_t;
 } // end anonymous namespace
 
-REGISTER_MAP_WITH_PROGRAMSTATE(ClassMap, SymbolRef, EquivalenceClass)
+REGISTER_SET_FACTORY_WITH_PROGRAMSTATE(SymbolSet, SymbolRef)
+REGISTER_SET_FACTORY_WITH_PROGRAMSTATE(ClassSet, EquivalenceClass)
+REGISTER_MAP_FACTORY_WITH_PROGRAMSTATE(ClassMap, BitWidthType, EquivalenceClass)
+
+REGISTER_MAP_WITH_PROGRAMSTATE(SymClassMap, SymbolRef, ClassMap)
 REGISTER_MAP_WITH_PROGRAMSTATE(ClassMembers, EquivalenceClass, SymbolSet)
 REGISTER_MAP_WITH_PROGRAMSTATE(ConstraintRange, EquivalenceClass, RangeSet)
-
-REGISTER_SET_FACTORY_WITH_PROGRAMSTATE(ClassSet, EquivalenceClass)
 REGISTER_MAP_WITH_PROGRAMSTATE(DisequalityMap, EquivalenceClass, ClassSet)
 
 namespace {
@@ -989,11 +991,10 @@
 return getRepresentativeSymbol()->getType();
   }
 
-  EquivalenceClass() = delete;
+  EquivalenceClass() = default;
   EquivalenceClass(const EquivalenceClass &) = default;
-  EquivalenceClass =(const EquivalenceClass &) = delete;
-  EquivalenceClass(EquivalenceClass &&) = default;
-  EquivalenceClass =(EquivalenceClass &&) = delete;
+  EquivalenceClass(SymbolRef Sym) : ID(reinterpret_cast(Sym)) {}
+  EquivalenceClass =(const EquivalenceClass &) = default;
 
   bool operator==(const EquivalenceClass ) const {
 return ID == Other.ID;
@@ -1010,9 +1011,6 @@
   void Profile(llvm::FoldingSetNodeID ) const { Profile(ID, this->ID); }
 
 private:
-  /* implicit */ EquivalenceClass(SymbolRef Sym)
-  : ID(reinterpret_cast(Sym)) {}
-
   /// This function is intended to be used ONLY within the class.
 

[PATCH] D138297: [Clang][OpenMP] Add default map type for target enter/exit data

2022-11-18 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev accepted this revision.
ABataev added a comment.
This revision is now accepted and ready to land.

LG


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

https://reviews.llvm.org/D138297

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


[PATCH] D138297: [Clang][OpenMP] Add default map type for target enter/exit data

2022-11-18 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
doru1004 marked an inline comment as done.
doru1004 added inline comments.



Comment at: clang/lib/Parse/ParseOpenMP.cpp:4367-4371
+  if (getLangOpts().OpenMP >= 52)
+if (DKind == OMPD_target_enter_data)
+  Data.ExtraModifier = OMPC_MAP_to;
+else if (DKind == OMPD_target_exit_data)
+  Data.ExtraModifier = OMPC_MAP_from;

ABataev wrote:
> Add braces, otherwise the compiler generates a warning
Thanks for pointing it out!


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

https://reviews.llvm.org/D138297

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


[PATCH] D138297: [Clang][OpenMP] Add default map type for target enter/exit data

2022-11-18 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
doru1004 updated this revision to Diff 476560.

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

https://reviews.llvm.org/D138297

Files:
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/target_enter_data_ast_print.cpp
  clang/test/OpenMP/target_enter_data_ast_print_openmp52.cpp
  clang/test/OpenMP/target_exit_data_ast_print.cpp
  clang/test/OpenMP/target_exit_data_ast_print_openmp52.cpp

Index: clang/test/OpenMP/target_exit_data_ast_print_openmp52.cpp
===
--- /dev/null
+++ clang/test/OpenMP/target_exit_data_ast_print_openmp52.cpp
@@ -0,0 +1,64 @@
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=52 -ast-print %s | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=52 -x c++ -std=c++11 -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=52 -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s
+
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+
+template 
+T tmain(T argc, T *argv) {
+  T i_def, i;
+
+  i = argc;
+#pragma omp target exit data map(i_def)
+
+#pragma omp target exit data map(from: i)
+
+  return 0;
+}
+
+// CHECK: template  T tmain(T argc, T *argv) {
+// CHECK-NEXT: T i_def, i;
+// CHECK-NEXT: i = argc;
+// CHECK-NEXT: #pragma omp target exit data map(from: i_def){{$}}
+// CHECK-NEXT: #pragma omp target exit data map(from: i){{$}}
+
+// CHECK: template<> int tmain(int argc, int *argv) {
+// CHECK-NEXT: int i_def, i;
+// CHECK-NEXT: i = argc;
+// CHECK-NEXT: #pragma omp target exit data map(from: i_def)
+// CHECK-NEXT: #pragma omp target exit data map(from: i)
+
+// CHECK: template<> char tmain(char argc, char *argv) {
+// CHECK-NEXT: char i_def, i;
+// CHECK-NEXT: i = argc;
+// CHECK-NEXT: #pragma omp target exit data map(from: i_def)
+// CHECK-NEXT: #pragma omp target exit data map(from: i)
+
+int main (int argc, char **argv) {
+  int b_def, b;
+  static int a_def, a;
+// CHECK: static int a_def, a;
+
+#pragma omp target exit data map(a_def)
+// CHECK:  #pragma omp target exit data map(from: a_def)
+  a_def=2;
+// CHECK-NEXT: a_def = 2;
+
+#pragma omp target exit data map(from: a)
+// CHECK:  #pragma omp target exit data map(from: a)
+  a=2;
+// CHECK-NEXT: a = 2;
+
+#pragma omp target exit data map(b_def)
+// CHECK-NEXT:  #pragma omp target exit data map(from: b_def)
+
+#pragma omp target exit data map(from: b)
+// CHECK-NEXT:  #pragma omp target exit data map(from: b)
+
+  return tmain(argc, ) + tmain(argv[0][0], argv[0]);
+}
+
+#endif
Index: clang/test/OpenMP/target_exit_data_ast_print.cpp
===
--- clang/test/OpenMP/target_exit_data_ast_print.cpp
+++ clang/test/OpenMP/target_exit_data_ast_print.cpp
@@ -6,6 +6,10 @@
 // RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=45 -x c++ -std=c++11 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=45 -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s
 
+// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-version=52 -ast-print %s | FileCheck %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=52 -x c++ -std=c++11 -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=52 -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s
+
 // RUN: %clang_cc1 -verify -fopenmp -ast-print %s | FileCheck %s
 // RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s
Index: clang/test/OpenMP/target_enter_data_ast_print_openmp52.cpp
===
--- /dev/null
+++ clang/test/OpenMP/target_enter_data_ast_print_openmp52.cpp
@@ -0,0 +1,65 @@
+// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-version=52 -ast-print %s | FileCheck %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=52 -x c++ -std=c++11 -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=52 -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck --check-prefix=CHECK --check-prefix=CHECK-52 %s
+
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+
+template 
+T tmain(T argc, T *argv) {
+  T i_def, i;
+
+  i = argc;
+
+#pragma omp target enter data map(i_def)
+
+#pragma omp target enter data map(to: i)
+
+  return 0;
+}
+
+// CHECK: template  T tmain(T argc, T *argv) {
+// CHECK-NEXT: T i_def, i;
+// CHECK-NEXT: i = argc;
+// CHECK-NEXT: #pragma omp target enter data map(to: i_def){{$}}
+// CHECK-NEXT: #pragma omp target enter data map(to: i){{$}}
+
+// CHECK: template<> int tmain(int argc, int *argv) {
+// CHECK-NEXT: int i_def, i;
+// CHECK-NEXT: i = argc;
+// CHECK-NEXT: #pragma omp target enter data map(to: i_def){{$}}
+// CHECK-NEXT: #pragma omp target enter data map(to: i)
+
+// CHECK: template<> char tmain(char argc, char *argv) {
+// CHECK-NEXT: char 

[PATCH] D138318: [-Wunsafe-buffer-usage] Improve pointer match pattern

2022-11-18 Thread Ziqing Luo via Phabricator via cfe-commits
ziqingluo-90 created this revision.
ziqingluo-90 added reviewers: NoQ, jkorous, t-rasmud, malavikasamak, 
aaron.ballman, gribozavr, xazax.hun.
Herald added a subscriber: rnkovacs.
Herald added a project: All.
ziqingluo-90 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch Improves the pointer-type matcher used by UnsafeBufferUsage checking 
by adding matchers for pointer types involving typedef, template instantiation, 
and decayed types.   Corresponding tests are added.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138318

Files:
  clang/lib/Analysis/UnsafeBufferUsage.cpp
  clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp

Index: clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
===
--- clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
+++ clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
@@ -1,4 +1,22 @@
-// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -verify %s
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -include %s -verify %s
+#ifndef INCLUDED
+#define INCLUDED
+#pragma clang system_header
+
+// no spanification warnings for system headers
+void foo(...);
+void testAsSystemHeader(char *p) {
+  ++p;
+
+  auto ap1 = p;
+  auto ap2 = 
+
+  foo(p[1],
+  ap1[1],
+  ap2[2][3]);
+}
+
+#else
 
 void testIncrement(char *p) {
   ++p; // expected-warning{{unchecked operation on raw buffer in expression}}
@@ -7,8 +25,6 @@
   p--; // expected-warning{{unchecked operation on raw buffer in expression}}
 }
 
-void foo(...);
-
 void * bar(void);
 char * baz(void);
 
@@ -33,7 +49,7 @@
 
   int a[10], b[10][10];
 
-  // not to warn subscripts on arrays
+  // Not to warn subscripts on arrays
   foo(a[0], a[1],
   0[a], 1[a],
   b[3][4],
@@ -46,17 +62,98 @@
 
   auto ap1 = a; // expected-warning{{variable 'ap1' participates in unchecked buffer operations}}
 
-  foo(ap1[0]);
+  foo(ap1[0]);  
 
   auto ap2 = p; // expected-warning{{variable 'ap2' participates in unchecked buffer operations}}
 
-  foo(ap2[0]);
+  foo(ap2[0]);  
 
-  auto ap3 = pp;
+  auto ap3 = pp;  // expected-warning{{variable 'ap3' participates in unchecked buffer operations}}
 
-  foo(pp[0][0]); // expected-warning2{{unchecked operation on raw buffer in expression}}
+  foo(ap3[0][0]); // expected-warning{{unchecked operation on raw buffer in expression}} 
 
   auto ap4 = *pp; // expected-warning{{variable 'ap4' participates in unchecked buffer operations}}
 
-  foo(ap4[0]);
+  foo(ap4[0]);  
+}
+
+void testQualifiedParameters(const int * p, const int * const q,
+			 const int a[10], const int b[10][10]) {
+  foo(p[1], 1[p], p[-1],   // expected-warning3{{unchecked operation on raw buffer in expression}} 
+  q[1], 1[q], q[-1],   // expected-warning3{{unchecked operation on raw buffer in expression}} 
+  a[1],// expected-warning{{unchecked operation on raw buffer in expression}} `a` is of pointer type
+  b[1][2]  // expected-warning{{unchecked operation on raw buffer in expression}} `b[1]` is of array type
+  );
+}
+
+struct T {
+  int a[10];
+  int * b;
+  struct {
+int a[10];
+int * b;
+  } c;
+};
+
+typedef struct T T_t;
+
+T_t funRetT();
+T_t * funRetTStar();
+
+void testStructMembers(struct T * sp, struct T s, T_t * sp2, T_t s2) {
+  foo(sp->a[1],
+  sp->b[1], // expected-warning{{unchecked operation on raw buffer in expression}} 
+  sp->c.a[1],
+  sp->c.b[1],   // expected-warning{{unchecked operation on raw buffer in expression}} 
+  s.a[1],
+  s.b[1],   // expected-warning{{unchecked operation on raw buffer in expression}} 
+  s.c.a[1],
+  s.c.b[1], // expected-warning{{unchecked operation on raw buffer in expression}} 
+  sp2->a[1],
+  sp2->b[1],// expected-warning{{unchecked operation on raw buffer in expression}} 
+  sp2->c.a[1],
+  sp2->c.b[1],  // expected-warning{{unchecked operation on raw buffer in expression}} 
+  s2.a[1],
+  s2.b[1],  // expected-warning{{unchecked operation on raw buffer in expression}} 
+  s2.c.a[1],
+  s2.c.b[1],   // expected-warning{{unchecked operation on raw buffer in expression}} 
+  funRetT().a[1],
+  funRetT().b[1],  // expected-warning{{unchecked operation on raw buffer in expression}}
+  funRetTStar()->a[1],
+  funRetTStar()->b[1]  // expected-warning{{unchecked operation on raw buffer in expression}}
+  );
 }
+
+int garray[10];
+int * gp = garray;
+int gvar = gp[1];  // TODO: this is not warned
+
+void testLambdaCaptureAndGlobal(int * p) {
+  int a[10];
+
+  auto Lam = [p, a]() {
+return p[1] // expected-warning2{{unchecked operation on raw buffer in expression}}
+  + a[1] + garray[1]
+  + gp[1];  // expected-warning2{{unchecked operation on raw buffer in expression}}
+  };
+}
+
+typedef T_t * T_ptr_t;
+
+void testTypedefs(T_ptr_t p) {
+  foo(p[1], 

[PATCH] D138263: [clang-format] Supress aligning of trailing namespace comments

2022-11-18 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment.

In D138263#3937223 , 
@HazardyKnusperkeks wrote:

> In D138263#3936536 , 
> @HazardyKnusperkeks wrote:
>
>> In D138263#3936007 , @owenpan 
>> wrote:
>>
>>> I suppose it's fairly easy to annotate the `l_brace` of a namespace? If so, 
>>> then wouldn't it be better to do that?
>>
>> But the `r_brace` has no `MatchingParen`, and I didn't want to go into that 
>> hole.
>
> One thing I thought about was adding a new `BlockKind`. But maybe I can look 
> into the code that sets that and can add the `MatchingParen`.

Instead, we can annotate the `r_brace` (in `UnwrappedLineParser::parseBlock`) 
after annotating the `l_brace`, which would enable us to keep the existing test 
case on line 4037 unchanged.




Comment at: clang/unittests/Format/FormatTest.cpp:4037
 "int i;\n"
-"} // my_namespace\n"
+"}  // my_namespace\n"
 "#endif // HEADER_GUARD",

This trailing comment was at the correct column and shouldn’t be aligned to the 
header guard comment below it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138263

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


[PATCH] D138263: [clang-format] Supress aligning of trailing namespace comments

2022-11-18 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks planned changes to this revision.
HazardyKnusperkeks added a comment.

Working on the annotation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138263

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


[PATCH] D137944: [ObjC][ARC] Teach the OptimizeSequences step of ObjCARCOpts about WinEH funclet tokens

2022-11-18 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

I can't say I'm familiar with ARC, but this seems right to me.




Comment at: llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp:767
+const ColorVector  = BlockColors.find(BB)->second;
+assert(CV.size() == 1 && "non-unique color for block!");
+BasicBlock *EHPadBB = CV.front();

I don't think the verifier changes you made guarantee this. I would consider 
strengthening this to `report_fatal_error`, since valid IR transforms can 
probably reach this code.



Comment at: llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp:2040-2041
+  DenseMap BlockColors;
+  if (F.hasPersonalityFn() &&
+  isScopedEHPersonality(classifyEHPersonality(F.getPersonalityFn(
+BlockColors = colorEHFunclets(F);

I think this code snippet probably deserves a Function helper, 
`F->hasScopedEHPersonality()`. Another part of me thinks we should cache the 
personality enum similar to the way we cache intrinsic ids, but I wouldn't want 
to increase Function memory usage, so that seems premature. "No action 
required", I guess.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137944

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


[PATCH] D138316: [ASTContext] Avoid duplicating address space map. NFCI

2022-11-18 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson created this revision.
arichardson added reviewers: clang, pcc.
Herald added a project: All.
arichardson requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

ASTContext was holding onto a pointer to the Clang->LLVM address space map
which is stored inside TargetInfo. Instead of doing this, we can forward to
TargetInfo instead. This change will allow us to eventually remove
getTargetAddressSpace() from ASTContext and only have this information in
TargetInfo.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138316

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/TargetInfo.cpp

Index: clang/lib/Basic/TargetInfo.cpp
===
--- clang/lib/Basic/TargetInfo.cpp
+++ clang/lib/Basic/TargetInfo.cpp
@@ -24,6 +24,30 @@
 using namespace clang;
 
 static const LangASMap DefaultAddrSpaceMap = {0};
+// The fake address space map must have a distinct entry for each
+// language-specific address space.
+static const LangASMap FakeAddrSpaceMap = {
+0,  // Default
+1,  // opencl_global
+3,  // opencl_local
+2,  // opencl_constant
+0,  // opencl_private
+4,  // opencl_generic
+5,  // opencl_global_device
+6,  // opencl_global_host
+7,  // cuda_device
+8,  // cuda_constant
+9,  // cuda_shared
+1,  // sycl_global
+5,  // sycl_global_device
+6,  // sycl_global_host
+3,  // sycl_local
+0,  // sycl_private
+10, // ptr32_sptr
+11, // ptr32_uptr
+12, // ptr64
+13, // hlsl_groupshared
+};
 
 // TargetInfo Constructor.
 TargetInfo::TargetInfo(const llvm::Triple ) : Triple(T) {
@@ -487,6 +511,9 @@
 
   if (Opts.MaxBitIntWidth)
 MaxBitIntWidth = Opts.MaxBitIntWidth;
+
+  if (Opts.FakeAddressSpaceMap)
+AddrSpaceMap = 
 }
 
 bool TargetInfo::initFeatureMap(
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -929,39 +929,6 @@
   return *ParentMapCtx.get();
 }
 
-static const LangASMap *getAddressSpaceMap(const TargetInfo ,
-   const LangOptions ) {
-  if (LOpts.FakeAddressSpaceMap) {
-// The fake address space map must have a distinct entry for each
-// language-specific address space.
-static const unsigned FakeAddrSpaceMap[] = {
-0,  // Default
-1,  // opencl_global
-3,  // opencl_local
-2,  // opencl_constant
-0,  // opencl_private
-4,  // opencl_generic
-5,  // opencl_global_device
-6,  // opencl_global_host
-7,  // cuda_device
-8,  // cuda_constant
-9,  // cuda_shared
-1,  // sycl_global
-5,  // sycl_global_device
-6,  // sycl_global_host
-3,  // sycl_local
-0,  // sycl_private
-10, // ptr32_sptr
-11, // ptr32_uptr
-12, // ptr64
-13, // hlsl_groupshared
-};
-return 
-  } else {
-return ();
-  }
-}
-
 static bool isAddrSpaceMapManglingEnabled(const TargetInfo ,
   const LangOptions ) {
   switch (LangOpts.getAddressSpaceMapMangling()) {
@@ -1292,7 +1259,6 @@
   this->AuxTarget = AuxTarget;
 
   ABI.reset(createCXXABI(Target));
-  AddrSpaceMap = getAddressSpaceMap(Target, LangOpts);
   AddrSpaceMapMangling = isAddrSpaceMapManglingEnabled(Target, LangOpts);
 
   // C99 6.2.5p19.
@@ -12228,10 +12194,7 @@
 }
 
 unsigned ASTContext::getTargetAddressSpace(LangAS AS) const {
-  if (isTargetAddressSpace(AS))
-return toTargetAddressSpace(AS);
-  else
-return (*AddrSpaceMap)[(unsigned)AS];
+  return getTargetInfo().getTargetAddressSpace(AS);
 }
 
 bool ASTContext::hasSameExpr(const Expr *X, const Expr *Y) const {
Index: clang/include/clang/AST/ASTContext.h
===
--- clang/include/clang/AST/ASTContext.h
+++ clang/include/clang/AST/ASTContext.h
@@ -640,9 +640,6 @@
   std::unique_ptr ABI;
   CXXABI *createCXXABI(const TargetInfo );
 
-  /// The logical -> physical address space map.
-  const LangASMap *AddrSpaceMap = nullptr;
-
   /// Address space map mangling must be used with language specific
   /// address spaces (e.g. OpenCL/CUDA)
   bool AddrSpaceMapMangling;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138081: [IR] Split out IR printing passes into IRPrinter

2022-11-18 Thread Alexander Shaposhnikov via Phabricator via cfe-commits
alexander-shaposhnikov added inline comments.



Comment at: utils/bazel/llvm-project-overlay/llvm/BUILD.bazel:918
 ":Core",
+":IRPrinter",
 ":IRReader",

tejohnson wrote:
> Per the follow on fix to D137768, I guess this one was unnecessary. Can you 
> check if there are any others added that are similarly unnecessary and can be 
> removed?
Yeah, correct. I'll check other places.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138081

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


[PATCH] D137818: Add support for querying SubstTemplateTypeParm types

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

LGTM, thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137818

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


[PATCH] D138088: [clang][docs] Use `option` directive in User's Manual

2022-11-18 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

Thank you for this cleanup! In general, I thin this looks correct. However, I 
know we've had to fix a bunch of options that cause the sphinx build to fail 
(IIRC, oftentimes due to duplicate options) and our precommit CI doesn't test 
the documentation build. Did you try building the docs locally to ensure there 
are no new warnings/errors from Sphinx?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138088

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


[PATCH] D137885: [modules] Support zstd in .pcm file

2022-11-18 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added inline comments.



Comment at: clang/lib/Serialization/ASTReader.cpp:1457
+  const llvm::compression::Format F =
+  Blob.size() >= 2 && memcmp(Blob.data(), "\x1f\x8b", 2) == 0
+  ? llvm::compression::Format::Zlib

MaskRay wrote:
> dblaikie wrote:
> > tschuett wrote:
> > > Could you put the magic number into a named constant?
> > & perhaps we should have/test a magic number for zstd too, so it's clear 
> > it's one or the other and not something else added in the future (since 
> > this isn't reving the bitcode version or anything - reading zstd compressed 
> > data with the library before this version will I guess/hopefully appear 
> > corrupted, but we could avoid that being the failure mode in the future 
> > when another compression scheme is added by checking explicitly for 
> > zstd/zlib now and reporting unknown compression scheme otherwise?)?
> > Could you put the magic number into a named constant?
> 
> I think a constant doesn't improve readability of this used-once value with a 
> comment.
> 
> > & perhaps we should have/test a magic number for zstd too, so it's clear 
> > it's one or the other and not something else added in the future
> 
> Since the version number keeps increasing, when a new algorithm is added, 
> they will see a version mismatch error...
> > & perhaps we should have/test a magic number for zstd too, so it's clear 
> > it's one or the other and not something else added in the future
> 
> Since the version number keeps increasing, when a new algorithm is added, 
> they will see a version mismatch error...

Oh, right, sorry, got muddled - thinking bitcode & then thinking LLVM IR 
bitcode guarantees, super stable, but the AST's super unstable/closely version 
locked anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137885

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


[PATCH] D138312: [OPENMP5.1] Initial support for message clause.

2022-11-18 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev accepted this revision.
ABataev added a comment.
This revision is now accepted and ready to land.

LG


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138312

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


[PATCH] D138091: [Clang] Fix Sema::ClassifyName so that it classifies EnumConstantDecl as NonType when they are brought into scope via using enum

2022-11-18 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added a comment.

Adding some more reviewers to get more eyes who may be familiar with this 
corner for the code.


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

https://reviews.llvm.org/D138091

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


[PATCH] D137885: [modules] Support zstd in .pcm file

2022-11-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay marked 2 inline comments as done.
MaskRay added inline comments.



Comment at: clang/lib/Serialization/ASTReader.cpp:1457
+  const llvm::compression::Format F =
+  Blob.size() >= 2 && memcmp(Blob.data(), "\x1f\x8b", 2) == 0
+  ? llvm::compression::Format::Zlib

dblaikie wrote:
> tschuett wrote:
> > Could you put the magic number into a named constant?
> & perhaps we should have/test a magic number for zstd too, so it's clear it's 
> one or the other and not something else added in the future (since this isn't 
> reving the bitcode version or anything - reading zstd compressed data with 
> the library before this version will I guess/hopefully appear corrupted, but 
> we could avoid that being the failure mode in the future when another 
> compression scheme is added by checking explicitly for zstd/zlib now and 
> reporting unknown compression scheme otherwise?)?
> Could you put the magic number into a named constant?

I think a constant doesn't improve readability of this used-once value with a 
comment.

> & perhaps we should have/test a magic number for zstd too, so it's clear it's 
> one or the other and not something else added in the future

Since the version number keeps increasing, when a new algorithm is added, they 
will see a version mismatch error...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137885

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


[clang] 7fc57d7 - Add more tests for C DRs and update the status page

2022-11-18 Thread Aaron Ballman via cfe-commits

Author: Aaron Ballman
Date: 2022-11-18T14:15:41-05:00
New Revision: 7fc57d7c97c64b64ef865d71343867ab30cfcf15

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

LOG: Add more tests for C DRs and update the status page

Added: 
clang/test/C/drs/dr464.c
clang/test/C/drs/dr466.c
clang/test/C/drs/dr483.c
clang/test/C/drs/dr491.c
clang/test/C/drs/dr494.c

Modified: 
clang/test/C/drs/dr4xx.c
clang/www/c_dr_status.html

Removed: 




diff  --git a/clang/test/C/drs/dr464.c b/clang/test/C/drs/dr464.c
new file mode 100644
index 0..a9c00b2eb4b19
--- /dev/null
+++ b/clang/test/C/drs/dr464.c
@@ -0,0 +1,19 @@
+/* RUN: %clang_cc1 -std=c89 -verify -pedantic -Wno-c11-extensions %s
+   RUN: %clang_cc1 -std=c99 -verify -pedantic -Wno-c11-extensions %s
+   RUN: %clang_cc1 -std=c11 -verify -pedantic %s
+   RUN: %clang_cc1 -std=c17 -verify -pedantic %s
+   RUN: %clang_cc1 -std=c2x -verify -pedantic %s
+ */
+
+/* expected-no-diagnostics */
+
+/* WG14 DR464: yes
+ * Clarifying the Behavior of the #line Directive
+ *
+ * Note: the behavior described by this DR allows for two 
diff erent
+ * interpretations, but WG14 N2322 (adopted for C2x) adds a recommended
+ * practice which is what we're testing our interpretation against.
+ */
+#line 1
+_Static_assert(__LI\
+NE__ == 1, "");

diff  --git a/clang/test/C/drs/dr466.c b/clang/test/C/drs/dr466.c
new file mode 100644
index 0..6b5811dbf9ddc
--- /dev/null
+++ b/clang/test/C/drs/dr466.c
@@ -0,0 +1,30 @@
+/* RUN: %clang_cc1 -std=c89 -Wno-gcc-compat -ast-dump -o -  %s | FileCheck %s
+   RUN: %clang_cc1 -std=c99 -ast-dump -o -  %s | FileCheck %s
+   RUN: %clang_cc1 -std=c11 -ast-dump -o -  %s | FileCheck %s
+   RUN: %clang_cc1 -std=c17 -ast-dump -o -  %s | FileCheck %s
+   RUN: %clang_cc1 -std=c2x -ast-dump -o -  %s | FileCheck %s
+ */
+
+/* WG14 DR466: yes
+ * Scope of a for loop control declaration
+ */
+int dr466(void) {
+  for (int i = 0; ; ) {
+long i = 1;   /* valid C, invalid C++ */
+// ...
+return i; /* (perhaps unexpectedly) returns 1 in C */
+  }
+}
+
+/*
+CHECK: FunctionDecl 0x{{.+}} dr466 'int (void)'
+CHECK-NEXT: CompoundStmt
+CHECK-NEXT: ForStmt
+CHECK-NEXT: DeclStmt
+CHECK-NEXT: VarDecl 0x{{.+}} {{.+}} i 'int'
+CHECK: CompoundStmt
+CHECK-NEXT: DeclStmt
+CHECK-NEXT: VarDecl [[ACTUAL:0x.+]]  col:{{.+}} used i 'long'
+CHECK: ReturnStmt
+CHECK: DeclRefExpr 0x{{.+}}  'long' lvalue Var [[ACTUAL]] 'i' 
'long'
+*/

diff  --git a/clang/test/C/drs/dr483.c b/clang/test/C/drs/dr483.c
new file mode 100644
index 0..2d4b7bc08723e
--- /dev/null
+++ b/clang/test/C/drs/dr483.c
@@ -0,0 +1,22 @@
+/* RUN: %clang_cc1 -std=c89 -verify -pedantic -Wno-c11-extensions %s
+   RUN: %clang_cc1 -std=c99 -verify -pedantic -Wno-c11-extensions %s
+   RUN: %clang_cc1 -std=c11 -verify -pedantic %s
+   RUN: %clang_cc1 -std=c17 -verify -pedantic %s
+   RUN: %clang_cc1 -std=c2x -verify -pedantic %s
+ */
+
+/* expected-no-diagnostics */
+
+/* WG14 DR483: yes
+ * __LINE__ and __FILE__ in macro replacement list
+ *
+ * The crux of this DR is to ensure that __LINE__ (and __FILE__) use in a macro
+ * replacement list report the line and file of the expansion of that macro,
+ * not the line and file of the macro definition itself.
+ */
+#line 500
+#define MAC() __LINE__
+
+#line 1000
+_Static_assert(MAC() == 1000, "");
+

diff  --git a/clang/test/C/drs/dr491.c b/clang/test/C/drs/dr491.c
new file mode 100644
index 0..d59c27b6afd94
--- /dev/null
+++ b/clang/test/C/drs/dr491.c
@@ -0,0 +1,30 @@
+/* RUN: %clang_cc1 -std=c89 -verify -Wreserved-macro-identifier %s
+   RUN: %clang_cc1 -std=c99 -verify -Wreserved-macro-identifier %s
+   RUN: %clang_cc1 -std=c11 -verify -Wreserved-macro-identifier %s
+   RUN: %clang_cc1 -std=c17 -verify -Wreserved-macro-identifier %s
+   RUN: %clang_cc1 -std=c2x -verify -Wreserved-macro-identifier %s
+ */
+
+/* WG14 DR491: partial
+ * Concern with Keywords that Match Reserved Identifiers
+ *
+ * Claiming this as partial because we do not reject code using a reserved
+ * identifier, but our reserved identifier code incorrectly identifies some
+ * keywords as reserved identifiers for macro names, but not others.
+ */
+
+#define const const
+#define int int
+#define restrict restrict
+
+/* FIXME: none of these should diagnose the macro name as a reserved
+ * identifier per C2x 6.4.2p7 (similar wording existed in earlier standard
+ * versions).
+ */
+#define _Static_assert _Static_assert  /* expected-warning {{macro name is a 
reserved identifier}} */
+#define _Alignof(x) _Alignof(x)/* expected-warning {{macro name is a 
reserved identifier}} */
+#define _Bool _Bool/* expected-warning {{macro name is a 
reserved identifier}} */
+#define __has_c_attribute 

[PATCH] D138312: [OPENMP5.1] Initial support for message clause.

2022-11-18 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:11048
   SeverityClauses.empty() ? nullptr : (*SeverityClauses.begin());
+  auto MessageClauses =
+  OMPExecutableDirective::getClausesOfKind(Clauses);

ABataev wrote:
> Use real type instead of auto
Thanks.  How about using getSingleClause.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138312

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


[PATCH] D138312: [OPENMP5.1] Initial support for message clause.

2022-11-18 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 updated this revision to Diff 476534.
jyu2 added a comment.

Thanks Alexey for the review.  Address his comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138312

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/CodeGen/CGStmtOpenMP.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/error_ast_print.cpp
  clang/test/OpenMP/error_message.cpp
  clang/tools/libclang/CIndex.cpp
  flang/lib/Semantics/check-omp-structure.cpp
  llvm/include/llvm/Frontend/OpenMP/OMP.td

Index: llvm/include/llvm/Frontend/OpenMP/OMP.td
===
--- llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -307,6 +307,9 @@
 def OMPC_Severity : Clause<"severity"> {
   let clangClass = "OMPSeverityClause";
 }
+def OMPC_Message : Clause<"message"> {
+  let clangClass = "OMPMessageClause";
+}
 def OMPC_Allocate : Clause<"allocate"> {
   let clangClass = "OMPAllocateClause";
   let flangClass = "OmpAllocateClause";
@@ -536,7 +539,8 @@
 def OMP_Error : Directive<"error"> {
   let allowedClauses = [
 VersionedClause,
-VersionedClause
+VersionedClause,
+VersionedClause
   ];
 }
 def OMP_TaskWait : Directive<"taskwait"> {
Index: flang/lib/Semantics/check-omp-structure.cpp
===
--- flang/lib/Semantics/check-omp-structure.cpp
+++ flang/lib/Semantics/check-omp-structure.cpp
@@ -1870,6 +1870,7 @@
 CHECK_SIMPLE_CLAUSE(Nocontext, OMPC_nocontext)
 CHECK_SIMPLE_CLAUSE(At, OMPC_at)
 CHECK_SIMPLE_CLAUSE(Severity, OMPC_severity)
+CHECK_SIMPLE_CLAUSE(Message, OMPC_message)
 CHECK_SIMPLE_CLAUSE(Filter, OMPC_filter)
 CHECK_SIMPLE_CLAUSE(When, OMPC_when)
 CHECK_SIMPLE_CLAUSE(AdjustArgs, OMPC_adjust_args)
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -2447,6 +2447,8 @@
 
 void OMPClauseEnqueue::VisitOMPSeverityClause(const OMPSeverityClause *) {}
 
+void OMPClauseEnqueue::VisitOMPMessageClause(const OMPMessageClause *) {}
+
 void OMPClauseEnqueue::VisitOMPDeviceClause(const OMPDeviceClause *C) {
   Visitor->AddStmt(C->getDevice());
 }
Index: clang/test/OpenMP/error_message.cpp
===
--- clang/test/OpenMP/error_message.cpp
+++ clang/test/OpenMP/error_message.cpp
@@ -94,6 +94,26 @@
 #pragma omp error at(execution) severity(warning) // no error, diagnosic at runtime
 #pragma omp error at(compilation) severity(fatal) // expected-error {{ERROR}}
 #pragma omp error at(execution) severity(fatal) // no error, error at runtime
+
+#pragma omp error message("GPU compiler is needed.") // expected-error {{GPU compiler is needed}}
+#pragma omp error at(compilation) message("GPU compiler is needed.") // expected-error {{GPU compiler is needed}}
+#pragma omp error at(execution) message("GPU compiler is needed.") // no error
+// expected-warning@+1 {{GPU compiler is needed.}}
+#pragma omp error severity(warning) message("GPU compiler is needed.") // expected-warning {{GPU compiler is needed.}}
+#pragma omp error severity(fatal) message("GPU compiler is needed.") // expected-error {{GPU compiler is needed}}
+// expected-warning@+1 {{GPU compiler is needed.}}
+#pragma omp error at(compilation) severity(warning) message("GPU compiler is needed.") // expected-warning {{GPU compiler is needed.}}
+#pragma omp error at(compilation) severity(fatal) message("GPU compiler is needed.") // expected-error {{GPU compiler is needed.}}
+#pragma omp error at(execution) severity(warning) message("GPU compiler is needed.") // no warning warning will emit at runtime.
+#pragma omp error at(execution) severity(fatal) message("GPU compiler is needed.") // no warning warning will emit at runtime.
+
+// expected-error@+1 {{GPU compiler is needed.}}
+#pragma omp error message("GPU compiler is needed.") message("GPU compiler is needed.") // expected-error {{directive '#pragma omp error' cannot contain more than one 'message' clause}}
+  int a;
+// expected-warning@+1 {{expected string literal in 'clause message' - ignoring}}
+#pragma omp error message(a) // expected-error {{ERROR}}
+// expected-error@+1 {{ERROR}}
+#pragma omp error message() // expected-error {{expected expression}}
   return T();
 }
 
Index: clang/test/OpenMP/error_ast_print.cpp
===
--- clang/test/OpenMP/error_ast_print.cpp
+++ 

[clang] f01b68e - [Hexagon] Add checks for immediate arguments for remaining builtins

2022-11-18 Thread Krzysztof Parzyszek via cfe-commits

Author: Krzysztof Parzyszek
Date: 2022-11-18T11:09:41-08:00
New Revision: f01b68e4bee7ccf92f125b1100367804b43f6a7e

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

LOG: [Hexagon] Add checks for immediate arguments for remaining builtins

Checks for builtins for the following instructions were aded:
  V6_v6mpyhubs10
  V6_v6mpyhubs10_vxx
  V6_v6mpyvubs10
  V6_v6mpyvubs10_vxx
  V6_vlutvvbi
  V6_vlutvvb_oracci
  V6_vlutvwhi
  V6_vlutvwh_oracci

Added: 


Modified: 
clang/lib/Sema/SemaChecking.cpp
clang/test/Headers/hexagon-hvx-headers.c

Removed: 




diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 75fb822f392dd..e3b1d5f7f9e45 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -3629,6 +3629,31 @@ bool Sema::CheckHexagonBuiltinArgument(unsigned 
BuiltinID, CallExpr *TheCall) {
 { Hexagon::BI__builtin_HEXAGON_V6_vrsadubi_acc,   {{ 3, false, 1,  0 }} },
 { Hexagon::BI__builtin_HEXAGON_V6_vrsadubi_acc_128B,
   {{ 3, false, 1,  0 }} },
+
+{ Hexagon::BI__builtin_HEXAGON_V6_v6mpyhubs10,{{ 2, false, 2,  0 }} },
+{ Hexagon::BI__builtin_HEXAGON_V6_v6mpyhubs10_128B,
+  {{ 2, false, 2,  0 }} },
+{ Hexagon::BI__builtin_HEXAGON_V6_v6mpyhubs10_vxx,
+  {{ 3, false, 2,  0 }} },
+{ Hexagon::BI__builtin_HEXAGON_V6_v6mpyhubs10_vxx_128B,
+  {{ 3, false, 2,  0 }} },
+{ Hexagon::BI__builtin_HEXAGON_V6_v6mpyvubs10,{{ 2, false, 2,  0 }} },
+{ Hexagon::BI__builtin_HEXAGON_V6_v6mpyvubs10_128B,
+  {{ 2, false, 2,  0 }} },
+{ Hexagon::BI__builtin_HEXAGON_V6_v6mpyvubs10_vxx,
+  {{ 3, false, 2,  0 }} },
+{ Hexagon::BI__builtin_HEXAGON_V6_v6mpyvubs10_vxx_128B,
+  {{ 3, false, 2,  0 }} },
+{ Hexagon::BI__builtin_HEXAGON_V6_vlutvvbi,   {{ 2, false, 3,  0 }} },
+{ Hexagon::BI__builtin_HEXAGON_V6_vlutvvbi_128B,  {{ 2, false, 3,  0 }} },
+{ Hexagon::BI__builtin_HEXAGON_V6_vlutvvb_oracci, {{ 3, false, 3,  0 }} },
+{ Hexagon::BI__builtin_HEXAGON_V6_vlutvvb_oracci_128B,
+  {{ 3, false, 3,  0 }} },
+{ Hexagon::BI__builtin_HEXAGON_V6_vlutvwhi,   {{ 2, false, 3,  0 }} },
+{ Hexagon::BI__builtin_HEXAGON_V6_vlutvwhi_128B,  {{ 2, false, 3,  0 }} },
+{ Hexagon::BI__builtin_HEXAGON_V6_vlutvwh_oracci, {{ 3, false, 3,  0 }} },
+{ Hexagon::BI__builtin_HEXAGON_V6_vlutvwh_oracci_128B,
+  {{ 3, false, 3,  0 }} },
   };
 
   // Use a dynamically initialized static to sort the table exactly once on

diff  --git a/clang/test/Headers/hexagon-hvx-headers.c 
b/clang/test/Headers/hexagon-hvx-headers.c
index afea9a6bee298..835dde4956a1d 100644
--- a/clang/test/Headers/hexagon-hvx-headers.c
+++ b/clang/test/Headers/hexagon-hvx-headers.c
@@ -33,5 +33,5 @@ void test_hvx_protos(float a, unsigned int b) {
   HVX_VectorPair c;
   // CHECK-64: call <32 x i32> @llvm.hexagon.V6.v6mpyhubs10
   // CHECK:call <64 x i32> @llvm.hexagon.V6.v6mpyhubs10.128B
-  c = Q6_Ww_v6mpy_WubWbI_h(c, c, 12);
+  c = Q6_Ww_v6mpy_WubWbI_h(c, c, 2);
 }



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


[PATCH] D138078: [SelectionDAGISel] split critical indirect edges from callbr w/ outputs

2022-11-18 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers updated this revision to Diff 476530.
nickdesaulniers retitled this revision from "[CodeGenPrepare] split critical 
indirect edges from callbr w/ outputs" to "[SelectionDAGISel] split critical 
indirect edges from callbr w/ outputs".
nickdesaulniers edited the summary of this revision.
nickdesaulniers added a comment.

- move critical edge splitting from CodeGenPrepare to SelectionDAGISel


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138078

Files:
  clang/docs/ReleaseNotes.rst
  llvm/docs/LangRef.rst
  llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
  llvm/lib/Transforms/Utils/BreakCriticalEdges.cpp
  llvm/test/CodeGen/AArch64/callbr-split-critedge.ll
  llvm/test/CodeGen/X86/callbr-asm-outputs.ll

Index: llvm/test/CodeGen/X86/callbr-asm-outputs.ll
===
--- llvm/test/CodeGen/X86/callbr-asm-outputs.ll
+++ llvm/test/CodeGen/X86/callbr-asm-outputs.ll
@@ -38,36 +38,46 @@
 ; CHECK-NEXT:pushl %esi
 ; CHECK-NEXT:movl {{[0-9]+}}(%esp), %edi
 ; CHECK-NEXT:movl {{[0-9]+}}(%esp), %esi
-; CHECK-NEXT:movl $-1, %eax
 ; CHECK-NEXT:cmpl %edi, %esi
-; CHECK-NEXT:jge .LBB1_2
+; CHECK-NEXT:jge .LBB1_3
 ; CHECK-NEXT:  # %bb.1: # %if.then
 ; CHECK-NEXT:#APP
 ; CHECK-NEXT:testl %esi, %esi
 ; CHECK-NEXT:testl %edi, %esi
-; CHECK-NEXT:jne .LBB1_4
+; CHECK-NEXT:jne .LBB1_2
 ; CHECK-NEXT:#NO_APP
-; CHECK-NEXT:jmp .LBB1_3
-; CHECK-NEXT:  .LBB1_2: # %if.else
+; CHECK-NEXT:jmp .LBB1_4
+; CHECK-NEXT:  .LBB1_2: # Block address taken
+; CHECK-NEXT:# %if.then.label_true_crit_edge
+; CHECK-NEXT:# Label of block must be emitted
+; CHECK-NEXT:jmp .LBB1_8
+; CHECK-NEXT:  .LBB1_3: # %if.else
 ; CHECK-NEXT:#APP
 ; CHECK-NEXT:testl %esi, %edi
 ; CHECK-NEXT:testl %esi, %edi
-; CHECK-NEXT:jne .LBB1_5
+; CHECK-NEXT:jne .LBB1_9
 ; CHECK-NEXT:#NO_APP
-; CHECK-NEXT:  .LBB1_3:
+; CHECK-NEXT:  .LBB1_4:
 ; CHECK-NEXT:movl %esi, %eax
 ; CHECK-NEXT:addl %edi, %eax
-; CHECK-NEXT:  .LBB1_5: # Block address taken
-; CHECK-NEXT:# %return
-; CHECK-NEXT:# Label of block must be emitted
+; CHECK-NEXT:  .LBB1_5: # %return
 ; CHECK-NEXT:popl %esi
 ; CHECK-NEXT:popl %edi
 ; CHECK-NEXT:retl
-; CHECK-NEXT:  .LBB1_4: # Block address taken
-; CHECK-NEXT:# %label_true
+; CHECK-NEXT:  .LBB1_7: # Block address taken
+; CHECK-NEXT:# %if.else.label_true_crit_edge
 ; CHECK-NEXT:# Label of block must be emitted
+; CHECK-NEXT:  .LBB1_8: # %label_true
 ; CHECK-NEXT:movl $-2, %eax
 ; CHECK-NEXT:jmp .LBB1_5
+; CHECK-NEXT:  .LBB1_9: # Block address taken
+; CHECK-NEXT:# %if.else.return_crit_edge
+; CHECK-NEXT:# Label of block must be emitted
+; CHECK-NEXT:  .LBB1_6: # Block address taken
+; CHECK-NEXT:# %if.then.return_crit_edge
+; CHECK-NEXT:# Label of block must be emitted
+; CHECK-NEXT:movl $-1, %eax
+; CHECK-NEXT:jmp .LBB1_5
 entry:
   %cmp = icmp slt i32 %out1, %out2
   br i1 %cmp, label %if.then, label %if.else
@@ -122,7 +132,10 @@
 ; CHECK-NEXT:popl %edi
 ; CHECK-NEXT:retl
 ; CHECK-NEXT:  .LBB2_6: # Block address taken
-; CHECK-NEXT:# %indirect
+; CHECK-NEXT:# %true.indirect_crit_edge
+; CHECK-NEXT:# Label of block must be emitted
+; CHECK-NEXT:  .LBB2_7: # Block address taken
+; CHECK-NEXT:# %false.indirect_crit_edge
 ; CHECK-NEXT:# Label of block must be emitted
 ; CHECK-NEXT:movl $42, %eax
 ; CHECK-NEXT:jmp .LBB2_5
@@ -148,31 +161,37 @@
 define i32 @test4(i32 %out1, i32 %out2) {
 ; CHECK-LABEL: test4:
 ; CHECK:   # %bb.0: # %entry
-; CHECK-NEXT:movl $-1, %eax
-; CHECK-NEXT:movl {{[0-9]+}}(%esp), %ecx
+; CHECK-NEXT:movl {{[0-9]+}}(%esp), %eax
 ; CHECK-NEXT:#APP
-; CHECK-NEXT:testl %ecx, %ecx
-; CHECK-NEXT:testl %edx, %ecx
+; CHECK-NEXT:testl %eax, %eax
+; CHECK-NEXT:testl %ecx, %eax
 ; CHECK-NEXT:jne .LBB3_3
 ; CHECK-NEXT:#NO_APP
 ; CHECK-NEXT:  # %bb.1: # %asm.fallthrough
 ; CHECK-NEXT:#APP
-; CHECK-NEXT:testl %ecx, %edx
-; CHECK-NEXT:testl %ecx, %edx
-; CHECK-NEXT:jne .LBB3_4
+; CHECK-NEXT:testl %eax, %ecx
+; CHECK-NEXT:testl %eax, %ecx
+; CHECK-NEXT:jne .LBB3_5
 ; CHECK-NEXT:#NO_APP
 ; CHECK-NEXT:  # %bb.2: # %asm.fallthrough2
-; CHECK-NEXT:addl %edx, %ecx
-; CHECK-NEXT:movl %ecx, %eax
+; CHECK-NEXT:addl %ecx, %eax
+; CHECK-NEXT:retl
 ; CHECK-NEXT:  .LBB3_4: # Block address taken
-; CHECK-NEXT:# %return
+; CHECK-NEXT:# %entry.return_crit_edge
+; CHECK-NEXT:# Label of block must be emitted
+; CHECK-NEXT:  .LBB3_5: # Block address taken
+; CHECK-NEXT:# %asm.fallthrough.return_crit_edge
 ; CHECK-NEXT:# Label of block must be emitted
+; CHECK-NEXT:movl $-1, %eax
 ; CHECK-NEXT:retl
+; CHECK-NEXT:  .LBB3_6: # Block address taken
+; 

[PATCH] D138312: [OPENMP5.1] Initial support for message clause.

2022-11-18 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:11048
   SeverityClauses.empty() ? nullptr : (*SeverityClauses.begin());
+  auto MessageClauses =
+  OMPExecutableDirective::getClausesOfKind(Clauses);

Use real type instead of auto


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138312

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


[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-18 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n added a comment.

In D137181#3935951 , @owenpan wrote:

> In D137181#3935856 , @goldstein.w.n 
> wrote:
>
>> In D137181#3935752 , @owenpan 
>> wrote:
>>
>>> Please mark review comments as done if you have addressed them. Can you 
>>> also clean up the test cases, removing overlapping/redundant ones, making 
>>> sure a test case doesn't end with a newline (e.g., line 5380), etc?
>>
>> Regarding newlines. I have new line before changing the style. Otherwise 
>> done. (let me know if you want me to remove those).
>
> Sorry, it's line 5379 and several other places that have an ending newline. 
> (Just search for `\n",`.) You only need to remove them in the new test cases. 
> We can fix the existing ones in a separate NFC patch.

Done. Sorry I had thought you mean the newline in the file between the test 
cases.

>> Regarding removing redundant test cases. I think I've had each of them fail 
>> independently at some point (although many when I was doing the previous 
>> PPLevel tracking) so I wouldn't call any of them truly redundant.
>
> I know, but IMO some of the failures (e.g. the `switch` statement) during 
> development shouldn't automatically go in. This file already has a humongous 
> size. Maybe do the best you can here.
>
>> I could remove either the `PPDIS_BeforeHash` or `PPDIS_AfterHash` though as 
>> the they really no independent logic in this commit.
>
> Yes, please. Or better yet, alternate a little bit between the two.

Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137181

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


[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-18 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n updated this revision to Diff 476528.
goldstein.w.n added a comment.



1. Updating D137181 : [clang-format] Don't 
use 'PPIndentWidth' inside multi-line macros #
2. Enter a brief description of the changes included in this update.
3. The first line is used as subject, next lines as comment. #
4. If you intended to create a new revision, use:
5. $ arc diff --create

Cleanup tests file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137181

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -5040,6 +5040,222 @@
"  int y = 0;\n"
"}",
style);
+
+  style.IndentPPDirectives = FormatStyle::PPDIS_None;
+  verifyFormat("#ifdef foo\n"
+   "#define bar() \\\n"
+   "if (A) {  \\\n"
+   "B();  \\\n"
+   "} \\\n"
+   "C();\n"
+   "#endif",
+   style);
+  verifyFormat("if (emacs) {\n"
+   "#ifdef is\n"
+   "#define lit   \\\n"
+   "if (af) { \\\n"
+   "return duh(); \\\n"
+   "}\n"
+   "#endif\n"
+   "}",
+   style);
+  verifyFormat("#if abc\n"
+   "#ifdef foo\n"
+   "#define bar()\\\n"
+   "if (A) { \\\n"
+   "if (B) { \\\n"
+   "C(); \\\n"
+   "}\\\n"
+   "}\\\n"
+   "D();\n"
+   "#endif\n"
+   "#endif",
+   style);
+  verifyFormat("#ifndef foo\n"
+   "#define foo\n"
+   "if (emacs) {\n"
+   "#ifdef is\n"
+   "#define lit   \\\n"
+   "if (af) { \\\n"
+   "return duh(); \\\n"
+   "}\n"
+   "#endif\n"
+   "}\n"
+   "#endif",
+   style);
+  verifyFormat("#if 1\n"
+   "#define X  \\\n"
+   "{  \\\n"
+   "x; \\\n"
+   "x; \\\n"
+   "}\n"
+   "#endif",
+   style);
+  verifyFormat("#define X  \\\n"
+   "{  \\\n"
+   "x; \\\n"
+   "x; \\\n"
+   "}",
+   style);
+
+  style.PPIndentWidth = 2;
+  verifyFormat("#ifdef foo\n"
+   "#define bar() \\\n"
+   "if (A) {  \\\n"
+   "B();  \\\n"
+   "} \\\n"
+   "C();\n"
+   "#endif",
+   style);
+  style.IndentWidth = 8;
+  verifyFormat("#ifdef foo\n"
+   "#define bar()\\\n"
+   "if (A) { \\\n"
+   "B(); \\\n"
+   "}\\\n"
+   "C();\n"
+   "#endif",
+   style);
+
+  style.IndentWidth = 1;
+  style.PPIndentWidth = 4;
+  verifyFormat("#if 1\n"
+   "#define X \\\n"
+   " {\\\n"
+   "  x;  \\\n"
+   "  x;  \\\n"
+   " }\n"
+   "#endif",
+   style);
+  verifyFormat("#define X \\\n"
+   " {\\\n"
+   "  x;  \\\n"
+   "  x;  \\\n"
+   " }",
+   style);
+
+  style.IndentWidth = 4;
+  style.PPIndentWidth = 1;
+  style.IndentPPDirectives = FormatStyle::PPDIS_AfterHash;
+  verifyFormat("#ifdef foo\n"
+   "# define bar() \\\n"
+   " if (A) {  \\\n"
+   " B();  \\\n"
+   " } \\\n"
+   " C();\n"
+   "#endif",
+   style);
+
+  style.IndentPPDirectives = FormatStyle::PPDIS_BeforeHash;
+  verifyFormat("if (emacs) {\n"
+   "#ifdef is\n"
+   " #define lit   \\\n"
+   " if (af) { \\\n"
+   " return duh(); \\\n"
+   " }\n"
+   "#endif\n"
+   "}",
+   style);
+
+  style.IndentPPDirectives = FormatStyle::PPDIS_AfterHash;
+  verifyFormat("#if abc\n"
+   "# ifdef foo\n"
+   "#  define bar()\\\n"
+   "  if (A) { \\\n"
+   "  

[PATCH] D138296: [clang] Avoid duplicating ProgramAddressSpace in TargetInfo. NFCI

2022-11-18 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

In D138296#3937486 , @arichardson 
wrote:

> In D138296#3937224 , @eandrews 
> wrote:
>
>> Functionally this looks ok to me. However I am not sure if CodeGenTypes is 
>> the 'right' place for this function to live, considering that other 
>> functions with similar functionality are in ASTContext - including overloads 
>> of getTargetAddressSpace( ). @erichkeane @aaron.ballman could you please 
>> take a look?
>
> My view is that the parts that interact with LLVM IR should really live in 
> CodeGen/ and not Basic/ or AST/. I will see how difficult it would be to move 
> the remaining target (LLVM IR) address space handling code to CodeGen/

Yeah, I don't think there's a good reason for some of the address-space stuff 
that currently lives in Basic to be there instead of in CodeGen.  Basic/AST 
need to understand what address spaces exist, their sizes, and what 
relationships they have with each other, and that's it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138296

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


[PATCH] D138312: [OPENMP5.1] Initial support for message clause.

2022-11-18 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 created this revision.
jyu2 added reviewers: ABataev, jdoerfert, mikerice.
jyu2 added a project: OpenMP.
Herald added subscribers: arphaman, guansong, yaxunl.
Herald added projects: Flang, All.
jyu2 requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, sstefan1.
Herald added projects: clang, LLVM.

Initial support for message clause.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138312

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/CodeGen/CGStmtOpenMP.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/error_ast_print.cpp
  clang/test/OpenMP/error_message.cpp
  clang/tools/libclang/CIndex.cpp
  flang/lib/Semantics/check-omp-structure.cpp
  llvm/include/llvm/Frontend/OpenMP/OMP.td

Index: llvm/include/llvm/Frontend/OpenMP/OMP.td
===
--- llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -307,6 +307,9 @@
 def OMPC_Severity : Clause<"severity"> {
   let clangClass = "OMPSeverityClause";
 }
+def OMPC_Message : Clause<"message"> {
+  let clangClass = "OMPMessageClause";
+}
 def OMPC_Allocate : Clause<"allocate"> {
   let clangClass = "OMPAllocateClause";
   let flangClass = "OmpAllocateClause";
@@ -536,7 +539,8 @@
 def OMP_Error : Directive<"error"> {
   let allowedClauses = [
 VersionedClause,
-VersionedClause
+VersionedClause,
+VersionedClause
   ];
 }
 def OMP_TaskWait : Directive<"taskwait"> {
Index: flang/lib/Semantics/check-omp-structure.cpp
===
--- flang/lib/Semantics/check-omp-structure.cpp
+++ flang/lib/Semantics/check-omp-structure.cpp
@@ -1870,6 +1870,7 @@
 CHECK_SIMPLE_CLAUSE(Nocontext, OMPC_nocontext)
 CHECK_SIMPLE_CLAUSE(At, OMPC_at)
 CHECK_SIMPLE_CLAUSE(Severity, OMPC_severity)
+CHECK_SIMPLE_CLAUSE(Message, OMPC_message)
 CHECK_SIMPLE_CLAUSE(Filter, OMPC_filter)
 CHECK_SIMPLE_CLAUSE(When, OMPC_when)
 CHECK_SIMPLE_CLAUSE(AdjustArgs, OMPC_adjust_args)
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -2447,6 +2447,8 @@
 
 void OMPClauseEnqueue::VisitOMPSeverityClause(const OMPSeverityClause *) {}
 
+void OMPClauseEnqueue::VisitOMPMessageClause(const OMPMessageClause *) {}
+
 void OMPClauseEnqueue::VisitOMPDeviceClause(const OMPDeviceClause *C) {
   Visitor->AddStmt(C->getDevice());
 }
Index: clang/test/OpenMP/error_message.cpp
===
--- clang/test/OpenMP/error_message.cpp
+++ clang/test/OpenMP/error_message.cpp
@@ -94,6 +94,26 @@
 #pragma omp error at(execution) severity(warning) // no error, diagnosic at runtime
 #pragma omp error at(compilation) severity(fatal) // expected-error {{ERROR}}
 #pragma omp error at(execution) severity(fatal) // no error, error at runtime
+
+#pragma omp error message("GPU compiler is needed.") // expected-error {{GPU compiler is needed}}
+#pragma omp error at(compilation) message("GPU compiler is needed.") // expected-error {{GPU compiler is needed}}
+#pragma omp error at(execution) message("GPU compiler is needed.") // no error
+// expected-warning@+1 {{GPU compiler is needed.}}
+#pragma omp error severity(warning) message("GPU compiler is needed.") // expected-warning {{GPU compiler is needed.}}
+#pragma omp error severity(fatal) message("GPU compiler is needed.") // expected-error {{GPU compiler is needed}}
+// expected-warning@+1 {{GPU compiler is needed.}}
+#pragma omp error at(compilation) severity(warning) message("GPU compiler is needed.") // expected-warning {{GPU compiler is needed.}}
+#pragma omp error at(compilation) severity(fatal) message("GPU compiler is needed.") // expected-error {{GPU compiler is needed.}}
+#pragma omp error at(execution) severity(warning) message("GPU compiler is needed.") // no warning warning will emit at runtime.
+#pragma omp error at(execution) severity(fatal) message("GPU compiler is needed.") // no warning warning will emit at runtime.
+
+// expected-error@+1 {{GPU compiler is needed.}}
+#pragma omp error message("GPU compiler is needed.") message("GPU compiler is needed.") // expected-error {{directive '#pragma omp error' cannot contain more than one 'message' clause}}
+  int a;
+// expected-warning@+1 {{expected string literal in 'clause message' - ignoring}}
+#pragma omp error message(a) // expected-error {{ERROR}}
+// expected-error@+1 {{ERROR}}
+#pragma omp error message() // expected-error {{expected expression}}
   return T();
 }
 

[PATCH] D137885: [modules] Support zstd in .pcm file

2022-11-18 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added inline comments.



Comment at: clang/lib/Serialization/ASTReader.cpp:1457
+  const llvm::compression::Format F =
+  Blob.size() >= 2 && memcmp(Blob.data(), "\x1f\x8b", 2) == 0
+  ? llvm::compression::Format::Zlib

tschuett wrote:
> Could you put the magic number into a named constant?
& perhaps we should have/test a magic number for zstd too, so it's clear it's 
one or the other and not something else added in the future (since this isn't 
reving the bitcode version or anything - reading zstd compressed data with the 
library before this version will I guess/hopefully appear corrupted, but we 
could avoid that being the failure mode in the future when another compression 
scheme is added by checking explicitly for zstd/zlib now and reporting unknown 
compression scheme otherwise?)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137885

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


[PATCH] D138247: PR58819: Correct linkage and mangling of lambdas in inline static member initializers

2022-11-18 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.

In D138247#3936928 , @probinson wrote:

> If this is specifically for C++17, I believe Sony doesn't officially support 
> that yet although I am checking.

Cool, thanks!

> It looks like this is only part of the fix for #58819? The original report 
> also had a `static int n` with an internal-linkage name.

Nah, it's the full fix - but the static member handling works correctly once 
you fix the type mangling itself, so the bug is in the type linkage/mangling, 
the static member was just the canary in the coal mine/a more visible effect of 
the bug. I could add test coverage for it, but by default I prefer to test more 
narrowly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138247

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


[PATCH] D138137: [CodeGen][ARM] Fix ARMABIInfo::EmitVAAarg crash with empty record type variadic arg

2022-11-18 Thread Lin Yurong via Phabricator via cfe-commits
yronglin added a comment.

Thank you so much for your review, @rjmccall , can you land this patch for me? 
Please use 'yronglin ' to commit the change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138137

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


[PATCH] D138296: [clang] Avoid duplicating ProgramAddressSpace in TargetInfo. NFCI

2022-11-18 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson added a comment.

In D138296#3937224 , @eandrews wrote:

> Functionally this looks ok to me. However I am not sure if CodeGenTypes is 
> the 'right' place for this function to live, considering that other functions 
> with similar functionality are in ASTContext - including overloads of 
> getTargetAddressSpace( ). @erichkeane @aaron.ballman could you please take a 
> look?

My view is that the parts that interact with LLVM IR should really live in 
CodeGen/ and not Basic/ or AST/. I will see how difficult it would be to move 
the remaining target (LLVM IR) address space handling code to CodeGen/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138296

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


[clang] 7c47669 - [Hexagon] Add clang flags for v71, v71t, v73

2022-11-18 Thread Krzysztof Parzyszek via cfe-commits

Author: Krzysztof Parzyszek
Date: 2022-11-18T09:39:47-08:00
New Revision: 7c476697e29988adcd25036d4e187cd2efcf0c6a

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

LOG: [Hexagon] Add clang flags for v71, v71t, v73

Added: 


Modified: 
clang/include/clang/Driver/Options.td
clang/lib/Basic/Targets/Hexagon.cpp
clang/test/Driver/hexagon-hvx.c
clang/test/Driver/hexagon-toolchain-elf.c
clang/test/Misc/target-invalid-cpu-note.c
clang/test/Preprocessor/hexagon-predefines.c

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index b5287300ecf08..d67dd6f99a9c4 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4470,6 +4470,12 @@ def mv68 : Flag<["-"], "mv68">, 
Group,
   Alias, AliasArgs<["hexagonv68"]>;
 def mv69 : Flag<["-"], "mv69">, Group,
   Alias, AliasArgs<["hexagonv69"]>;
+def mv71 : Flag<["-"], "mv71">, Group,
+  Alias, AliasArgs<["hexagonv71"]>;
+def mv71t : Flag<["-"], "mv71t">, Group,
+  Alias, AliasArgs<["hexagonv71t"]>;
+def mv73 : Flag<["-"], "mv73">, Group,
+  Alias, AliasArgs<["hexagonv73"]>;
 def mhexagon_hvx : Flag<["-"], "mhvx">, Group,
   HelpText<"Enable Hexagon Vector eXtensions">;
 def mhexagon_hvx_EQ : Joined<["-"], "mhvx=">,

diff  --git a/clang/lib/Basic/Targets/Hexagon.cpp 
b/clang/lib/Basic/Targets/Hexagon.cpp
index 161369242926e..11e63f0853f7e 100644
--- a/clang/lib/Basic/Targets/Hexagon.cpp
+++ b/clang/lib/Basic/Targets/Hexagon.cpp
@@ -71,6 +71,15 @@ void HexagonTargetInfo::getTargetDefines(const LangOptions 
,
   } else if (CPU == "hexagonv69") {
 Builder.defineMacro("__HEXAGON_V69__");
 Builder.defineMacro("__HEXAGON_ARCH__", "69");
+  } else if (CPU == "hexagonv71") {
+Builder.defineMacro("__HEXAGON_V71__");
+Builder.defineMacro("__HEXAGON_ARCH__", "71");
+  } else if (CPU == "hexagonv71t") {
+Builder.defineMacro("__HEXAGON_V71T__");
+Builder.defineMacro("__HEXAGON_ARCH__", "71");
+  } else if (CPU == "hexagonv73") {
+Builder.defineMacro("__HEXAGON_V73__");
+Builder.defineMacro("__HEXAGON_ARCH__", "73");
   }
 
   if (hasFeature("hvx-length64b")) {
@@ -222,6 +231,8 @@ static constexpr CPUSuffix Suffixes[] = {
 {{"hexagonv65"}, {"65"}}, {{"hexagonv66"},  {"66"}},
 {{"hexagonv67"}, {"67"}}, {{"hexagonv67t"}, {"67t"}},
 {{"hexagonv68"}, {"68"}}, {{"hexagonv69"},  {"69"}},
+{{"hexagonv71"}, {"71"}}, {{"hexagonv71t"},  {"71t"}},
+{{"hexagonv73"}, {"73"}},
 };
 
 const char *HexagonTargetInfo::getHexagonCPUSuffix(StringRef Name) {

diff  --git a/clang/test/Driver/hexagon-hvx.c b/clang/test/Driver/hexagon-hvx.c
index 7533e0da9f12e..49cd86a928f89 100644
--- a/clang/test/Driver/hexagon-hvx.c
+++ b/clang/test/Driver/hexagon-hvx.c
@@ -28,6 +28,12 @@
 // RUN:  2>&1 | FileCheck -check-prefix=CHECK-HVX-OFF %s
 // RUN: %clang -c %s -### -target hexagon-unknown-elf -mv69 \
 // RUN:  2>&1 | FileCheck -check-prefix=CHECK-HVX-OFF %s
+// RUN: %clang -c %s -### -target hexagon-unknown-elf -mv71 \
+// RUN:  2>&1 | FileCheck -check-prefix=CHECK-HVX-OFF %s
+// RUN: %clang -c %s -### -target hexagon-unknown-elf -mv71t \
+// RUN:  2>&1 | FileCheck -check-prefix=CHECK-HVX-OFF %s
+// RUN: %clang -c %s -### -target hexagon-unknown-elf -mv73 \
+// RUN:  2>&1 | FileCheck -check-prefix=CHECK-HVX-OFF %s
 
 // Infer HVX version from flag:
 
@@ -38,6 +44,8 @@
 // CHECK-HVX-V67: "-target-feature" "+hvxv67"
 // CHECK-HVX-V68: "-target-feature" "+hvxv68"
 // CHECK-HVX-V69: "-target-feature" "+hvxv69"
+// CHECK-HVX-V71: "-target-feature" "+hvxv71"
+// CHECK-HVX-V73: "-target-feature" "+hvxv73"
 
 // Direct version flag:
 // RUN: %clang -c %s -### -target hexagon-unknown-elf -mhvx=v60 \
@@ -54,6 +62,10 @@
 // RUN:  2>&1 | FileCheck -check-prefix=CHECK-HVX-V68 %s
 // RUN: %clang -c %s -### -target hexagon-unknown-elf -mhvx=v69 \
 // RUN:  2>&1 | FileCheck -check-prefix=CHECK-HVX-V69 %s
+// RUN: %clang -c %s -### -target hexagon-unknown-elf -mhvx=v71 \
+// RUN:  2>&1 | FileCheck -check-prefix=CHECK-HVX-V71 %s
+// RUN: %clang -c %s -### -target hexagon-unknown-elf -mhvx=v73 \
+// RUN:  2>&1 | FileCheck -check-prefix=CHECK-HVX-V73 %s
 // Infer HVX version from CPU version:
 // RUN: %clang -c %s -### -target hexagon-unknown-elf -mv60 -mhvx \
 // RUN:  2>&1 | FileCheck -check-prefix=CHECK-HVX-V60 %s
@@ -71,6 +83,10 @@
 // RUN:  2>&1 | FileCheck -check-prefix=CHECK-HVX-V68 %s
 // RUN: %clang -c %s -### -target hexagon-unknown-elf -mv69 -mhvx \
 // RUN:  2>&1 | FileCheck -check-prefix=CHECK-HVX-V69 %s
+// RUN: %clang -c %s -### -target hexagon-unknown-elf -mv71 -mhvx \
+// RUN:  2>&1 | FileCheck -check-prefix=CHECK-HVX-V71 %s
+// RUN: %clang -c %s -### -target hexagon-unknown-elf -mv73 -mhvx \
+// RUN:  2>&1 | FileCheck 

[PATCH] D138258: clang/cmake: Fix incorrectly disabling tests when LLVM_EXTERNAL_LIT is used

2022-11-18 Thread Tom Stellard via Phabricator via cfe-commits
tstellar added inline comments.



Comment at: clang/CMakeLists.txt:97
 # Seek installed Lit.
-find_program(LLVM_LIT
- NAMES llvm-lit lit.py lit
- PATHS "${LLVM_MAIN_SRC_DIR}/utils/lit"
- DOC "Path to lit.py")
+if (NOT LLVM_EXTERNAL_LIT)
+  find_program(LLVM_EXTERNAL_LIT

kwk wrote:
> mgorny wrote:
> > I don't think you need to do this if you rename the var, `find_program()` 
> > doesn't seem to do any searching when the var is already set.
> > 
> > I've tested with a dummy CMakeLists:
> > 
> > ```
> > set(FOO /bin/true)
> > find_program(FOO NAMES foo)
> > message(FATAL_ERROR ${FOO})
> > ```
> > 
> > gives `/bin/true` for me.
> > I don't think you need to do this if you rename the var, `find_program()` 
> > doesn't seem to do any searching when the var is already set.
> 
> This might be true but it is counter intuitive to assume that `find_program` 
> does nothing. So for readability I'd keep the `if (NOT LLVM_EXTERNAL_LIT)`. I 
> know it is not needed but with the `if` it won't look like 
> `find_program(LLVM_EXTERNAL_LIT` is the only/central place to set the 
> variable `LLVM_EXTERNAL_LIT`. With the guard around it, it becomes more 
> obvious that this is more of  a fallback.
@mgorny  The [[ http://devdoc.net/linux/cmake-3.9.6/command/find_program.html | 
documentation ]] confirms this too:

"Once one of the calls succeeds the result variable will be set and stored in 
the cache so that no call will search again."


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138258

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


[PATCH] D138258: clang/cmake: Fix incorrectly disabling tests when LLVM_EXTERNAL_LIT is used

2022-11-18 Thread Tom Stellard via Phabricator via cfe-commits
tstellar added a comment.

In D138258#3936260 , @kwk wrote:

> As much as I would like this to be fixed. I vote against this patch because 
> in `lld/CMakeLists.txt` there's an almost (if not entirely) identical piece 
> of code 
> 
>  that screams to be outsourced into a `/cmake/Modules/FindLit.cmake` (to be 
> created). I'll have a look at this and see if I can come up with a patch for 
> this. Afterall `/cmake` is the central place to distribute shared CMake code 
> between subprojects, right @phosek (didn't you create `/cmake` in the first 
> place)?

My eventual goal is to move this out into a separate CMake file, but I was 
hoping to simplify the code first before doing this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138258

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


[PATCH] D137205: [clang-tidy] Add performance-unnecessary-copy-on-last-use check

2022-11-18 Thread Oliver Stöneberg via Phabricator via cfe-commits
firewave added a comment.

In D137205#3937298 , @Febbe wrote:

> Does only a warning appear? Or also a fix? Currently, only a warning should 
> appear, but this is not fixable in c++11.

There's no fix-it. I didn't realize that before.

Maybe this should also be added to the documentation as a known limitation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137205

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


  1   2   >