date:20230811

[PATCH] D157680: [X86]Support options -mno-gather -mno-scatter

2023-08-11 Thread Phoebe Wang via Phabricator via cfe-commits

pengfei added inline comments.



Comment at: llvm/lib/Target/X86/X86.td:437
+: SubtargetFeature<"prefer-no-gather", "PreferGather", "false",
+   "Indicates if gather prefer to be disabled">;
+def FeaturePreferNoScatter

XinWang10 wrote:
> skan wrote:
> > Does "Prefer no gather instructions" sound better?
> > 
> > I think these two should be put under "X86 Subtarget Tuning features"?
> I think the two options are to mitigate security issues. Could refer to link 
> in summary.
It depends on if the micro code was applied. We should assume user care of this 
option should have applied the micro code. So it's more like a performance 
turning rather than mitigation. And you cannot disable all gather/scatter 
instructions with these options.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157680

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

[PATCH] D157332: [clang] Make init for empty no_unique_address fields a no-op write

2023-08-11 Thread Martin Storsjö via Phabricator via cfe-commits

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

Thanks a lot for the guidance!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157332

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

[PATCH] D157332: [clang] Make init for empty no_unique_address fields a no-op write

2023-08-11 Thread Martin Storsjö via Phabricator via cfe-commits

mstorsjo updated this revision to Diff 549575.
mstorsjo added a comment.

Move the existing comment into the new if statement, add a comment to the new 
if. Add a comment to the second part of the testcase, which probably is good to 
keep anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157332

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CodeGenCXX/ctor-empty-nounique.cpp


Index: clang/test/CodeGenCXX/ctor-empty-nounique.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/ctor-empty-nounique.cpp
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple x86_64-windows-gnu -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64le-windows-gnu -emit-llvm -o - %s | 
FileCheck %s
+
+// An empty struct is handled as a struct with a dummy i8, on all targets.
+// Most targets treat an empty struct return value as essentially void - but
+// some don't. (Currently, at least x86_64-windows-* and powerpc64le-* don't
+// treat it as void.)
+//
+// When intializing a struct with such a no_unique_address member, make sure we
+// don't write the dummy i8 into the struct where there's no space allocated 
for
+// it.
+//
+// This can only be tested with targets that don't treat empty struct returns 
as
+// void.
+
+struct S {};
+S f();
+struct Z {
+  int x;
+  [[no_unique_address]] S y;
+  Z();
+};
+Z::Z() : x(111), y(f()) {}
+
+// CHECK: define {{.*}} @_ZN1ZC2Ev
+// CHECK: %call = call i8 @_Z1fv()
+// CHECK-NEXT: ret void
+
+
+// Check that the constructor for an empty member gets called with the right
+// 'this' pointer.
+
+struct S2 {
+  S2();
+};
+struct Z2 {
+  int x;
+  [[no_unique_address]] S2 y;
+  Z2();
+};
+Z2::Z2() : x(111) {}
+
+// CHECK: define {{.*}} @_ZN2Z2C2Ev(ptr {{.*}} %this)
+// CHECK: %this.addr = alloca ptr
+// CHECK: store ptr %this, ptr %this.addr
+// CHECK: %this1 = load ptr, ptr %this.addr
+// CHECK: call void @_ZN2S2C1Ev(ptr {{.*}} %this1)
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -13,6 +13,7 @@
 
 #include "CGCall.h"
 #include "ABIInfo.h"
+#include "ABIInfoImpl.h"
 #include "CGBlocks.h"
 #include "CGCXXABI.h"
 #include "CGCleanup.h"
@@ -5777,9 +5778,13 @@
 DestIsVolatile = false;
   }
 
-  // If the value is offset in memory, apply the offset now.
-  Address StorePtr = emitAddressAtOffset(*this, DestPtr, RetAI);
-  CreateCoercedStore(CI, StorePtr, DestIsVolatile, *this);
+  // An empty record can overlap other data (if declared with 
no_unique_address);
+  // omit the store for such types - as there is no actual data to store.
+  if (!isEmptyRecord(getContext(), RetTy, true)) {
+// If the value is offset in memory, apply the offset now.
+Address StorePtr = emitAddressAtOffset(*this, DestPtr, RetAI);
+CreateCoercedStore(CI, StorePtr, DestIsVolatile, *this);
+  }
 
   return convertTempToRValue(DestPtr, RetTy, SourceLocation());
 }


Index: clang/test/CodeGenCXX/ctor-empty-nounique.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/ctor-empty-nounique.cpp
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple x86_64-windows-gnu -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64le-windows-gnu -emit-llvm -o - %s | FileCheck %s
+
+// An empty struct is handled as a struct with a dummy i8, on all targets.
+// Most targets treat an empty struct return value as essentially void - but
+// some don't. (Currently, at least x86_64-windows-* and powerpc64le-* don't
+// treat it as void.)
+//
+// When intializing a struct with such a no_unique_address member, make sure we
+// don't write the dummy i8 into the struct where there's no space allocated for
+// it.
+//
+// This can only be tested with targets that don't treat empty struct returns as
+// void.
+
+struct S {};
+S f();
+struct Z {
+  int x;
+  [[no_unique_address]] S y;
+  Z();
+};
+Z::Z() : x(111), y(f()) {}
+
+// CHECK: define {{.*}} @_ZN1ZC2Ev
+// CHECK: %call = call i8 @_Z1fv()
+// CHECK-NEXT: ret void
+
+
+// Check that the constructor for an empty member gets called with the right
+// 'this' pointer.
+
+struct S2 {
+  S2();
+};
+struct Z2 {
+  int x;
+  [[no_unique_address]] S2 y;
+  Z2();
+};
+Z2::Z2() : x(111) {}
+
+// CHECK: define {{.*}} @_ZN2Z2C2Ev(ptr {{.*}} %this)
+// CHECK: %this.addr = alloca ptr
+// CHECK: store ptr %this, ptr %this.addr
+// CHECK: %this1 = load ptr, ptr %this.addr
+// CHECK: call void @_ZN2S2C1Ev(ptr {{.*}} %this1)
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -13,6 +13,7 @@
 
 #include "CGCall.h"
 #include "ABIInfo.h"
+#include "ABIInfoImpl.h"
 #include

[PATCH] D157757: [Headers] Replace __need_STDDEF_H_misc with specific __need_ macros

2023-08-11 Thread Ian Anderson via Phabricator via cfe-commits

iana updated this revision to Diff 549572.
iana added a comment.

Let clang-format update stddef.h


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157757

Files:
  clang/lib/Headers/stddef.h
  clang/test/Headers/stddef.c
  clang/test/Headers/stddefneeds.c

Index: clang/test/Headers/stddefneeds.c
===
--- /dev/null
+++ clang/test/Headers/stddefneeds.c
@@ -0,0 +1,169 @@
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.9.0 -verify=c99 -std=c99 %s
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.9.0 -verify=c23 -std=c23 %s
+
+// Use C99 to verify that __need_ can be used to get types that wouldn't normally be available.
+
+struct astruct { char member; };
+
+ptrdiff_t p0; // c99-error{{unknown}} c23-error{{unknown}}
+size_t s0; // c99-error{{unknown}} c23-error{{unknown}}
+rsize_t r0; // c99-error{{unknown}} c23-error{{unknown}}
+wchar_t wc0; // c99-error{{unknown}} c23-error{{unknown}}
+void *v0 = NULL; // c99-error{{undeclared}} c23-error{{undeclared}}
+nullptr_t n0; // c99-error{{unknown}} c23-error{{unknown}}
+static void f0(void) { unreachable(); } // c99-error{{undeclared}} c23-error{{undeclared}}
+max_align_t m0; // c99-error{{unknown}} c23-error{{unknown}}
+size_t o0 = offsetof(struct astruct, member); // c99-error{{unknown}} c99-error{{undeclared}} c99-error{{expected expression}} c99-error{{undeclared}} \
+ c23-error{{unknown}} c23-error{{undeclared}} c23-error{{expected expression}} c23-error{{undeclared}}
+wint_t wi0; // c99-error{{unknown}} c23-error{{unknown}}
+
+#define __need_ptrdiff_t
+#include 
+
+ptrdiff_t p1;
+size_t s1; // c99-error{{unknown}} c23-error{{unknown}}
+rsize_t r1; // c99-error{{unknown}} c23-error{{unknown}}
+wchar_t wc1; // c99-error{{unknown}} c23-error{{unknown}}
+void *v1 = NULL; // c99-error{{undeclared}} c23-error{{undeclared}}
+nullptr_t n1; // c99-error{{unknown}} c23-error{{unknown}}
+static void f1(void) { unreachable(); } // c99-error{{undeclared}} c23-error{{undeclared}}
+max_align_t m1; // c99-error{{unknown}} c23-error{{unknown}}
+size_t o1 = offsetof(struct astruct, member); // c99-error{{unknown}} c99-error{{undeclared}} c99-error{{expected expression}} \
+ c23-error{{unknown}} c23-error{{undeclared}} c23-error{{expected expression}} c23-error{{undeclared}}
+wint_t wi1; // c99-error{{unknown}} c23-error{{unknown}}
+
+#define __need_size_t
+#include 
+
+ptrdiff_t p2;
+size_t s2;
+rsize_t r2; // c99-error{{unknown}} c23-error{{unknown}}
+// c99-note@stddef.h:*{{'size_t' declared here}} c23-note@stddef.h:*{{'size_t' declared here}}
+wchar_t wc2; // c99-error{{unknown}} c23-error{{unknown}}
+void *v2 = NULL; // c99-error{{undeclared}} c23-error{{undeclared}}
+nullptr_t n2; // c99-error{{unknown}} c23-error{{unknown}}
+static void f2(void) { unreachable(); } // c99-error{{undeclared}} c23-error{{undeclared}}
+max_align_t m2; // c99-error{{unknown}} c23-error{{unknown}}
+size_t o2 = offsetof(struct astruct, member); // c99-error{{undeclared}} c99-error{{expected expression}} \
+ c23-error{{undeclared}} c23-error{{expected expression}} c23-error{{undeclared}}
+wint_t wi2; // c99-error{{unknown}} c23-error{{unknown}}
+
+#define __need_rsize_t
+#include 
+
+ptrdiff_t p3;
+size_t s3;
+rsize_t r3;
+wchar_t wc3; // c99-error{{unknown}} c23-error{{unknown}}
+void *v3 = NULL; // c99-error{{undeclared}} c23-error{{undeclared}}
+nullptr_t n3; // c99-error{{unknown}} c23-error{{unknown}}
+static void f3(void) { unreachable(); } // c99-error{{undeclared}} c23-error{{undeclared}}
+max_align_t m3; // c99-error{{unknown}} c23-error{{unknown}}
+size_t o3 = offsetof(struct astruct, member); // c99-error{{undeclared}} c99-error{{expected expression}} \
+ c23-error{{undeclared}} c23-error{{expected expression}} c23-error{{undeclared}}
+wint_t wi3; // c99-error{{unknown}} c23-error{{unknown}}
+
+#define __need_wchar_t
+#include 
+
+ptrdiff_t p4;
+size_t s4;
+rsize_t r4;
+wchar_t wc4;
+void *v4 = NULL; // c99-error{{undeclared}} c23-error{{undeclared}}
+nullptr_t n4; // c99-error{{unknown}} c23-error{{unknown}}
+static void f4(void) { unreachable(); } // c99-error{{undeclared}} c23-error{{undeclared}}
+max_align_t m4; // c99-error{{unknown}} c23-error{{unknown}}
+size_t o4 = offsetof(struct astruct, member); // c99-error{{undeclared}} c99-error{{expected expression}} \
+ c23-error{{undeclared}} c23-error{{expected expression}} c23-error{{undeclared}}
+wint_t wi4; // c99-error{{unknown}} c23-error{{unknown}}
+
+#define __need_NULL
+#include 
+
+ptrdiff_t p5;
+size_t s5;
+rsize_t r5;
+wchar_t wc5;
+void *v5 = NULL;
+nullptr_t n5; // c99-error{{unknown}}

[clang] c053345 - [clang] Fix typos in documentation

2023-08-11 Thread Kazu Hirata via cfe-commits


Author: Kazu Hirata
Date: 2023-08-11T21:44:33-07:00
New Revision: c053345b059bd3fa7f0e5f51498c71095b7a1d88

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

LOG: [clang] Fix typos in documentation

Added: 


Modified: 
clang/docs/analyzer/checkers.rst

Removed: 




diff  --git a/clang/docs/analyzer/checkers.rst 
b/clang/docs/analyzer/checkers.rst
index d3a4ebcaf02025..b7860af80c200d 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -2375,8 +2375,8 @@ The malicious data is injected at the taint source (e.g. 
``getenv()`` call)
 which is then propagated through function calls and being used as arguments of
 sensitive operations, also called as taint sinks (e.g. ``system()`` call).
 
-One can defend agains this type of vulnerability by always checking and
-santizing the potentially malicious, untrusted user input.
+One can defend against this type of vulnerability by always checking and
+sanitizing the potentially malicious, untrusted user input.
 
 The goal of the checker is to discover and show to the user these potential
 taint source-sink pairs and the propagation call chain.
@@ -2438,7 +2438,7 @@ Unfortunately, the checker cannot discover automatically 
that the programmer
 have performed data sanitation, so it still emits the warning.
 
 One can get rid of this superflous warning by telling by specifying the
-sanitation functions in the taint configuation file (see
+sanitation functions in the taint configuration file (see
 :doc:`user-docs/TaintAnalysisConfiguration`).
 
 .. code-block:: YAML



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

[PATCH] D157680: [X86]Support options -mno-gather -mno-scatter

2023-08-11 Thread Wang, Xin via Phabricator via cfe-commits

XinWang10 marked 4 inline comments as done.
XinWang10 added inline comments.



Comment at: llvm/lib/Target/X86/X86.td:437
+: SubtargetFeature<"prefer-no-gather", "PreferGather", "false",
+   "Indicates if gather prefer to be disabled">;
+def FeaturePreferNoScatter

skan wrote:
> Does "Prefer no gather instructions" sound better?
> 
> I think these two should be put under "X86 Subtarget Tuning features"?
I think the two options are to mitigate security issues. Could refer to link in 
summary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157680

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

[PATCH] D157680: [X86]Support options -mno-gather -mno-scatter

2023-08-11 Thread Wang, Xin via Phabricator via cfe-commits

XinWang10 updated this revision to Diff 549571.
XinWang10 added a comment.

- resolve comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157680

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Arch/X86.cpp
  clang/test/Driver/x86-no-gather-no-scatter.cpp
  llvm/lib/Target/X86/X86.td
  llvm/lib/Target/X86/X86TargetTransformInfo.cpp
  llvm/lib/Target/X86/X86TargetTransformInfo.h
  llvm/test/CodeGen/X86/x86-prefer-no-gather-no-scatter.ll

Index: llvm/test/CodeGen/X86/x86-prefer-no-gather-no-scatter.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/x86-prefer-no-gather-no-scatter.ll
@@ -0,0 +1,199 @@
+; Check that if option prefer-no-gather/scatter can disable gather/scatter instructions.
+; RUN: llc -mattr=+avx2,+fast-gather %s -o - | FileCheck %s --check-prefixes=GATHER
+; RUN: llc -mattr=+avx2,+fast-gather,+prefer-no-gather %s -o - | FileCheck %s --check-prefixes=NO-GATHER
+; RUN: llc -mtriple=x86_64-unknown-linux-gnu  -mattr=+avx512vl,+avx512dq < %s | FileCheck %s --check-prefix=SCATTER
+; RUN: llc -mtriple=x86_64-unknown-linux-gnu  -mattr=+avx512vl,+avx512dq,+prefer-no-gather < %s | FileCheck %s --check-prefix=SCATTER-NO-GATHER
+; RUN: llc -mtriple=x86_64-unknown-linux-gnu  -mattr=+avx512vl,+avx512dq,+prefer-no-scatter < %s | FileCheck %s --check-prefix=GATHER-NO-SCATTER
+; RUN: llc -mtriple=x86_64-unknown-linux-gnu  -mattr=+avx512vl,+avx512dq,+prefer-no-gather,+prefer-no-scatter < %s | FileCheck %s --check-prefix=NO-SCATTER-GATHER
+
+@A = global [1024 x i8] zeroinitializer, align 128
+@B = global [1024 x i64] zeroinitializer, align 128
+@C = global [1024 x i64] zeroinitializer, align 128
+
+; This tests the function that if prefer-no-gather can disable lowerMGather
+define void @test() #0 {
+; GATHER-LABEL: test:
+; GATHER: vpgatherdq
+;
+; NO-GATHER-LABEL: test:
+; NO-GATHER-NOT: vpgatherdq
+;
+; GATHER-NO-SCATTER-LABEL: test:
+; GATHER-NO-SCATTER: vpgatherdq
+;
+; NO-SCATTER-GATHER-LABEL: test:
+; NO-SCATTER-GATHER-NOT: vpgatherdq
+iter.check:
+  br i1 false, label %vec.epilog.scalar.ph, label %vector.main.loop.iter.check
+
+vector.main.loop.iter.check:  ; preds = %iter.check
+  br i1 false, label %vec.epilog.ph, label %vector.ph
+
+vector.ph:; preds = %vector.main.loop.iter.check
+  br label %vector.body
+
+vector.body:  ; preds = %vector.body, %vector.ph
+  %index = phi i64 [ 0, %vector.ph ], [ %index.next, %vector.body ]
+  %0 = add i64 %index, 0
+  %1 = getelementptr inbounds [1024 x i8], ptr @A, i64 0, i64 %0
+  %2 = getelementptr inbounds i8, ptr %1, i32 0
+  %wide.load = load <32 x i8>, ptr %2, align 1
+  %3 = sext <32 x i8> %wide.load to <32 x i64>
+  %4 = getelementptr inbounds [1024 x i64], ptr @B, i64 0, <32 x i64> %3
+  %wide.masked.gather = call <32 x i64> @llvm.masked.gather.v32i64.v32p0(<32 x ptr> %4, i32 8, <32 x i1> , <32 x i64> poison)
+  %5 = getelementptr inbounds [1024 x i64], ptr @C, i64 0, i64 %0
+  %6 = getelementptr inbounds i64, ptr %5, i32 0
+  store <32 x i64> %wide.masked.gather, ptr %6, align 8
+  %index.next = add nuw i64 %index, 32
+  %7 = icmp eq i64 %index.next, 1024
+  br i1 %7, label %middle.block, label %vector.body, !llvm.loop !0
+
+middle.block: ; preds = %vector.body
+  %cmp.n = icmp eq i64 1024, 1024
+  br i1 %cmp.n, label %for.cond.cleanup, label %vec.epilog.iter.check
+
+vec.epilog.iter.check:; preds = %middle.block
+  br i1 true, label %vec.epilog.scalar.ph, label %vec.epilog.ph
+
+vec.epilog.ph:; preds = %vector.main.loop.iter.check, %vec.epilog.iter.check
+  %vec.epilog.resume.val = phi i64 [ 1024, %vec.epilog.iter.check ], [ 0, %vector.main.loop.iter.check ]
+  br label %vec.epilog.vector.body
+
+vec.epilog.vector.body:   ; preds = %vec.epilog.vector.body, %vec.epilog.ph
+  %index2 = phi i64 [ %vec.epilog.resume.val, %vec.epilog.ph ], [ %index.next5, %vec.epilog.vector.body ]
+  %8 = add i64 %index2, 0
+  %9 = getelementptr inbounds [1024 x i8], ptr @A, i64 0, i64 %8
+  %10 = getelementptr inbounds i8, ptr %9, i32 0
+  %wide.load3 = load <16 x i8>, ptr %10, align 1
+  %11 = sext <16 x i8> %wide.load3 to <16 x i64>
+  %12 = getelementptr inbounds [1024 x i64], ptr @B, i64 0, <16 x i64> %11
+  %wide.masked.gather4 = call <16 x i64> @llvm.masked.gather.v16i64.v16p0(<16 x ptr> %12, i32 8, <16 x i1> , <16 x i64> poison)
+  %13 = getelementptr inbounds [1024 x i64], ptr @C, i64 0, i64 %8
+  %14 = getelementptr inbounds i64, ptr %13, i32 0
+  store <16 x i64> %wide.masked.gather4, ptr %14, align 8
+  %index.next5 = add nuw i64 %index2, 16
+  %15 = icmp eq i64 %index.next5, 1024
+  br i1 %15, label %vec.epilog.middle.block, label %vec.epilog.vector.body, !llvm.loop !2

[PATCH] D155610: [Clang][Sema] Fix display of characters on static assertion failure

2023-08-11 Thread Hubert Tong via Phabricator via cfe-commits

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16899
+  uint32_t CodePoint = 
static_cast(V.getInt().getZExtValue());
+  PrintCharLiteralPrefix(BTy->getKind(), OS);
+  OS << '\'';

hubert.reinterpretcast wrote:
> cor3ntin wrote:
> > Looking at the diagnostics, I don't think it makes sense to print a prefix 
> > here. You could just leave that part out.
> Why is removing the prefix better? The types can matter (characters outside 
> the basic character set are allowed to have negative `char` values). Also, 
> moving forward, the value of a character need not be the same in the various 
> encodings.
Some fun with signedness (imagine a more realistic example with `ISO-8859-1` 
ordinary character encoding with a signed `char` type):
```
$ clang -Xclang -fwchar-type=short -xc++ -<<<$'static_assert(L"\\uFF10"[0] == 
U\'\\uFF10\');'
:1:15: error: static assertion failed due to requirement 'L"\xFF10"[0] 
== U'\uff10''
1 | static_assert(L"\uFF10"[0] == U'\uFF10');
  |   ^
:1:28: note: expression evaluates to ''０' (0xFF10) == '０' (0xFF10)'
1 | static_assert(L"\uFF10"[0] == U'\uFF10');
  |   ~^~~~
1 error generated.
Return:  0x01:1   Fri Aug 11 23:49:02 2023 EDT
```


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

https://reviews.llvm.org/D155610

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

[PATCH] D156237: Complete the implementation of P2361 Unevaluated string literals

2023-08-11 Thread Hubert Tong via Phabricator via cfe-commits

hubert.reinterpretcast added inline comments.



Comment at: clang/test/Parser/c2x-attributes.c:29
+[[deprecated(L"abc")]] void unevaluated_string(void);
+// expected-warning@-1 {{encoding prefix 'L' on an unevaluated string literal 
has no effec}}
+

"Typo" fix.



Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:2336
 
+// Emits the list of arguments that shoulkd be parsed as unevaluated string
+// literals for each attributes

Typo fix



Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:2349
+  if (isVariadicStringLiteralArgument(Args[N])) {
+for (; N < sizeof(uint32_t); N++)
+  Bits |= (1 << N);

barannikov88 wrote:
> maskTrailingZeros might also be useful
@cor3ntin, this comment appears unaddressed.

Additionally, (just for future reference) `++N` is suggested for the loop: 
https://llvm.org/docs/CodingStandards.html#prefer-preincrement.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156237

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

[PATCH] D157767: [Driver] move Haiku header search path management to the driver

2023-08-11 Thread Brad Smith via Phabricator via cfe-commits

brad created this revision.
brad added a reviewer: nielx.
brad added a project: clang.
Herald added a project: All.
brad requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.

Here is a first cut at moving the Haiku header search path management to the 
driver as has been done for many other OS's to date.

InitHeaderSearch::AddDefaultCIncludePaths() currently does not skip adding 
/usr/local/include as well as /usr/include on Haiku. I installed
Haiku in a VM and see that neither path exists on an initial install. The other 
header paths and contents do exist out of the box. I have
not included those paths when moving the header path management to the driver 
so far.

I have not  built this on Haiku. I am looking for an actual Haiku user to test 
this.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157767

Files:
  clang/lib/Driver/ToolChains/Haiku.cpp
  clang/lib/Driver/ToolChains/Haiku.h
  clang/lib/Lex/InitHeaderSearch.cpp

Index: clang/lib/Lex/InitHeaderSearch.cpp
===
--- clang/lib/Lex/InitHeaderSearch.cpp
+++ clang/lib/Lex/InitHeaderSearch.cpp
@@ -279,42 +279,6 @@
 AddPath(P, System, false);
 break;
   }
-
-  case llvm::Triple::Haiku:
-AddPath("/boot/system/non-packaged/develop/headers", System, false);
-AddPath("/boot/system/develop/headers/os", System, false);
-AddPath("/boot/system/develop/headers/os/app", System, false);
-AddPath("/boot/system/develop/headers/os/arch", System, false);
-AddPath("/boot/system/develop/headers/os/device", System, false);
-AddPath("/boot/system/develop/headers/os/drivers", System, false);
-AddPath("/boot/system/develop/headers/os/game", System, false);
-AddPath("/boot/system/develop/headers/os/interface", System, false);
-AddPath("/boot/system/develop/headers/os/kernel", System, false);
-AddPath("/boot/system/develop/headers/os/locale", System, false);
-AddPath("/boot/system/develop/headers/os/mail", System, false);
-AddPath("/boot/system/develop/headers/os/media", System, false);
-AddPath("/boot/system/develop/headers/os/midi", System, false);
-AddPath("/boot/system/develop/headers/os/midi2", System, false);
-AddPath("/boot/system/develop/headers/os/net", System, false);
-AddPath("/boot/system/develop/headers/os/opengl", System, false);
-AddPath("/boot/system/develop/headers/os/storage", System, false);
-AddPath("/boot/system/develop/headers/os/support", System, false);
-AddPath("/boot/system/develop/headers/os/translation", System, false);
-AddPath("/boot/system/develop/headers/os/add-ons/graphics", System, false);
-AddPath("/boot/system/develop/headers/os/add-ons/input_server", System, false);
-AddPath("/boot/system/develop/headers/os/add-ons/mail_daemon", System, false);
-AddPath("/boot/system/develop/headers/os/add-ons/registrar", System, false);
-AddPath("/boot/system/develop/headers/os/add-ons/screen_saver", System, false);
-AddPath("/boot/system/develop/headers/os/add-ons/tracker", System, false);
-AddPath("/boot/system/develop/headers/os/be_apps/Deskbar", System, false);
-AddPath("/boot/system/develop/headers/os/be_apps/NetPositive", System, false);
-AddPath("/boot/system/develop/headers/os/be_apps/Tracker", System, false);
-AddPath("/boot/system/develop/headers/3rdparty", System, false);
-AddPath("/boot/system/develop/headers/bsd", System, false);
-AddPath("/boot/system/develop/headers/glibc", System, false);
-AddPath("/boot/system/develop/headers/posix", System, false);
-AddPath("/boot/system/develop/headers",  System, false);
-break;
   case llvm::Triple::RTEMS:
 break;
   case llvm::Triple::Win32:
@@ -388,6 +352,7 @@
   case llvm::Triple::PS4:
   case llvm::Triple::PS5:
   case llvm::Triple::Fuchsia:
+  case llvm::Triple::Haiku:
   case llvm::Triple::Hurd:
   case llvm::Triple::Linux:
   case llvm::Triple::Solaris:
Index: clang/lib/Driver/ToolChains/Haiku.h
===
--- clang/lib/Driver/ToolChains/Haiku.h
+++ clang/lib/Driver/ToolChains/Haiku.h
@@ -26,6 +26,9 @@
 return getTriple().getArch() == llvm::Triple::x86_64;
   }
 
+  void AddClangSystemIncludeArgs(
+  const llvm::opt::ArgList ,
+  llvm::opt::ArgStringList ) const override;
   void addLibCxxIncludePaths(
   const llvm::opt::ArgList ,
   llvm::opt::ArgStringList ) const override;
Index: clang/lib/Driver/ToolChains/Haiku.cpp
===
--- clang/lib/Driver/ToolChains/Haiku.cpp
+++ clang/lib/Driver/ToolChains/Haiku.cpp
@@ -8,6 +8,8 @@
 
 #include "Haiku.h"
 #include "CommonArgs.h"
+#include "clang/Config/config.h"
+#include "llvm/Support/Path.h"
 
 using namespace clang::driver;
 using namespace clang::driver::toolchains;
@@ -21,6 +23,70 @@
 
 }
 
+void Haiku::AddClangSystemIncludeArgs(const llvm::opt::ArgList ,

[PATCH] D155986: [clang][AMDGPU]: Don't use byval for struct arguments in function ABI

2023-08-11 Thread Jon Chesterfield via Phabricator via cfe-commits

JonChesterfield added a comment.

Thanks! Happy to see function calls getting cheaper


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155986

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

[PATCH] D157331: [clang] Implement C23

2023-08-11 Thread Zijun Zhao via Phabricator via cfe-commits

ZijunZhao added a comment.

In D157331#4576540 , @aaron.ballman 
wrote:

> In D157331#4575224 , @ZijunZhao 
> wrote:
>
>> Another followup question: I check 
>> https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2683.pdf and I only add 
>> Core Proposal here. Do I need to add Supplemental Proposal, like some types?
>
> Ah, this is a bit confusing! tl;dr: No need to add the supplemental proposal.
>
> The way to trace this down yourself is:
>
> - The working draft 
> (https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3096.pdf) has a list of 
> what papers were adopted and applied to the standard. You'll find N2683 under 
> the June 2021 virtual meeting heading. That's how you know which paper was 
> adopted.
> - You can look at the paper to see the proposed wording, prior art, 
> motivation, etc. That gets you 95% of the way but you need to know what words 
> actually went into the standard. So use the original paper to give you ideas 
> on what to implement, what to test, etc, but verify against the wording in 
> the standard.
> - Because this was discussed at the June 2021 virtual meeting, you should 
> look at those meeting minutes 
> (https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2802.pdf) to see what was 
> adopted. You can search for `2683` to find the relevant discussion in the 
> minutes. Most importantly, you can see what polls were taken. The two polls 
> of interest were:
>
> Straw Poll: Does the committee wish to adopt the Core proposal from N2683 
> into C23? 13-0-5 Core proposal goes into C23.
> Straw Poll: Does the committee want the "ckd_" identifiers from N2683's Core 
> proposal in future library directions to be potentially reserved identifiers? 
> 18-0-0
>
> This is how we know that only the core proposal was added, and not the 
> supplemental proposal. Checking that belief against the standard wording 
> itself verifies that only the core proposal was added and the supplemental 
> bits are left out.
>
> (Hopefully that explanation makes some sense, but if you have more questions, 
> just ask!)

Yes, that's very helpful! Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157331

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

[clang] 182de29 - Add argument to static_assert() call to work with compilers that default to c++14 or earlier.

2023-08-11 Thread Douglas Yung via cfe-commits


Author: Douglas Yung
Date: 2023-08-11T17:54:58-07:00
New Revision: 182de296f7bac49c655304d5c9f6b0edadf48302

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

LOG: Add argument to static_assert() call to work with compilers that default 
to c++14 or earlier.

Added: 


Modified: 
clang/test/Sema/ms_predefined_expr.cpp

Removed: 




diff  --git a/clang/test/Sema/ms_predefined_expr.cpp 
b/clang/test/Sema/ms_predefined_expr.cpp
index 27025e0de7ff54..5919ee964ccab9 100644
--- a/clang/test/Sema/ms_predefined_expr.cpp
+++ b/clang/test/Sema/ms_predefined_expr.cpp
@@ -51,8 +51,8 @@ constexpr bool equal(const T ()[N], const T ()[N]) {
   return true;
 }
 
-#define ASSERT_EQ(X, Y) static_assert(equal(X, Y))
-#define ASSERT_EQ_TY(X, Y) static_assert(is_same)
+#define ASSERT_EQ(X, Y) static_assert(equal(X, Y), "")
+#define ASSERT_EQ_TY(X, Y) static_assert(is_same, "")
 
 #define _WIDE(s) L##s
 #define WIDE(s)  _WIDE(s)
@@ -158,7 +158,7 @@ constexpr size_t operator""_len(const char*, size_t len) {
 }
 
 void test_udliteral() {
-  static_assert(__FUNCTION__ ""_len == 14); // expected-warning{{expansion of 
predefined identifier '__FUNCTION__' to a string literal is a Microsoft 
extension}}
+  static_assert(__FUNCTION__ ""_len == 14, ""); // expected-warning{{expansion 
of predefined identifier '__FUNCTION__' to a string literal is a Microsoft 
extension}}
 }
 
 void test_static_assert() {



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

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

2023-08-11 Thread Aiden Grossman via Phabricator via cfe-commits

aidengrossman added a comment.

The main issue here is that there's no way to reliably detect whether the built 
in is defined (can't check if a function is defined in the preprocessor, 
preprocessor macro isn't exposed correctly in all configurations), which ends 
up breaking some configurations. I wouldn't be opposed to exposing things more 
often, but I'm fine with the builtins being exposed under `-fms-extensions`, we 
just need to expose the preprocessor macro when we expose the builtins.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150646

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

[PATCH] D155610: [Clang][Sema] Fix display of characters on static assertion failure

2023-08-11 Thread Hubert Tong via Phabricator via cfe-commits

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16930-16936
+case BuiltinType::Char_S:
+case BuiltinType::Char_U:
+case BuiltinType::Char8:
+case BuiltinType::Char16:
+case BuiltinType::Char32:
+case BuiltinType::WChar_S:
+case BuiltinType::WChar_U: {

Add FIXME for `char` and `wchar_t` cases that this assumes Unicode literal 
encodings.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16942-16945
+  OS << "' (0x"
+ << llvm::format_hex_no_prefix(CodePoint, /*Width=*/2,
+   /*Upper=*/true)
+ << ')';

@aaron.ballman, hex output hides signedness. I think we want hex //and// 
decimal.



Comment at: clang/test/SemaCXX/static-assert.cpp:287
+  static_assert((char16_t)L'ゆ' == L"C̵̭̯̠̎͌ͅť̺"[1], ""); // expected-error 
{{failed}} \
+  // expected-note {{evaluates 
to ''ゆ' (0x3086) == '̵' (0x335)'}}
+  static_assert(L"＼／"[1] == u'\xFFFD', ""); // expected-error {{failed}} \

The C++23 escaped string formatting facility would not generate a trailing 
combining character like this. I recommend following suit.

Info on U+0335: https://util.unicode.org/UnicodeJsps/character.jsp?a=0335



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

https://reviews.llvm.org/D155610

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

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

2023-08-11 Thread Reid Kleckner via Phabricator via cfe-commits

rnk added a comment.

I think MSVC defines `_MSC_EXTENSIONS` under one of its compatibility modes as 
well, so it's not a good indicator of "is this compiler MSVC-like". I think we 
should be exposing the `__cpudex` builtin any time we are being MSVC-like, not 
under `fms-compatibility`. Would that make things sensible again?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150646

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

[PATCH] D157757: [Headers] Replace __need_STDDEF_H_misc with specific __need_ macros

2023-08-11 Thread Ian Anderson via Phabricator via cfe-commits

iana updated this revision to Diff 549547.
iana marked an inline comment as done.
iana added a comment.

Update the tests to handle diagnostics from stddef.h
Fix stddef.h to support defining __STDC_WANT_LIB_EXT1__ if stddef.h has already 
been included


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157757

Files:
  clang/lib/Headers/stddef.h
  clang/test/Headers/stddef.c
  clang/test/Headers/stddefneeds.c

Index: clang/test/Headers/stddefneeds.c
===
--- /dev/null
+++ clang/test/Headers/stddefneeds.c
@@ -0,0 +1,169 @@
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.9.0 -verify=c99 -std=c99 %s
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.9.0 -verify=c23 -std=c23 %s
+
+// Use C99 to verify that __need_ can be used to get types that wouldn't normally be available.
+
+struct astruct { char member; };
+
+ptrdiff_t p0; // c99-error{{unknown}} c23-error{{unknown}}
+size_t s0; // c99-error{{unknown}} c23-error{{unknown}}
+rsize_t r0; // c99-error{{unknown}} c23-error{{unknown}}
+wchar_t wc0; // c99-error{{unknown}} c23-error{{unknown}}
+void *v0 = NULL; // c99-error{{undeclared}} c23-error{{undeclared}}
+nullptr_t n0; // c99-error{{unknown}} c23-error{{unknown}}
+static void f0(void) { unreachable(); } // c99-error{{undeclared}} c23-error{{undeclared}}
+max_align_t m0; // c99-error{{unknown}} c23-error{{unknown}}
+size_t o0 = offsetof(struct astruct, member); // c99-error{{unknown}} c99-error{{undeclared}} c99-error{{expected expression}} c99-error{{undeclared}} \
+ c23-error{{unknown}} c23-error{{undeclared}} c23-error{{expected expression}} c23-error{{undeclared}}
+wint_t wi0; // c99-error{{unknown}} c23-error{{unknown}}
+
+#define __need_ptrdiff_t
+#include 
+
+ptrdiff_t p1;
+size_t s1; // c99-error{{unknown}} c23-error{{unknown}}
+rsize_t r1; // c99-error{{unknown}} c23-error{{unknown}}
+wchar_t wc1; // c99-error{{unknown}} c23-error{{unknown}}
+void *v1 = NULL; // c99-error{{undeclared}} c23-error{{undeclared}}
+nullptr_t n1; // c99-error{{unknown}} c23-error{{unknown}}
+static void f1(void) { unreachable(); } // c99-error{{undeclared}} c23-error{{undeclared}}
+max_align_t m1; // c99-error{{unknown}} c23-error{{unknown}}
+size_t o1 = offsetof(struct astruct, member); // c99-error{{unknown}} c99-error{{undeclared}} c99-error{{expected expression}} \
+ c23-error{{unknown}} c23-error{{undeclared}} c23-error{{expected expression}} c23-error{{undeclared}}
+wint_t wi1; // c99-error{{unknown}} c23-error{{unknown}}
+
+#define __need_size_t
+#include 
+
+ptrdiff_t p2;
+size_t s2;
+rsize_t r2; // c99-error{{unknown}} c23-error{{unknown}}
+// c99-note@stddef.h:*{{'size_t' declared here}} c23-note@stddef.h:*{{'size_t' declared here}}
+wchar_t wc2; // c99-error{{unknown}} c23-error{{unknown}}
+void *v2 = NULL; // c99-error{{undeclared}} c23-error{{undeclared}}
+nullptr_t n2; // c99-error{{unknown}} c23-error{{unknown}}
+static void f2(void) { unreachable(); } // c99-error{{undeclared}} c23-error{{undeclared}}
+max_align_t m2; // c99-error{{unknown}} c23-error{{unknown}}
+size_t o2 = offsetof(struct astruct, member); // c99-error{{undeclared}} c99-error{{expected expression}} \
+ c23-error{{undeclared}} c23-error{{expected expression}} c23-error{{undeclared}}
+wint_t wi2; // c99-error{{unknown}} c23-error{{unknown}}
+
+#define __need_rsize_t
+#include 
+
+ptrdiff_t p3;
+size_t s3;
+rsize_t r3;
+wchar_t wc3; // c99-error{{unknown}} c23-error{{unknown}}
+void *v3 = NULL; // c99-error{{undeclared}} c23-error{{undeclared}}
+nullptr_t n3; // c99-error{{unknown}} c23-error{{unknown}}
+static void f3(void) { unreachable(); } // c99-error{{undeclared}} c23-error{{undeclared}}
+max_align_t m3; // c99-error{{unknown}} c23-error{{unknown}}
+size_t o3 = offsetof(struct astruct, member); // c99-error{{undeclared}} c99-error{{expected expression}} \
+ c23-error{{undeclared}} c23-error{{expected expression}} c23-error{{undeclared}}
+wint_t wi3; // c99-error{{unknown}} c23-error{{unknown}}
+
+#define __need_wchar_t
+#include 
+
+ptrdiff_t p4;
+size_t s4;
+rsize_t r4;
+wchar_t wc4;
+void *v4 = NULL; // c99-error{{undeclared}} c23-error{{undeclared}}
+nullptr_t n4; // c99-error{{unknown}} c23-error{{unknown}}
+static void f4(void) { unreachable(); } // c99-error{{undeclared}} c23-error{{undeclared}}
+max_align_t m4; // c99-error{{unknown}} c23-error{{unknown}}
+size_t o4 = offsetof(struct astruct, member); // c99-error{{undeclared}} c99-error{{expected expression}} \
+ c23-error{{undeclared}} c23-error{{expected expression}} c23-error{{undeclared}}
+wint_t wi4; // c99-error{{unknown}} c23-error{{unknown}}
+

[PATCH] D155986: [clang][AMDGPU]: Don't use byval for struct arguments in function ABI

2023-08-11 Thread Changpeng Fang via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd77c62053c94: [clang][AMDGPU]: Dont use byval for 
struct arguments in function ABI (authored by cfang).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D155986?vs=549545=549546#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155986

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/Targets/AMDGPU.cpp
  clang/test/CodeGenCUDA/kernel-args.cu
  clang/test/CodeGenCXX/amdgcn-func-arg.cpp
  clang/test/CodeGenOpenCL/addr-space-struct-arg.cl
  clang/test/CodeGenOpenCL/amdgpu-abi-struct-arg-byref.cl
  clang/test/CodeGenOpenCL/amdgpu-abi-struct-coerce.cl
  clang/test/CodeGenOpenCL/byval.cl
  llvm/docs/AMDGPUUsage.rst

Index: llvm/docs/AMDGPUUsage.rst
===
--- llvm/docs/AMDGPUUsage.rst
+++ llvm/docs/AMDGPUUsage.rst
@@ -13812,6 +13812,10 @@
 9.  All other registers are unspecified.
 10. Any necessary ``s_waitcnt`` has been performed to ensure memory is available
 to the function.
+11: Use pass-by-reference (byref) in stead of pass-by-value (byval) for struct
+arguments in C ABI. Callee is responsible for allocating stack memory and
+copying the value of the struct if modified. Note that the backend still
+supports byval for struct arguments.
 
 On exit from a function:
 
Index: clang/test/CodeGenOpenCL/byval.cl
===
--- clang/test/CodeGenOpenCL/byval.cl
+++ clang/test/CodeGenOpenCL/byval.cl
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -emit-llvm -o - -triple amdgcn %s | FileCheck %s
-
+// RUN: %clang_cc1 -emit-llvm -o - -triple i686-pc-darwin %s | FileCheck -check-prefix=X86 %s
+// RUN: %clang_cc1 -emit-llvm -o - -triple amdgcn %s | FileCheck -check-prefix=AMDGCN %s
 struct A {
   int x[100];
 };
@@ -8,8 +8,10 @@
 
 int g() {
   struct A a;
-  // CHECK: call i32 @f(ptr addrspace(5) noundef byval{{.*}}%a)
+  // X86:call i32 @f(ptr noundef nonnull byval(%struct.A) align 4 %a)
+  // AMDGCN: call i32 @f(ptr addrspace(5) noundef byref{{.*}}%a)
   return f(a);
 }
 
-// CHECK: declare i32 @f(ptr addrspace(5) noundef byval{{.*}})
+// X86:   declare i32 @f(ptr noundef byval(%struct.A) align 4)
+// AMDGCN: declare i32 @f(ptr addrspace(5) noundef byref{{.*}})
Index: clang/test/CodeGenOpenCL/amdgpu-abi-struct-coerce.cl
===
--- clang/test/CodeGenOpenCL/amdgpu-abi-struct-coerce.cl
+++ clang/test/CodeGenOpenCL/amdgpu-abi-struct-coerce.cl
@@ -448,11 +448,11 @@
 // CHECK: define{{.*}} void @func_reg_state_lo(<4 x i32> noundef %arg0, <4 x i32> noundef %arg1, <4 x i32> noundef %arg2, i32 noundef %arg3, i32 %s.coerce0, float %s.coerce1, i32 %s.coerce2)
 void func_reg_state_lo(int4 arg0, int4 arg1, int4 arg2, int arg3, struct_arg_t s) { }
 
-// CHECK: define{{.*}} void @func_reg_state_hi(<4 x i32> noundef %arg0, <4 x i32> noundef %arg1, <4 x i32> noundef %arg2, i32 noundef %arg3, i32 noundef %arg4, ptr addrspace(5) nocapture noundef readnone byval(%struct.struct_arg) align 4 %s)
+// CHECK: define{{.*}} void @func_reg_state_hi(<4 x i32> noundef %arg0, <4 x i32> noundef %arg1, <4 x i32> noundef %arg2, i32 noundef %arg3, i32 noundef %arg4, ptr addrspace(5) nocapture noundef readnone byref(%struct.struct_arg) align 4 %{{.*}})
 void func_reg_state_hi(int4 arg0, int4 arg1, int4 arg2, int arg3, int arg4, struct_arg_t s) { }
 
 // XXX - Why don't the inner structs flatten?
-// CHECK: define{{.*}} void @func_reg_state_num_regs_nested_struct(<4 x i32> noundef %arg0, i32 noundef %arg1, i32 %arg2.coerce0, %struct.nested %arg2.coerce1, i32 %arg3.coerce0, %struct.nested %arg3.coerce1, ptr addrspace(5) nocapture noundef readnone byval(%struct.num_regs_nested_struct) align 8 %arg4)
+// CHECK: define{{.*}} void @func_reg_state_num_regs_nested_struct(<4 x i32> noundef %arg0, i32 noundef %arg1, i32 %arg2.coerce0, %struct.nested %arg2.coerce1, i32 %arg3.coerce0, %struct.nested %arg3.coerce1, ptr addrspace(5) nocapture noundef readnone byref(%struct.num_regs_nested_struct) align 8 %{{.*}})
 void func_reg_state_num_regs_nested_struct(int4 arg0, int arg1, num_regs_nested_struct arg2, num_regs_nested_struct arg3, num_regs_nested_struct arg4) { }
 
 // CHECK: define{{.*}} void @func_double_nested_struct_arg(<4 x i32> noundef %arg0, i32 noundef %arg1, i32 %arg2.coerce0, %struct.double_nested %arg2.coerce1, i16 %arg2.coerce2)
@@ -467,7 +467,7 @@
 // CHECK: define{{.*}} void @func_large_struct_padding_arg_direct(i8 %arg.coerce0, i32 %arg.coerce1, i8 %arg.coerce2, i32 %arg.coerce3, i8 %arg.coerce4, i8 %arg.coerce5, i16 %arg.coerce6, i16 %arg.coerce7, [3 x i8] %arg.coerce8, i64 %arg.coerce9, i32 %arg.coerce10, i8

[clang] d77c620 - [clang][AMDGPU]: Don't use byval for struct arguments in function ABI

2023-08-11 Thread Changpeng Fang via cfe-commits


Author: Changpeng Fang
Date: 2023-08-11T16:37:42-07:00
New Revision: d77c62053c944652846c00a35c921e14b43b1877

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

LOG: [clang][AMDGPU]: Don't use byval for struct arguments in function ABI

Summary:
  Byval requires allocating additional stack space, and always requires an 
implicit copy to be inserted in codegen,
where it can be difficult to optimize. In this work, we use 
byref/IndirectAliased promotion method instead of
byval with the implicit copy semantics.

Reviewers:
  arsenm

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

Added: 
clang/test/CodeGenOpenCL/amdgpu-abi-struct-arg-byref.cl

Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/CodeGen/CGCall.cpp
clang/lib/CodeGen/Targets/AMDGPU.cpp
clang/test/CodeGenCUDA/kernel-args.cu
clang/test/CodeGenCXX/amdgcn-func-arg.cpp
clang/test/CodeGenOpenCL/addr-space-struct-arg.cl
clang/test/CodeGenOpenCL/amdgpu-abi-struct-coerce.cl
clang/test/CodeGenOpenCL/byval.cl
llvm/docs/AMDGPUUsage.rst

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 860bcceeef21ff..cd7beff546c932 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -193,6 +193,10 @@ Target Specific Changes
 
 AMDGPU Support
 ^^
+- Use pass-by-reference (byref) in stead of pass-by-value (byval) for struct
+  arguments in C ABI. Callee is responsible for allocating stack memory and
+  copying the value of the struct if modified. Note that AMDGPU backend still
+  supports byval for struct arguments.
 
 X86 Support
 ^^^

diff  --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 28c3bc7c9f70f6..2b5121a7b23063 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -2156,7 +2156,8 @@ static bool DetermineNoUndef(QualType QTy, CodeGenTypes 
,
  const llvm::DataLayout , const ABIArgInfo ,
  bool CheckCoerce = true) {
   llvm::Type *Ty = Types.ConvertTypeForMem(QTy);
-  if (AI.getKind() == ABIArgInfo::Indirect)
+  if (AI.getKind() == ABIArgInfo::Indirect ||
+  AI.getKind() == ABIArgInfo::IndirectAliased)
 return true;
   if (AI.getKind() == ABIArgInfo::Extend)
 return true;
@@ -5126,12 +5127,15 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo 
,
   auto LV = I->getKnownLValue();
   auto AS = LV.getAddressSpace();
 
-  if (!ArgInfo.getIndirectByVal() ||
+  bool isByValOrRef =
+  ArgInfo.isIndirectAliased() || ArgInfo.getIndirectByVal();
+
+  if (!isByValOrRef ||
   (LV.getAlignment() < getContext().getTypeAlignInChars(I->Ty))) {
 NeedCopy = true;
   }
   if (!getLangOpts().OpenCL) {
-if ((ArgInfo.getIndirectByVal() &&
+if ((isByValOrRef &&
 (AS != LangAS::Default &&
  AS != CGM.getASTAllocaAddressSpace( {
   NeedCopy = true;
@@ -5139,7 +5143,7 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo 
,
   }
   // For OpenCL even if RV is located in default or alloca address 
space
   // we don't want to perform address space cast for it.
-  else if ((ArgInfo.getIndirectByVal() &&
+  else if ((isByValOrRef &&
 Addr.getType()->getAddressSpace() != IRFuncTy->
   getParamType(FirstIRArg)->getPointerAddressSpace())) {
 NeedCopy = true;

diff  --git a/clang/lib/CodeGen/Targets/AMDGPU.cpp 
b/clang/lib/CodeGen/Targets/AMDGPU.cpp
index 6e40c0a6607fae..1e7b036de82efd 100644
--- a/clang/lib/CodeGen/Targets/AMDGPU.cpp
+++ b/clang/lib/CodeGen/Targets/AMDGPU.cpp
@@ -248,6 +248,12 @@ ABIArgInfo AMDGPUABIInfo::classifyArgumentType(QualType Ty,
 return ABIArgInfo::getDirect();
   }
 }
+
+// Use pass-by-reference in stead of pass-by-value for struct arguments in
+// function ABI.
+return ABIArgInfo::getIndirectAliased(
+getContext().getTypeAlignInChars(Ty),
+getContext().getTargetAddressSpace(LangAS::opencl_private));
   }
 
   // Otherwise just do the default thing.

diff  --git a/clang/test/CodeGenCUDA/kernel-args.cu 
b/clang/test/CodeGenCUDA/kernel-args.cu
index 5f064694223b55..bcce729f14481c 100644
--- a/clang/test/CodeGenCUDA/kernel-args.cu
+++ b/clang/test/CodeGenCUDA/kernel-args.cu
@@ -9,14 +9,14 @@ struct A {
   float *p;
 };
 
-// AMDGCN: define{{.*}} amdgpu_kernel void @_Z6kernel1A(ptr addrspace(4) 
byref(%struct.A) align 8 %{{.+}})
+// AMDGCN: define{{.*}} amdgpu_kernel void @_Z6kernel1A(ptr addrspace(4) 
noundef byref(%struct.A) align 8 %{{.+}})
 // NVPTX: define{{.*}} void @_Z6kernel1A(ptr noundef

[PATCH] D156737: clang: Add __builtin_elementwise_sqrt

2023-08-11 Thread Matt Arsenault via Phabricator via cfe-commits

arsenm closed this revision.
arsenm added a comment.

9e3d9c9eae03910d93e2312e1e0845433c779998 



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

https://reviews.llvm.org/D156737

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

[clang] 9e3d9c9 - clang: Add __builtin_elementwise_sqrt

2023-08-11 Thread Matt Arsenault via cfe-commits


Author: Matt Arsenault
Date: 2023-08-11T19:32:39-04:00
New Revision: 9e3d9c9eae03910d93e2312e1e0845433c779998

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

LOG: clang: Add __builtin_elementwise_sqrt

This will be used in the opencl builtin headers to provide direct
intrinsic access with proper !fpmath metadata.

https://reviews.llvm.org/D156737

Added: 


Modified: 
clang/docs/LanguageExtensions.rst
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/Builtins.def
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/Sema/SemaChecking.cpp
clang/test/CodeGen/builtins-elementwise-math.c
clang/test/CodeGen/strictfp-elementwise-bulitins.cpp
clang/test/CodeGenCUDA/correctly-rounded-div.cu
clang/test/CodeGenOpenCL/fpmath.cl
clang/test/Sema/builtins-elementwise-math.c
clang/test/SemaCXX/builtins-elementwise-math.cpp

Removed: 




diff  --git a/clang/docs/LanguageExtensions.rst 
b/clang/docs/LanguageExtensions.rst
index 56c277983a7403..c771e3457af2b2 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -643,6 +643,8 @@ Unless specified otherwise operation(±0) = ±0 and 
operation(±infinity) = ±in
  T __builtin_elementwise_bitreverse(T x) return the integer represented 
after reversing the bits of x integer types
  T __builtin_elementwise_exp(T x)returns the base-e exponential, 
e^x, of the specified value  floating point types
  T __builtin_elementwise_exp2(T x)   returns the base-2 exponential, 
2^x, of the specified value  floating point types
+
+ T __builtin_elementwise_sqrt(T x)   return the square root of a 
floating-point numberfloating point types
  T __builtin_elementwise_roundeven(T x)  round x to the nearest integer 
value in floating point format,   floating point types
  rounding halfway cases to even 
(that is, to the nearest value
  that is an even integer), 
regardless of the current rounding

diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a80f57d9bb71ac..860bcceeef21ff 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -239,6 +239,7 @@ Floating Point Support in Clang
 - Add ``__builtin_set_flt_rounds`` builtin for X86, x86_64, Arm and AArch64 
only.
 - Add ``__builtin_elementwise_pow`` builtin for floating point types only.
 - Add ``__builtin_elementwise_bitreverse`` builtin for integer types only.
+- Add ``__builtin_elementwise_sqrt`` builtin for floating point types only.
 
 AST Matchers
 

diff  --git a/clang/include/clang/Basic/Builtins.def 
b/clang/include/clang/Basic/Builtins.def
index 843cc7f334f564..83e4259ea037b9 100644
--- a/clang/include/clang/Basic/Builtins.def
+++ b/clang/include/clang/Basic/Builtins.def
@@ -694,6 +694,7 @@ BUILTIN(__builtin_elementwise_round, "v.", "nct")
 BUILTIN(__builtin_elementwise_rint, "v.", "nct")
 BUILTIN(__builtin_elementwise_nearbyint, "v.", "nct")
 BUILTIN(__builtin_elementwise_sin, "v.", "nct")
+BUILTIN(__builtin_elementwise_sqrt, "v.", "nct")
 BUILTIN(__builtin_elementwise_trunc, "v.", "nct")
 BUILTIN(__builtin_elementwise_canonicalize, "v.", "nct")
 BUILTIN(__builtin_elementwise_copysign, "v.", "nct")

diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 6cd6f6fe37ebc7..5a183d3553279e 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -2530,7 +2530,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl 
GD, unsigned BuiltinID,
 case Builtin::BI__builtin_sqrtf:
 case Builtin::BI__builtin_sqrtf16:
 case Builtin::BI__builtin_sqrtl:
-case Builtin::BI__builtin_sqrtf128: {
+case Builtin::BI__builtin_sqrtf128:
+case Builtin::BI__builtin_elementwise_sqrt: {
   llvm::Value *Call = emitUnaryMaybeConstrainedFPBuiltin(
   *this, E, Intrinsic::sqrt, Intrinsic::experimental_constrained_sqrt);
   SetSqrtFPAccuracy(Call);

diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 984e43c1fcfad5..dc45e8d61cea73 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -2642,6 +2642,7 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, 
unsigned BuiltinID,
   case Builtin::BI__builtin_elementwise_rint:
   case Builtin::BI__builtin_elementwise_nearbyint:
   case Builtin::BI__builtin_elementwise_sin:
+  case Builtin::BI__builtin_elementwise_sqrt:
   case Builtin::BI__builtin_elementwise_trunc:
   case Builtin::BI__builtin_elementwise_canonicalize: {
 if (PrepareBuiltinElementwiseMathOneArgCall(TheCall))

diff  --git a/clang/test/CodeGen/builtins-elementwise-math.c

[PATCH] D155610: [Clang][Sema] Fix display of characters on static assertion failure

2023-08-11 Thread Hubert Tong via Phabricator via cfe-commits

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16899
+  uint32_t CodePoint = 
static_cast(V.getInt().getZExtValue());
+  PrintCharLiteralPrefix(BTy->getKind(), OS);
+  OS << '\'';

cor3ntin wrote:
> Looking at the diagnostics, I don't think it makes sense to print a prefix 
> here. You could just leave that part out.
Why is removing the prefix better? The types can matter (characters outside the 
basic character set are allowed to have negative `char` values). Also, moving 
forward, the value of a character need not be the same in the various encodings.


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

https://reviews.llvm.org/D155610

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

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

2023-08-11 Thread Reid Kleckner via Phabricator via cfe-commits

rnk added a comment.

What are the next steps here? Have concerns been addressed? The CI failures 
seem unrelated (libcxx tests is an AIX bot out of disk, clang CI is an 
interpreter test that seems unrelated, but please confirm).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112921

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

[PATCH] D157757: [Headers] Replace __need_STDDEF_H_misc with specific __need_ macros

2023-08-11 Thread Ian Anderson via Phabricator via cfe-commits

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



Comment at: clang/test/Headers/stddef.c:20-23
+// rsize_t will only be defined if __STDC_WANT_LIB_EXT1__ is set to >= 1.
+// It would be nice to test the default undefined behavior, but that emits
+// a note coming from stddef.h "rsize_t, did you mean size_t?" that can't be
+// `c99-note`d away.

efriedma wrote:
> iana wrote:
> > Does anyone know a trick to catch the note from stddef.h?
> I think the syntax is something like `expected-warning@stddef.h:10`?  See 
> rGfcc699ae ,  There's also a wildcard syntax; see rG05eedf1f.
Ah ha!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157757

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

[PATCH] D156726: Make globals with mutable members non-constant, even in custom sections

2023-08-11 Thread Reid Kleckner via Phabricator via cfe-commits

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

Thanks! My concerns are addressed, but please confirm that Eli's are too.




Comment at: clang/lib/Sema/SemaDecl.cpp:14254
 int SectionFlags = ASTContext::PSF_Read;
-if (var->getType().isConstQualified()) {
-  if (HasConstInit)

efriedma wrote:
> dblaikie wrote:
> > efriedma wrote:
> > > dblaikie wrote:
> > > > rnk wrote:
> > > > > rsmith wrote:
> > > > > > efriedma wrote:
> > > > > > > rnk wrote:
> > > > > > > > I think this is not compatible with MSVC. MSVC uses simple 
> > > > > > > > logic, it doesn't look for mutable: 
> > > > > > > > https://gcc.godbolt.org/z/sj6d4saxx
> > > > > > > > 
> > > > > > > > The const mutable struct appears in the myrdata section in that 
> > > > > > > > example.
> > > > > > > > 
> > > > > > > > I think the solution is to separate the flag logic from the 
> > > > > > > > pragma stack selection logic, which has to remain 
> > > > > > > > MSVC-compatible.
> > > > > > > MSVC apparently looks at whether the variable is marked "const", 
> > > > > > > and nothing else; it doesn't look at mutable, it doesn't look at 
> > > > > > > whether the variable has a constant initializer.  So the current 
> > > > > > > code isn't right either; if we're trying to implement 
> > > > > > > MSVC-compatible logic, we shouldn't check HasConstInit.
> > > > > > > 
> > > > > > > That said, I'm not sure how precisely/in what modes we want to 
> > > > > > > precisely emulate MSVC.  Probably anything we do here will be 
> > > > > > > confusing.
> > > > > > We should at least issue a warning if the sensible logic and the 
> > > > > > MSVC-compatible calculation differ. @rnk, do you know how important 
> > > > > > it is to follow the MSVC semantics in this regard?
> > > > > I think it depends on whether you think that users are primarily 
> > > > > using these pragmas to override the default rdata/bss/data sections 
> > > > > without any care about precisely what goes where, or if they are 
> > > > > using them to do something finer grained.
> > > > > 
> > > > > If I had to guess, I'd say it's more likely the former, given that 
> > > > > `__declspec(allocate)` and `#pragma(section)` exist to handle cases 
> > > > > where users are putting specific globals into specific sections.
> > > > > 
> > > > > Which, if we follow Richard's suggestion, would mean warning when we 
> > > > > put a global marked `const` into a writable section when 
> > > > > `ConstSegStack` is non-empty. That seems reasonable. 
> > > > > `-Wmicrosoft-const-seg` for the new warning group?
> > > > Does the MSVC situation only apply to custom sections? (presumably when 
> > > > not customizing the section, MSVC gets it right and can support a const 
> > > > global with a runtime initializer, mutable member, or mutating dtor?)
> > > > 
> > > > I think this code still needs to be modified, since this is the code 
> > > > that drives the error about incompatible sections. So it'll need to 
> > > > behave differently depending on the target platform?
> > > Yes, the MSVC situation is specifically if you specify `#pragma 
> > > const_seg`; without the pragma, it does what you'd expect.
> > Went with the "let's do the thing that the user probably wants, but isn't 
> > what MSVC does, and warn when that difference comes up" - if that's OK with 
> > everyone.
> > 
> > (always open to wordsmithing the warning - and if we want to, can go to the 
> > extra layer and specifically diagnose which reason (mutable members, 
> > non-const init) - and I can't quite figure out the best phrasing to say 
> > "we're putting it in section X insetad of section Y, because Z, but 
> > Microsoft would use X because A" or something... it's all a bit of a 
> > mouthful)
> Describing which reason actually applies would make the warning a lot easier 
> to read.
That is true, but I think very few people will see this diagnostic. I'm not 
sure it's worth the added code complexity to implement that improvement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156726

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

[PATCH] D157757: [Headers] Replace __need_STDDEF_H_misc with specific __need_ macros

2023-08-11 Thread Eli Friedman via Phabricator via cfe-commits

efriedma added inline comments.



Comment at: clang/test/Headers/stddef.c:20-23
+// rsize_t will only be defined if __STDC_WANT_LIB_EXT1__ is set to >= 1.
+// It would be nice to test the default undefined behavior, but that emits
+// a note coming from stddef.h "rsize_t, did you mean size_t?" that can't be
+// `c99-note`d away.

iana wrote:
> Does anyone know a trick to catch the note from stddef.h?
I think the syntax is something like `expected-warning@stddef.h:10`?  See 
rGfcc699ae ,  There's also a wildcard syntax; see rG05eedf1f.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157757

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

[PATCH] D157762: [WIP] Implement [[msvc::no_unique_address]]

2023-08-11 Thread Amy Huang via Phabricator via cfe-commits

akhuang created this revision.
Herald added a reviewer: aaron.ballman.
Herald added a project: All.
akhuang requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This attribute should match the behavior of MSVC's [[msvc::no_unique_address]] 
attribute.

Bug: https://github.com/llvm/llvm-project/issues/49358


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157762

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/AST/Decl.cpp
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/lib/CodeGen/ABIInfoImpl.cpp
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/CodeGen/CGExprConstant.cpp
  clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/test/Layout/ms-no-unique-address.cpp

Index: clang/test/Layout/ms-no-unique-address.cpp
===
--- /dev/null
+++ clang/test/Layout/ms-no-unique-address.cpp
@@ -0,0 +1,254 @@
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only -triple x86_64-pc-win32 -fdump-record-layouts %s | FileCheck %s
+
+namespace Empty {
+  struct A {};
+  struct B { [[msvc::no_unique_address]] A a; char b; };
+  static_assert(sizeof(B) == 1);
+
+  // CHECK:*** Dumping AST Record Layout
+  // CHECK:  0 | struct Empty::B
+  // CHECK-NEXT: 0 |   struct Empty::A a (empty)
+  // CHECK-NEXT: 0 |   char b
+  // CHECK-NEXT:   | [sizeof=1, align=1,
+  // CHECK-NEXT:   |  nvsize=1, nvalign=1]
+
+  struct C {};
+  struct D {
+[[msvc::no_unique_address]] A a;
+[[msvc::no_unique_address]] C c;
+char d;
+  };
+  static_assert(sizeof(D) == 1);
+
+  // CHECK:*** Dumping AST Record Layout
+  // CHECK:  0 | struct Empty::D
+  // CHECK-NEXT: 0 |   struct Empty::A a (empty)
+  // CHECK-NEXT: 0 |   struct Empty::C c (empty)
+  // CHECK-NEXT: 0 |   char d
+  // CHECK-NEXT:   | [sizeof=1, align=1,
+  // CHECK-NEXT:   |  nvsize=1, nvalign=1]
+
+  struct E {
+[[msvc::no_unique_address]] A a1;
+[[msvc::no_unique_address]] A a2;
+char e;
+  };
+  static_assert(sizeof(E) == 2);
+
+  // CHECK:*** Dumping AST Record Layout
+  // CHECK:  0 | struct Empty::E
+  // CHECK-NEXT: 0 |   struct Empty::A a1 (empty)
+  // CHECK-NEXT: 1 |   struct Empty::A a2 (empty)
+  // CHECK-NEXT: 0 |   char e
+  // CHECK-NEXT:   | [sizeof=2, align=1,
+  // CHECK-NEXT:   |  nvsize=2, nvalign=1]
+
+  struct F {
+~F();
+[[msvc::no_unique_address]] A a1;
+[[msvc::no_unique_address]] A a2;
+char f;
+  };
+  static_assert(sizeof(F) == 2);
+
+  // CHECK:*** Dumping AST Record Layout
+  // CHECK:  0 | struct Empty::F
+  // CHECK-NEXT: 0 |   struct Empty::A a1 (empty)
+  // CHECK-NEXT: 1 |   struct Empty::A a2 (empty)
+  // CHECK-NEXT: 0 |   char f
+  // CHECK-NEXT:   | [sizeof=2, align=1,
+  // CHECK-NEXT:   |  nvsize=2, nvalign=1]
+
+  struct G { [[msvc::no_unique_address]] A a; ~G(); };
+  static_assert(sizeof(G) == 1);
+
+  // CHECK:*** Dumping AST Record Layout
+  // CHECK:  0 | struct Empty::G
+  // CHECK-NEXT: 0 |   struct Empty::A a (empty)
+  // CHECK-NEXT:   | [sizeof=1, align=1,
+  // CHECK-NEXT:   |  nvsize=1, nvalign=1]
+
+  struct H {
+[[msvc::no_unique_address]] A a;
+[[msvc::no_unique_address]] A b;
+~H();
+  };
+  static_assert(sizeof(H) == 2);
+
+  // CHECK:*** Dumping AST Record Layout
+  // CHECK:  0 | struct Empty::H
+  // CHECK-NEXT: 0 |   struct Empty::A a (empty)
+  // CHECK-NEXT: 1 |   struct Empty::A b (empty)
+  // CHECK-NEXT:   | [sizeof=2, align=1,
+  // CHECK-NEXT:   |  nvsize=2, nvalign=1]
+
+  struct OversizedEmpty : A {
+~OversizedEmpty();
+[[msvc::no_unique_address]] A a;
+  };
+  static_assert(sizeof(OversizedEmpty) == 2);
+
+  // CHECK:*** Dumping AST Record Layout
+  // CHECK:  0 | struct Empty::OversizedEmpty
+  // CHECK-NEXT: 0 |   struct Empty::A (base) (empty)
+  // CHECK-NEXT: 1 |   struct Empty::A a (empty)
+  // CHECK-NEXT:   | [sizeof=2, align=1,
+  // CHECK-NEXT:   |  nvsize=2, nvalign=1]
+
+  struct HasOversizedEmpty {
+[[msvc::no_unique_address]] OversizedEmpty m;
+  };
+  static_assert(sizeof(HasOversizedEmpty) == 2);
+
+  // CHECK:*** Dumping AST Record Layout
+  // CHECK:  0 | struct Empty::HasOversizedEmpty
+  // CHECK-NEXT: 0 |   struct Empty::OversizedEmpty m (empty)
+  // CHECK-NEXT: 0 | struct Empty::A (base) (empty)
+  // CHECK-NEXT: 1 | struct Empty::A a (empty)
+  // CHECK-NEXT:   | [sizeof=2, align=1,
+  // CHECK-NEXT:   |  nvsize=2, nvalign=1]
+
+  struct EmptyWithNonzeroDSize {
+[[msvc::no_unique_address]] A a;
+int x;
+[[msvc::no_unique_address]] A b;
+int y;
+[[msvc::no_unique_address]] A c;
+  };
+  static_assert(sizeof(EmptyWithNonzeroDSize) == 8);
+
+  // CHECK:*** Dumping AST Record Layout
+  // CHECK:  0 |

[PATCH] D157757: [Headers] Replace __need_STDDEF_H_misc with specific __need_ macros

2023-08-11 Thread Ian Anderson via Phabricator via cfe-commits

iana added inline comments.



Comment at: clang/test/Headers/stddefneeds.c:85-87
+// __need_nullptr_t generates an error in https://reviews.llvm.org/D157757/new/

https://reviews.llvm.org/D157757

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

[PATCH] D157750: Properly handle -fsplit-machine-functions for fatbinary compilation

2023-08-11 Thread Artem Belevich via Phabricator via cfe-commits

tra added inline comments.

Comment at: clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c:9
+
+// Check that -fsplit-machine-functions is passed to both x86 and cuda 
compilation and does not cause driver error.
+// MFS2: -fsplit-machine-functions

shenhan wrote:
> tra wrote:
> > We will still see a warning, right? So, for someone compiling with 
> > `-Werror` that's going to be a problem.
> > 
> > Also, if the warning is issued from the top-level driver, we may not even 
> > be able to suppress it when we disable splitting on GPU side with 
> > `-Xarch_device -fno-split-machine-functions`.
> > 
> > 
> > We will still see a warning, right?
> Yes, there still will be a warning. We've discussed it and we think that pass 
> -fsplit-machine-functions in this case is not a proper usage and a warning is 
> warranted, and it is not good that skip doing split silently while uses 
> explicitly ask for it.
> 
> > Also, if the warning is issued from the top-level driver
> The warning will not be issued from the top-level driver, it will be issued 
> when configuring optimization passes.
> So:
> 
> 
>   - -fsplit-machine-functions -Xarch_device -fno-split-machine-functions
> Will enable MFS for host, disable MFS for gpus and without any warnings.
> 
>   - -Xarch_host -fsplit-machine-functions
> The same as the above
> 
>   - -Xarch_host -fsplit-machine-functions -Xarch_device 
> -fno-split-machine-functions
> The same as the above
> 
> We've discussed it and we think that pass -fsplit-machine-functions in this 
> case is not a proper usage and a warning is warranted, and it is not good 
> that skip doing split silently while uses explicitly ask for it.

I would agree with that assertion if we were talking exclusively about CUDA 
compilation.
However, a common real world use pattern is that the flags are set globally for 
all C++ compilations, and then CUDA compilations within the project need to do 
whatever they need to to keep things working. The original user intent was for 
the option to affect the host compilation. There's no inherent assumption that 
it will do anything useful for the GPU.

In number of similar cases in the past we did settle on silently ignoring some 
top-level flags that we do expect to encounter in real projects, but which made 
no sense for the GPU. E.g. sanitizers. If the project is built w/ sanitizer 
enabled, the idea is to sanitize the host code, The GPU code continues to be 
built w/o sanitizer enabled. 

Anyways, as long as we have a way to deal with it it's not a big deal one way 
or another.

> -fsplit-machine-functions -Xarch_device -fno-split-machine-functions
> Will enable MFS for host, disable MFS for gpus and without any warnings.

OK. This will work.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157750

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

[PATCH] D157757: [Headers] Replace __need_STDDEF_H_misc with specific __need_ macros

2023-08-11 Thread Ian Anderson via Phabricator via cfe-commits

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



Comment at: clang/test/Headers/stddef.c:20-23
+// rsize_t will only be defined if __STDC_WANT_LIB_EXT1__ is set to >= 1.
+// It would be nice to test the default undefined behavior, but that emits
+// a note coming from stddef.h "rsize_t, did you mean size_t?" that can't be
+// `c99-note`d away.

Does anyone know a trick to catch the note from stddef.h?



Comment at: clang/test/Headers/stddefneeds.c:35-37
+// If you do these individually, the compiler will add a note
+// coming from stddef.h "rsize_t, did you mean size_t?" that
+// can't be `c99-note`d away.

(same here)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157757

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

[PATCH] D157757: [Headers] Replace __need_STDDEF_H_misc with specific __need_ macros

2023-08-11 Thread Eli Friedman via Phabricator via cfe-commits

efriedma accepted this revision.
efriedma 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/D157757/new/

https://reviews.llvm.org/D157757

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

[PATCH] D157757: [Headers] Replace __need_STDDEF_H_misc with specific __need_ macros

2023-08-11 Thread Ian Anderson via Phabricator via cfe-commits

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



Comment at: clang/lib/Headers/stddef.h:112
 #if defined(_MSC_EXTENSIONS) && defined(_NATIVE_NULLPTR_SUPPORTED)
-namespace std { typedef decltype(nullptr) nullptr_t; }
-using ::std::nullptr_t;
+// __need_NULL would previously define nullptr_t for C++, add it here.
+#define __need_nullptr_t

efriedma wrote:
> I think this code was pulled into the `__need_NULL` conditional by accident; 
> I doubt anyone actually depends on the precise interaction here.  Just move 
> to the same place as the other `__need_` defines.
 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157757

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

[PATCH] D157757: [Headers] Replace __need_STDDEF_H_misc with specific __need_ macros

2023-08-11 Thread Ian Anderson via Phabricator via cfe-commits

iana updated this revision to Diff 549534.
iana added a comment.

Don't define nullptr_t in C++ if __need_NULL is set without __need_nullptr_t


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157757

Files:
  clang/lib/Headers/stddef.h
  clang/test/Headers/stddef.c
  clang/test/Headers/stddefneeds.c

Index: clang/test/Headers/stddefneeds.c
===
--- /dev/null
+++ clang/test/Headers/stddefneeds.c
@@ -0,0 +1,160 @@
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.9.0 -verify=c99 -std=c99 %s
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.9.0 -verify=c23 -std=c23 %s
+
+// Use C99 to verify that __need_ can be used to get types that wouldn't normally be available.
+
+struct astruct { char member; };
+
+ptrdiff_t p0; // c99-error{{unknown}} c23-error{{unknown}}
+size_t s0; // c99-error{{unknown}} c23-error{{unknown}}
+rsize_t r0; // c99-error{{unknown}} c23-error{{unknown}}
+wchar_t wc0; // c99-error{{unknown}} c23-error{{unknown}}
+void *v0 = NULL; // c99-error{{undeclared}} c23-error{{undeclared}}
+nullptr_t n0; // c99-error{{unknown}} c23-error{{unknown}}
+static void f0(void) { unreachable(); } // c99-error{{undeclared}} c23-error{{undeclared}}
+max_align_t m0; // c99-error{{unknown}} c23-error{{unknown}}
+size_t o0 = offsetof(struct astruct, member); // c99-error{{unknown}} c99-error{{undeclared}} c99-error{{expected expression}} c99-error{{undeclared}} \
+ c23-error{{unknown}} c23-error{{undeclared}} c23-error{{expected expression}} c23-error{{undeclared}}
+wint_t wi0; // c99-error{{unknown}} c23-error{{unknown}}
+
+#define __need_ptrdiff_t
+#include 
+
+ptrdiff_t p1;
+size_t s1; // c99-error{{unknown}} c23-error{{unknown}}
+rsize_t r1; // c99-error{{unknown}} c23-error{{unknown}}
+wchar_t wc1; // c99-error{{unknown}} c23-error{{unknown}}
+void *v1 = NULL; // c99-error{{undeclared}} c23-error{{undeclared}}
+nullptr_t n1; // c99-error{{unknown}} c23-error{{unknown}}
+static void f1(void) { unreachable(); } // c99-error{{undeclared}} c23-error{{undeclared}}
+max_align_t m1; // c99-error{{unknown}} c23-error{{unknown}}
+size_t o1 = offsetof(struct astruct, member); // c99-error{{unknown}} c99-error{{undeclared}} c99-error{{expected expression}} \
+ c23-error{{unknown}} c23-error{{undeclared}} c23-error{{expected expression}} c23-error{{undeclared}}
+wint_t wi1; // c99-error{{unknown}} c23-error{{unknown}}
+
+// If you do these individually, the compiler will add a note
+// coming from stddef.h "rsize_t, did you mean size_t?" that
+// can't be `c99-note`d away.
+#define __need_size_t
+#define __need_rsize_t
+#include 
+
+ptrdiff_t p2;
+size_t s2;
+rsize_t r2;
+wchar_t wc2; // c99-error{{unknown}} c23-error{{unknown}}
+void *v2 = NULL; // c99-error{{undeclared}} c23-error{{undeclared}}
+nullptr_t n2; // c99-error{{unknown}} c23-error{{unknown}}
+static void f2(void) { unreachable(); } // c99-error{{undeclared}} c23-error{{undeclared}}
+max_align_t m2; // c99-error{{unknown}} c23-error{{unknown}}
+size_t o2 = offsetof(struct astruct, member); // c99-error{{undeclared}} c99-error{{expected expression}} \
+ c23-error{{undeclared}} c23-error{{expected expression}} c23-error{{undeclared}}
+wint_t wi2; // c99-error{{unknown}} c23-error{{unknown}}
+
+#define __need_wchar_t
+#include 
+
+ptrdiff_t p3;
+size_t s3;
+rsize_t r3;
+wchar_t wc3;
+void *v3 = NULL; // c99-error{{undeclared}} c23-error{{undeclared}}
+nullptr_t n3; // c99-error{{unknown}} c23-error{{unknown}}
+static void f3(void) { unreachable(); } // c99-error{{undeclared}} c23-error{{undeclared}}
+max_align_t m3; // c99-error{{unknown}} c23-error{{unknown}}
+size_t o3 = offsetof(struct astruct, member); // c99-error{{undeclared}} c99-error{{expected expression}} \
+ c23-error{{undeclared}} c23-error{{expected expression}} c23-error{{undeclared}}
+wint_t wi3; // c99-error{{unknown}} c23-error{{unknown}}
+
+#define __need_NULL
+#include 
+
+ptrdiff_t p4;
+size_t s4;
+rsize_t r4;
+wchar_t wc4;
+void *v4 = NULL;
+nullptr_t n4; // c99-error{{unknown}} c23-error{{unknown}}
+static void f4(void) { unreachable(); } // c99-error{{undeclared}} c23-error{{undeclared}}
+max_align_t m4; // c99-error{{unknown}} c23-error{{unknown}}
+size_t o4 = offsetof(struct astruct, member); // c99-error{{undeclared}} c99-error{{expected expression}} \
+ c23-error{{undeclared}} c23-error{{expected expression}} c23-error{{undeclared}}
+wint_t wi4; // c99-error{{unknown}} c23-error{{unknown}}
+
+#if __STDC_VERSION__ >= 202311L
+// __need_nullptr_t generates an error in 
+
+ptrdiff_t p5;
+size_t s5;
+rsize_t r5;
+wchar_t wc5;
+void *v5 = NULL;
+nullptr_t n5; //

[clang] 368a0ac - Revert part of "[test][asan] Disable new test from D157552 on Android"

2023-08-11 Thread Vitaly Buka via cfe-commits


Author: Vitaly Buka
Date: 2023-08-11T15:07:29-07:00
New Revision: 368a0acea56e8f89860f5a64b4d0587bce6cff54

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

LOG: Revert part of "[test][asan] Disable new test from D157552 on Android"

Undo unrelated edit.

This reverts commit 6b7b45c213d44cb49708e22fb31df524298495d2.

Added: 


Modified: 
clang/cmake/caches/PGO-stage2.cmake

Removed: 




diff  --git a/clang/cmake/caches/PGO-stage2.cmake 
b/clang/cmake/caches/PGO-stage2.cmake
index 6bceeb72327368..b9b2f62e9cae46 100644
--- a/clang/cmake/caches/PGO-stage2.cmake
+++ b/clang/cmake/caches/PGO-stage2.cmake
@@ -1,4 +1,3 @@
 set(CMAKE_BUILD_TYPE RELEASE CACHE STRING "")
-set(LLVM_ENABLE_LLD ON CACHE BOOL "")
 set(LLVM_ENABLE_PROJECTS "clang;lld" CACHE STRING "")
 set(LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx;libcxxabi" CACHE STRING "")



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

[PATCH] D157750: Properly handle -fsplit-machine-functions for fatbinary compilation

2023-08-11 Thread Matt Arsenault via Phabricator via cfe-commits

arsenm added inline comments.



Comment at: llvm/lib/CodeGen/TargetPassConfig.cpp:1281-1282
+else
+  WithColor::warning()
+  << "-fsplit-machine-functions is only valid for X86.\n";
   }

You cannot spam warnings here. The other instance of printing here looks like a 
new addition and should be removed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157750

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

[PATCH] D157757: [Headers] Replace __need_STDDEF_H_misc with specific __need_ macros

2023-08-11 Thread Eli Friedman via Phabricator via cfe-commits

efriedma added inline comments.



Comment at: clang/lib/Headers/stddef.h:112
 #if defined(_MSC_EXTENSIONS) && defined(_NATIVE_NULLPTR_SUPPORTED)
-namespace std { typedef decltype(nullptr) nullptr_t; }
-using ::std::nullptr_t;
+// __need_NULL would previously define nullptr_t for C++, add it here.
+#define __need_nullptr_t

I think this code was pulled into the `__need_NULL` conditional by accident; I 
doubt anyone actually depends on the precise interaction here.  Just move to 
the same place as the other `__need_` defines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157757

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

[PATCH] D157750: Properly handle -fsplit-machine-functions for fatbinary compilation

2023-08-11 Thread Han Shen via Phabricator via cfe-commits

shenhan added inline comments.



Comment at: llvm/lib/CodeGen/TargetPassConfig.cpp:1278
 }
-addPass(createMachineFunctionSplitterPass());
+if (TM->getTargetTriple().isX86())
+  addPass(createMachineFunctionSplitterPass());

snehasish wrote:
> Can you coordinate with @dhoekwater ? He has some patches in flight for 
> AArch64. 
> 
> I think D157157 is the one which modifies the same logic.
Thanks. Yes, I'll coordinate with @dhoekwater before resolving this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157750

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

[PATCH] D157750: Properly handle -fsplit-machine-functions for fatbinary compilation

2023-08-11 Thread Han Shen via Phabricator via cfe-commits

shenhan added inline comments.



Comment at: clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c:9
+
+// Check that -fsplit-machine-functions is passed to both x86 and cuda 
compilation and does not cause driver error.
+// MFS2: -fsplit-machine-functions

tra wrote:
> We will still see a warning, right? So, for someone compiling with `-Werror` 
> that's going to be a problem.
> 
> Also, if the warning is issued from the top-level driver, we may not even be 
> able to suppress it when we disable splitting on GPU side with `-Xarch_device 
> -fno-split-machine-functions`.
> 
> 
> We will still see a warning, right?
Yes, there still will be a warning. We've discussed it and we think that pass 
-fsplit-machine-functions in this case is not a proper usage and a warning is 
warranted, and it is not good that skip doing split silently while uses 
explicitly ask for it.

> Also, if the warning is issued from the top-level driver
The warning will not be issued from the top-level driver, it will be issued 
when configuring optimization passes.
So:


  - -fsplit-machine-functions -Xarch_device -fno-split-machine-functions
Will enable MFS for host, disable MFS for gpus and without any warnings.

  - -Xarch_host -fsplit-machine-functions
The same as the above

  - -Xarch_host -fsplit-machine-functions -Xarch_device 
-fno-split-machine-functions
The same as the above



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157750

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

[PATCH] D157747: Support Unicode Microsoft predefined macros

2023-08-11 Thread Richard Dzenis via Phabricator via cfe-commits

RIscRIpt added a comment.

This implementation preserves support for identifiers `L__FUNCSIG__` and 
`L__FUNCTION__` (which have never been supported by MSVC), and adds other 
identifiers for all supported encodings: L, u8, u, U.

If breaking backwards compatibility in clang (with Microsoft Extensions) is 
allowed, then I'd prefer other (more correct) solution: support for 
`__LPREFIX`-family macros ( see https://godbolt.org/z/eacYqh1ab ), and remove 
token/identifiers like `L__FUNCTION__`; relevant issue 
https://github.com/llvm/llvm-project/issues/27402
I'll try to implement this alternative approach.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157747

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

[PATCH] D157757: [Headers] Replace __need_STDDEF_H_misc with specific __need_ macros

2023-08-11 Thread Ian Anderson via Phabricator via cfe-commits

iana created this revision.
iana added reviewers: aaron.ballman, rsmith, steplong, efriedma, jyknight, 
erichkeane.
Herald added a subscriber: ributzka.
Herald added a project: All.
iana requested review of this revision.
Herald added subscribers: cfe-commits, wangpc.
Herald added a project: clang.

Apple needs __need_ macros for rsize_t and offsetof. Add those, and also ones 
for nullptr_t, unreachable, max_align_t.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157757

Files:
  clang/lib/Headers/stddef.h
  clang/test/Headers/stddef.c
  clang/test/Headers/stddefneeds.c

Index: clang/test/Headers/stddefneeds.c
===
--- /dev/null
+++ clang/test/Headers/stddefneeds.c
@@ -0,0 +1,160 @@
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.9.0 -verify=c99 -std=c99 %s
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.9.0 -verify=c23 -std=c23 %s
+
+// Use C99 to verify that __need_ can be used to get types that wouldn't normally be available.
+
+struct astruct { char member; };
+
+ptrdiff_t p0; // c99-error{{unknown}} c23-error{{unknown}}
+size_t s0; // c99-error{{unknown}} c23-error{{unknown}}
+rsize_t r0; // c99-error{{unknown}} c23-error{{unknown}}
+wchar_t wc0; // c99-error{{unknown}} c23-error{{unknown}}
+void *v0 = NULL; // c99-error{{undeclared}} c23-error{{undeclared}}
+nullptr_t n0; // c99-error{{unknown}} c23-error{{unknown}}
+static void f0(void) { unreachable(); } // c99-error{{undeclared}} c23-error{{undeclared}}
+max_align_t m0; // c99-error{{unknown}} c23-error{{unknown}}
+size_t o0 = offsetof(struct astruct, member); // c99-error{{unknown}} c99-error{{undeclared}} c99-error{{expected expression}} c99-error{{undeclared}} \
+ c23-error{{unknown}} c23-error{{undeclared}} c23-error{{expected expression}} c23-error{{undeclared}}
+wint_t wi0; // c99-error{{unknown}} c23-error{{unknown}}
+
+#define __need_ptrdiff_t
+#include 
+
+ptrdiff_t p1;
+size_t s1; // c99-error{{unknown}} c23-error{{unknown}}
+rsize_t r1; // c99-error{{unknown}} c23-error{{unknown}}
+wchar_t wc1; // c99-error{{unknown}} c23-error{{unknown}}
+void *v1 = NULL; // c99-error{{undeclared}} c23-error{{undeclared}}
+nullptr_t n1; // c99-error{{unknown}} c23-error{{unknown}}
+static void f1(void) { unreachable(); } // c99-error{{undeclared}} c23-error{{undeclared}}
+max_align_t m1; // c99-error{{unknown}} c23-error{{unknown}}
+size_t o1 = offsetof(struct astruct, member); // c99-error{{unknown}} c99-error{{undeclared}} c99-error{{expected expression}} \
+ c23-error{{unknown}} c23-error{{undeclared}} c23-error{{expected expression}} c23-error{{undeclared}}
+wint_t wi1; // c99-error{{unknown}} c23-error{{unknown}}
+
+// If you do these individually, the compiler will add a note
+// coming from stddef.h "rsize_t, did you mean size_t?" that
+// can't be `c99-note`d away.
+#define __need_size_t
+#define __need_rsize_t
+#include 
+
+ptrdiff_t p2;
+size_t s2;
+rsize_t r2;
+wchar_t wc2; // c99-error{{unknown}} c23-error{{unknown}}
+void *v2 = NULL; // c99-error{{undeclared}} c23-error{{undeclared}}
+nullptr_t n2; // c99-error{{unknown}} c23-error{{unknown}}
+static void f2(void) { unreachable(); } // c99-error{{undeclared}} c23-error{{undeclared}}
+max_align_t m2; // c99-error{{unknown}} c23-error{{unknown}}
+size_t o2 = offsetof(struct astruct, member); // c99-error{{undeclared}} c99-error{{expected expression}} \
+ c23-error{{undeclared}} c23-error{{expected expression}} c23-error{{undeclared}}
+wint_t wi2; // c99-error{{unknown}} c23-error{{unknown}}
+
+#define __need_wchar_t
+#include 
+
+ptrdiff_t p3;
+size_t s3;
+rsize_t r3;
+wchar_t wc3;
+void *v3 = NULL; // c99-error{{undeclared}} c23-error{{undeclared}}
+nullptr_t n3; // c99-error{{unknown}} c23-error{{unknown}}
+static void f3(void) { unreachable(); } // c99-error{{undeclared}} c23-error{{undeclared}}
+max_align_t m3; // c99-error{{unknown}} c23-error{{unknown}}
+size_t o3 = offsetof(struct astruct, member); // c99-error{{undeclared}} c99-error{{expected expression}} \
+ c23-error{{undeclared}} c23-error{{expected expression}} c23-error{{undeclared}}
+wint_t wi3; // c99-error{{unknown}} c23-error{{unknown}}
+
+#define __need_NULL
+#include 
+
+ptrdiff_t p4;
+size_t s4;
+rsize_t r4;
+wchar_t wc4;
+void *v4 = NULL;
+nullptr_t n4; // c99-error{{unknown}} c23-error{{unknown}}
+static void f4(void) { unreachable(); } // c99-error{{undeclared}} c23-error{{undeclared}}
+max_align_t m4; // c99-error{{unknown}} c23-error{{unknown}}
+size_t o4 = offsetof(struct astruct, member); // c99-error{{undeclared}} c99-error{{expected expression}} \
+ c23-error{{undeclared}} c23-error{{expected expression}} c23-error{{undeclared}}
+wint_t wi4; //

[PATCH] D157332: [clang] Make init for empty no_unique_address fields a no-op write

2023-08-11 Thread Eli Friedman via Phabricator via cfe-commits

efriedma added a comment.

This approach looks fine.




Comment at: clang/lib/CodeGen/CGCall.cpp:5781
 
   // If the value is offset in memory, apply the offset now.
+  if (!isEmptyRecord(getContext(), RetTy, true)) {

The isEmptyRecord call could use a comment briefly explaining that empty 
records can overlap with other data.

The existing comment about the offset probably belongs inside the if statement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157332

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

[PATCH] D157332: [clang] Make init for empty no_unique_address fields a no-op write

2023-08-11 Thread Martin Storsjö via Phabricator via cfe-commits

mstorsjo updated this revision to Diff 549517.
mstorsjo added a comment.

Updated to just check isEmptyRecord in EmitCall.

The second part of the test probably is kinda redundant/pointless at this 
point, and I guess the test comment needs updating too; can do that later when 
the implementation is fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157332

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CodeGenCXX/ctor-empty-nounique.cpp


Index: clang/test/CodeGenCXX/ctor-empty-nounique.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/ctor-empty-nounique.cpp
@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 -triple x86_64-windows-gnu -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64le-windows-gnu -emit-llvm -o - %s | 
FileCheck %s
+
+// An empty struct is handled as a struct with a dummy i8, on all targets.
+// Most targets treat an empty struct return value as essentially void - but
+// some don't. (Currently, at least x86_64-windows-* and powerpc64le-* don't
+// treat it as void.)
+//
+// When intializing a struct with such a no_unique_address member, make sure we
+// don't write the dummy i8 into the struct where there's no space allocated 
for
+// it.
+//
+// This can only be tested with targets that don't treat empty struct returns 
as
+// void.
+
+struct S {};
+S f();
+struct Z {
+  int x;
+  [[no_unique_address]] S y;
+  Z();
+};
+Z::Z() : x(111), y(f()) {}
+
+// CHECK: define {{.*}} @_ZN1ZC2Ev
+// CHECK: %call = call i8 @_Z1fv()
+// CHECK-NEXT: ret void
+
+struct S2 {
+  S2();
+};
+struct Z2 {
+  int x;
+  [[no_unique_address]] S2 y;
+  Z2();
+};
+Z2::Z2() : x(111) {}
+
+// CHECK: define {{.*}} @_ZN2Z2C2Ev(ptr {{.*}} %this)
+// CHECK: %this.addr = alloca ptr
+// CHECK: store ptr %this, ptr %this.addr
+// CHECK: %this1 = load ptr, ptr %this.addr
+// CHECK: call void @_ZN2S2C1Ev(ptr {{.*}} %this1)
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -13,6 +13,7 @@
 
 #include "CGCall.h"
 #include "ABIInfo.h"
+#include "ABIInfoImpl.h"
 #include "CGBlocks.h"
 #include "CGCXXABI.h"
 #include "CGCleanup.h"
@@ -5778,8 +5779,10 @@
   }
 
   // If the value is offset in memory, apply the offset now.
-  Address StorePtr = emitAddressAtOffset(*this, DestPtr, RetAI);
-  CreateCoercedStore(CI, StorePtr, DestIsVolatile, *this);
+  if (!isEmptyRecord(getContext(), RetTy, true)) {
+Address StorePtr = emitAddressAtOffset(*this, DestPtr, RetAI);
+CreateCoercedStore(CI, StorePtr, DestIsVolatile, *this);
+  }
 
   return convertTempToRValue(DestPtr, RetTy, SourceLocation());
 }


Index: clang/test/CodeGenCXX/ctor-empty-nounique.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/ctor-empty-nounique.cpp
@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 -triple x86_64-windows-gnu -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64le-windows-gnu -emit-llvm -o - %s | FileCheck %s
+
+// An empty struct is handled as a struct with a dummy i8, on all targets.
+// Most targets treat an empty struct return value as essentially void - but
+// some don't. (Currently, at least x86_64-windows-* and powerpc64le-* don't
+// treat it as void.)
+//
+// When intializing a struct with such a no_unique_address member, make sure we
+// don't write the dummy i8 into the struct where there's no space allocated for
+// it.
+//
+// This can only be tested with targets that don't treat empty struct returns as
+// void.
+
+struct S {};
+S f();
+struct Z {
+  int x;
+  [[no_unique_address]] S y;
+  Z();
+};
+Z::Z() : x(111), y(f()) {}
+
+// CHECK: define {{.*}} @_ZN1ZC2Ev
+// CHECK: %call = call i8 @_Z1fv()
+// CHECK-NEXT: ret void
+
+struct S2 {
+  S2();
+};
+struct Z2 {
+  int x;
+  [[no_unique_address]] S2 y;
+  Z2();
+};
+Z2::Z2() : x(111) {}
+
+// CHECK: define {{.*}} @_ZN2Z2C2Ev(ptr {{.*}} %this)
+// CHECK: %this.addr = alloca ptr
+// CHECK: store ptr %this, ptr %this.addr
+// CHECK: %this1 = load ptr, ptr %this.addr
+// CHECK: call void @_ZN2S2C1Ev(ptr {{.*}} %this1)
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -13,6 +13,7 @@
 
 #include "CGCall.h"
 #include "ABIInfo.h"
+#include "ABIInfoImpl.h"
 #include "CGBlocks.h"
 #include "CGCXXABI.h"
 #include "CGCleanup.h"
@@ -5778,8 +5779,10 @@
   }
 
   // If the value is offset in memory, apply the offset now.
-  Address StorePtr = emitAddressAtOffset(*this, DestPtr, RetAI);
-  CreateCoercedStore(CI, StorePtr, DestIsVolatile, *this);
+  if (!isEmptyRecord(getContext(), RetTy, true)) {
+Address StorePtr = emitAddressAtOffset(*this, DestPtr, RetAI);

[PATCH] D157750: Properly handle -fsplit-machine-functions for fatbinary compilation

2023-08-11 Thread Artem Belevich via Phabricator via cfe-commits

tra added a comment.






Comment at: clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c:9
+
+// Check that -fsplit-machine-functions is passed to both x86 and cuda 
compilation and does not cause driver error.
+// MFS2: -fsplit-machine-functions

We will still see a warning, right? So, for someone compiling with `-Werror` 
that's going to be a problem.

Also, if the warning is issued from the top-level driver, we may not even be 
able to suppress it when we disable splitting on GPU side with `-Xarch_device 
-fno-split-machine-functions`.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157750

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

[PATCH] D157750: Properly handle -fsplit-machine-functions for fatbinary compilation

2023-08-11 Thread Snehasish Kumar via Phabricator via cfe-commits

snehasish added a subscriber: dhoekwater.
snehasish added inline comments.



Comment at: llvm/lib/CodeGen/TargetPassConfig.cpp:1278
 }
-addPass(createMachineFunctionSplitterPass());
+if (TM->getTargetTriple().isX86())
+  addPass(createMachineFunctionSplitterPass());

Can you coordinate with @dhoekwater ? He has some patches in flight for 
AArch64. 

I think D157157 is the one which modifies the same logic.



Comment at: llvm/test/CodeGen/X86/mfs-triple.ll:8
+
+define void @foo4(i1 zeroext %0, i1 zeroext %1) nounwind {
+  br i1 %0, label %3, label %7

Any reason why we can't use the bitcode already in 
test/CodeGen/machine-function-splitter.ll? (Going to be moved to 
test/Generic/machine-function-splitter.ll in D157563)

IMO we can just reuse the basic test and add these run and check lines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157750

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

[PATCH] D157661: [clang][NFC] Robustify testcase

2023-08-11 Thread Nathan Sidwell via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe789bcbb967f: [clang][NFC] Robustify testcase (authored by 
urnathan).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157661

Files:
  clang/test/CodeGen/fixup-depth-overflow.c


Index: clang/test/CodeGen/fixup-depth-overflow.c
===
--- clang/test/CodeGen/fixup-depth-overflow.c
+++ clang/test/CodeGen/fixup-depth-overflow.c
@@ -24,3 +24,5 @@
 
 // CHECK-LABEL: define {{(dso_local )?}}void @f
 // CHECK-NOT: cleanup
+// CHECK: {{^!0 = }}
+// We might see 'cleanup' as part of a pathname or versioning in metadata


Index: clang/test/CodeGen/fixup-depth-overflow.c
===
--- clang/test/CodeGen/fixup-depth-overflow.c
+++ clang/test/CodeGen/fixup-depth-overflow.c
@@ -24,3 +24,5 @@
 
 // CHECK-LABEL: define {{(dso_local )?}}void @f
 // CHECK-NOT: cleanup
+// CHECK: {{^!0 = }}
+// We might see 'cleanup' as part of a pathname or versioning in metadata
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] e789bcb - [clang][NFC] Robustify testcase

2023-08-11 Thread Nathan Sidwell via cfe-commits


Author: Nathan Sidwell
Date: 2023-08-11T15:59:52-04:00
New Revision: e789bcbb967f391d4f641bc8fa0403a45039a592

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

LOG: [clang][NFC] Robustify testcase

Protect this testcase from 'cleanup' appearing in metadata due to a pathname.

Reviewed By: jroelofs, bruno

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

Added: 


Modified: 
clang/test/CodeGen/fixup-depth-overflow.c

Removed: 




diff  --git a/clang/test/CodeGen/fixup-depth-overflow.c 
b/clang/test/CodeGen/fixup-depth-overflow.c
index 1ae2a41b1c2628..6662eb95bb2d4e 100644
--- a/clang/test/CodeGen/fixup-depth-overflow.c
+++ b/clang/test/CodeGen/fixup-depth-overflow.c
@@ -24,3 +24,5 @@ void f(int x) {
 
 // CHECK-LABEL: define {{(dso_local )?}}void @f
 // CHECK-NOT: cleanup
+// CHECK: {{^!0 = }}
+// We might see 'cleanup' as part of a pathname or versioning in metadata



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

[PATCH] D157632: [Profile] Allow online merging with debug info correlation.

2023-08-11 Thread Ellis Hoag via Phabricator via cfe-commits

ellis added a comment.

In D157632#4580576 , @zequanwu wrote:

> BTW, I noticed something strange with `-pgo-function-entry-coverage` when 
> merging via llvm-profdata.

This is intentional. The two raw profiles individually store blocks as either 
covered or not, but when we merge them they get converted to counters and 
accumulated.
https://github.com/llvm/llvm-project/blob/6a0feb1503e21432e63d93b44357bad43f8026d1/llvm/lib/ProfileData/InstrProf.cpp#L726
Then in `PGOUseFunc::populateCoverage()` we annotate blocks as alive if their 
counter value is non-zero.
https://github.com/llvm/llvm-project/blob/1aee2434ce4c9b5785cbc8f72cbbbd64f9e85297/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp#L1399
My logic was that the indexed profile represents these counters as ints, so we 
might as well accumulate them. Also, this makes the implementation simpler.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157632

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

[PATCH] D156821: [CodeGen] [ubsan] Respect integer overflow handling in abs builtin

2023-08-11 Thread Eli Friedman via Phabricator via cfe-commits

efriedma added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2697
+bool SanitizeBuiltin = SanOpts.has(SanitizerKind::Builtin);
+bool SanitizeOverflow = SanOpts.has(SanitizerKind::SignedIntegerOverflow);
+

artem wrote:
> efriedma wrote:
> > Putting the behavior under both "builtin" and "signed-integer-overflow" 
> > feels a little weird; is there any precedent for that?
> The problem is, we are instrumenting a builtin function, so on the one hand 
> it is reasonable to be controlled by `-fsanitize=builtin`. On the other hand, 
> GCC treats abs() as an arithmetic operation, so it is being instrumented by 
> `-fsanitize=signed-integer-overflow` (`abs(INT_MIN)` causes a negation 
> overflow).
> 
> I have decided to enable instrumentation under any of the flags, but I am not 
> sure whether it is the right choice.
I'd prefer to just follow gcc here, I think, if there isn't any strong reason 
to pick a different approach.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2702
+case LangOptions::SOB_Defined:
+  Result = Builder.CreateBinaryIntrinsic(
+  Intrinsic::abs, EmitScalarExpr(E->getArg(0)), Builder.getFalse(),

artem wrote:
> efriedma wrote:
> > Can we land the change to directly generate calls to llvm.abs() in the 
> > default case separately? This might end up impacting generated code for a 
> > variety of workloads, and I'd prefer to have a more clear bisect point.
> I used llvm.abs() for code simplicity, since middle-end combines the 
> instructions to it anyways. I think this part can be dropped entirely because 
> the intrinsic is not the best possible option either (the compiler emits 
> conditional move and fails to elide extra sign checks if the argument is 
> known to be non-negative).
Sure, that works too.


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

https://reviews.llvm.org/D156821

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

[PATCH] D157750: Properly handle -fsplit-machine-functions for fatbinary compilation

2023-08-11 Thread Han Shen via Phabricator via cfe-commits

shenhan created this revision.
shenhan added reviewers: xur, snehasish.
Herald added subscribers: mattd, asavonic, pengfei, hiraditya.
Herald added a project: All.
shenhan requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, MaskRay.
Herald added projects: clang, LLVM.

When building a fatbinary, the driver invokes the compiler multiple times with 
different "--target". (For example, with "-x cuda --cuda-gpu-arch=sm_70" flags, 
clang will be invoded twice, once with --target=x86_64_, once with 
--target=sm_70) If we use -fsplit-machine-functions or 
-fno-split-machine-functions for such invocation, the driver reports an error.

This CL changes the behavior so:

1. "-fsplit-machine-functions" is now passed to all targets, for non-X86 
targets, the flag is a NOOP and causes a warning.
2. "-fno-split-machine-functions" now negates -fsplit-machine-functions (if 
-fno-split-machine-functions appears after any -fsplit-machine-functions) for 
any target triple, previously, it causes an error.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157750

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c
  clang/test/Driver/fsplit-machine-functions.c
  clang/test/Driver/fsplit-machine-functions2.c
  llvm/lib/CodeGen/TargetPassConfig.cpp
  llvm/test/CodeGen/X86/mfs-triple.ll

Index: llvm/test/CodeGen/X86/mfs-triple.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/mfs-triple.ll
@@ -0,0 +1,38 @@
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -debug-pass=Structure -fs-profile-file=%S/Inputs/fsloader-mfs.afdo -enable-fs-discriminator=true -improved-fs-discriminator=true -split-machine-functions 2>&1 | FileCheck %s --check-prefix=MFS_ON
+; RUN: llc < %s -mtriple=aarch64-unknown-linux-gnu -debug-pass=Structure -fs-profile-file=%S/Inputs/fsloader-mfs.afdo -enable-fs-discriminator=true -improved-fs-discriminator=true -split-machine-functions 2>&1 | FileCheck %s --check-prefix=MFS_WARN
+
+; MFS_ON: Machine Function Splitter Transformation
+; MFS_WARN: warning: -fsplit-machine-functions is only valid for X86.
+; MFS_WARN_NO: Machine Function Splitter Transformation
+
+define void @foo4(i1 zeroext %0, i1 zeroext %1) nounwind {
+  br i1 %0, label %3, label %7
+
+3:
+  %4 = call i32 @bar()
+  br label %7
+
+5:
+  %6 = call i32 @baz()
+  br label %7
+
+7:
+  br i1 %1, label %8, label %10
+
+8:
+  %9 = call i32 @bam()
+  br label %12
+
+10:
+  %11 = call i32 @baz()
+  br label %12
+
+12:
+  %13 = tail call i32 @qux()
+  ret void
+}
+
+declare i32 @bar()
+declare i32 @baz()
+declare i32 @bam()
+declare i32 @qux()
Index: llvm/lib/CodeGen/TargetPassConfig.cpp
===
--- llvm/lib/CodeGen/TargetPassConfig.cpp
+++ llvm/lib/CodeGen/TargetPassConfig.cpp
@@ -1275,7 +1275,11 @@
"performance.";
   }
 }
-addPass(createMachineFunctionSplitterPass());
+if (TM->getTargetTriple().isX86())
+  addPass(createMachineFunctionSplitterPass());
+else
+  WithColor::warning()
+  << "-fsplit-machine-functions is only valid for X86.\n";
   }
 
   addPostBBSections();
Index: clang/test/Driver/fsplit-machine-functions2.c
===
--- clang/test/Driver/fsplit-machine-functions2.c
+++ clang/test/Driver/fsplit-machine-functions2.c
@@ -7,6 +7,9 @@
 // Test the mix of -fsplit-machine-functions and -fno-split-machine-functions
 // RUN: %clang -### -target x86_64-unknown-linux -flto -fsplit-machine-functions -fno-split-machine-functions %s 2>&1 | FileCheck %s -check-prefix=CHECK-NOPASS
 // RUN: %clang -### -target x86_64-unknown-linux -flto -fno-split-machine-functions -fsplit-machine-functions %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS
+// Check that for non-X86, passing no-split-machine-functions does not cause error.
+// RUN: %clang -### -target aarch64-unknown-linux -flto -fsplit-machine-functions -fno-split-machine-functions %s 2>&1 | FileCheck %s -check-prefix=CHECK-NOPASS2
 
 // CHECK-PASS:  "-plugin-opt=-split-machine-functions"
 // CHECK-NOPASS-NOT:"-plugin-opt=-split-machine-functions"
+// CHECK-NOPASS2-NOT:   "-plugin-opt=-split-machine-functions"
Index: clang/test/Driver/fsplit-machine-functions.c
===
--- clang/test/Driver/fsplit-machine-functions.c
+++ clang/test/Driver/fsplit-machine-functions.c
@@ -1,8 +1,8 @@
 // RUN: %clang -### -target x86_64 -fprofile-use=default.profdata -fsplit-machine-functions %s -c 2>&1 | FileCheck -check-prefix=CHECK-OPT %s
 // RUN: %clang -### -target x86_64 -fsplit-machine-functions %s -c 2>&1 | FileCheck -check-prefix=CHECK-OPT %s
 // RUN: %clang -### -target x86_64 -fprofile-use=default.profdata -fsplit-machine-functions

[PATCH] D157332: [clang] Make init for empty no_unique_address fields a no-op write

2023-08-11 Thread Eli Friedman via Phabricator via cfe-commits

efriedma added a comment.

You can't, in general, check whether a type is stored in a no_unique_address 
field.  Consider the following; the bad store is in the constructor S2, but the 
relevant no_unique_address attribute doesn't even show up until the definition 
of S3.

  struct S {};
  S f();
  struct S2 : public S { S2();};
  S2::S2() : S(f()) {}
  struct S3 { int x; [[no_unique_address]] S2 y; S3(); };
  S3::S3() : x(1), y() {}

So we have to suppress all stores of empty types, whether or not we see a 
no_unique_address attribute.

Given that, I don't think the isDummy field is necessary; the important part is 
just whether the type is empty, and you can derive that directly from the type 
itself.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157332

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

[PATCH] D156737: clang: Add __builtin_elementwise_sqrt

2023-08-11 Thread Matt Arsenault via Phabricator via cfe-commits

arsenm updated this revision to Diff 549490.
arsenm added a comment.

Release note


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

https://reviews.llvm.org/D156737

Files:
  clang/docs/LanguageExtensions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/Builtins.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtins-elementwise-math.c
  clang/test/CodeGen/strictfp-elementwise-bulitins.cpp
  clang/test/CodeGenCUDA/correctly-rounded-div.cu
  clang/test/CodeGenOpenCL/fpmath.cl
  clang/test/Sema/builtins-elementwise-math.c
  clang/test/SemaCXX/builtins-elementwise-math.cpp

Index: clang/test/SemaCXX/builtins-elementwise-math.cpp
===
--- clang/test/SemaCXX/builtins-elementwise-math.cpp
+++ clang/test/SemaCXX/builtins-elementwise-math.cpp
@@ -111,6 +111,13 @@
   static_assert(!is_const::value);
 }
 
+void test_builtin_elementwise_sqrt() {
+  const float a = 42.0;
+  float b = 42.3;
+  static_assert(!is_const::value);
+  static_assert(!is_const::value);
+}
+
 void test_builtin_elementwise_log() {
   const float a = 42.0;
   float b = 42.3;
Index: clang/test/Sema/builtins-elementwise-math.c
===
--- clang/test/Sema/builtins-elementwise-math.c
+++ clang/test/Sema/builtins-elementwise-math.c
@@ -601,6 +601,27 @@
   // expected-error@-1 {{1st argument must be a floating point type (was 'unsigned4' (vector of 4 'unsigned int' values))}}
 }
 
+void test_builtin_elementwise_sqrt(int i, float f, double d, float4 v, int3 iv, unsigned u, unsigned4 uv) {
+
+  struct Foo s = __builtin_elementwise_sqrt(f);
+  // expected-error@-1 {{initializing 'struct Foo' with an expression of incompatible type 'float'}}
+
+  i = __builtin_elementwise_sqrt();
+  // expected-error@-1 {{too few arguments to function call, expected 1, have 0}}
+
+  i = __builtin_elementwise_sqrt(i);
+  // expected-error@-1 {{1st argument must be a floating point type (was 'int')}}
+
+  i = __builtin_elementwise_sqrt(f, f);
+  // expected-error@-1 {{too many arguments to function call, expected 1, have 2}}
+
+  u = __builtin_elementwise_sqrt(u);
+  // expected-error@-1 {{1st argument must be a floating point type (was 'unsigned int')}}
+
+  uv = __builtin_elementwise_sqrt(uv);
+  // expected-error@-1 {{1st argument must be a floating point type (was 'unsigned4' (vector of 4 'unsigned int' values))}}
+}
+
 void test_builtin_elementwise_trunc(int i, float f, double d, float4 v, int3 iv, unsigned u, unsigned4 uv) {
 
   struct Foo s = __builtin_elementwise_trunc(f);
Index: clang/test/CodeGenOpenCL/fpmath.cl
===
--- clang/test/CodeGenOpenCL/fpmath.cl
+++ clang/test/CodeGenOpenCL/fpmath.cl
@@ -28,6 +28,21 @@
   return __builtin_sqrtf(a);
 }
 
+float elementwise_sqrt_f32(float a) {
+  // CHECK-LABEL: @elementwise_sqrt_f32
+  // NODIVOPT: call float @llvm.sqrt.f32(float %{{.+}}), !fpmath ![[MD_SQRT:[0-9]+]]
+  // DIVOPT: call float @llvm.sqrt.f32(float %{{.+}}){{$}}
+  return __builtin_elementwise_sqrt(a);
+}
+
+float4 elementwise_sqrt_v4f32(float4 a) {
+  // CHECK-LABEL: @elementwise_sqrt_v4f32
+  // NODIVOPT: call <4 x float> @llvm.sqrt.v4f32(<4 x float> %{{.+}}), !fpmath ![[MD_SQRT:[0-9]+]]
+  // DIVOPT: call <4 x float> @llvm.sqrt.v4f32(<4 x float> %{{.+}}){{$}}
+  return __builtin_elementwise_sqrt(a);
+}
+
+
 #if __OPENCL_C_VERSION__ >=120
 void printf(constant char* fmt, ...);
 
@@ -61,6 +76,18 @@
   return __builtin_sqrt(a);
 }
 
+double elementwise_sqrt_f64(double a) {
+  // CHECK-LABEL: @elementwise_sqrt_f64
+  // CHECK: call double @llvm.sqrt.f64(double %{{.+}}){{$}}
+  return __builtin_elementwise_sqrt(a);
+}
+
+double4 elementwise_sqrt_v4f64(double4 a) {
+  // CHECK-LABEL: @elementwise_sqrt_v4f64
+  // CHECK: call <4 x double> @llvm.sqrt.v4f64(<4 x double> %{{.+}}){{$}}
+  return __builtin_elementwise_sqrt(a);
+}
+
 #endif
 
 // NODIVOPT: ![[MD_FDIV]] = !{float 2.50e+00}
Index: clang/test/CodeGenCUDA/correctly-rounded-div.cu
===
--- clang/test/CodeGenCUDA/correctly-rounded-div.cu
+++ clang/test/CodeGenCUDA/correctly-rounded-div.cu
@@ -46,4 +46,18 @@
   return __builtin_sqrt(a);
 }
 
+// COMMON-LABEL: @_Z28test_builtin_elementwise_f32f
+// NCRDIV: call contract float @llvm.sqrt.f32(float %{{.+}}), !fpmath ![[MD:[0-9]+]]
+// CRDIV: call contract float @llvm.sqrt.f32(float %{{.+}}){{$}}
+__device__ float test_builtin_elementwise_f32(float a) {
+  return __builtin_elementwise_sqrt(a);
+}
+
+// COMMON-LABEL: @_Z28test_builtin_elementwise_f64d
+// COMMON: call contract double @llvm.sqrt.f64(double %{{.+}}){{$}}
+// COMMON-NOT: !fpmath
+__device__ double test_builtin_elementwise_f64(double a) {
+  return __builtin_elementwise_sqrt(a);
+}
+
 // NCRSQRT: ![[MD]] = !{float 2.50e+00}
Index:

[PATCH] D157040: [OpenMP][IR] Set correct alignment for internal variables

2023-08-11 Thread Congzhe Cao via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc384e79675fe: [OpenMP][IR] Set correct alignment for 
internal variables (authored by erjin, committed by congzhe).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157040

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp


Index: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
===
--- llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -2744,7 +2744,15 @@
   PointerType *CriticalNamePtrTy =
   PointerType::getUnqual(ArrayType::get(Type::getInt32Ty(Ctx), 8));
   EXPECT_EQ(CriticalEndCI->getArgOperand(2), 
CriticalEntryCI->getArgOperand(2));
-  EXPECT_EQ(CriticalEndCI->getArgOperand(2)->getType(), CriticalNamePtrTy);
+  GlobalVariable *GV =
+  dyn_cast(CriticalEndCI->getArgOperand(2));
+  ASSERT_NE(GV, nullptr);
+  EXPECT_EQ(GV->getType(), CriticalNamePtrTy);
+  const DataLayout  = M->getDataLayout();
+  const llvm::Align TypeAlign = DL.getABITypeAlign(CriticalNamePtrTy);
+  const llvm::Align PtrAlign = 
DL.getPointerABIAlignment(GV->getAddressSpace());
+  if (const llvm::MaybeAlign Alignment = GV->getAlign())
+EXPECT_EQ(*Alignment, std::max(TypeAlign, PtrAlign));
 }
 
 TEST_F(OpenMPIRBuilderTest, OrderedDirectiveDependSource) {
Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -4493,7 +4493,10 @@
 M, Ty, /*IsConstant=*/false, GlobalValue::CommonLinkage,
 Constant::getNullValue(Ty), Elem.first(),
 /*InsertBefore=*/nullptr, GlobalValue::NotThreadLocal, AddressSpace);
-GV->setAlignment(M.getDataLayout().getABITypeAlign(Ty));
+const DataLayout  = M.getDataLayout();
+const llvm::Align TypeAlign = DL.getABITypeAlign(Ty);
+const llvm::Align PtrAlign = DL.getPointerABIAlignment(AddressSpace);
+GV->setAlignment(std::max(TypeAlign, PtrAlign));
 Elem.second = GV;
   }
 
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -2161,11 +2161,7 @@
 llvm::Value *CGOpenMPRuntime::getCriticalRegionLock(StringRef CriticalName) {
   std::string Prefix = Twine("gomp_critical_user_", CriticalName).str();
   std::string Name = getName({Prefix, "var"});
-  llvm::GlobalVariable *G = 
OMPBuilder.getOrCreateInternalVariable(KmpCriticalNameTy, Name);
-  llvm::Align PtrAlign = 
OMPBuilder.M.getDataLayout().getPointerABIAlignment(G->getAddressSpace());
-  if (PtrAlign > llvm::Align(G->getAlignment()))
-G->setAlignment(PtrAlign);
-  return G;
+  return OMPBuilder.getOrCreateInternalVariable(KmpCriticalNameTy, Name);
 }
 
 namespace {


Index: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
===
--- llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -2744,7 +2744,15 @@
   PointerType *CriticalNamePtrTy =
   PointerType::getUnqual(ArrayType::get(Type::getInt32Ty(Ctx), 8));
   EXPECT_EQ(CriticalEndCI->getArgOperand(2), CriticalEntryCI->getArgOperand(2));
-  EXPECT_EQ(CriticalEndCI->getArgOperand(2)->getType(), CriticalNamePtrTy);
+  GlobalVariable *GV =
+  dyn_cast(CriticalEndCI->getArgOperand(2));
+  ASSERT_NE(GV, nullptr);
+  EXPECT_EQ(GV->getType(), CriticalNamePtrTy);
+  const DataLayout  = M->getDataLayout();
+  const llvm::Align TypeAlign = DL.getABITypeAlign(CriticalNamePtrTy);
+  const llvm::Align PtrAlign = DL.getPointerABIAlignment(GV->getAddressSpace());
+  if (const llvm::MaybeAlign Alignment = GV->getAlign())
+EXPECT_EQ(*Alignment, std::max(TypeAlign, PtrAlign));
 }
 
 TEST_F(OpenMPIRBuilderTest, OrderedDirectiveDependSource) {
Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -4493,7 +4493,10 @@
 M, Ty, /*IsConstant=*/false, GlobalValue::CommonLinkage,
 Constant::getNullValue(Ty), Elem.first(),
 /*InsertBefore=*/nullptr, GlobalValue::NotThreadLocal, AddressSpace);
-GV->setAlignment(M.getDataLayout().getABITypeAlign(Ty));
+const DataLayout  = M.getDataLayout();
+const llvm::Align TypeAlign = DL.getABITypeAlign(Ty);
+const llvm::Align PtrAlign = DL.getPointerABIAlignment(AddressSpace);
+GV->setAlignment(std::max(TypeAlign, PtrAlign));
 Elem.second = GV;
   }
 
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp

[clang] c384e79 - [OpenMP][IR] Set correct alignment for internal variables

2023-08-11 Thread via cfe-commits


Author: Hao Jin
Date: 2023-08-11T15:20:15-04:00
New Revision: c384e79675fe44cd2651dbd9d506d0a421c37533

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

LOG: [OpenMP][IR] Set correct alignment for internal variables

OpenMP runtime functions assume the pointers are aligned to sizeof(pointer),
but it is being aligned incorrectly. Fix with the proper alignment in the IR 
builder.

Reviewed By: tianshilei1992

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

Added: 


Modified: 
clang/lib/CodeGen/CGOpenMPRuntime.cpp
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index 02410c4d2b4250..fad8faad68c479 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -2161,11 +2161,7 @@ Address 
CGOpenMPRuntime::emitThreadIDAddress(CodeGenFunction ,
 llvm::Value *CGOpenMPRuntime::getCriticalRegionLock(StringRef CriticalName) {
   std::string Prefix = Twine("gomp_critical_user_", CriticalName).str();
   std::string Name = getName({Prefix, "var"});
-  llvm::GlobalVariable *G = 
OMPBuilder.getOrCreateInternalVariable(KmpCriticalNameTy, Name);
-  llvm::Align PtrAlign = 
OMPBuilder.M.getDataLayout().getPointerABIAlignment(G->getAddressSpace());
-  if (PtrAlign > llvm::Align(G->getAlignment()))
-G->setAlignment(PtrAlign);
-  return G;
+  return OMPBuilder.getOrCreateInternalVariable(KmpCriticalNameTy, Name);
 }
 
 namespace {

diff  --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp 
b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index 1694df8f482e67..3eedef17b0d080 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -4493,7 +4493,10 @@ OpenMPIRBuilder::getOrCreateInternalVariable(Type *Ty, 
const StringRef ,
 M, Ty, /*IsConstant=*/false, GlobalValue::CommonLinkage,
 Constant::getNullValue(Ty), Elem.first(),
 /*InsertBefore=*/nullptr, GlobalValue::NotThreadLocal, AddressSpace);
-GV->setAlignment(M.getDataLayout().getABITypeAlign(Ty));
+const DataLayout  = M.getDataLayout();
+const llvm::Align TypeAlign = DL.getABITypeAlign(Ty);
+const llvm::Align PtrAlign = DL.getPointerABIAlignment(AddressSpace);
+GV->setAlignment(std::max(TypeAlign, PtrAlign));
 Elem.second = GV;
   }
 

diff  --git a/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp 
b/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
index 51af5eb392f5ae..0ec7b751be60e3 100644
--- a/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ b/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -2744,7 +2744,15 @@ TEST_F(OpenMPIRBuilderTest, CriticalDirective) {
   PointerType *CriticalNamePtrTy =
   PointerType::getUnqual(ArrayType::get(Type::getInt32Ty(Ctx), 8));
   EXPECT_EQ(CriticalEndCI->getArgOperand(2), 
CriticalEntryCI->getArgOperand(2));
-  EXPECT_EQ(CriticalEndCI->getArgOperand(2)->getType(), CriticalNamePtrTy);
+  GlobalVariable *GV =
+  dyn_cast(CriticalEndCI->getArgOperand(2));
+  ASSERT_NE(GV, nullptr);
+  EXPECT_EQ(GV->getType(), CriticalNamePtrTy);
+  const DataLayout  = M->getDataLayout();
+  const llvm::Align TypeAlign = DL.getABITypeAlign(CriticalNamePtrTy);
+  const llvm::Align PtrAlign = 
DL.getPointerABIAlignment(GV->getAddressSpace());
+  if (const llvm::MaybeAlign Alignment = GV->getAlign())
+EXPECT_EQ(*Alignment, std::max(TypeAlign, PtrAlign));
 }
 
 TEST_F(OpenMPIRBuilderTest, OrderedDirectiveDependSource) {



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

[PATCH] D156726: Make globals with mutable members non-constant, even in custom sections

2023-08-11 Thread Eli Friedman via Phabricator via cfe-commits

efriedma added a comment.

Sure, diverging from MSVC here seems fine.  I agree it's unlikely anyone would 
actually want to put a variable that will be modified in a "const" section.




Comment at: clang/lib/Sema/SemaDecl.cpp:14254
 int SectionFlags = ASTContext::PSF_Read;
-if (var->getType().isConstQualified()) {
-  if (HasConstInit)

dblaikie wrote:
> efriedma wrote:
> > dblaikie wrote:
> > > rnk wrote:
> > > > rsmith wrote:
> > > > > efriedma wrote:
> > > > > > rnk wrote:
> > > > > > > I think this is not compatible with MSVC. MSVC uses simple logic, 
> > > > > > > it doesn't look for mutable: https://gcc.godbolt.org/z/sj6d4saxx
> > > > > > > 
> > > > > > > The const mutable struct appears in the myrdata section in that 
> > > > > > > example.
> > > > > > > 
> > > > > > > I think the solution is to separate the flag logic from the 
> > > > > > > pragma stack selection logic, which has to remain MSVC-compatible.
> > > > > > MSVC apparently looks at whether the variable is marked "const", 
> > > > > > and nothing else; it doesn't look at mutable, it doesn't look at 
> > > > > > whether the variable has a constant initializer.  So the current 
> > > > > > code isn't right either; if we're trying to implement 
> > > > > > MSVC-compatible logic, we shouldn't check HasConstInit.
> > > > > > 
> > > > > > That said, I'm not sure how precisely/in what modes we want to 
> > > > > > precisely emulate MSVC.  Probably anything we do here will be 
> > > > > > confusing.
> > > > > We should at least issue a warning if the sensible logic and the 
> > > > > MSVC-compatible calculation differ. @rnk, do you know how important 
> > > > > it is to follow the MSVC semantics in this regard?
> > > > I think it depends on whether you think that users are primarily using 
> > > > these pragmas to override the default rdata/bss/data sections without 
> > > > any care about precisely what goes where, or if they are using them to 
> > > > do something finer grained.
> > > > 
> > > > If I had to guess, I'd say it's more likely the former, given that 
> > > > `__declspec(allocate)` and `#pragma(section)` exist to handle cases 
> > > > where users are putting specific globals into specific sections.
> > > > 
> > > > Which, if we follow Richard's suggestion, would mean warning when we 
> > > > put a global marked `const` into a writable section when 
> > > > `ConstSegStack` is non-empty. That seems reasonable. 
> > > > `-Wmicrosoft-const-seg` for the new warning group?
> > > Does the MSVC situation only apply to custom sections? (presumably when 
> > > not customizing the section, MSVC gets it right and can support a const 
> > > global with a runtime initializer, mutable member, or mutating dtor?)
> > > 
> > > I think this code still needs to be modified, since this is the code that 
> > > drives the error about incompatible sections. So it'll need to behave 
> > > differently depending on the target platform?
> > Yes, the MSVC situation is specifically if you specify `#pragma const_seg`; 
> > without the pragma, it does what you'd expect.
> Went with the "let's do the thing that the user probably wants, but isn't 
> what MSVC does, and warn when that difference comes up" - if that's OK with 
> everyone.
> 
> (always open to wordsmithing the warning - and if we want to, can go to the 
> extra layer and specifically diagnose which reason (mutable members, 
> non-const init) - and I can't quite figure out the best phrasing to say 
> "we're putting it in section X insetad of section Y, because Z, but Microsoft 
> would use X because A" or something... it's all a bit of a mouthful)
Describing which reason actually applies would make the warning a lot easier to 
read.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156726

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

[PATCH] D157747: Support Unicode Microsoft predefined macros

2023-08-11 Thread Richard Dzenis via Phabricator via cfe-commits

RIscRIpt created this revision.
Herald added subscribers: PiotrZSL, carlosgalvezp, martong.
Herald added a reviewer: shafik.
Herald added a reviewer: njames93.
Herald added a project: All.
RIscRIpt requested review of this revision.
Herald added projects: clang, clang-tools-extra.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157747

Files:
  clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.cpp
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/Stmt.h
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/Lex/LiteralSupport.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/TreeTransform.h
  clang/test/AST/Interp/literals.cpp
  clang/test/Sema/ms_predefined_expr.cpp

Index: clang/test/Sema/ms_predefined_expr.cpp
===
--- clang/test/Sema/ms_predefined_expr.cpp
+++ clang/test/Sema/ms_predefined_expr.cpp
@@ -1,16 +1,31 @@
-// RUN: %clang_cc1 %s -fsyntax-only -Wmicrosoft -verify -fms-extensions
+// RUN: %clang_cc1 %s -fsyntax-only -Wmicrosoft -verify -fms-extensions -std=c++20
 
 using size_t = __SIZE_TYPE__;
 
 // Test array initialization
 void array_init() {
- const char a[] = __FUNCTION__; // expected-warning{{initializing an array from a '__FUNCTION__' predefined identifier is a Microsoft extension}}
- const char b[] = __FUNCDNAME__; // expected-warning{{initializing an array from a '__FUNCDNAME__' predefined identifier is a Microsoft extension}}
- const char c[] = __FUNCSIG__; // expected-warning{{initializing an array from a '__FUNCSIG__' predefined identifier is a Microsoft extension}}
- const char d[] = __func__; // expected-warning{{initializing an array from a '__func__' predefined identifier is a Microsoft extension}}
- const char e[] = __PRETTY_FUNCTION__; // expected-warning{{initializing an array from a '__PRETTY_FUNCTION__' predefined identifier is a Microsoft extension}}
- const wchar_t f[] = L__FUNCTION__; // expected-warning{{initializing an array from a 'L__FUNCTION__' predefined identifier is a Microsoft extension}}
- const wchar_t g[] = L__FUNCSIG__; // expected-warning{{initializing an array from a 'L__FUNCSIG__' predefined identifier is a Microsoft extension}}
+ const char a_f[] = __func__; // expected-warning{{initializing an array from a '__func__' predefined identifier is a Microsoft extension}}
+ const char a_P[] = __PRETTY_FUNCTION__; // expected-warning{{initializing an array from a '__PRETTY_FUNCTION__' predefined identifier is a Microsoft extension}}
+
+ const char a_F[] = __FUNCTION__;  // expected-warning{{initializing an array from a '__FUNCTION__' predefined identifier is a Microsoft extension}}
+ const char a_D[] = __FUNCDNAME__; // expected-warning{{initializing an array from a '__FUNCDNAME__' predefined identifier is a Microsoft extension}}
+ const char a_S[] = __FUNCSIG__;   // expected-warning{{initializing an array from a '__FUNCSIG__' predefined identifier is a Microsoft extension}}
+
+ const wchar_t L_F[] = L__FUNCTION__;  // expected-warning{{initializing an array from a 'L__FUNCTION__' predefined identifier is a Microsoft extension}}
+ const wchar_t L_D[] = L__FUNCDNAME__; // expected-warning{{initializing an array from a 'L__FUNCDNAME__' predefined identifier is a Microsoft extension}}
+ const wchar_t L_S[] = L__FUNCSIG__;   // expected-warning{{initializing an array from a 'L__FUNCSIG__' predefined identifier is a Microsoft extension}}
+
+ const char8_t u8_F[] = u8__FUNCTION__;  // expected-warning{{initializing an array from a 'u8__FUNCTION__' predefined identifier is a Microsoft extension}}
+ const char8_t u8_D[] = u8__FUNCDNAME__; // expected-warning{{initializing an array from a 'u8__FUNCDNAME__' predefined identifier is a Microsoft extension}}
+ const char8_t u8_S[] = u8__FUNCSIG__;   // expected-warning{{initializing an array from a 'u8__FUNCSIG__' predefined identifier is a Microsoft extension}}
+
+ const char16_t u_F[] = u__FUNCTION__;  // expected-warning{{initializing an array from a 'u__FUNCTION__' predefined identifier is a Microsoft extension}}
+ const char16_t u_D[] = u__FUNCDNAME__; // expected-warning{{initializing an array from a 'u__FUNCDNAME__' predefined identifier is a Microsoft extension}}
+ const char16_t u_S[] = u__FUNCSIG__;   // expected-warning{{initializing an array from a 'u__FUNCSIG__' predefined identifier is a Microsoft extension}}
+
+ const char32_t U_F[] = U__FUNCTION__;  // expected-warning{{initializing an array from a 'U__FUNCTION__' predefined identifier is a Microsoft extension}}
+ const char32_t U_D[] = U__FUNCDNAME__; // expected-warning{{initializing an array from a 'U__FUNCDNAME__'

[PATCH] D155794: [OpenMP][OpenMPIRBuilder] Migrate setPropertyExecutionMode() from Clang to OpenMPIRBuilder.

2023-08-11 Thread Raghu via Phabricator via cfe-commits

raghavendhra abandoned this revision.
raghavendhra added a comment.

setPropertyExecutionMode function is no longer present / used in clang because 
of a change in https://reviews.llvm.org/D142569 So abandoning this patch to 
migrate that function into OMPIRBuilder.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155794

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

[PATCH] D156565: Diagnose use of VLAs in C++ by default

2023-08-11 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In D156565#4580716 , @Endill wrote:

> In D156565#4547909 , @aaron.ballman 
> wrote:
>
>> In D156565#4543503 , 
>> @aaron.ballman wrote:
>>
>>> In D156565#4543414 , @jrtc27 
>>> wrote:
>>>
 Given GCC defines GNU C++ and regards this as a feature (unless you use 
 things like -pedantic to ask for ISO C++), does it make sense to enable 
 this for GNU C++?
>>>
>>> I think GCC should enable -Wvla by default in GNU C++ as well, for the same 
>>> reasons I'm proposing it for Clang. I've filed an issue for it at 
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110848
>>
>> The GCC conversation is leaning towards only diagnosing by default in C++ 
>> mode but not in GNU++ mode. I'm still trying to persuade them to diagnose in 
>> both modes one last time, but if it looks like they're firm about not 
>> diagnosing in GNU++ mode, I can live with that (for now). It at least 
>> improves our security posture a bit, so it's definitely a win.
>
> I think that we should warn by default in GNU mode regardless of GCC 
> decision. As for the porting concern, I think it falls into "comprehensive 
> diagnostics" selling point you mentioned earlier, which I totally agree with.

The current discussion on the GCC issue is to diagnose by default in C++ mode 
and add `-Wvla` to `-Wall` in GNU++ mode, which perhaps is a nice compromise. 
I'm waiting to see if any further discussion happens on that issue, but if 
folks have opinions on that approach, I'd love to hear them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156565

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

[PATCH] D157130: [RISCV] Check type size for lax conversions between RVV builtin types and VectorType::RVVFixedLengthDataVector.

2023-08-11 Thread Craig Topper via Phabricator via cfe-commits

craig.topper added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157130

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

[PATCH] D155773: [llvm][MemoryBuiltins] Add alloca support to getInitialValueOfAllocation

2023-08-11 Thread John McIver via Phabricator via cfe-commits

jmciver added inline comments.



Comment at: llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp:809-811
+  Updater.AddAvailableValue(
+  Alloca.getParent(),
+  getInitialValueOfAllocation(, nullptr, VectorTy));

arsenm wrote:
> This is very specifically handling alloca, not any random allocation like 
> function
@arsenm thanks for the feedback. I added functionality to 
`getInitalValueOfAllocation` to handle `alloca` instructions specifically. This 
is being done as preliminary to some possible refactorizations allowing 
uninitialized memory to move to poison semantics. The behavior for these 
changes would be the same for `alloca` and allocation like functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155773

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

[PATCH] D157738: [OpenMP] Emit offloading entries for indirect target variables

2023-08-11 Thread Joseph Huber via Phabricator via cfe-commits

jhuber6 created this revision.
jhuber6 added reviewers: jdoerfert, tianshilei1992, ye-luo, ABataev, 
RaviNarayanaswamy.
Herald added subscribers: guansong, hiraditya, yaxunl.
Herald added a project: All.
jhuber6 requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, jplehr, sstefan1.
Herald added projects: clang, LLVM.

OpenMP 5.1 allows emission of the `indirect` clause on declare target
functions, see 
https://www.openmp.org/spec-html/5.1/openmpsu70.html#x98-1080002.14.7.
The intended use of this is to permit calling device functions via their
associated host pointer. In order to do this the first step will be
building a map associating these variables. Doing this will require the
same offloading entry handling we use for other kernels and globals.

We intentionally emit a new global on the device side. Although it's
possible to look up the device function's address directly, this would
require changing the visibility and would prevent us from making static
functions indirect. Also, the CUDA toolchain will optimize out unused
functions and using a global prevents that. The downside is that the
runtime will need to read the global and copy its value, but there
shouldn't be any other costs.

Note that this patch just performs the codegen, currently this new
offloading entry type is unused and will be ignored by the runtime.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157738

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/OpenMP/target_indirect_codegen.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -5525,9 +5525,10 @@
 
 void OpenMPIRBuilder::createOffloadEntry(Constant *ID, Constant *Addr,
  uint64_t Size, int32_t Flags,
- GlobalValue::LinkageTypes) {
+ GlobalValue::LinkageTypes,
+ StringRef Name) {
   if (!Config.isGPU()) {
-emitOffloadingEntry(ID, Addr->getName(), Size, Flags);
+emitOffloadingEntry(ID, Name.empty() ? Addr->getName() : Name, Size, Flags);
 return;
   }
   // TODO: Add support for global variables on the device after declare target
@@ -5687,13 +5688,20 @@
 
   // Hidden or internal symbols on the device are not externally visible.
   // We should not attempt to register them by creating an offloading
-  // entry.
+  // entry. Indirect variables are handled separately on the device.
   if (auto *GV = dyn_cast(CE->getAddress()))
-if (GV->hasLocalLinkage() || GV->hasHiddenVisibility())
+if ((GV->hasLocalLinkage() || GV->hasHiddenVisibility()) &&
+Flags != OffloadEntriesInfoManager::OMPTargetGlobalVarEntryIndirect)
   continue;
 
-  createOffloadEntry(CE->getAddress(), CE->getAddress(), CE->getVarSize(),
- Flags, CE->getLinkage());
+  // Indirect globals need to use a special name that doesn't match the name
+  // of the associated host global.
+  if (Flags == OffloadEntriesInfoManager::OMPTargetGlobalVarEntryIndirect)
+createOffloadEntry(CE->getAddress(), CE->getAddress(), CE->getVarSize(),
+   Flags, CE->getLinkage(), CE->getVarName());
+  else
+createOffloadEntry(CE->getAddress(), CE->getAddress(), CE->getVarSize(),
+   Flags, CE->getLinkage());
 
 } else {
   llvm_unreachable("Unsupported entry kind.");
@@ -6038,8 +6046,13 @@
   }
   return;
 }
-OffloadEntriesDeviceGlobalVar.try_emplace(VarName, OffloadingEntriesNum,
-  Addr, VarSize, Flags, Linkage);
+if (Flags == OffloadEntriesInfoManager::OMPTargetGlobalVarEntryIndirect)
+  OffloadEntriesDeviceGlobalVar.try_emplace(VarName, OffloadingEntriesNum,
+Addr, VarSize, Flags, Linkage,
+VarName.str());
+else
+  OffloadEntriesDeviceGlobalVar.try_emplace(
+  VarName, OffloadingEntriesNum, Addr, VarSize, Flags, Linkage, "");
 ++OffloadingEntriesNum;
   }
 }
Index: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
===
--- llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -326,6 +326,8 @@
 OMPTargetGlobalVarEntryEnter = 0x2,
 /// Mark the entry as having no declare target entry kind.
 OMPTargetGlobalVarEntryNone = 0x3,
+/// Mark the entry as a declare target indirect global.
+

[PATCH] D155064: [clang][SemaCXX] Diagnose tautological uses of consteval if and is_constant_evaluated

2023-08-11 Thread Christopher Di Bella via Phabricator via cfe-commits

cjdb added inline comments.



Comment at: clang/test/SemaCXX/warn-constant-evaluated-constexpr.cpp:38
 constexpr int fn5() {
-  if constexpr (__builtin_is_constant_evaluated()) // expected-warning 
{{'__builtin_is_constant_evaluated' will always evaluate to 'true' in a 
manifestly constant-evaluated expression}}
+  if constexpr (__builtin_is_constant_evaluated()) // expected-warning 
{{'__builtin_is_constant_evaluated' will always evaluate to true in this 
context}}
 return 0;

This should also generate a fix-it hint, since we can automate the fix.



Comment at: clang/test/SemaCXX/warn-tautological-meta-constant.cpp:18
+  } else {
+if constexpr (std::is_constant_evaluated()) { // expected-warning {{always 
evaluate to true}}
+  return 0;

I'm not a fan of this diagnostic text: it doesn't offer insight into what's 
gone wrong or provide actionable feedback on how to fix the code.


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

https://reviews.llvm.org/D155064

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

[PATCH] D155978: [SPIRV] Add SPIR-V logical triple.

2023-08-11 Thread Anastasia Stulova via Phabricator via cfe-commits

Anastasia added inline comments.

Comment at: llvm/unittests/TargetParser/TripleTest.cpp:1307
+  EXPECT_TRUE(T.isSPIRV());
+
   T.setArch(Triple::spirv32);

pmatos wrote:
> Keenuts wrote:
> > pmatos wrote:
> > > I am not well-versed in SPIRV for gfx but if we are using 32bits in SPIRV 
> > > logical, can't we reuse spirv32? Is there some sort of strong reason not 
> > > to or adding spirv for logical spirv as an alternative to spirv32 and 
> > > spirv64 is just easier?
> > This is a very valid question! And I'm not sure of the best option.
> > My understanding is: in logical SPIR-V, there are no notion of pointer 
> > size, or architecture size. We cannot offset pointers by a sizeof(), or do 
> > such things.
> > 
> > But the options I had didn't seem to allow this kind of "undefined 
> > architecture".
> > I chose 32bits here because the IDs are 32bit. But pointer addresses are 
> > not, so returning 32bit is not quite right either.
> > 
> > 
> This is a tricky one tbh - I would have to do a bit more research to have a 
> more insighful reply here. Maybe @mpaszkowski or @Anastasia can add more.
I think to do this properly in LLVM would require an IR extension or something, 
it is maybe worth starting RFC to discuss this.

Out of curiosity do you have any code that can be compiled from any GFX source 
language that would need a pointer size to be emitted in IR? If there is no 
code that can be written in such a way then using one fixed pointer width could 
be ok. Then the SPIR-V backend should just recognise it and lower as required. 
Otherwise target configurable types might be useful: 
https://llvm.org/docs/LangRef.html#target-extension-type

In general we tried to avoid adding bitwidth neutral SPIR-V triple originally 
just because LLVM always requires a pointer width. We were thinking of adding 
vulkan as a component to the existing SPIR-V 34|64 targets 
https://discourse.llvm.org/t/setting-version-environment-and-extensions-for-spir-v-target.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155978

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

[PATCH] D156565: Diagnose use of VLAs in C++ by default

2023-08-11 Thread Vlad Serebrennikov via Phabricator via cfe-commits

Endill added a comment.

In D156565#4547909 , @aaron.ballman 
wrote:

> In D156565#4543503 , @aaron.ballman 
> wrote:
>
>> In D156565#4543414 , @jrtc27 wrote:
>>
>>> Given GCC defines GNU C++ and regards this as a feature (unless you use 
>>> things like -pedantic to ask for ISO C++), does it make sense to enable 
>>> this for GNU C++?
>>
>> I think GCC should enable -Wvla by default in GNU C++ as well, for the same 
>> reasons I'm proposing it for Clang. I've filed an issue for it at 
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110848
>
> The GCC conversation is leaning towards only diagnosing by default in C++ 
> mode but not in GNU++ mode. I'm still trying to persuade them to diagnose in 
> both modes one last time, but if it looks like they're firm about not 
> diagnosing in GNU++ mode, I can live with that (for now). It at least 
> improves our security posture a bit, so it's definitely a win.

I think that we should warn by default in GNU mode regardless of GCC decision. 
As for the porting concern, I think it falls into "comprehensive diagnostics" 
selling point you mentioned earlier, which I totally agree with.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156565

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

[PATCH] D156737: clang: Add __builtin_elementwise_sqrt

2023-08-11 Thread Joshua Batista via Phabricator via cfe-commits

bob80905 added a comment.

Shouldn't this new builtin be mentioned in Release Notes?


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

https://reviews.llvm.org/D156737

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

[PATCH] D157197: [clang][CodeGen][OpenMP] Fix if-clause for 'target teams loop'

2023-08-11 Thread Alexey Bataev via Phabricator via cfe-commits

ABataev added inline comments.



Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:1570-1575
+  // If we are here with a 'target teams loop' then we are emitting the
+  // 'parallel' region of the 'target teams distribute parallel for'
+  // emitted in place of the 'target teams loop'. Based on the specification
+  // noted above, an if-clause associated with a 'target teams loop', be it
+  // 'if(val)' or an 'if(target:val)', will apply only to 'target' and not
+  // the 'parallel' of the 'target teams distribute parallel for'.

ddpagan wrote:
> ABataev wrote:
> > It does not match the spec. 
> > ```
> > For a combined or composite construct, if no directive-name-modifier is 
> > specified then the if clause applies to all constituent constructs to which 
> > an if clause can apply.
> > ```
> > So, if(val) should be applied to both target and parallel regions, no?
> > It does not match the spec. 
> > ```
> > For a combined or composite construct, if no directive-name-modifier is 
> > specified then the if clause applies to all constituent constructs to which 
> > an if clause can apply.
> > ```
> > So, if(val) should be applied to both target and parallel regions, no?
> 
> Hi Alexey - Question for you: does revising the comment above at lines 
> 1570-1575 to the following text help explain in a better way what's being 
> done, and why?
> 
>   If we are handling a 'target teams distribute parallel for' explicitly 
> written
>   in the source, and it has an 'if(val)' clause, the if condition is applied 
> to
>   both 'target' and 'parallel' regions according to
>   OpenMP 5.2 [3.4, if Clause, Semantics, 15-18].
> 
>   However, if we are mapping an explicit 'target teams loop if(val)' onto a
>   'target teams distribute parallel for if(val)', to preserve the 'if' 
> semantics
>   as specified by the user with the 'target teams loop', we apply it just to
>   the 'target' region.
It does not match the spec. Why we shall handle it this way?


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

https://reviews.llvm.org/D157197

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

[PATCH] D157610: [include-cleaner][clangd][clang-tidy] Ignore resource dir during include-cleaner analysis.

2023-08-11 Thread Viktoriia Bakalova via Phabricator via cfe-commits

VitaNuo added a comment.

Thanks for the comments!




Comment at: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp:122
   llvm::DenseSet SeenSymbols;
+  std::string ResourceDir = HS->getHeaderSearchOpts().ResourceDir;
   // FIXME: Find a way to have less code duplication between include-cleaner

kadircet wrote:
> let's use `HS->getModuleMap().getBuiltinDir()` then we can get away with just 
> comparing that pointer to `H.physical()->getLastRef().getDir()` (same applies 
> to all the other places as well)
This only works in `clangd` code for me. I get `nullptr` for 
`HS->getModuleMap().getBuiltinDir()` in other places (Clang Tidy check and the 
library).



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:309
   continue;
+auto Dir = llvm::StringRef{MFI.Resolved}.rsplit('/').first;
+if (Dir == AST.getPreprocessor()

kadircet wrote:
> let's move this into `mayConsiderUnused`, we also convert this include into a 
> FileEntry in there, so we can directly compare the directory agian.
Ok moved to `mayConsiderUnused` and using the file entry now, but see above 
regarding comparing the directories.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157610

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

[PATCH] D153701: [WIP][Clang] Implement P2718R0 "Lifetime extension in range-based for loops"

2023-08-11 Thread Yurong via Phabricator via cfe-commits

yronglin updated this revision to Diff 549449.
yronglin added a comment.

Only create addational metarialized temporary in for-range-init.
FIXME: Need to handle function default argument, and add more test to make sure 
the generated LLVM IR is correct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153701

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/AST/ast-dump-for-range-lifetime.cpp

Index: clang/test/AST/ast-dump-for-range-lifetime.cpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-for-range-lifetime.cpp
@@ -0,0 +1,68 @@
+// RUN: %clang_cc1 -std=c++23 -triple x86_64-linux-gnu -fcxx-exceptions -ast-dump %s \
+// RUN: | FileCheck -strict-whitespace %s
+
+namespace p2718r0 {
+struct T {
+  int a[10] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
+  T() {}
+  ~T() {}
+  const int *begin() const { return a; }
+  const int *end() const { return a + 10; }
+};
+
+const T (const T ) { return t; }
+T g() { return T(); }
+
+void foo() {
+  // CHECK: FunctionDecl {{.*}} foo 'void ()'
+  // CHECK: `-CXXForRangeStmt {{.*}}
+  // CHECK-NEXT:|-<<>>
+  // CHECK-NEXT:|-DeclStmt {{.*}}
+  // CHECK-NEXT:| `-VarDecl {{.*}} implicit used __range1 'const T &' cinit
+  // CHECK-NEXT:|   `-ExprWithCleanups {{.*}} 'const T':'const p2718r0::T' lvalue
+  // CHECK-NEXT:| `-CallExpr {{.*}} 'const T':'const p2718r0::T' lvalue
+  // CHECK-NEXT:|   |-ImplicitCastExpr {{.*}} 'const T &(*)(const T &)' 
+  // CHECK-NEXT:|   | `-DeclRefExpr {{.*}} 'const T &(const T &)' lvalue Function {{.*}} 'f1' 'const T &(const T &)'
+  // CHECK-NEXT:|   `-MaterializeTemporaryExpr {{.*}} 'const T':'const p2718r0::T' lvalue extended by Var {{.*}} '__range1' 'const T &'
+  // CHECK-NEXT:| `-ImplicitCastExpr {{.*}} 'const T':'const p2718r0::T' 
+  // CHECK-NEXT:|   `-CXXBindTemporaryExpr {{.*}}  'T':'p2718r0::T' (CXXTemporary {{.*}})
+  // CHECK-NEXT:| `-CallExpr {{.*}} 'T':'p2718r0::T'
+  // CHECK-NEXT:|   `-ImplicitCastExpr {{.*}} 'T (*)()' 
+  // CHECK-NEXT:| `-DeclRefExpr {{.*}} 'T ()' lvalue Function {{.*}} 'g' 'T ()'
+  [[maybe_unused]] int sum = 0;
+  for (auto e : f1(g()))
+sum += e;
+}
+
+struct LockGuard {
+LockGuard(int) {}
+
+~LockGuard() {}
+};
+
+void f() {
+  int v[] = {42, 17, 13};
+  int M = 0;
+
+  // CHECK: FunctionDecl {{.*}} f 'void ()'
+  // CHECK: `-CXXForRangeStmt {{.*}}
+  // CHECK-NEXT:   |-<<>>
+  // CHECK-NEXT:   |-DeclStmt {{.*}}
+  // CHECK-NEXT:   | `-VarDecl {{.*}} col:16 implicit used __range1 'int (&)[3]' cinit
+  // CHECK-NEXT:   |   `-ExprWithCleanups {{.*}} 'int[3]' lvalue
+  // CHECK-NEXT:   | `-BinaryOperator {{.*}} 'int[3]' lvalue ','
+  // CHECK-NEXT:   |   |-CXXStaticCastExpr {{.*}}'void' static_cast 
+  // CHECK-NEXT:   |   | `-MaterializeTemporaryExpr {{.*}} 'LockGuard':'p2718r0::LockGuard' xvalue extended by Var {{.*}} '__range1' 'int (&)[3]'
+  // CHECK-NEXT:   |   |   `-CXXFunctionalCastExpr {{.*}} 'LockGuard':'p2718r0::LockGuard' functional cast to LockGuard 
+  // CHECK-NEXT:   |   | `-CXXBindTemporaryExpr {{.*}} 'LockGuard':'p2718r0::LockGuard' (CXXTemporary {{.*}})
+  // CHECK-NEXT:   |   |   `-CXXConstructExpr {{.*}} 'LockGuard':'p2718r0::LockGuard' 'void (int)'
+  // CHECK-NEXT:   |   | `-ImplicitCastExpr {{.*}} 'int' 
+  // CHECK-NEXT:   |   |   `-DeclRefExpr {{.*}} 'int' lvalue Var {{.*}} 'M' 'int'
+  // CHECK-NEXT:   |   `-DeclRefExpr {{.*}} 'int[3]' lvalue Var {{.*}} 'v' 'int[3]'
+  for (int x : static_cast(LockGuard(M)), v) // lock released in C++ 2020
+  {
+LockGuard guard(M); // OK in C++ 2020, now deadlocks
+  }
+}
+
+} // namespace p2718r0
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -27,6 +27,7 @@
 #include "clang/AST/TypeOrdering.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Lex/Preprocessor.h"
+#include "clang/Sema/EnterExpressionEvaluationContext.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Ownership.h"
@@ -2529,10 +2530,20 @@
   VarDecl *RangeVar = BuildForRangeVarDecl(*this, RangeLoc,
Context.getAutoRRefDeductType(),
std::string("__range") + DepthStr);
-  if (FinishForRangeVarDecl(*this, RangeVar, Range, RangeLoc,
-diag::err_for_range_deduction_failure)) {
-ActOnInitializerError(LoopVar);
-return StmtError();
+  {
+EnterExpressionEvaluationContext RangeVarContext(
+*this,

[PATCH] D157197: [clang][CodeGen][OpenMP] Fix if-clause for 'target teams loop'

2023-08-11 Thread David Pagan via Phabricator via cfe-commits

ddpagan added inline comments.

Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:1570-1575
+  // If we are here with a 'target teams loop' then we are emitting the
+  // 'parallel' region of the 'target teams distribute parallel for'
+  // emitted in place of the 'target teams loop'. Based on the specification
+  // noted above, an if-clause associated with a 'target teams loop', be it
+  // 'if(val)' or an 'if(target:val)', will apply only to 'target' and not
+  // the 'parallel' of the 'target teams distribute parallel for'.

ABataev wrote:
> It does not match the spec. 
> ```
> For a combined or composite construct, if no directive-name-modifier is 
> specified then the if clause applies to all constituent constructs to which 
> an if clause can apply.
> ```
> So, if(val) should be applied to both target and parallel regions, no?
> It does not match the spec. 
> ```
> For a combined or composite construct, if no directive-name-modifier is 
> specified then the if clause applies to all constituent constructs to which 
> an if clause can apply.
> ```
> So, if(val) should be applied to both target and parallel regions, no?

Hi Alexey - Question for you: does revising the comment above at lines 
1570-1575 to the following text help explain in a better way what's being done, 
and why?

  If we are handling a 'target teams distribute parallel for' explicitly written
  in the source, and it has an 'if(val)' clause, the if condition is applied to
  both 'target' and 'parallel' regions according to
  OpenMP 5.2 [3.4, if Clause, Semantics, 15-18].

  However, if we are mapping an explicit 'target teams loop if(val)' onto a
  'target teams distribute parallel for if(val)', to preserve the 'if' semantics
  as specified by the user with the 'target teams loop', we apply it just to
  the 'target' region.

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

https://reviews.llvm.org/D157197

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

[PATCH] D155773: [llvm][MemoryBuiltins] Add alloca support to getInitialValueOfAllocation

2023-08-11 Thread Matt Arsenault via Phabricator via cfe-commits

arsenm added inline comments.



Comment at: llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp:809-811
+  Updater.AddAvailableValue(
+  Alloca.getParent(),
+  getInitialValueOfAllocation(, nullptr, VectorTy));

This is very specifically handling alloca, not any random allocation like 
function


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155773

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

[PATCH] D157610: [include-cleaner][clangd][clang-tidy] Ignore resource dir during include-cleaner analysis.

2023-08-11 Thread Viktoriia Bakalova via Phabricator via cfe-commits

VitaNuo updated this revision to Diff 549443.
VitaNuo marked 4 inline comments as done.
VitaNuo added a comment.

Address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157610

Files:
  clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/test/include-cleaner-batch-fix.test
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
  clang-tools-extra/include-cleaner/lib/Analysis.cpp
  clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp

Index: clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
@@ -273,6 +273,30 @@
   EXPECT_THAT(Results.Unused, testing::IsEmpty());
 }
 
+TEST_F(AnalyzeTest, ResourceDirIsIgnored) {
+  Inputs.ExtraArgs.push_back("-resource-dir");
+  Inputs.ExtraArgs.push_back("./resources");
+  Inputs.Code = R"cpp(
+#include "resources/amintrin.h"
+#include "resources/imintrin.h"
+void baz() {
+  bar();
+}
+  )cpp";
+  Inputs.ExtraFiles["resources/amintrin.h"] = "";
+  Inputs.ExtraFiles["resources/emintrin.h"] = guard(R"cpp(
+void bar();
+  )cpp");
+  Inputs.ExtraFiles["resources/imintrin.h"] = guard(R"cpp(
+#include "emintrin.h"
+  )cpp");
+  TestAST AST(Inputs);
+  auto Results = analyze({}, {}, PP.Includes, , AST.sourceManager(),
+ AST.preprocessor().getHeaderSearchInfo());
+  EXPECT_THAT(Results.Unused, testing::IsEmpty());
+  EXPECT_THAT(Results.Missing, testing::IsEmpty());
+}
+
 TEST(FixIncludes, Basic) {
   llvm::StringRef Code = R"cpp(#include "d.h"
 #include "a.h"
Index: clang-tools-extra/include-cleaner/lib/Analysis.cpp
===
--- clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -17,6 +17,7 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Format/Format.h"
 #include "clang/Lex/HeaderSearch.h"
+#include "clang/Lex/HeaderSearchOptions.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "clang/Tooling/Inclusions/StandardLibrary.h"
 #include "llvm/ADT/ArrayRef.h"
@@ -68,12 +69,18 @@
   llvm::StringSet<> Missing;
   if (!HeaderFilter)
 HeaderFilter = [](llvm::StringRef) { return false; };
+  std::string ResourceDir = HS.getHeaderSearchOpts().ResourceDir;
   walkUsed(ASTRoots, MacroRefs, PI, SM,
[&](const SymbolReference , llvm::ArrayRef Providers) {
  bool Satisfied = false;
  for (const Header  : Providers) {
-   if (H.kind() == Header::Physical && H.physical() == MainFile)
+   if (H.kind() == Header::Physical &&
+   (H.physical() == MainFile ||
+H.physical()->getLastRef().getDir().getName() ==
+ResourceDir)) {
  Satisfied = true;
+ continue;
+   }
for (const Include *I : Inc.match(H)) {
  Used.insert(I);
  Satisfied = true;
@@ -88,7 +95,8 @@
   AnalysisResults Results;
   for (const Include  : Inc.all()) {
 if (Used.contains() || !I.Resolved ||
-HeaderFilter(I.Resolved->getFileEntry().tryGetRealPathName()))
+HeaderFilter(I.Resolved->getFileEntry().tryGetRealPathName()) ||
+I.Resolved->getDir().getName() == ResourceDir)
   continue;
 if (PI) {
   if (PI->shouldKeep(*I.Resolved))
Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -574,6 +574,29 @@
   EXPECT_THAT(Findings.UnusedIncludes, IsEmpty());
 }
 
+TEST(IncludeCleaner, ResourceDirIsIgnored) {
+  auto TU = TestTU::withCode(R"cpp(
+#include 
+#include 
+void baz() {
+  bar();
+}
+  )cpp");
+  TU.ExtraArgs.push_back("-resource-dir");
+  TU.ExtraArgs.push_back(testPath("resources"));
+  TU.AdditionalFiles["resources/include/amintrin.h"] = "";
+  TU.AdditionalFiles["resources/include/imintrin.h"] = guard(R"cpp(
+#include 
+  )cpp");
+  TU.AdditionalFiles["resources/include/emintrin.h"] = guard(R"cpp(
+void bar();
+  )cpp");
+  auto AST = TU.build();
+  auto Findings = computeIncludeCleanerFindings(AST);
+  EXPECT_THAT(Findings.UnusedIncludes, IsEmpty());
+  EXPECT_THAT(Findings.MissingIncludes, IsEmpty());
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/test/include-cleaner-batch-fix.test
===
--- clang-tools-extra/clangd/test/include-cleaner-batch-fix.test

[PATCH] D150647: [WIP][analyzer] Fix EnumCastOutOfRangeChecker C++17 handling

2023-08-11 Thread Balázs Benics via Phabricator via cfe-commits

steakhal added a comment.

Isn't D153954  superseded this one? If so, 
consider abandoning it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150647

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

[PATCH] D155773: [llvm][MemoryBuiltins] Add alloca support to getInitialValueOfAllocation

2023-08-11 Thread John McIver via Phabricator via cfe-commits

jmciver added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155773

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

[PATCH] D154603: [analyzer][clangsa] Add new option to alpha.security.cert.InvalidPtrChecker

2023-08-11 Thread Balázs Benics via Phabricator via cfe-commits

steakhal added a comment.

I'm sorry starting the review of this one only now, but I'm quite booked.
Is it still relevant? If so, I'll continue.




Comment at: clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp:116-117
+const NoteTag *Note =
+C.getNoteTag([Region, FunctionName, Message](PathSensitiveBugReport 
,
+ llvm::raw_ostream ) {
+  if (!BR.isInteresting(Region))

`FunctionName` and `Message` will dangle inside the NoteTag.



Comment at: clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp:125
 
-  const NoteTag *Note =
-  C.getNoteTag([SymbolicEnvPtrRegion, FunctionName](
-   PathSensitiveBugReport , llvm::raw_ostream ) {
-if (!BR.isInteresting(SymbolicEnvPtrRegion))
-  return;
-Out << '\'' << FunctionName
-<< "' call may invalidate the environment parameter of 'main'";
-  });
+  ExplodedNode *CurrentChainEnd = nullptr;
+

donat.nagy wrote:
> Perhaps add a comment that clarifies that passing a `nullptr` as the 
> ExplodedNode to `addTransition` is equivalent to specifying the current node. 
> I remember this because I was studying its implementation recently, but I 
> would've been surprised and suspicious otherwise.
If `nullptr` is equivalent with `C.getPredecessor()` inside `addTransition()`, 
why not simply initialize it to that value instead of to `nullptr`?



Comment at: clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp:214
+  // Model 'getenv' calls
+  CallDescription GetEnvCall{{"getenv"}, 1};
+  if (GetEnvCall.matches(Call)) {

We should hoist this into a field, to only construct it once.



Comment at: clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp:216-217
+  if (GetEnvCall.matches(Call)) {
+State =
+State->add(Call.getReturnValue().getAsRegion());
+   C.addTransition(State);

What ensures that `Call.getReturnValue().getAsRegion()` is not null?



Comment at: clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp:218
+State->add(Call.getReturnValue().getAsRegion());
+   C.addTransition(State);
+  }

donat.nagy wrote:
> I fear that this state transition will go "sideways" and the later state 
> transitions (which add the note tags) will branch off instead of building 
> onto this. IIUC calling `CheckerContext::addTransition` registers the 
> transition without updating the "current ExplodedNode" field of 
> `CheckerContext`, so you need to explicitly store and pass around the 
> ExplodedNode returned by it if you want to build on it.
> 
> This is an ugly and counter-intuitive API, and I also ran into a very similar 
> issue a few weeks ago (@Szelethus helped me).
I think the usage here is correct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154603

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

[PATCH] D157632: [Profile] Allow online merging with debug info correlation.

2023-08-11 Thread Zequan Wu via Phabricator via cfe-commits

zequanwu added a comment.

BTW, I noticed something strange with `-pgo-function-entry-coverage` when 
merging via llvm-profdata.
In this file `compiler-rt/test/profile/Linux/instrprof-debug-info-correlate.c`, 
I ran the following:

  // RUN: %clang_pgogen -o %t.cov.normal -mllvm --pgo-function-entry-coverage 
-mllvm --disable-vp=true %S/../Inputs/instrprof-debug-info-correlate-main.cpp 
%S/../Inputs/instrprof-debug-info-correlate-foo.cpp
  // RUN: rm -rf %t.dir && mkdir %t.dir
  // RUN: env LLVM_PROFILE_FILE=%t.dir/%m.profraw %run %t.cov.normal
  // RUN: env LLVM_PROFILE_FILE=%t.dir/%m.profraw %run %t.cov.normal
  // RUN: llvm-profdata merge -o %t.cov.normal.profdata %t.dir/
  // RUN: llvm-profdata show --all-functions --counts %t.cov.normal.profdata
  // It shows block counts 1.
  Counters:
main:
  Hash: 0x06d15c67b2c35b9c
  Counters: 1
  Block counts: [1]
_Z3fooi:
  Hash: 0x0209aa3e3852da94
  Counters: 1
  Block counts: [1]
_Z3bari:
  Hash: 0x0209aa3e1d398548
  Counters: 1
  Block counts: [1]
_Z6unusedi:
  Hash: 0x0a4d0ad3efff
  Counters: 1
  Block counts: [0]
  Instrumentation level: IR  entry_first = 0
  Functions shown: 4
  Total functions: 4
  Maximum function count: 1
  Maximum internal block count: 0
  
  // RUN: rm -rf %t.dir && mkdir %t.dir
  // RUN: env LLVM_PROFILE_FILE=%t.dir/%t.cov-1.profraw %run %t.cov.normal
  // RUN: env LLVM_PROFILE_FILE=%t.dir/%t.cov-2.profraw %run %t.cov.normal
  // RUN: llvm-profdata merge -o %t.cov.normal.profdata %t.dir/
  // RUN: llvm-profdata show --all-functions --counts %t.cov.normal.profdata
  // It shows block counts 2, as opposed to 1.
  Counters:
main:
  Hash: 0x06d15c67b2c35b9c
  Counters: 1
  Block counts: [2]
_Z3fooi:
  Hash: 0x0209aa3e3852da94
  Counters: 1
  Block counts: [2]
_Z3bari:
  Hash: 0x0209aa3e1d398548
  Counters: 1
  Block counts: [2]
_Z6unusedi:
  Hash: 0x0a4d0ad3efff
  Counters: 1
  Block counts: [0]
  Instrumentation level: IR  entry_first = 0
  Functions shown: 4
  Total functions: 4
  Maximum function count: 2
  Maximum internal block count: 0


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157632

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

[PATCH] D157684: [clang][ASTImporter] Repeated friend templates are partially imported

2023-08-11 Thread Ding Fei via Phabricator via cfe-commits

danix800 updated this revision to Diff 549436.
danix800 added a comment.

Turn off `Complain` mode on `IsEquivalentFriend` checking.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157684

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp

Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -4054,6 +4054,25 @@
  ->lookup(ToRecordOfFriend->getDeclName())
  .empty());
   }
+
+  void testRepeatedFriendImport(const char *Code) {
+Decl *ToTu = getToTuDecl(Code, Lang_CXX03);
+Decl *FromTu = getTuDecl(Code, Lang_CXX03, "from.cc");
+
+auto *ToFriend1 = FirstDeclMatcher().match(ToTu, friendDecl());
+auto *ToFriend2 = LastDeclMatcher().match(ToTu, friendDecl());
+auto *FromFriend1 =
+FirstDeclMatcher().match(FromTu, friendDecl());
+auto *FromFriend2 =
+LastDeclMatcher().match(FromTu, friendDecl());
+
+FriendDecl *ToImportedFriend1 = Import(FromFriend1, Lang_CXX03);
+FriendDecl *ToImportedFriend2 = Import(FromFriend2, Lang_CXX03);
+
+EXPECT_NE(ToImportedFriend1, ToImportedFriend2);
+EXPECT_EQ(ToFriend1, ToImportedFriend1);
+EXPECT_EQ(ToFriend2, ToImportedFriend2);
+  }
 };
 
 TEST_P(ImportFriendClasses, ImportOfFriendRecordDoesNotMergeDefinition) {
@@ -4343,21 +4362,7 @@
 friend class X;
   };
   )";
-  Decl *ToTu = getToTuDecl(Code, Lang_CXX03);
-  Decl *FromTu = getTuDecl(Code, Lang_CXX03, "from.cc");
-
-  auto *ToFriend1 = FirstDeclMatcher().match(ToTu, friendDecl());
-  auto *ToFriend2 = LastDeclMatcher().match(ToTu, friendDecl());
-  auto *FromFriend1 =
-  FirstDeclMatcher().match(FromTu, friendDecl());
-  auto *FromFriend2 = LastDeclMatcher().match(FromTu, friendDecl());
-
-  FriendDecl *ToImportedFriend1 = Import(FromFriend1, Lang_CXX03);
-  FriendDecl *ToImportedFriend2 = Import(FromFriend2, Lang_CXX03);
-
-  EXPECT_NE(ToImportedFriend1, ToImportedFriend2);
-  EXPECT_EQ(ToFriend1, ToImportedFriend1);
-  EXPECT_EQ(ToFriend2, ToImportedFriend2);
+  testRepeatedFriendImport(Code);
 }
 
 TEST_P(ImportFriendClasses, ImportOfRepeatedFriendDecl) {
@@ -4368,21 +4373,31 @@
 friend void f();
   };
   )";
-  Decl *ToTu = getToTuDecl(Code, Lang_CXX03);
-  Decl *FromTu = getTuDecl(Code, Lang_CXX03, "from.cc");
-
-  auto *ToFriend1 = FirstDeclMatcher().match(ToTu, friendDecl());
-  auto *ToFriend2 = LastDeclMatcher().match(ToTu, friendDecl());
-  auto *FromFriend1 =
-  FirstDeclMatcher().match(FromTu, friendDecl());
-  auto *FromFriend2 = LastDeclMatcher().match(FromTu, friendDecl());
+  testRepeatedFriendImport(Code);
+}
 
-  FriendDecl *ToImportedFriend1 = Import(FromFriend1, Lang_CXX03);
-  FriendDecl *ToImportedFriend2 = Import(FromFriend2, Lang_CXX03);
+TEST_P(ImportFriendClasses, ImportOfRepeatedFriendFunctionTemplateDecl) {
+  const char *Code =
+  R"(
+template 
+class Container {
+  template  friend void m();
+  template  friend void m();
+};
+  )";
+  testRepeatedFriendImport(Code);
+}
 
-  EXPECT_NE(ToImportedFriend1, ToImportedFriend2);
-  EXPECT_EQ(ToFriend1, ToImportedFriend1);
-  EXPECT_EQ(ToFriend2, ToImportedFriend2);
+TEST_P(ImportFriendClasses, ImportOfRepeatedFriendClassTemplateDecl) {
+  const char *Code =
+  R"(
+template 
+class Container {
+  template  friend class X;
+  template  friend class X;
+};
+  )";
+  testRepeatedFriendImport(Code);
 }
 
 TEST_P(ASTImporterOptionSpecificTestBase, FriendFunInClassTemplate) {
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -4064,22 +4064,33 @@
   unsigned int IndexOfDecl;
 };
 
-template 
-static FriendCountAndPosition getFriendCountAndPosition(
-const FriendDecl *FD,
-llvm::function_ref GetCanTypeOrDecl) {
+static bool IsEquivalentFriend(ASTImporter , FriendDecl *FD1,
+   FriendDecl *FD2) {
+  if ((!FD1->getFriendType()) != (!FD2->getFriendType()))
+return false;
+
+  if (auto *TSI = FD1->getFriendType())
+return Importer.IsStructurallyEquivalent(
+TSI->getType(), FD2->getFriendType()->getType(), /*Complain=*/false);
+
+  StructuralEquivalenceContext Ctx(
+  FD1->getASTContext(), FD2->getASTContext(),
+  Importer.getNonEquivalentDecls(), getStructuralEquivalenceKind(Importer),
+  /* StrictTypeSpelling = */ false, /* Complain = */ false);
+  return Ctx.IsEquivalent(FD1, FD2);
+}
+
+static FriendCountAndPosition getFriendCountAndPosition(ASTImporter ,
+FriendDecl *FD) {
   unsigned int FriendCount = 0;
   std::optional

[PATCH] D156032: Implement CWG2137 (list-initialization from objects of the same type)

2023-08-11 Thread Mark de Wever via Phabricator via cfe-commits

Mordante accepted this revision as: libc++.
Mordante added a comment.

I only looked at the libc++ changes. The libc++ AIX CI failures are unrelated 
to your patch. LGTM modulo one nit.




Comment at: 
libcxx/test/std/utilities/utility/pairs/pairs.pair/ctor.pair_U_V_move.pass.cpp:134
+ */
+//test_pair_rv();
+static_assert(std::is_constructible,

Please remove this line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156032

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

[PATCH] D152436: [clang][analyzer] Move checker alpha.unix.StdCLibraryFunctions out of alpha.

2023-08-11 Thread Balázs Benics via Phabricator via cfe-commits

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

As I don't use this checker, thus I cannot speak of my experience.
However, the reasons look solid and I'm fine with moving this checker to 
`unix.StdCLibraryFunctions`.
Let some time for other reviewers to object before landing this. Lets say, one 
week from now.
@xazax.hun @NoQ?

As next steps, I'd be glad set the default value of `ModelPOSIX`  to `true`. I 
don't see much harm doing so.
(And maybe getting rid of that checker option entirely.)




Comment at: clang/docs/analyzer/checkers.rst:979-982
+.. _unix-StdCLibraryFunctions:
+
+unix.StdCLibraryFunctions (C)
+"""




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152436

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

[PATCH] D155081: Specify the developer policy around links to external resources

2023-08-11 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: llvm/docs/DeveloperPolicy.rst:359
+  If the patch fixes a bug in GitHub Issues, we encourage adding
+  "Fixes https://github.com/llvm/llvm-project/issues/12345; to automate closing
+  the issue in GitHub. If the patch has been reviewed, we encourage adding a

rZhBoYao wrote:
> probinson wrote:
> > mehdi_amini wrote:
> > > ldionne wrote:
> > > > smeenai wrote:
> > > > > aaron.ballman wrote:
> > > > > > arsenm wrote:
> > > > > > > I haven't quite figured out what the exact syntaxes which are 
> > > > > > > automatically recognized. It seems to recognize "Fixes #Nxyz"
> > > > > > Yup, it does support that form as well. I had heard more than once 
> > > > > > during code review that folks seem to prefer the full link because 
> > > > > > it's easier to click on that from the commit message than it is to 
> > > > > > navigate to the fix from the number alone. That seemed like a 
> > > > > > pretty good reason to recommend the full form, but I don't have 
> > > > > > strong opinions.
> > > > > +1 for encouraging the full link
> > > > Perhaps we could encourage using `https://llvm.org/PR12345` instead? 
> > > > Does anybody know whether `llvm.org/PRXXX` is something that we intend 
> > > > to keep around with the Github transition or not?
> > > @arsenm: It's documented 
> > > https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword
> > > And for linking cross-repo: 
> > > https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/autolinked-references-and-urls#issues-and-pull-requests
> > > Perhaps we could encourage using `https://llvm.org/PR12345` instead? Does 
> > > anybody know whether `llvm.org/PRXXX` is something that we intend to keep 
> > > around with the Github transition or not?
> > 
> > Currently the PRxxx links are to the old bugzillas, not the Github issues. 
> > It might be sad to lose that.
> If the full link is preferred, can you update the first bullet point in [[ 
> https://llvm.org/docs/BugLifeCycle.html#resolving-closing-bugs | the 
> Resolving/Closing bugs section of LLVM Bug Life Cycle ]]?
Given that the RFC was specifically about links and not about bug lifecycle, 
I'd rather change that policy in a different patch (I suspect someone would 
have to make a full RFC to make the change; I don't feel strongly enough to go 
through that process myself). It might make more sense to link to that section 
of the documentation from here instead of spelling out something that may sound 
like a conflicting policy. e.g.,
```
If the patch fixes a bug in GitHub Issues, we encourage adding a reference to 
the issue being closed, as described `here 
`_.
```


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

https://reviews.llvm.org/D155081

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

[clang-tools-extra] 887bece - [clang-tidy][NFC] Update clang-analyzer-cplusplus.PlacementNew doc

2023-08-11 Thread Piotr Zegar via cfe-commits


Author: Piotr Zegar
Date: 2023-08-11T16:06:45Z
New Revision: 887bece66c80fe2bc3f9010f34f4fccf0a2f964e

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

LOG: [clang-tidy][NFC] Update clang-analyzer-cplusplus.PlacementNew doc

Re-generate documentation for cplusplus.PlacementNew
using gen-static-analyzer-docs.py

Added: 


Modified: 

clang-tools-extra/docs/clang-tidy/checks/clang-analyzer/cplusplus.PlacementNew.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst

Removed: 




diff  --git 
a/clang-tools-extra/docs/clang-tidy/checks/clang-analyzer/cplusplus.PlacementNew.rst
 
b/clang-tools-extra/docs/clang-tidy/checks/clang-analyzer/cplusplus.PlacementNew.rst
index 7f75e2d5315dae..c5d19a7deeb2e8 100644
--- 
a/clang-tools-extra/docs/clang-tidy/checks/clang-analyzer/cplusplus.PlacementNew.rst
+++ 
b/clang-tools-extra/docs/clang-tidy/checks/clang-analyzer/cplusplus.PlacementNew.rst
@@ -1,4 +1,6 @@
 .. title:: clang-tidy - clang-analyzer-cplusplus.PlacementNew
+.. meta::
+   :http-equiv=refresh: 
5;URL=https://clang.llvm.org/docs/analyzer/checkers.html#cplusplus-placementnew
 
 clang-analyzer-cplusplus.PlacementNew
 =
@@ -6,5 +8,7 @@ clang-analyzer-cplusplus.PlacementNew
 Check if default placement new is provided with pointers to sufficient storage
 capacity.
 
-The clang-analyzer-cplusplus.PlacementNew check is an alias of
-Clang Static Analyzer cplusplus.PlacementNew.
+The clang-analyzer-cplusplus.PlacementNew check is an alias, please see
+`Clang Static Analyzer Available Checkers
+`_
+for more information.

diff  --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst 
b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 6dfdf34e9d026f..bbacd7458c6c3d 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -421,7 +421,7 @@ Clang-Tidy Checks
`clang-analyzer-cplusplus.Move `_, 
Clang Static Analyzer cplusplus.Move,
`clang-analyzer-cplusplus.NewDelete 
`_, `Clang Static Analyzer 
cplusplus.NewDelete 
`_,
`clang-analyzer-cplusplus.NewDeleteLeaks 
`_, `Clang Static Analyzer 
cplusplus.NewDeleteLeaks 
`_,
-   `clang-analyzer-cplusplus.PlacementNew 
`_, Clang Static Analyzer 
cplusplus.PlacementNew,
+   `clang-analyzer-cplusplus.PlacementNew 
`_, `Clang Static Analyzer 
cplusplus.PlacementNew 
`_,
`clang-analyzer-cplusplus.PureVirtualCall 
`_, Clang Static Analyzer 
cplusplus.PureVirtualCall,
`clang-analyzer-cplusplus.StringChecker 
`_, `Clang Static Analyzer 
cplusplus.StringChecker 
`_,
`clang-analyzer-deadcode.DeadStores 
`_, `Clang Static Analyzer 
deadcode.DeadStores 
`_,



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

[PATCH] D157702: [clang][analyzer][NFC] Change PlacementNewChecker into PlacementNew in documentation

2023-08-11 Thread Piotr Zegar via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa3a66de37fc0: [clang][analyzer][NFC] Change 
PlacementNewChecker into PlacementNew in… (authored by PiotrZSL).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157702

Files:
  clang/docs/analyzer/checkers.rst


Index: clang/docs/analyzer/checkers.rst
===
--- clang/docs/analyzer/checkers.rst
+++ clang/docs/analyzer/checkers.rst
@@ -321,10 +321,10 @@
int *p = new int;
  } // warn
 
-.. _cplusplus-PlacementNewChecker:
+.. _cplusplus-PlacementNew:
 
-cplusplus.PlacementNewChecker (C++)
-"""
+cplusplus.PlacementNew (C++)
+
 Check if default placement new is provided with pointers to sufficient storage 
capacity.
 
 .. code-block:: cpp


Index: clang/docs/analyzer/checkers.rst
===
--- clang/docs/analyzer/checkers.rst
+++ clang/docs/analyzer/checkers.rst
@@ -321,10 +321,10 @@
int *p = new int;
  } // warn
 
-.. _cplusplus-PlacementNewChecker:
+.. _cplusplus-PlacementNew:
 
-cplusplus.PlacementNewChecker (C++)
-"""
+cplusplus.PlacementNew (C++)
+
 Check if default placement new is provided with pointers to sufficient storage capacity.
 
 .. code-block:: cpp
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157649: [clang-tidy] Fix crash when diagnostic is emit with invalid location

2023-08-11 Thread Piotr Zegar via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGefd44f80a5a8: [clang-tidy] Fix crash when diagnostic is emit 
with invalid location (authored by PiotrZSL).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157649

Files:
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp


Index: clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
@@ -25,6 +25,7 @@
 // RUN: not clang-tidy -checks='-*,modernize-use-override' 
%T/diagnostics/input.cpp -- -DCOMPILATION_ERROR 2>&1 | FileCheck 
-check-prefix=CHECK6 -implicit-check-not='{{warning:|error:}}' %s
 // RUN: clang-tidy 
-checks='-*,modernize-use-override,clang-diagnostic-macro-redefined' %s -- 
-DMACRO_FROM_COMMAND_LINE -std=c++20 | FileCheck -check-prefix=CHECK4 
-implicit-check-not='{{warning:|error:}}' %s
 // RUN: clang-tidy 
-checks='-*,modernize-use-override,clang-diagnostic-macro-redefined,clang-diagnostic-literal-conversion'
 %s -- -DMACRO_FROM_COMMAND_LINE -std=c++20 -Wno-macro-redefined | FileCheck 
--check-prefix=CHECK7 -implicit-check-not='{{warning:|error:}}' %s
+// RUN: not clang-tidy -checks='-*,modernize-use-override' %s -- -std=c++20 
-DPR64602 | FileCheck -check-prefix=CHECK8 
-implicit-check-not='{{warning:|error:}}' %s
 
 // CHECK1: error: no input files [clang-diagnostic-error]
 // CHECK1: error: no such file or directory: '{{.*}}nonexistent.cpp' 
[clang-diagnostic-error]
@@ -54,3 +55,18 @@
   // CHECK6: :[[@LINE-1]]:3: error: cannot take the address of an rvalue of 
type 'int' [clang-diagnostic-error]
 }
 #endif
+
+#ifdef PR64602 // Should not crash
+template 
+struct S
+{
+auto foo(auto);
+};
+
+template <>
+auto S<>::foo(auto)
+{
+return 1;
+}
+// CHECK8: error: template parameter list matching the non-templated nested 
type 'S<>' should be empty ('template<>') [clang-diagnostic-error]
+#endif
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -437,8 +437,11 @@
 SmallString<100> Message;
 Info.FormatDiagnostic(Message);
 FullSourceLoc Loc;
-if (Info.getLocation().isValid() && Info.hasSourceManager())
+if (Info.hasSourceManager())
   Loc = FullSourceLoc(Info.getLocation(), Info.getSourceManager());
+else if (Context.DiagEngine->hasSourceManager())
+  Loc = FullSourceLoc(Info.getLocation(),
+  Context.DiagEngine->getSourceManager());
 Converter.emitDiagnostic(Loc, DiagLevel, Message, Info.getRanges(),
  Info.getFixItHints());
   }


Index: clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
@@ -25,6 +25,7 @@
 // RUN: not clang-tidy -checks='-*,modernize-use-override' %T/diagnostics/input.cpp -- -DCOMPILATION_ERROR 2>&1 | FileCheck -check-prefix=CHECK6 -implicit-check-not='{{warning:|error:}}' %s
 // RUN: clang-tidy -checks='-*,modernize-use-override,clang-diagnostic-macro-redefined' %s -- -DMACRO_FROM_COMMAND_LINE -std=c++20 | FileCheck -check-prefix=CHECK4 -implicit-check-not='{{warning:|error:}}' %s
 // RUN: clang-tidy -checks='-*,modernize-use-override,clang-diagnostic-macro-redefined,clang-diagnostic-literal-conversion' %s -- -DMACRO_FROM_COMMAND_LINE -std=c++20 -Wno-macro-redefined | FileCheck --check-prefix=CHECK7 -implicit-check-not='{{warning:|error:}}' %s
+// RUN: not clang-tidy -checks='-*,modernize-use-override' %s -- -std=c++20 -DPR64602 | FileCheck -check-prefix=CHECK8 -implicit-check-not='{{warning:|error:}}' %s
 
 // CHECK1: error: no input files [clang-diagnostic-error]
 // CHECK1: error: no such file or directory: '{{.*}}nonexistent.cpp' [clang-diagnostic-error]
@@ -54,3 +55,18 @@
   // CHECK6: :[[@LINE-1]]:3: error: cannot take the address of an rvalue of type 'int' [clang-diagnostic-error]
 }
 #endif
+
+#ifdef PR64602 // Should not crash
+template 
+struct S
+{
+auto foo(auto);
+};
+
+template <>
+auto S<>::foo(auto)
+{
+return 1;
+}
+// CHECK8: error: template parameter list matching the non-templated nested type 'S<>' should be empty ('template<>') [clang-diagnostic-error]
+#endif
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++

[clang-tools-extra] efd44f8 - [clang-tidy] Fix crash when diagnostic is emit with invalid location

2023-08-11 Thread Piotr Zegar via cfe-commits


Author: Piotr Zegar
Date: 2023-08-11T16:04:15Z
New Revision: efd44f80a5a8194b9fe26ff3244ce702cd8dab73

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

LOG: [clang-tidy] Fix crash when diagnostic is emit with invalid location

Fix crash when diagnostic is emit with invalid location,
but with attached valid ranges. Diagnostic can contain
invalid location, but SourceManager attached to it still
can be valid, use it in such case or fallback to known
SourceManager.

Fixes: #64602

Reviewed By: aaron.ballman

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp 
b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index 019d9b1f18a5a9..41a210a83a84f3 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -437,8 +437,11 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic(
 SmallString<100> Message;
 Info.FormatDiagnostic(Message);
 FullSourceLoc Loc;
-if (Info.getLocation().isValid() && Info.hasSourceManager())
+if (Info.hasSourceManager())
   Loc = FullSourceLoc(Info.getLocation(), Info.getSourceManager());
+else if (Context.DiagEngine->hasSourceManager())
+  Loc = FullSourceLoc(Info.getLocation(),
+  Context.DiagEngine->getSourceManager());
 Converter.emitDiagnostic(Loc, DiagLevel, Message, Info.getRanges(),
  Info.getFixItHints());
   }

diff  --git a/clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp 
b/clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
index c87496292b9db2..547f634a101c58 100644
--- a/clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
@@ -25,6 +25,7 @@
 // RUN: not clang-tidy -checks='-*,modernize-use-override' 
%T/diagnostics/input.cpp -- -DCOMPILATION_ERROR 2>&1 | FileCheck 
-check-prefix=CHECK6 -implicit-check-not='{{warning:|error:}}' %s
 // RUN: clang-tidy 
-checks='-*,modernize-use-override,clang-diagnostic-macro-redefined' %s -- 
-DMACRO_FROM_COMMAND_LINE -std=c++20 | FileCheck -check-prefix=CHECK4 
-implicit-check-not='{{warning:|error:}}' %s
 // RUN: clang-tidy 
-checks='-*,modernize-use-override,clang-diagnostic-macro-redefined,clang-diagnostic-literal-conversion'
 %s -- -DMACRO_FROM_COMMAND_LINE -std=c++20 -Wno-macro-redefined | FileCheck 
--check-prefix=CHECK7 -implicit-check-not='{{warning:|error:}}' %s
+// RUN: not clang-tidy -checks='-*,modernize-use-override' %s -- -std=c++20 
-DPR64602 | FileCheck -check-prefix=CHECK8 
-implicit-check-not='{{warning:|error:}}' %s
 
 // CHECK1: error: no input files [clang-diagnostic-error]
 // CHECK1: error: no such file or directory: '{{.*}}nonexistent.cpp' 
[clang-diagnostic-error]
@@ -54,3 +55,18 @@ void f(int a) {
   // CHECK6: :[[@LINE-1]]:3: error: cannot take the address of an rvalue of 
type 'int' [clang-diagnostic-error]
 }
 #endif
+
+#ifdef PR64602 // Should not crash
+template 
+struct S
+{
+auto foo(auto);
+};
+
+template <>
+auto S<>::foo(auto)
+{
+return 1;
+}
+// CHECK8: error: template parameter list matching the non-templated nested 
type 'S<>' should be empty ('template<>') [clang-diagnostic-error]
+#endif



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

[clang] a3a66de - [clang][analyzer][NFC] Change PlacementNewChecker into PlacementNew in documentation

2023-08-11 Thread Piotr Zegar via cfe-commits


Author: Piotr Zegar
Date: 2023-08-11T16:04:15Z
New Revision: a3a66de37fc038d46d7146c9dfe55f26fd3a

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

LOG: [clang][analyzer][NFC] Change PlacementNewChecker into PlacementNew in 
documentation

Check name according to Checkers.td is actually a PlacementNew.

Reviewed By: steakhal

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

Added: 


Modified: 
clang/docs/analyzer/checkers.rst

Removed: 




diff  --git a/clang/docs/analyzer/checkers.rst 
b/clang/docs/analyzer/checkers.rst
index 97b5369ac86c9b..d3a4ebcaf02025 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -321,10 +321,10 @@ Check for memory leaks. Traces memory managed by 
new/delete.
int *p = new int;
  } // warn
 
-.. _cplusplus-PlacementNewChecker:
+.. _cplusplus-PlacementNew:
 
-cplusplus.PlacementNewChecker (C++)
-"""
+cplusplus.PlacementNew (C++)
+
 Check if default placement new is provided with pointers to sufficient storage 
capacity.
 
 .. code-block:: cpp



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

[PATCH] D157632: [Profile] Allow online merging with debug info correlation.

2023-08-11 Thread Zequan Wu via Phabricator via cfe-commits

zequanwu added a comment.

In D157632#4578582 , @ellis wrote:

> I've just published https://reviews.llvm.org/D157664, so you'll want to 
> rebase ontop of it if it lands soon. I would also like to see some more tests 
> added to `instrprof-merge-error.c` to make sure two different binaries can't 
> merge profiles together with `--debug-info-correlate`. I was thinking the 
> test would be something like this.
>
>   // RUN: %clang_pgogen -o %t/a -g -mllvm --debug-info-correlate -mllvm 
> --disable-vp=true %t/main.c
>   // RUN: %clang_pgogen -o %t/b -g -mllvm --debug-info-correlate -mllvm 
> --disable-vp=true %t/main.c %t/foo.c
>   // RUN: env LLVM_PROFILE_FILE=%t/default_%m.profdata %run %t/a
>   // This next line should fail to merge because the counter sections have 
> different sizes
>   // RUN: env LLVM_PROFILE_FILE=%t/default_%m.profdata %run %t/b
>   
>   //--- main.c
>   int main() { return 0; }
>   //--- foo.c
>   int foo() { return 4; }

In this example, `%m` will translated to two different values so they won't be 
merged. As I mentioned in the inline comment, 
`__llvm_profile_check_compatibility` will verify that for us.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157632

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

[PATCH] D157104: [analyzer] Improve underflow handling in ArrayBoundV2

2023-08-11 Thread Balázs Benics via Phabricator via cfe-commits

steakhal accepted this revision.
steakhal added a comment.

Looks safe and good. I'm interested in the diff though.
Let me know once you have the results.
I wanna have a look before we land this.




Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:184
+// symbolic regions on the heap (which may be introduced by checkers that
+// call SValBuilder::getConjuredHeapSymbolVal()) and non-symbolic regions
+// (e.g. a field subregion of a symbolic region) in unknown space.

Such as the MallocChecker.



Comment at: clang/test/Analysis/out-of-bounds.c:176
 }
-

Try not to introduce unrelated hunks, as it might introduce more conflicts for 
downstream users than absolutely necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157104

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

[PATCH] D157632: [Profile] Allow online merging with debug info correlation.

2023-08-11 Thread Zequan Wu via Phabricator via cfe-commits

zequanwu added inline comments.



Comment at: compiler-rt/lib/profile/InstrProfilingMerge.c:129
+  // enabled.
+  if (Header->DataSize == 0) {
+if (!(__llvm_profile_get_version() & VARIANT_MASK_DBG_CORRELATE)) {

ellis wrote:
> Since we don't have the data section, we need to be confident that existing 
> profile comes from the same binary (so that the counter section is 
> identical). Can we add some extra checks here? I'm thinking we can verify 
> that some fields in the headers match and that the variant flags are 
> identical.
The [[ 
https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/profile/InstrProfiling.h#L105-L107
 | comment ]] says the caller of this function is responsible for the check. 
And we [[ 
https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/profile/InstrProfilingMerge.c#L48
 | verifies ]] that fields in both headers math before merging at 
doProfileMerging.



Comment at: compiler-rt/lib/profile/InstrProfilingMerge.c:134-136
+for (SrcCounter = SrcCountersStart,
+DstCounter = __llvm_profile_begin_counters();
+ SrcCounter < SrcCountersEnd;) {

ellis wrote:
> Can you add a check to make sure src and dst have the same number of counters 
> (`SrcCountersEnd - SrcCountersStart`)?
Added the check at `__llvm_profile_check_compatibility`. 

I use in-memory `__llvm_profile_counter_entry_size()` to calculate in-file 
SrcCountersEnd, because the header only tells number of counters.



Comment at: compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c:26
+// RUN: %clang_pgogen -o %t -g -mllvm --debug-info-correlate -mllvm 
--disable-vp=true %S/../Inputs/instrprof-debug-info-correlate-main.cpp 
%S/../Inputs/instrprof-debug-info-correlate-foo.cpp
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.proflite %run %t
+// RUN: llvm-profdata merge -o %t.profdata --debug-info=%t.dSYM %t.profdir/

ellis wrote:
> We need to run this line twice to correctly test 
> `__llvm_profile_merge_from_buffer()`
Added.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157632

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

[PATCH] D157632: [Profile] Allow online merging with debug info correlation.

2023-08-11 Thread Zequan Wu via Phabricator via cfe-commits

zequanwu updated this revision to Diff 549424.
zequanwu marked 3 inline comments as done.
zequanwu added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157632

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  compiler-rt/lib/profile/InstrProfilingMerge.c
  compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c
  compiler-rt/test/profile/Linux/instrprof-debug-info-correlate.c
  compiler-rt/test/profile/instrprof-merge-error.c

Index: compiler-rt/test/profile/instrprof-merge-error.c
===
--- compiler-rt/test/profile/instrprof-merge-error.c
+++ compiler-rt/test/profile/instrprof-merge-error.c
@@ -1,11 +1,5 @@
 // RUN: rm -rf %t; mkdir %t
 
-// RUN: %clang_pgogen -o %t/dbg -g -mllvm --debug-info-correlate -mllvm --disable-vp=true %s
-// RUN: env LLVM_PROFILE_FILE=%t/dbg_%m.profdata %run %t/dbg 2>&1 | count 0
-// RUN: env LLVM_PROFILE_FILE=%t/dbg_%m.profdata %run %t/dbg 2>&1 | FileCheck %s --check-prefix=DBG
-
-// DBG: Debug info correlation does not support profile merging at runtime.
-
 // RUN: %clang_pgogen -o %t/timeprof -mllvm -pgo-temporal-instrumentation %s
 // RUN: env LLVM_PROFILE_FILE=%t/timeprof_%m.profdata %run %t/timeprof 2>&1 | count 0
 // RUN: env LLVM_PROFILE_FILE=%t/timeprof_%m.profdata %run %t/timeprof 2>&1 | FileCheck %s --check-prefix=TIMEPROF
Index: compiler-rt/test/profile/Linux/instrprof-debug-info-correlate.c
===
--- compiler-rt/test/profile/Linux/instrprof-debug-info-correlate.c
+++ compiler-rt/test/profile/Linux/instrprof-debug-info-correlate.c
@@ -24,3 +24,29 @@
 // RUN: llvm-profdata merge -o %t.cov.normal.profdata %t.cov.profraw
 
 // RUN: diff <(llvm-profdata show %t.cov.normal.profdata) <(llvm-profdata show %t.cov.profdata)
+
+// Test debug info correlate with online merging.
+
+// RUN: rm -rf %t.profdir && mkdir %t.profdir
+
+// RUN: env LLVM_PROFILE_FILE=%t-1.profraw %run %t.normal
+// RUN: env LLVM_PROFILE_FILE=%t-2.profraw %run %t.normal
+// RUN: llvm-profdata merge -o %t.normal.profdata %t-1.profraw %t-2.profraw
+
+// RUN: %clang_pgogen -o %t -g -mllvm --debug-info-correlate -mllvm --disable-vp=true %S/../Inputs/instrprof-debug-info-correlate-main.cpp %S/../Inputs/instrprof-debug-info-correlate-foo.cpp
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.proflite %run %t
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.proflite %run %t
+// RUN: llvm-profdata merge -o %t.profdata --debug-info=%t %t.profdir/
+
+// RUN: diff <(llvm-profdata show --all-functions --counts %t.normal.profdata) <(llvm-profdata show --all-functions --counts %t.profdata)
+// RUN: diff %t.normal.profdata %t.profdata
+
+// RUN: rm -rf %t.profdir && mkdir %t.profdir
+
+// RUN: %clang_pgogen -o %t.cov -g -mllvm --debug-info-correlate -mllvm -pgo-function-entry-coverage -mllvm --disable-vp=true %S/../Inputs/instrprof-debug-info-correlate-main.cpp %S/../Inputs/instrprof-debug-info-correlate-foo.cpp
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.cov.proflite %run %t.cov
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.cov.proflite %run %t.cov
+// RUN: llvm-profdata merge -o %t.cov.profdata --debug-info=%t.cov %t.profdir/
+
+// RUN: diff <(llvm-profdata show --all-functions --counts %t.cov.normal.profdata) <(llvm-profdata show --all-functions --counts %t.cov.profdata)
+// RUN: diff %t.cov.normal.profdata %t.cov.profdata
Index: compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c
===
--- compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c
+++ compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c
@@ -18,3 +18,27 @@
 // RUN: llvm-profdata merge -o %t.cov.normal.profdata %t.cov.profraw
 
 // RUN: diff %t.cov.normal.profdata %t.cov.profdata
+
+// Test debug info correlate with online merging.
+
+// RUN: rm -rf %t.profdir && mkdir %t.profdir
+
+// RUN: env LLVM_PROFILE_FILE=%t-1.profraw %run %t.normal
+// RUN: env LLVM_PROFILE_FILE=%t-2.profraw %run %t.normal
+// RUN: llvm-profdata merge -o %t.normal.profdata %t-1.profraw %t-2.profraw
+
+// RUN: %clang_pgogen -o %t -g -mllvm --debug-info-correlate -mllvm --disable-vp=true %S/../Inputs/instrprof-debug-info-correlate-main.cpp %S/../Inputs/instrprof-debug-info-correlate-foo.cpp
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.proflite %run %t
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.proflite %run %t
+// RUN: llvm-profdata merge -o %t.profdata --debug-info=%t.dSYM %t.profdir/
+
+// RUN: diff %t.normal.profdata %t.profdata
+
+// RUN: rm -rf %t.profdir && mkdir %t.profdir
+
+// RUN: %clang_pgogen -o %t.cov -g -mllvm --debug-info-correlate -mllvm -pgo-function-entry-coverage -mllvm --disable-vp=true %S/../Inputs/instrprof-debug-info-correlate-main.cpp %S/../Inputs/instrprof-debug-info-correlate-foo.cpp
+// RUN: env

[PATCH] D157702: [clang][analyzer][NFC] Change PlacementNewChecker into PlacementNew in documentation

2023-08-11 Thread Balázs Benics via Phabricator via cfe-commits

steakhal added a comment.

In D157702#4580384 , @PiotrZSL wrote:

> In D157702#4580310 , @steakhal 
> wrote:
>
>> Do we have other typos like this?
>
> I don't think so, only mess in overall documentation: 
> https://github.com/llvm/llvm-project/issues/64614
> I updated gen-static-analyzer-docs.py, and it shown me that some 
> documentation is missing: 
> https://clang.llvm.org/extra/clang-tidy/checks/list.html.
> You can see it, as there are no links for some checks.

Yea, it's a complete mess, I agree.
I have no stakes on the docs, but I know that Ericsson folks do. They might 
wanna chim in. @donat.nagy @Szelethus


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157702

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

[PATCH] D141714: Fix ast print of variables with attributes

2023-08-11 Thread Timo Stripf via Phabricator via cfe-commits

strimo378 added a comment.

@aaron.ballman I had today a teams meeting with @giulianobelinassi and we 
discussed both patches as well as that we want to work together in improving 
AST print. Both patches are fine for us since they improve the attribute 
printing. The features of the respective other patch can be easily integrated 
with minimal changes in a subsequent smaller patch. We further think that there 
are a lot more cases of attributes that we need to consider that we do not know 
atm.

I think we should go one with patch that has the higher probability to get 
landed soon. Since this branch had a lot more reviews, I suggest we go further 
with this patch. I further suggest to keep it simple and remove the tblgen part 
and add it in an subsequent patch once we know all cases and requirements. What 
do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141714

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

[PATCH] D155081: Specify the developer policy around links to external resources

2023-08-11 Thread Tom Honermann via Phabricator via cfe-commits

tahonermann added a comment.

This looks good to me.


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

https://reviews.llvm.org/D155081

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

[PATCH] D157702: [clang][analyzer][NFC] Change PlacementNewChecker into PlacementNew in documentation

2023-08-11 Thread Piotr Zegar via Phabricator via cfe-commits

PiotrZSL added a comment.

In D157702#4580310 , @steakhal wrote:

> Do we have other typos like this?

I don't think so, only mess in overall documentation: 
https://github.com/llvm/llvm-project/issues/64614
I updated gen-static-analyzer-docs.py, and it shown me that some documentation 
is missing: https://clang.llvm.org/extra/clang-tidy/checks/list.html.
You can see it, as there are no links for some checks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157702

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

[PATCH] D157727: [clang] Fix copy-and-paste error in CXXRecordDecl tabledef

2023-08-11 Thread Christian Blichmann via Phabricator via cfe-commits

cblichmann created this revision.
cblichmann added reviewers: clang, rsmith, zahiraam.
cblichmann added a project: clang.
Herald added a project: All.
cblichmann requested review of this revision.
Herald added a subscriber: cfe-commits.

This change just updates the comment in the tabledef, no change in observable 
behavior.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157727

Files:
  clang/include/clang/AST/CXXRecordDeclDefinitionBits.def


Index: clang/include/clang/AST/CXXRecordDeclDefinitionBits.def
===
--- clang/include/clang/AST/CXXRecordDeclDefinitionBits.def
+++ clang/include/clang/AST/CXXRecordDeclDefinitionBits.def
@@ -99,7 +99,7 @@
 /// True when there are protected non-static data members.
 FIELD(HasProtectedFields, 1, NO_MERGE)
 
-/// True when there are private non-static data members.
+/// True when there are public non-static data members.
 FIELD(HasPublicFields, 1, NO_MERGE)
 
 /// True if this class (or any subobject) has mutable fields.


Index: clang/include/clang/AST/CXXRecordDeclDefinitionBits.def
===
--- clang/include/clang/AST/CXXRecordDeclDefinitionBits.def
+++ clang/include/clang/AST/CXXRecordDeclDefinitionBits.def
@@ -99,7 +99,7 @@
 /// True when there are protected non-static data members.
 FIELD(HasProtectedFields, 1, NO_MERGE)
 
-/// True when there are private non-static data members.
+/// True when there are public non-static data members.
 FIELD(HasPublicFields, 1, NO_MERGE)
 
 /// True if this class (or any subobject) has mutable fields.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157702: [clang][analyzer][NFC] Change PlacementNewChecker into PlacementNew in documentation

2023-08-11 Thread Balázs Benics via Phabricator via cfe-commits

steakhal added a comment.

Do we have other typos like this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157702

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

[PATCH] D157702: [clang][analyzer][NFC] Change PlacementNewChecker into PlacementNew in documentation

2023-08-11 Thread Balázs Benics via Phabricator via cfe-commits

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

Correct. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157702

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

[PATCH] D155081: Specify the developer policy around links to external resources

2023-08-11 Thread PoYao Chang via Phabricator via cfe-commits

rZhBoYao added inline comments.



Comment at: llvm/docs/DeveloperPolicy.rst:359
+  If the patch fixes a bug in GitHub Issues, we encourage adding
+  "Fixes https://github.com/llvm/llvm-project/issues/12345; to automate closing
+  the issue in GitHub. If the patch has been reviewed, we encourage adding a

probinson wrote:
> mehdi_amini wrote:
> > ldionne wrote:
> > > smeenai wrote:
> > > > aaron.ballman wrote:
> > > > > arsenm wrote:
> > > > > > I haven't quite figured out what the exact syntaxes which are 
> > > > > > automatically recognized. It seems to recognize "Fixes #Nxyz"
> > > > > Yup, it does support that form as well. I had heard more than once 
> > > > > during code review that folks seem to prefer the full link because 
> > > > > it's easier to click on that from the commit message than it is to 
> > > > > navigate to the fix from the number alone. That seemed like a pretty 
> > > > > good reason to recommend the full form, but I don't have strong 
> > > > > opinions.
> > > > +1 for encouraging the full link
> > > Perhaps we could encourage using `https://llvm.org/PR12345` instead? Does 
> > > anybody know whether `llvm.org/PRXXX` is something that we intend to keep 
> > > around with the Github transition or not?
> > @arsenm: It's documented 
> > https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword
> > And for linking cross-repo: 
> > https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/autolinked-references-and-urls#issues-and-pull-requests
> > Perhaps we could encourage using `https://llvm.org/PR12345` instead? Does 
> > anybody know whether `llvm.org/PRXXX` is something that we intend to keep 
> > around with the Github transition or not?
> 
> Currently the PRxxx links are to the old bugzillas, not the Github issues. It 
> might be sad to lose that.
If the full link is preferred, can you update the first bullet point in [[ 
https://llvm.org/docs/BugLifeCycle.html#resolving-closing-bugs | the 
Resolving/Closing bugs section of LLVM Bug Life Cycle ]]?


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

https://reviews.llvm.org/D155081

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

[PATCH] D157517: [AArch64][SVE] Add asm predicate constraint Uph

2023-08-11 Thread Matt Devereau via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc52d9509d40d: [AArch64][SVE] Add asm predicate constraint 
Uph (authored by MattDevereau).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157517

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/test/CodeGen/aarch64-sve-inline-asm-datatypes.c
  llvm/docs/LangRef.rst
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/test/CodeGen/AArch64/aarch64-sve-asm.ll

Index: llvm/test/CodeGen/AArch64/aarch64-sve-asm.ll
===
--- llvm/test/CodeGen/AArch64/aarch64-sve-asm.ll
+++ llvm/test/CodeGen/AArch64/aarch64-sve-asm.ll
@@ -68,3 +68,14 @@
   %1 = tail call  asm "incp $0.s, $1", "=w,@3Upa,0"( %Pg,  %Zn)
   ret  %1
 }
+
+; Function Attrs: nounwind readnone
+; CHECK: [[ARG1:%[0-9]+]]:zpr = COPY $z1
+; CHECK: [[ARG2:%[0-9]+]]:zpr = COPY $z0
+; CHECK: [[ARG3:%[0-9]+]]:ppr = COPY $p0
+; CHECK: [[ARG4:%[0-9]+]]:ppr_p8to15 = COPY [[ARG3]]
+; CHECK: INLINEASM {{.*}} [[ARG4]]
+define  @test_svfadd_f16_Uph_constraint( %Pg,  %Zn,  %Zm) {
+  %1 = tail call  asm "fadd $0.h, $1/m, $2.h, $3.h", "=w,@3Uph,w,w"( %Pg,  %Zn,  %Zm)
+  ret  %1
+}
Index: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
===
--- llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -9987,19 +9987,31 @@
   return "r";
 }
 
-enum PredicateConstraint {
-  Upl,
-  Upa,
-  Invalid
-};
+enum PredicateConstraint { Uph, Upl, Upa, Invalid };
 
 static PredicateConstraint parsePredicateConstraint(StringRef Constraint) {
-  PredicateConstraint P = PredicateConstraint::Invalid;
-  if (Constraint == "Upa")
-P = PredicateConstraint::Upa;
-  if (Constraint == "Upl")
-P = PredicateConstraint::Upl;
-  return P;
+  return StringSwitch(Constraint)
+  .Case("Uph", PredicateConstraint::Uph)
+  .Case("Upl", PredicateConstraint::Upl)
+  .Case("Upa", PredicateConstraint::Upa)
+  .Default(PredicateConstraint::Invalid);
+}
+
+static const TargetRegisterClass *
+getPredicateRegisterClass(PredicateConstraint Constraint, EVT VT) {
+  if (!VT.isScalableVector() || VT.getVectorElementType() != MVT::i1)
+return nullptr;
+
+  switch (Constraint) {
+  default:
+return nullptr;
+  case PredicateConstraint::Uph:
+return ::PPR_p8to15RegClass;
+  case PredicateConstraint::Upl:
+return ::PPR_3bRegClass;
+  case PredicateConstraint::Upa:
+return ::PPRRegClass;
+  }
 }
 
 // The set of cc code supported is from
@@ -10191,13 +10203,8 @@
 }
   } else {
 PredicateConstraint PC = parsePredicateConstraint(Constraint);
-if (PC != PredicateConstraint::Invalid) {
-  if (!VT.isScalableVector() || VT.getVectorElementType() != MVT::i1)
-return std::make_pair(0U, nullptr);
-  bool restricted = (PC == PredicateConstraint::Upl);
-  return restricted ? std::make_pair(0U, ::PPR_3bRegClass)
-: std::make_pair(0U, ::PPRRegClass);
-}
+if (const TargetRegisterClass *RegClass = getPredicateRegisterClass(PC, VT))
+  return std::make_pair(0U, RegClass);
   }
   if (StringRef("{cc}").equals_insensitive(Constraint) ||
   parseConstraintCode(Constraint) != AArch64CC::Invalid)
Index: llvm/docs/LangRef.rst
===
--- llvm/docs/LangRef.rst
+++ llvm/docs/LangRef.rst
@@ -4997,7 +4997,8 @@
 - ``w``: A 32, 64, or 128-bit floating-point, SIMD or SVE vector register.
 - ``x``: Like w, but restricted to registers 0 to 15 inclusive.
 - ``y``: Like w, but restricted to SVE vector registers Z0 to Z7 inclusive.
-- ``Upl``: One of the low eight SVE predicate registers (P0 to P7)
+- ``Uph``: One of the upper eight SVE predicate registers (P8 to P15)
+- ``Upl``: One of the lower eight SVE predicate registers (P0 to P7)
 - ``Upa``: Any of the SVE predicate registers (P0 to P15)
 
 AMDGPU:
Index: clang/test/CodeGen/aarch64-sve-inline-asm-datatypes.c
===
--- clang/test/CodeGen/aarch64-sve-inline-asm-datatypes.c
+++ clang/test/CodeGen/aarch64-sve-inline-asm-datatypes.c
@@ -168,6 +168,30 @@
 SVBOOL_TEST_UPL(__SVInt64_t, d) ;
 // CHECK: call  asm sideeffect "fadd $0.d, $1.d, $2.d, $3.d\0A", "=w,@3Upl,w,w"( %in1,  %in2,  %in3)
 
+#define SVBOOL_TEST_UPH(DT, KIND)\
+__SVBool_t func_bool_uph_##KIND(__SVBool_t in1, DT in2, DT in3)\
+{\
+  __SVBool_t out;\
+  asm volatile (\
+"fadd %[out]." #KIND ", %[in1]." #KIND ", %[in2]." #KIND ", %[in3]." #KIND "\n"\
+: [out] "=w" (out)\
+:  [in1] "Uph" (in1),\
+  [in2] "w" (in2),\
+  [in3] "w" (in3)\
+:);\
+  return out;\
+}
+
+SVBOOL_TEST_UPH(__SVInt8_t, b) ;
+// CHECK: call  asm sideeffect "fadd $0.b, $1.b, $2.b, $3.b\0A", "=w,@3Uph,w,w"( %in1,  %in2,  %in3)
+SVBOOL_TEST_UPH(__SVInt16_t, h) ;

[clang] c52d950 - [AArch64][SVE] Add asm predicate constraint Uph

2023-08-11 Thread Matt Devereau via cfe-commits


Author: Matt Devereau
Date: 2023-08-11T14:48:19Z
New Revision: c52d9509d40d3048914b144618232213e6076e05

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

LOG: [AArch64][SVE] Add asm predicate constraint Uph

Some instructions such as multi-vector LD1 only accept a range
of PN8-PN15 predicate-as-counter. This new constraint allows more
refined parsing and better decision making when parsing these
instructions from ASM, instead of defaulting to Upa which incorrectly
uses the whole range of registers P0-P15 from the register class PPR.

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

Added: 


Modified: 
clang/lib/Basic/Targets/AArch64.cpp
clang/test/CodeGen/aarch64-sve-inline-asm-datatypes.c
llvm/docs/LangRef.rst
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
llvm/test/CodeGen/AArch64/aarch64-sve-asm.ll

Removed: 




diff  --git a/clang/lib/Basic/Targets/AArch64.cpp 
b/clang/lib/Basic/Targets/AArch64.cpp
index 7c4cc5fb33f886..6c43c8b592622d 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -1288,8 +1288,9 @@ bool AArch64TargetInfo::validateAsmConstraint(
 Info.setAllowsRegister();
 return true;
   case 'U':
-if (Name[1] == 'p' && (Name[2] == 'l' || Name[2] == 'a')) {
-  // SVE predicate registers ("Upa"=P0-15, "Upl"=P0-P7)
+if (Name[1] == 'p' &&
+(Name[2] == 'l' || Name[2] == 'a' || Name[2] == 'h')) {
+  // SVE predicate registers ("Upa"=P0-15, "Upl"=P0-P7, "Uph"=P8-P15)
   Info.setAllowsRegister();
   Name += 2;
   return true;

diff  --git a/clang/test/CodeGen/aarch64-sve-inline-asm-datatypes.c 
b/clang/test/CodeGen/aarch64-sve-inline-asm-datatypes.c
index 5c1e931a727124..14a29dfac2c7bd 100644
--- a/clang/test/CodeGen/aarch64-sve-inline-asm-datatypes.c
+++ b/clang/test/CodeGen/aarch64-sve-inline-asm-datatypes.c
@@ -168,6 +168,30 @@ SVBOOL_TEST_UPL(__SVInt32_t, s) ;
 SVBOOL_TEST_UPL(__SVInt64_t, d) ;
 // CHECK: call  asm sideeffect "fadd $0.d, $1.d, $2.d, 
$3.d\0A", "=w,@3Upl,w,w"( %in1,  %in2, 
 %in3)
 
+#define SVBOOL_TEST_UPH(DT, KIND)\
+__SVBool_t func_bool_uph_##KIND(__SVBool_t in1, DT in2, DT in3)\
+{\
+  __SVBool_t out;\
+  asm volatile (\
+"fadd %[out]." #KIND ", %[in1]." #KIND ", %[in2]." #KIND ", %[in3]." #KIND 
"\n"\
+: [out] "=w" (out)\
+:  [in1] "Uph" (in1),\
+  [in2] "w" (in2),\
+  [in3] "w" (in3)\
+:);\
+  return out;\
+}
+
+SVBOOL_TEST_UPH(__SVInt8_t, b) ;
+// CHECK: call  asm sideeffect "fadd $0.b, $1.b, $2.b, 
$3.b\0A", "=w,@3Uph,w,w"( %in1,  %in2, 
 %in3)
+SVBOOL_TEST_UPH(__SVInt16_t, h) ;
+// CHECK: call  asm sideeffect "fadd $0.h, $1.h, $2.h, 
$3.h\0A", "=w,@3Uph,w,w"( %in1,  %in2, 
 %in3)
+SVBOOL_TEST_UPH(__SVInt32_t, s) ;
+// CHECK: call  asm sideeffect "fadd $0.s, $1.s, $2.s, 
$3.s\0A", "=w,@3Uph,w,w"( %in1,  %in2, 
 %in3)
+SVBOOL_TEST_UPH(__SVInt64_t, d) ;
+// CHECK: call  asm sideeffect "fadd $0.d, $1.d, $2.d, 
$3.d\0A", "=w,@3Uph,w,w"( %in1,  %in2, 
 %in3)
+
+
 #define SVFLOAT_TEST(DT,KIND)\
 DT func_float_##DT##KIND(DT inout1, DT in2)\
 {\

diff  --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index f024d009966a8d..f7f5cc193a149c 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -4997,7 +4997,8 @@ AArch64:
 - ``w``: A 32, 64, or 128-bit floating-point, SIMD or SVE vector register.
 - ``x``: Like w, but restricted to registers 0 to 15 inclusive.
 - ``y``: Like w, but restricted to SVE vector registers Z0 to Z7 inclusive.
-- ``Upl``: One of the low eight SVE predicate registers (P0 to P7)
+- ``Uph``: One of the upper eight SVE predicate registers (P8 to P15)
+- ``Upl``: One of the lower eight SVE predicate registers (P0 to P7)
 - ``Upa``: Any of the SVE predicate registers (P0 to P15)
 
 AMDGPU:

diff  --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp 
b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
index 948419f29b48e9..d0f4789d198058 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -9987,19 +9987,31 @@ const char *AArch64TargetLowering::LowerXConstraint(EVT 
ConstraintVT) const {
   return "r";
 }
 
-enum PredicateConstraint {
-  Upl,
-  Upa,
-  Invalid
-};
+enum PredicateConstraint { Uph, Upl, Upa, Invalid };
 
 static PredicateConstraint parsePredicateConstraint(StringRef Constraint) {
-  PredicateConstraint P = PredicateConstraint::Invalid;
-  if (Constraint == "Upa")
-P = PredicateConstraint::Upa;
-  if (Constraint == "Upl")
-P = PredicateConstraint::Upl;
-  return P;
+  return StringSwitch(Constraint)
+  .Case("Uph", PredicateConstraint::Uph)
+  .Case("Upl", PredicateConstraint::Upl)
+  .Case("Upa", PredicateConstraint::Upa)
+  .Default(PredicateConstraint::Invalid);
+}
+
+static const

[PATCH] D157680: [X86]Support options -mno-gather -mno-scatter

2023-08-11 Thread Kan Shengchen via Phabricator via cfe-commits

skan added inline comments.



Comment at: llvm/lib/Target/X86/X86.td:437
+: SubtargetFeature<"prefer-no-gather", "PreferGather", "false",
+   "Indicates if gather prefer to be disabled">;
+def FeaturePreferNoScatter

Does "Prefer no gather instructions" sound better?

I think these two should be put under "X86 Subtarget Tuning features"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157680

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

[PATCH] D157637: [ASTImporter][NFC] Fix typo in testcase

2023-08-11 Thread Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGafff91de082d: [ASTImporter][NFC] Fix typo in testcase 
(authored by dingfei fd...@feysh.com).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157637

Files:
  clang/unittests/AST/StructuralEquivalenceTest.cpp


Index: clang/unittests/AST/StructuralEquivalenceTest.cpp
===
--- clang/unittests/AST/StructuralEquivalenceTest.cpp
+++ clang/unittests/AST/StructuralEquivalenceTest.cpp
@@ -992,8 +992,8 @@
 
 TEST_F(StructuralEquivalenceRecordContextTest, NamespaceInlineTopLevel) {
   auto Decls =
-  makeNamedDecls("inline namespace A { class X; } }",
- "inline namespace B { class X; } }", Lang_CXX17, "X");
+  makeNamedDecls("inline namespace A { class X; }",
+ "inline namespace B { class X; }", Lang_CXX17, "X");
   EXPECT_TRUE(testStructuralMatch(Decls));
 }
 


Index: clang/unittests/AST/StructuralEquivalenceTest.cpp
===
--- clang/unittests/AST/StructuralEquivalenceTest.cpp
+++ clang/unittests/AST/StructuralEquivalenceTest.cpp
@@ -992,8 +992,8 @@
 
 TEST_F(StructuralEquivalenceRecordContextTest, NamespaceInlineTopLevel) {
   auto Decls =
-  makeNamedDecls("inline namespace A { class X; } }",
- "inline namespace B { class X; } }", Lang_CXX17, "X");
+  makeNamedDecls("inline namespace A { class X; }",
+ "inline namespace B { class X; }", Lang_CXX17, "X");
   EXPECT_TRUE(testStructuralMatch(Decls));
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157584: [clang][Sema] Skip access check on arrays of zero-length element

2023-08-11 Thread Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0f73a2406a16: [clang][Sema] Skip access check on arrays of 
zero-length element (authored by dingfei fd...@feysh.com).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157584

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/array-bounds-zero-length-elem-gh64564.c


Index: clang/test/Sema/array-bounds-zero-length-elem-gh64564.c
===
--- /dev/null
+++ clang/test/Sema/array-bounds-zero-length-elem-gh64564.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -triple i686-apple-darwin -verify %s
+
+int a[][0]; // expected-warning {{tentative array definition assumed to have 
one element}}
+void gh64564_1(void) {
+  int b = a[0x1][0];
+}
+
+typedef struct {} S;
+S s[]; // expected-warning {{tentative array definition assumed to have one 
element}}
+void gh64564_2(void) {
+  S t = s[0x1];
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -17146,7 +17146,7 @@
   ASTC.getTypeSizeInCharsIfKnown(EffectiveType);
   // PR50741 - If EffectiveType has unknown size (e.g., if it's a void
   // pointer) bounds-checking isn't meaningful.
-  if (!ElemCharUnits)
+  if (!ElemCharUnits || ElemCharUnits->isZero())
 return;
   llvm::APInt ElemBytes(index.getBitWidth(), ElemCharUnits->getQuantity());
   // If index has more active bits than address space, we already know
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -185,6 +185,8 @@
   terminated. Clang should now also recover better when an @end is missing
   between blocks.
   `Issue 64065 `_
+- Fixed a crash when check array access on zero-length element.
+  `Issue 64564 `_
 
 Target Specific Changes
 ---


Index: clang/test/Sema/array-bounds-zero-length-elem-gh64564.c
===
--- /dev/null
+++ clang/test/Sema/array-bounds-zero-length-elem-gh64564.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -triple i686-apple-darwin -verify %s
+
+int a[][0]; // expected-warning {{tentative array definition assumed to have one element}}
+void gh64564_1(void) {
+  int b = a[0x1][0];
+}
+
+typedef struct {} S;
+S s[]; // expected-warning {{tentative array definition assumed to have one element}}
+void gh64564_2(void) {
+  S t = s[0x1];
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -17146,7 +17146,7 @@
   ASTC.getTypeSizeInCharsIfKnown(EffectiveType);
   // PR50741 - If EffectiveType has unknown size (e.g., if it's a void
   // pointer) bounds-checking isn't meaningful.
-  if (!ElemCharUnits)
+  if (!ElemCharUnits || ElemCharUnits->isZero())
 return;
   llvm::APInt ElemBytes(index.getBitWidth(), ElemCharUnits->getQuantity());
   // If index has more active bits than address space, we already know
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -185,6 +185,8 @@
   terminated. Clang should now also recover better when an @end is missing
   between blocks.
   `Issue 64065 `_
+- Fixed a crash when check array access on zero-length element.
+  `Issue 64564 `_
 
 Target Specific Changes
 ---
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

1 2 >

1 - 100 of 174 matches

Mail list logo